fix: enforce the access manager guards on the assignment resource and instance methods

[howieandersen]

Seven assignment resource and instance methods in AssignmentService had their access guards inverted and keyed on the wrong id: if (await HasRole(assignment.Id, ...)) throw instead of if (!await HasRole(assignment.FromId, ...)) throw, and the same for the paired HasResource/HasPackage checks. Keyed on the assignment id the lookups never match anything, so the guards could not fire and the methods were fail open. This applies the correct pattern from AddAssignmentPackage to all seven method pairs: AddAssignmentResource, AddAssignmentInstance, UpsertAssignmentResource, UpsertAssignmentInstance, RemoveAssignmentPackage, RemoveAssignmentResource and RemoveAssignmentInstance. Exception types and messages are unchanged.

None of the seven methods has a caller outside IAssignmentService today, so no live endpoint changes behaviour; this closes the trap before the API gets wired up.

New integration test class AssignmentServiceAccessManagerGuardTest covers grant and deny paths against the real Postgres fixture, including a manager without the package hitting the second guard. Verified by mutation: with the old guards in place the three deny tests fail.

One limitation: the HasResource guard only becomes precise once the resource prune from #3899 lands (PR #3925); until then it proves the caller has some connection to the From party, which is why the deny tests target the role and package guards. Even with the prune there is an open question for review: HasResource does not load delegation resources, so a caller whose only hold on the resource is a client delegation will not pass the guard.

Closes #3111

[Copilot]

Pull request overview

This pull request fixes authorization guard logic in AssignmentService for assignment resource/package/instance mutation methods, ensuring AccessManager/ownership checks are evaluated against the correct FromId and with the correct polarity, and adds integration tests to prevent regression.

Changes:

• Fix inverted and incorrectly-keyed HasRole guard checks (now !HasRole(assignment.FromId, ...)) across affected methods.
• Fix incorrectly-keyed HasResource / HasPackage guards to use assignment.FromId (instead of assignment.Id).
• Add a new integration test class validating allow/deny paths for the corrected guards against the Postgres fixture.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/apps/Altinn.AccessManagement/src/Altinn.AccessMgmt.Core/Services/AssignmentService.cs	Corrects AccessManager/resource/package guard checks to use FromId and proper polarity for multiple assignment mutation methods.
src/apps/Altinn.AccessManagement/test/Altinn.AccessMgmt.Core.Tests/Integration/Services/AssignmentServiceAccessManagerGuardTest.cs	Adds integration coverage for key guard allow/deny paths using the real database fixture.

Suppressed comments (2)

src/apps/Altinn.AccessManagement/src/Altinn.AccessMgmt.Core/Services/AssignmentService.cs:578

• The UnauthorizedAccessException message in UpsertAssignmentInstance says "add resource to assignment", but this method upserts an instance. With the guard now active, the message should reflect the operation.

        // Check if user has AccessManager (Tilgangsstyrer) role
        if (!await HasRole(assignment.FromId, user.Id, RoleConstants.AccessManager, cancellationToken))
        {
            throw new UnauthorizedAccessException("User does not have permission to add resource to assignment");
        }

src/apps/Altinn.AccessManagement/src/Altinn.AccessMgmt.Core/Services/AssignmentService.cs:475

• The PR adds integration coverage for AddAssignmentResource and RemoveAssignmentPackage, but the same guard fix was applied to AddAssignmentInstance, UpsertAssignmentResource, UpsertAssignmentInstance, RemoveAssignmentResource, and RemoveAssignmentInstance. There are currently no tests calling those methods, so regressions in those guard paths would go undetected.

    /// <inheritdoc />
    public async Task<bool> AddAssignmentInstance(Guid userId, Guid assignmentId, Guid resourceId, string instanceId, string policyPath, string policyVersion, CancellationToken cancellationToken = default)
    {
        var user = await db.Entities.AsNoTracking().SingleAsync(t => t.Id == userId, cancellationToken);
        var assignment = await db.Assignments.AsNoTracking().SingleAsync(t => t.Id == assignmentId, cancellationToken);
        var resource = await db.Resources.AsNoTracking().SingleAsync(t => t.Id == resourceId, cancellationToken);

        // Check if user has AccessManager (Tilgangsstyrer) role
        if (!await HasRole(assignment.FromId, user.Id, RoleConstants.AccessManager, cancellationToken))
        {
            throw new UnauthorizedAccessException("User does not have permission to add resource to assignment");
        }

        // Check if user has access to the resource
        if (!await HasResource(assignment.FromId, user.Id, resource.Id, cancellationToken))
        {
            throw new UnauthorizedAccessException($"User '{user.Name}' does not have resource '{resource.Name}' for '{assignment.FromId}'");
        }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.