[PM-40209] Add PAM access claim and ManageAccessRules permission - #8160
[PM-40209] Add PAM access claim and ManageAccessRules permission#8160Hinton wants to merge 1 commit into
Conversation
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the PAM identity/authorization spine: the Code Review DetailsNo findings. Notes considered and intentionally not raised as findings:
|
c846d2e to
e0485bd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8160 +/- ##
===========================================
+ Coverage 15.10% 63.06% +47.95%
===========================================
Files 1417 2315 +898
Lines 61422 100530 +39108
Branches 4901 9045 +4144
===========================================
+ Hits 9279 63395 +54116
+ Misses 51978 34945 -17033
- Partials 165 2190 +2025 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The identity/authorization spine for Privileged Access Manager, mirroring the
Secrets Manager shape throughout: an `accesspam` claim beside
`accesssecretsmanager`, and the one PAM custom permission, `ManageAccessRules`.
Holding AccessPam *is* the access grant — there is no separate "can access PAM
items" permission. ManageAccessRules only gates Access Rule authorship.
Claims.PamAccess is deliberately kept out of UserIdentityClaimTypes: it is an
authorization claim, so it must be rebuilt from the database on every token
issuance rather than carried across a refresh.
Consumption is left to the IOrganizationRequirement pattern (ADR-0022) in
PM-40211, so this change only touches claim emission and the claims-parsing path
the requirement handler reads. CurrentContext is untouched — no AccessPam(orgId)
or ManageAccessRules(orgId) helpers, and no changes to its hand-rolled claims
parsers — since that is the legacy path requirements replace.
Permissions.ManageAccessRules gains a ClaimsMap entry, which makes both
custom-permission claim emission in BuildIdentityClaims and the grant-elevation
check in OrganizationUserValidationService automatic. Note that
OrganizationService.ValidateCustomPermissionsGrant enumerates permissions
explicitly against ICurrentContext and therefore does not cover
ManageAccessRules; closing that gap needs the CurrentContext helpers this story
excludes.
Two additions beyond the strict claims spine:
- ApiResources declares Claims.PamAccess on the `api` resource, where
SecretsManagerAccess already sits. Not load-bearing — ProfileService issues
claims regardless of RequestedClaimTypes — but it keeps the declared resource
claims honest.
- The public API's hand-rolled PermissionsModel gains ManageAccessRules. ToData()
rebuilds the whole permissions object, so without it a PUT to
/public/members/{id} would silently clear ManageAccessRules on a Custom member.
ToData_RoundTripsEveryPermission is a reflection-driven guard that now fails
whenever any permission goes missing from that mirror, rather than only fixing
this one instance.
Tests cover the AccessPam/UsePam/Enabled gating in CurrentContextOrganization
(and the previously uncovered Secrets Manager equivalent), access-claim emission
for both products, and the claim round trip through OrganizationClaimsExtensions.
e0485bd to
76f8d09
Compare
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40209
📔 Objective
The identity/authorization spine for PAM, mirroring the Secrets Manager shape throughout: an
accesspamclaim besideaccesssecretsmanager, and the one PAM custom permission,ManageAccessRules. HoldingAccessPamis the access grant — there is no separate "can access PAM items" permission;ManageAccessRulesonly gates Access Rule authorship.Consumption is left to the
IOrganizationRequirementpattern (ADR-0022) in PM-40211, so this only touches claim emission and the claims-parsing path the requirement handler reads.Worth flagging for review:
CurrentContextis untouched — noAccessPam(orgId)/ManageAccessRules(orgId)helpers, no changes to its hand-rolled claims parsers. That's the legacy path requirements replace.PamAccessis deliberately not inUserIdentityClaimTypes. It's an authorization claim, so it must be rebuilt from the database on every issuance rather than carried across a refresh.ApiResourcesdeclaresClaims.PamAccesswhereSecretsManagerAccessalready sits (not load-bearing, but keeps the declared resource claims honest). The public API's hand-rolledPermissionsModelgainsManageAccessRules, becauseToData()rebuilds the whole object and would otherwise silently clear it onPUT /public/members/{id};ToData_RoundTripsEveryPermissionguards that mirror against future drift.OrganizationService.ValidateCustomPermissionsGrantenumerates permissions againstICurrentContext, so it doesn't coverManageAccessRules— a Custom member withManageUserscould grant it without holding it. The newerOrganizationUserValidationServiceusesClaimsMapand is covered. Closing the legacy path needs theCurrentContexthelpers this story excludes.Depends on PM-40208 (the
OrganizationUser.AccessPamcolumn), already merged.