Skip to content

Auth/PM-42167 - SSO - Add Staged Org User Support - #8228

Open
JaredSnider-Bitwarden wants to merge 10 commits into
mainfrom
auth/pm-42167/sso-add-staged-org-users-support
Open

Auth/PM-42167 - SSO - Add Staged Org User Support#8228
JaredSnider-Bitwarden wants to merge 10 commits into
mainfrom
auth/pm-42167/sso-add-staged-org-users-support

Conversation

@JaredSnider-Bitwarden

@JaredSnider-Bitwarden JaredSnider-Bitwarden commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-42167
Clients PR: bitwarden/clients#22562

📔 Objective

Adds SSO support for organization users in the Staged status (PM34423StagedStatus flag-gated).

  • New user JIT-provisioning against a Staged row — creates the BW user and completes the join in one flow, mutating the Staged row in place. Seat check runs before user creation so rejection preserves prior state.
  • Existing BW user SSO against a Staged row — promotes Staged → Invited, dispatches the invite email, and returns a dedicated StagedOrgUserInviteAcceptanceRequired SSO redirect error code (client PR renders a "check your email" terminal page).

Also extracts a shared seat-availability check for both paths and adds Staged to the existing Key Connector guard so KC users hitting a Staged row get the KC error instead of falling through to promotion.

📸 Screenshots

See Clients PR: bitwarden/clients#22562

Promotes a Staged OrgUser row to Invited when a brand-new user JIT-provisions
against it via SSO (AutoProvisionUserAsync Scenario 3). Behind the
PM34423StagedStatus feature flag.

Extracts the pre-existing seat-availability check into a private helper and
calls it from both the fresh-JIT site and the new Staged-promotion site.
Staged rows don't count against occupied seats but Invited does, so the
promotion must run the same "enough seats or autoscale" check.

Integration tests cover: the Staged-promotion happy path, promotion blocked
at seat cap (self-hosted throw and cloud autoscale-fails throw), and all six
branches of the seat-availability check as lock-in for the extraction.
…tations

Scenario 3 of AutoProvisionUserAsync mutates the pre-existing OrganizationUser
row (UserId always, Status when the Staged promotion fires). The
OrganizationUser_Update sproc uses whatever RevisionDate the entity carries,
so callers must bump it explicitly — otherwise the row's watermark stays at
its Staged-creation timestamp and watermark-driven consumers miss the change.

Also updates the Staged-promotion integration test to capture the seeded
row's initial RevisionDate and assert the refreshed value has advanced.
…ged OrgUsers

When an existing BW user attempts SSO against a Staged OrganizationUser row
matching their email, promote the row to Invited, send the standard invite
email, and redirect the SSO callback to /login with InviteAcceptanceRequired
so the user completes acceptance via the normal token flow.

Behind the PM34423StagedStatus feature flag. Extracted into a
PromoteStagedOrgUserAndSendInviteAsync helper that verifies seat capacity
(autoscales on cloud when possible), flips status, bumps RevisionDate, logs
OrganizationUser_Invited, and dispatches ISendOrganizationInvitesCommand.
UserId stays null to match the standard admin-invite shape.

Integration tests cover: promote+invite happy path, self-hosted seat cap
throws NoSeatsAvailable (no invite sent, row unmutated), cloud autoscale
fails NoSeatsAvailable (same).

TODOs left inline for product/AC to weigh in on (a) whether
OrganizationUser_Invited is the right event type when the trigger is a user
SSO login rather than an admin action and (b) whether the client redirect
should carry a Staged-specific error code distinct from InviteAcceptanceRequired.
@JaredSnider-Bitwarden JaredSnider-Bitwarden added ai-review Request a Claude code review t:feature Change Type - Feature Development labels Aug 18, 2026
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed at head 1d2960f6, focusing on the new rollback commit on top of the previously-approved 228a10de. PromoteStagedOrgUserAndSendInviteAsync now snapshots Status/RevisionDate before the Staged → Invited flip and restores them via a second ReplaceAsync if SendInvitesAsync throws, logging (exception only, no PII) before the revert so the original failure is preserved even if the compensating write fails, then rethrowing. I traced the compensating path against SendOrganizationInvitesCommand — a Staged row reachable by the email/user lookup always resolves an email, so the silent ValidateInvitedUsersAsync skip is unreachable here, and a partially-delivered invite is self-healing because the next SSO attempt re-runs the promotion. The accompanying integration test asserts the row is back to Staged with a null UserId after a simulated send failure; the two previously-raised findings remain fixed and no new findings surfaced.

Code Review Details

No findings at or above the reporting threshold.

Considered and dropped: the seat cap grown by AutoAddSeatsAsync inside EnsureSeatAvailableAsync is not reverted when the invite send later fails, but this matches the pre-existing behavior of the JIT-provisioning path in the same method (autoscale is likewise not unwound if user registration fails), and the reverted status frees the occupied seat so a retry does not autoscale again.

Comment thread bitwarden_license/src/Sso/Controllers/AccountController.cs Outdated
Comment thread bitwarden_license/src/Sso/Controllers/AccountController.cs Outdated
Comment thread bitwarden_license/src/Sso/Controllers/AccountController.cs Dismissed
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.95833% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.33%. Comparing base (ac309aa) to head (1d2960f).

Files with missing lines Patch % Lines
...n_license/src/Sso/Controllers/AccountController.cs 98.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8228      +/-   ##
==========================================
+ Coverage   63.29%   63.33%   +0.03%     
==========================================
  Files        2401     2402       +1     
  Lines      104043   104113      +70     
  Branches     9426     9431       +5     
==========================================
+ Hits        65857    65938      +81     
+ Misses      35930    35921       -9     
+ Partials     2256     2254       -2     

☔ 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.

…motion

Introduces SsoAuthnStagedOrgUserRequiresInviteAcceptanceException + a
StagedOrgUserInviteAcceptanceRequired redirect error code so the web client
can prompt the user to check their email for the freshly-sent invite,
rather than reusing ssoOrgInviteAcceptanceRequired which implies the user
should already have an invite in hand.
@JaredSnider-Bitwarden JaredSnider-Bitwarden changed the title Auth/pm 42167/sso add staged org users support Auth/PM-42167 - SSO - Add Staged Org User Support Aug 19, 2026
Product confirmed no user event should be emitted when an SSO login
promotes a Staged OrganizationUser to Invited; drop the
OrganizationUser_Invited event log call and its pending TODO.
Comment thread bitwarden_license/src/Sso/Controllers/AccountController.cs Outdated
Comment thread bitwarden_license/src/Sso/Utilities/SsoRedirectUrlBuilder.cs
…ment

Drops the stale OrganizationUser_Invited event log claim (removed in
3575878) and rewrites the summary to describe only what this method
mutates, not the internals of EnsureSeatAvailableAsync,
ISendOrganizationInvitesCommand, or the accept endpoint.
…d promotions

When JIT-provisioning a BW User against a Staged OrganizationUser row and
the org is at seat cap without autoscale headroom, the seat check previously
ran after RegisterSSOAutoProvisionedUserAsync — leaving a password-less BW
User row plus a welcome email for a login that never completed.

Move the seat check to the pre-user-creation gate so a rejection preserves
prior state. Adds regression coverage: at-cap-autoscale-succeeds branch,
feature-flag-off boundary at seat cap, and a two-phase test verifying
rejection preserves state and retry after cap increase provisions cleanly.
…at guard

Adds OrganizationUserStatusType.Staged to the existing Key Connector guard in
AccountController.ExternalCallback so a Key Connector user who hits a Staged
OrganizationUser row in the SSO-target org is cleanly rejected before the
Staged-promotion branch runs. Previously the Staged case bypassed the guard
and fell through to PromoteStagedOrgUserAndSendInviteAsync, consuming a seat
and mailing an invite the KC user could not accept.

Adds integration test ExternalCallback_WithExistingKeyConnectorUser_AndStagedOrgUser_ReturnsError
alongside the two existing KC guard tests, asserting the KC error message,
that no invite email is sent, and that the Staged row is not mutated.
@JaredSnider-Bitwarden
JaredSnider-Bitwarden marked this pull request as ready for review August 20, 2026 20:06
@JaredSnider-Bitwarden
JaredSnider-Bitwarden requested a review from a team as a code owner August 20, 2026 20:06
@JaredSnider-Bitwarden
JaredSnider-Bitwarden requested review from enmande and ike-kottlowski and removed request for enmande August 20, 2026 20:06
…nd failure

If SendInvitesAsync throws after the Staged row has been flipped to
Invited, revert the row so a seat isn't consumed for an invite the
user never received and the next SSO attempt cleanly re-runs the
promotion instead of dead-ending on the "accept your invite" redirect.

@ike-kottlowski ike-kottlowski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of band finding of the email failure seat consumption issue has been addressed.

Transition of User from staged to invited provided auto scale is enabled is accomplished. All other flows for JIT registration can complete with the promotion of the user to invited.

Everything is well tested, and code readability improvements are sound.

@bre-deploy
bre-deploy Bot deployed to EU-QA Cloud August 21, 2026 15:31 Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants