Repository navigation
[PM-46145] feat: Keep Privileged Controls sales-assisted while PAM billing support is off - #8820
cyprain-okeke wants to merge 24 commits into
Conversation
…-up without the flag
…the flag and point owners to sales
…Privileged Controls on sign-up
…d drop a duplicated flag-off test
Organizations given Privileged Access Management before seats existed are stored with UsePam on and no seat count. Now that the Admin Portal validates the section regardless of the flag, every save of such an organization (even a rename) was rejected with "seats must be at least 1". When the stored organization already had UsePam on with no seats and the save doesn't supply a seat count, skip the seats-required rule and keep the seat count unset. The other rules still apply, and a posted seat count is validated normally.
…n fails With pm-39883-pam-billing-support off, a sign-up on the sales-assisted initiation path that asks for Privileged Controls now fails with a BadRequest when the sales invitation token is missing, blank, unreadable, expired or issued to another email, instead of silently creating the organization without Privileged Controls. The token error messages come from the existing SalesAssistedRegistrationTokenable validator; neither the token nor the email is logged. Self-serve sign-ups with the flag off keep ignoring the request (the clients hide those entry points). With the flag on, Privileged Controls provisions whatever the token state.
…n saved in the Admin Portal The organization form no longer renders the Privileged Access Management section for provider-managed organizations, so the posted model always bound UsePam = false and any save cleared UsePam, PamSeats and MaxAutoscalePamSeats. The Edit POST now looks up the provider once and, for provider-managed organizations, keeps the stored PAM values instead of the posted ones and skips the PAM validation (nothing in that section can change). Edit_PrivilegedControls_ProviderManaged_Rejects is removed: a provider-managed organization's posted UsePam is now ignored rather than rejected, so the validator's provider check became unreachable and was dropped with it. The test is replaced by Edit_PrivilegedControls_ProviderManaged_PostedPamIgnored, which pins that such an organization still can't gain PAM from the form, plus Edit_PrivilegedControls_ProviderManaged_KeepsStoredValues for the round trip. ValidatePrivilegedControlsAsync becomes the synchronous ValidatePrivilegedControls since it no longer queries the provider.
…on the Admin edit page A failed Stripe call while loading the Privileged Controls seat line was logged and rendered exactly like "no mismatch". The edit page now shows a warning that the check could not be loaded, mirroring the billing-info load in the same action. The log still carries only the organization id. A warning already set by the billing-info load is kept rather than replaced.
… describe the gate accurately Tests: - Admin OrganizationsControllerTests: keep one explicit flag-off test per verb (GET loads seats, POST saves seats); drop the flag-off validation test, which duplicated Edit_PrivilegedControls_PlanDoesNotSupport_Rejects. - CreateOrganizationSubscriptionCommandTests: pin the literal sales-assisted initiation path to the "sales-assisted" trialInitiationPath Stripe metadata value, as OrganizationBillingServiceTests does for Finalize. - CloudOrganizationSignUpCommandTests: replace the repeated 11-seat comments with a PamSeatsAbovePasswordManagerSeats constant and one helper comment. Comments: - Document PM39883_Pam_Billing_Support as the self-serve gate. - OrganizationService guardrails: state why they aren't flag-gated and that the self-serve endpoints return 404 with the flag off. - CloudOrganizationSignUpCommand: drop the stale purchase-branch parenthetical. - OrganizationNoPaymentCreateRequest.SalesAssistedToken: describe the sales-path-only use and the rejection of a missing or invalid token. - Subscriptions.Organization README: describe this library's own use of TrialInitiationConstants. - TrialInitiationConstants: name the clients enum file it must match. - Remove the unused Bit.Core using from the sales-assisted trial view.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Admin Portal organization edit changes, where Privileged Access Management now saves regardless of the flag, the autoscale limit is always stored as null, the seat count is kept on untick, and provider-managed organizations are forced off. Also reviewed the new server check that rejects a seat count below the occupied count, and the hard Privileged Controls seat limit in |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8820 +/- ##
==========================================
- Coverage 74.83% 68.50% -6.34%
==========================================
Files 2750 2750
Lines 117618 117661 +43
Branches 10781 10787 +6
==========================================
- Hits 88022 80601 -7421
- Misses 26940 34519 +7579
+ Partials 2656 2541 -115 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…edit page and tighten the flag summary
…naged organizations on Admin Portal save
… sales invitations while the flag is off
amorask-bitwarden
left a comment
amorask-bitwarden
left a comment
There was a problem hiding this comment.
We tightened the GA scope after this went up, so some of this needs to change direction. For GA:
- The Admin Portal org edit page is the only way an org gets PAM. Staff set the checkbox and a seat count, and that's it.
- Every PAM org is a sales-initiated trial as far as the platform is concerned. Finance bills PAM outside the platform, so we don't read or write any Stripe PAM line with the flag off.
- Self-serve stays off behind the flag, as it is on
main.
The inline comments cover the specific changes. The main ones are removing the sales invitation path, hiding the autoscale limit, keeping the Password Manager seat checks behind the flag, and adding a server check that PAM seats aren't below the number in use.
One thing that can't go inline because the files aren't in this diff: PAM seats need to be a hard limit. In cloud, giving a member PAM access (single or bulk) should fail with a 400 and a "contact sales for more seats" message when it would go over PamSeats, the same way self-hosted already does (here). Restoring a revoked member who has PAM access should fail the same way, in cloud and self-hosted. This applies whether the flag is on or off; autoscale can replace it later when self-serve comes back.
…e flag is off With the flag off, the Admin Portal organization edit page is the only way an organization gets Privileged Access Management. Restore main's sign-up behaviour so sign-up ignores Privileged Controls, drop the sales invitation token check and its request field, return the initiation path constant to the billing services, and keep Privileged Controls behind the flag on the sales invitation form.
…he flag Members with Privileged Access Management already take up Password Manager seats, so the seat and autoscale-limit guardrails only matter for self-serve. Restore main's flag check on both so owners aren't blocked by a rule they can't resolve without contacting sales.
…eview - Only look up the Stripe Privileged Controls line with the flag on, and drop the failed-lookup warning; the occupied seat count still loads for the form. - Require a seat count whenever Privileged Access Management is on, removing the exemption for organizations saved before seats existed. - Reject a seat count below the number of members who already have access. - Keep the seat count and autoscale limit when staff turn it off, so they're there if it's turned back on; members keep their access. Provider-managed organizations still clear both. - Remove the autoscale limit from the form and always save it as empty, since autoscaling is self-serve; the two limit rules it fed are gone with it. - The form script no longer clears or overwrites a saved seat count when the checkbox is toggled.
…nting or restoring access Finance bills Privileged Controls outside the platform, so cloud no longer lets a grant go over the organization's seats. Granting access to one member or in bulk now fails with a 400 asking the organization to contact sales, matching how self-hosted already refused. Restoring a revoked member who has access is limited the same way in cloud and self-hosted; a bulk restore reports the error against each member with access and restores the rest. A self-hosted license with fewer seats than members who already have access is still accepted, so existing grants stay and only new ones are blocked.
…le limit as empty
|
All applied, see the inline replies. The hard seat limit (grants and restores, cloud and self-hosted, flag on or off) uses the existing seat-count query so Kyle's #8812 can swap in his command on rebase. Those commands are Admin Console-owned, so I've requested an AC reviewer. Clients #24476 mirrors this. |
|
There are no auth owned files in the change set. Removing Auth as reviewers. |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-46145
📔 Objective
Privileged Controls goes GA sales-assisted only. The existing
pm-39883-pam-billing-supportflag is the self-serve switch and stays off at GA. Sales-assisted means staff turning Privileged Access Management on from the Admin Portal organization edit page; the platform treats every such organization as a sales-initiated trial, and Finance bills it outside the platform. The scope follows the rewritten PM-46145 acceptance criteria and the review on this PR.With the flag off:
main: with the flag off it ignores a Privileged Controls request on every path, sales invitation included, and the sales-assisted trial invitation form does not offer it.main.Regardless of the flag:
UpdateOrganizationUserCommand,BulkEnablePamCommand,RestoreOrganizationUserCommand) and overlap Kyle's [PM-45107] feat: Autoscale Privileged Controls seats when members get access #8812, which replaces the seat-count query in the same three places; whichever lands second rebases.Companion
bitwarden/clientsPR: bitwarden/clients#24476. It hides the self-serve subscription sections behind the flag, shows Contact sales blocks with seats in use out of total, and leaves Privileged Controls out of every trial sign-up while the flag is off.QA: recorded end-to-end runs against local cloud + Stripe test mode with the flag off and on (Admin Portal including a legacy and a provider-managed organization, sales invitation, self-serve trial link, subscription page, gated endpoints, hard seat limit on grants and restores). Evidence in the ticket's QA notes.
🤖 AI-assisted review
Multi-agent local review before opening, a second pass on the open PR, and a third pass modelled on the Billing lead's review history. The lead's own review (request changes, 2026-11-06) then tightened the GA scope; every inline item and the body ask are applied in the commits from f36b6f7e9f onward. Earlier review findings about the sales invitation token, the Stripe-check warning and the legacy-organization exemption no longer apply because those paths were removed.
Deferred, recorded for the reviewer:
MaxAutoscalePamSeatson the edit model although it is never rendered or stored; left to keep churn down.