[PM-40211] Authorize Access Rule endpoints with IOrganizationRequirement

[Hinton]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-40211

📔 Objective

Declarative authorization on the access-rule routes (ADR-0022): reads require organization membership, writes require authority over rule authorship via the new ManageAccessRulesRequirement.

Worth flagging for review:

• Deviation: no named-policy bridge. The story specifies policy-name constants in Policies.cs registered in Api/Startup.cs, because Pam.csproj cannot reference Api and so could not reach AuthorizeAttribute<T>. PM-41272 has since extracted that code into src/Libraries/OrganizationAuthorization, which depends only on Core — so Pam references the library directly and attaches requirements per route. Policies.cs and Startup.cs are untouched.
• Requirements combine with, not replace, the group policy. They're carried as endpoint metadata, which AuthorizationMiddleware combines with the group-level Policies.Application; a test asserts both are present on every route. OrganizationRequirementHandler resolves the org from the {orgId:guid} group prefix and is already DI-registered.
• Enforcement is covered end to end. AccessRuleAuthorizationTests (Api.IntegrationTest) drives the real pipeline: non-members, plain members, Custom users without ManageAccessRules, and provider users all get 403; members and Owners get through. Presence in metadata is not enforcement, so the registration tests alone would not catch an AllowAnonymous, a group policy that replaced rather than combined, or a new route added without the attribute. Only denials are asserted, so these stay valid once the handler scaffolds are implemented.
• Scope: the Access Rule half only. The usage gate on access-request submission (which must check the member's AccessPam and the org's UsePam) and the CipherLeaseGate parity decision cannot land yet — SubmitAccessRequestCommand does not exist on main, since the access-request and cipher-lease handlers are still NotImplementedException scaffolds. There is no submission path to gate; that work belongs with the request/lease behaviour slices. For the same reason there were no imperative EnsureMemberAsync/EnsureAdminAsync checks to delete — AccessRuleEndpointsHandler never had them on main. PM-40211 should stay open for the remaining half.
• Only one of the two observable changes flagged in breakdown review note 8 applies here. Unauthorized calls return 403 from the middleware rather than 404 from a handler. The other — that BasePermissionRequirement/MemberOrProviderRequirement authorize provider users — is deliberately avoided: the group uses MemberRequirement, and ManageAccessRulesRequirement implements IOrganizationRequirement directly rather than deriving from BasePermissionRequirement. Providers are denied on both reads and writes — access rules gate who can lease credentials out of an organization, which is not theirs to read or change. Two tests pin this, including MapPamEndpoints_AccessRulesNeverAuthorizeProvidersByMembership.

Lockfile changes are the new project reference only — no platform churn.

Depends on #8160 and #8161 — review those first.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.55%. Comparing base (31ab8a6) to head (85c150d).

Additional details and impacted files

@@                   Coverage Diff                    @@
##           pam/enable-pam-flows    #8162      +/-   ##
========================================================
- Coverage                 67.92%   63.55%   -4.38%     
========================================================
  Files                      2338     2339       +1     
  Lines                    101555   101566      +11     
  Branches                   9186     9187       +1     
========================================================
- Hits                      68983    64547    -4436     
- Misses                    30265    34805    +4540     
+ Partials                   2307     2214      -93     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the declarative authorization added to the PAM access-rule routes: a group-level MemberRequirement plus ManageAccessRulesRequirement on the three write endpoints, the new ManageAccessRulesRequirement in PermissionRequirements.cs, and the OrganizationAuthorization project reference from Pam.csproj. Verified that Permissions.ManageAccessRules and its claim already exist, that OrganizationRequirementHandler is DI-registered in Api via AddAuthorizationHandlers(), that the {orgId:guid} group prefix satisfies HttpContextExtensions.GetOrganizationId(), and that ASP.NET's AuthorizationPolicy.CombineAsync accumulates the group's Policies.Application with the per-endpoint requirement metadata rather than replacing it — matching what the new tests assert. Lock files are consistent with the single project-reference addition across all six affected manifests; no new NuGet packages, so no AppSec dependency approval is required.

Code Review Details

No blocking findings.

Notes considered and intentionally not raised as findings:

• Reads being gated at plain organization membership is stated as the intended design in the PR description, and the handlers are still NotImplementedException scaffolds, so there is no live data exposure.
• A provider user who is also an org member would pass both gates on writes; this is inherent to BasePermissionRequirement's provider fallback across all existing consumers, not introduced here.
• The group documents 400/404 in OpenAPI but not 401/403; a documentation-only gap.

[eliykat]

This suggests that you don't actually want to reuse BasePermissionRequirement because you don't want to authorize providers. It works for most of our custom permissions, but you can always implement IOrganizationRequirement directly if you don't want that behavior.

For example:

public class ManageAccessRulesRequirement : IOrganizationRequirement
{
    public Task<bool> AuthorizeAsync(CurrentContextOrganization? organizationClaims,
        Func<Task<bool>> isProviderUserForOrg)
    {
        var authorized = organizationClaims is
            { Type: OrganizationUserType.Owner }
            or { Type: OrganizationUserType.Admin }
            or { Type: OrganizationUserType.Custom, Permissions.ManageAccessRules : true };

        return Task.FromResult(authorized);
    }
}

This can be used inside Authorize<T> in the same way, so no change to the consumers.

You can own this requirement if desired, because it reflects the authorization rules for your domain.

The merge-base changed after approval.

[eliykat]

AC no longer owns any files in this PR, so no approval required, but the authorization implementation looks good to me.

On more of a business logic / product note:

providers manage an organization's billing and configuration, but access rules gate who can lease credentials out of it, which is not theirs to read or change.

This might be overstating it a little - today providers can assign and remove access to items by managing collection assignments. They can't access items themselves, but it's well within their purpose to set up group and collection hierarchies for their clients to make sure they're using Bitwarden properly. I can imagine a use case where providers configure PAM rules for their clients as part of setting up their organization. It's OK if they're out of scope for the MVP, just wanted to clarify.