[PM-37469] UseRiskInsights organization ability: migration, plan mapping, admin toggle, license, and endpoint guard#7782
Conversation
Adds the plan-level RiskInsights ability flag so org-creation, upgrade, and restart paths can derive Organization.UseRiskInsights from the plan, consistent with every other org ability. [PM-37471]
Maps plan.HasRiskInsights onto the organization in the vNext upgrade command (the actual upgrade ability-mapping site; the ticket referenced CloudOrganizationSignUpCommand, which has no upgrade path). [PM-37471]
Covers the legacy/default upgrade fallback and the Stripe webhook plan migration path, which both flow through Organization.ChangePlan. Found in code review; closes a cross-path gap against the other Use* flags. [PM-37471]
Mirrors CloudOrganizationSignUpCommand so provider-created Enterprise orgs get the ability. Found in code review. [PM-37471]
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the new commits since the last review round: the Access Intelligence endpoint guard re-enablement, the self-host license Code Review DetailsNo new findings. Verified in this pass:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| UsersGetPremium = plan.UsersGetPremium || signup.PremiumAccessAddon, | ||
| UseCustomPermissions = plan.HasCustomPermissions, | ||
| UseScim = plan.HasScim, | ||
| UseRiskInsights = plan.HasRiskInsights, |
There was a problem hiding this comment.
UpgradePremiumToOrganizationCommand
Details and fix
The PR objective is to map plan.HasRiskInsights -> organization.UseRiskInsights at "every live plan-to-organization site." UpgradePremiumToOrganizationCommand.BuildOrganization (src/Core/Billing/Premium/Commands/UpgradePremiumToOrganizationCommand.cs:228-281) is one such site — it constructs a brand-new Organization from a target Plan and maps every other Has* ability the same way the two SignUp commands do (HasPolicies, HasMyItems, HasInviteLinks, HasSso, HasGroups, HasScim, HasCustomPermissions, etc.).
This endpoint is reachable today via POST /account/billing/vnext/upgrade (no feature flag — see AccountBillingVNextController.UpgradePremiumToOrganizationAsync) and UpgradePremiumToOrganizationRequest allows ProductTierType.Enterprise, mapping it to PlanType.EnterpriseAnnually. So a user upgrading a Premium subscription directly to Enterprise will end up with UseRiskInsights = false even though the Enterprise plan has HasRiskInsights = true.
Suggested fix — add to the BuildOrganization initializer around line 265:
UseScim = targetPlan.HasScim,
UseRiskInsights = targetPlan.HasRiskInsights,
Plan = targetPlan.Name,This matches the exact cross-path-gap pattern the PR description calls out for ChangePlan and ProviderClientOrganizationSignUpCommand.
There was a problem hiding this comment.
Good catch, fixed in a5cb547. Added UseRiskInsights = targetPlan.HasRiskInsights right after UseScim in BuildOrganization, mirroring the other SignUp commands, plus a test assertion in UpgradePremiumToOrganizationCommandTests. This was the last cross-path plan-to-organization site.
There was a problem hiding this comment.
Claude finished @AlexRubik's task in 1m 16s —— View job
🤖 Re-review
Overall Assessment: REQUEST CHANGES
The previously-flagged UpgradePremiumToOrganizationCommand gap is correctly closed in commit a5cb547 — UseRiskInsights = targetPlan.HasRiskInsights is mapped at src/Core/Billing/Premium/Commands/UpgradePremiumToOrganizationCommand.cs:266, immediately after the UseScim assignment, with a paired assertion in UpgradePremiumToOrganizationCommandTests. Confirmed all seven plan-to-organization sites in src/ now map UseRiskInsights:
| Site | Line |
|---|---|
CloudOrganizationSignUpCommand |
96 |
ProviderClientOrganizationSignUpCommand |
93 |
UpgradeOrganizationPlanVNextCommand |
178 |
OrganizationPlanExtensions (ChangePlan) |
41 |
RestartSubscriptionCommand |
100 |
UpgradePremiumToOrganizationCommand |
266 |
PlanAdapter |
— populates Plan.HasRiskInsights source-of-truth |
However, the two bitwarden_license/ findings from the previous review are still unaddressed. Both files map every other Use* ability from the plan but not UseRiskInsights:
⚠️ bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs:114—AddExistingOrganizationmapsUseScim,UseKeyConnector, etc. from the managed plan but skipsUseRiskInsights. Reachable viaPOST /providers/{providerId}/clients/existing(no feature flag). Addorganization.UseRiskInsights = plan.HasRiskInsights;after theUseScimassignment. Fix this →⚠️ bitwarden_license/src/Commercial.Core/Billing/Providers/Services/BusinessUnitConverter.cs:81—FinalizeConversionconverts an organization toEnterpriseAnnually/EnterpriseMonthlyand maps every otherUse*ability from the new plan but skipsUseRiskInsights. Reachable via the business-unit-conversion finalization endpoint (no feature flag). Addorganization.UseRiskInsights = updatedPlan.HasRiskInsights;after theUseScimassignment. Fix this →
These are the same class of cross-path gap the PR description and prior reviews called out for ChangePlan, ProviderClientOrganizationSignUpCommand, and UpgradePremiumToOrganizationCommand — the Enterprise plan flows through these two bitwarden_license/ paths (provider-managed orgs and business-unit conversion to Enterprise) without UseRiskInsights being set, leaving the same forward-going gap the PR is intended to close. Grepping bitwarden_license/ for UseRiskInsights returns no results, confirming neither site has been touched.
Tasks
- Verified
UpgradePremiumToOrganizationCommandfix froma5cb547 - Re-checked
ProviderBillingService.AddExistingOrganization— still missing - Re-checked
BusinessUnitConverter.FinalizeConversion— still missing - Scanned for other live plan-to-organization sites — all
src/sites covered - Posted re-review
· Branch
There was a problem hiding this comment.
Both bitwarden_license/ findings are now fixed:
ProviderBillingService.AddExistingOrganization→organization.UseRiskInsights = plan.HasRiskInsights;afterUseScim(commit 007099a)BusinessUnitConverter.FinalizeConversion→organization.UseRiskInsights = updatedPlan.HasRiskInsights;afterUseScim(commit e9c4d55)
Each has a paired test assertion (Commercial.Core.Test, 66 tests pass; build clean). Root cause of why these were missed earlier: prior passes scoped only src/. I re-scoped the whole repo by grepping every non-test plan-derived .Use* = *.Has* assignment across both src/ and bitwarden_license/; all eight plan-to-organization write sites now map UseRiskInsights and no gaps remain.
UpgradePremiumToOrganizationCommand.BuildOrganization (reachable via POST /account/billing/vnext/upgrade, which accepts Enterprise) mapped every other Has* ability but not HasRiskInsights. Found in PR review; closes the last cross-path gap so Premium->Enterprise upgrades get the ability set like every other plan-to-organization site. [PM-37471]
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7782 +/- ##
==========================================
+ Coverage 61.22% 65.71% +4.49%
==========================================
Files 2209 2209
Lines 97716 97736 +20
Branches 8815 8816 +1
==========================================
+ Hits 59824 64227 +4403
+ Misses 35768 31289 -4479
- Partials 2124 2220 +96 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ProviderBillingService.AddExistingOrganization (reachable via
POST /providers/{providerId}/clients/existing) mapped every other Use*
ability from the managed plan but not UseRiskInsights. Found in PR
re-review; closes the gap for provider-managed Enterprise orgs in the
commercial-licensed code path.
[PM-37471]
BusinessUnitConverter.FinalizeConversion converts an org to Enterprise and mapped every other Use* ability from the new plan but not UseRiskInsights. Found in PR re-review. [PM-37471]
sven-bitwarden
left a comment
sven-bitwarden
left a comment
There was a problem hiding this comment.
Looks good if this is all that needs done - does anything need to be added to any Views to support reading this back out? Just comparing what I've seen done for prior org abilities to this one.
cyprain-okeke
left a comment
cyprain-okeke
left a comment
There was a problem hiding this comment.
looks good for billing
Nope, no read-side changes needed. UseRiskInsights is already wired through all the read layers (views, SPs, EF queries, OrganizationAbility, API responses). This was all accounted for when the column was added. @sven-bitwarden |
Mirror Plan.HasRiskInsights into the Admin Portal org edit form: include it in GetPlansHelper's plan projection and check the box in togglePlanFeatures, matching every other Use* ability. Plan.HasRiskInsights is introduced by PM-37471 in this same PR; the checkbox, edit-model property, and controller persistence already exist on main. [PM-37473]
AlexRubik
dismissed stale reviews from cyprain-okeke and sven-bitwarden
via
5c0f164
|
Including this functionality from pm-37473: "When an admin selects an Enterprise plan in the Bitwarden Admin Portal organization form, UseRiskInsights should auto-tick alongside the other Enterprise abilities." Small change commit: 5c0f164 |
AlexRubik
changed the title
Set UseRiskInsights = 1 on existing Organization rows with Enterprise plan types (4, 5, 10, 11, 14, 15, 19, 20). Batched and idempotent. [PM-37470]
Adds equivalent UPDATE migration for MySQL, Postgres, and SQLite providers used by self-hosted instances. Mirrors the MSSQL script. [PM-37470]
Keep UPDATE TOP (N) [target] (and bare UPDATE [target] in the EF variants) as a single header line. SET and WHERE arguments still indent onto their own line. No functional change. [PM-37470]
Match the MSSQL DbScript, which only updates rows where UseRiskInsights is currently 0. Reduces write churn on already-correct rows and keeps cloud (MSSQL) and self-host (EF) UPDATE shape identical. [PM-37470]
OrganizationLicense.VerifyData() signed the UseRiskInsights claim into the license but never compared it against the organization, so self-host could not rely on the license to grant Access Intelligence. Add the comparison using the conditional HasClaim pattern (PM-33980) so pre-existing licenses that lack the claim continue to validate. [PM-37474]
Adds absent/present-matches/present-mismatches tests mirroring the existing UseMyItems coverage, verifying pre-existing licenses (claim absent) still validate and that a present claim is enforced. [PM-37474]
…dpoints (PM-37475)
AlexRubik
dismissed stale reviews from cyprain-okeke and sven-bitwarden
via
faa7129
AlexRubik
changed the title
|
3 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-37469
This PR consolidates all server-side subtasks of PM-37469 into one atomic change:
📔 Objective
The
UseRiskInsightsorganization ability gates Access Intelligence but predates the org-ability pattern, so it was never reliably set on Enterprise orgs. This PR bundles every server-side piece so the migration, the forward-going mapping, the license check, and the re-enabled guard go live in a single atomic deploy, with no window where the guard rejects an org that has not yet been backfilled. The Billing riskInsights pricing key is already live in QA and prod, so the mapping takes effect on deploy.Each subtask below links to its commits; the code is the source of truth.
Data migration (PM-37470)
Backfills
UseRiskInsights = 1for existing Enterprise orgs (plan types 4, 5, 10, 11, 14, 15, 19, 20) via an idempotent MSSQL DbScript plus EF migrations for MySQL/Postgres/SQLite. Commits: 57d1ecc, 45f371c, 6106a03, c543345, f1d9997; re-dated to sort after main in df2c02e.Plan mapping (PM-37471)
Adds
HasRiskInsightsto thePlanmodel (11d26a3) and populates it inPlanAdapterviaHasFeature("riskInsights")(fe367f7), then mapsplan.HasRiskInsights -> organization.UseRiskInsightsat all eight live write sites: org creation 081f76e / provider-client 9a19e0e, upgrade 8e1e943 / ChangePlan 1a75acb / premium-to-org a5cb547, restart 4643003, and the twobitwarden_license/paths 007099a / e9c4d55. Tests: 6c925e4. Inert until Billing registers theriskInsightskey.Admin Portal (PM-37473)
Selecting an Enterprise plan in the org edit form auto-ticks the Risk Insights checkbox (mirrors
UseInviteLinks, sourced from the samePlan.HasRiskInsights). Commit: 5c0f164.Self-host license (PM-37474)
Adds
UseRiskInsightstoOrganizationLicense.VerifyData()using the conditional claim pattern, so pre-existing self-host licenses that lack the claim still validate. Commits: d663eb0, test dcb950e.Endpoint guard (PM-37475)
Re-enables the
orgAbility.UseRiskInsightscheck inOrganizationReportsController.AuthorizeAsync()that was temporarily disabled under PM-37478. One controller serves both V1 and V2 (V2 gated byAccessIntelligenceVersion2), both routing through the same guard; the controller tests now mock the org ability. Commit: faa7129.📸 Screenshots
No UI except PM-37473: in the Admin Portal org edit form, selecting an Enterprise plan auto-ticks the Risk Insights checkbox (and a non-Enterprise plan unticks it); a manual override still persists on save.