[PM-40210] Add single + bulk PAM enable flows for AccessPam - #8161
Open
Hinton wants to merge 1 commit into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## pam/claims-permission-plumbing #8161 +/- ##
==================================================================
+ Coverage 63.06% 67.53% +4.47%
==================================================================
Files 2315 2315
Lines 100530 100561 +31
Branches 9045 9052 +7
==================================================================
+ Hits 63395 67910 +4515
+ Misses 34945 30364 -4581
- Partials 2190 2287 +97 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Hinton
commented
Aug 7, 2026
Comment on lines
+136
to
+142
| // Granting PAM access to a member of an organization without PAM would be inert: claim emission ANDs | ||
| // AccessPam with the organization's UsePam. Reject so the admin gets an actionable error instead. | ||
| // Only the grant is gated — revoking access stays possible on an organization whose entitlement has lapsed. | ||
| if (!originalOrganizationUser.AccessPam && organizationUser.AccessPam && !organization.UsePam) | ||
| { | ||
| throw new BadRequestException("To grant PAM access the organization must have PAM enabled."); | ||
| } |
Member
Author
There was a problem hiding this comment.
I guess we could technically skip this check as enabling pam for a user without it being enabled in the org does nothing. I'll leave it up to AC to decide if it's worth keeping. It would eventually be replaced by the billing seat logic.
Hinton
force-pushed
the
pam/enable-pam-flows
branch
from
August 7, 2026 11:11
708e586 to
43281c3
Compare
The member-management flows that grant and revoke AccessPam, the admin-controlled
per-member PAM access grant.
Both paths are plain field writes behind an org-level UsePam check. PAM has no
seats, so there is deliberately no analogue of the Secrets Manager pattern here —
no CountNewSmSeatsRequiredQuery, no UpdateSecretsManagerSubscriptionCommand, and
no autoscale-last ordering rule to respect.
The UsePam guard replaces SM's, which is not explicit in the member flows at all:
SM leans on CountNewSmSeatsRequiredQuery throwing "Organization does not use
Secrets Manager" as a side effect of counting seats. With no seat count to
piggyback on, PAM checks UsePam directly. AccessPam = true on a non-PAM org would
otherwise be inert but unvalidated, since claim emission ANDs AccessPam with
UsePam (PM-40209) — the guard exists so admins get an actionable error instead of
a silently ineffective toggle.
Only the grant is gated. Revoking access is not checked against UsePam, so
removing a member's access stays possible on an organization whose PAM
entitlement has lapsed. Editing a member who already holds access is likewise not
a grant and is not blocked.
The single-user path is implemented on both sides of the ChangeMemberEmailNoMp
flag, so the toggle behaves the same whichever command serves the request:
- v1 threads AccessPam through OrganizationUserUpdateRequestModel.ToOrganizationUser
and guards the transition alongside the other validation.
- v2 adds NewAccessPam to UpdateOrganizationUserRequest with an IsEnablingPam()
transition helper, applies it via UpdateOrganizationUser, and returns a typed
PamNotEnabled error from the validator.
The bulk path is a new PUT organizations/{orgId}/users/enable-pam mirroring
BulkEnableSecretsManagerAsync minus the seat block. SM's obsolete PATCH alias is
not replicated, and like SM's bulk path it logs no events (breakdown review
note 5). The generic OrganizationUser_Updated event still covers the single-user
path.
UpdateOrganizationUser gains an accessPam parameter; its only production caller is
the v2 command.
Hinton
marked this pull request as ready for review
August 7, 2026 11:36
Hinton
force-pushed
the
pam/enable-pam-flows
branch
from
August 7, 2026 11:36
43281c3 to
b937e2f
Compare
BTreston
requested changes
Aug 7, 2026
BTreston
left a comment
BTreston
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me, just a few 🎨 requests to avoid hard coded error strings.
Contributor
There was a problem hiding this comment.
🎨 Can we update the hardcoded error strings to follow the pattern in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateUser/v2/Errors.cs
Contributor
There was a problem hiding this comment.
🎨 Can we update these string too
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40210
📔 Objective
The member-management flows that grant and revoke
AccessPam, the admin-controlled per-member PAM access grant. Both paths are plain field writes behind an org-levelUsePamcheck.Worth flagging for review:
CountNewSmSeatsRequiredQuery, noUpdateSecretsManagerSubscriptionCommand, no autoscale-last ordering rule. The story's earlierAllocatePamSeatsversion is superseded.UsePamguard. SM has no explicit check in these flows; it leans onCountNewSmSeatsRequiredQuerythrowing "Organization does not use Secrets Manager" as a side effect of counting seats. With no seat count to piggyback on, PAM checksUsePamdirectly — otherwiseAccessPam = trueon a non-PAM org is inert but unvalidated, since claim emission ANDs the two (PM-40209), and the admin gets a silently ineffective toggle instead of an error.UsePam, so removing a member's access stays possible on an org whose entitlement has lapsed. Editing a member who already holds access is likewise not a grant.ChangeMemberEmailNoMp, so the toggle behaves identically whichever serves the request. v2 gets anIsEnablingPam()transition helper and a typedPamNotEnabledvalidator error; v1 guards inline with the other validation.PUT organizations/{orgId}/users/enable-pam, mirroringBulkEnableSecretsManagerAsyncminus the seat block. SM's obsoletePATCHalias is not replicated, and like SM's bulk path it logs no events (breakdown review note 5).UpdateOrganizationUsergains anaccessPamparameter; its only production caller is the v2 command.Depends on #8160 (claims plumbing) — review that first.