Skip to content

[PM-40523] feat: Add endpoint to return organization private key to admins#8006

Merged
r-tome merged 5 commits into
mainfrom
ac/pm-40523-private-key-endpoint
Jul 22, 2026
Merged

[PM-40523] feat: Add endpoint to return organization private key to admins#8006
r-tome merged 5 commits into
mainfrom
ac/pm-40523-private-key-endpoint

Conversation

@r-tome

@r-tome r-tome commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-40523

📔 Objective

Adds GET /organizations/{orgId}/private-key, letting admin-level users (owners, admins, providers, and the "Manage Users" custom permission) fetch an organization's encrypted private key.

This supports KM's fix for the account-recovery-enrollment vulnerability (PM-40307): when an admin generates an invite link, the client needs to derive the org's public-key fingerprint from a source that can't be silently swapped by a malicious server. Fetching the encrypted (tamper-evident) private key and deriving the public key/fingerprint from it client-side achieves that, instead of trusting a bare public key fetched directly from the server.

Access is gated by [Authorize<ManageUsersRequirement>], matching the ticket's explicit authorization spec. This deliberately widens access for the "Manage Users" custom permission (previously only "Manage account recovery" could read the org private key) — a known, intentional expansion discussed in PM-40523. Also gated behind the GenerateInviteLink feature flag, matching the rest of this epic's admin-side invite-link endpoints.

…dmins

Adds GET /organizations/{orgId}/private-key so admin-level users
(owners, admins, providers, and the "Manage Users" custom permission)
can fetch an organization's encrypted private key.

Supports KM's fix for the account-recovery-enrollment vulnerability
(PM-40307): admins generating an invite link need to derive the org's
public-key fingerprint from a source that can't be silently swapped by
a malicious server. Fetching the encrypted, tamper-evident private key
and deriving the public key/fingerprint from it client-side achieves
that, instead of trusting a bare public key fetched from the server.

Gated by ManageUsersRequirement per the ticket's authorization spec
(deliberately widens "Manage Users" access to the org private key,
pending AppSec sign-off) and by the GenerateInviteLink feature flag,
matching the rest of this epic's admin-side invite-link endpoints.
@r-tome r-tome added the ai-review Request a Claude code review label Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new GET /organizations/{orgId}/private-key endpoint, its OrganizationPrivateKeyResponseModel, and the accompanying controller tests. The endpoint returns the organization's asymmetric private key encrypted with the org symmetric key (Protected Data), which preserves the zero-knowledge invariant — the server never sees plaintext key material. Access is correctly gated by both the GenerateInviteLink feature flag and [Authorize<ManageUsersRequirement>], and the {orgId} route parameter name is exactly what the requirement handler resolves via GetOrganizationId(). The intentional widening to the "Manage Users" custom permission is documented in the PR and tracked in PM-40523.

Code Review Details

No blocking findings. The change mirrors the existing sibling GetPublicKey endpoint and reuses the same encrypted Organization.PrivateKey field already exposed by OrganizationKeysResponseModel. The controller null-checks the organization and throws NotFoundException, and the response model is null-safe with a defensive ArgumentNullException guard. Tests cover both the found and not-found paths; authorization is enforced declaratively and validated by the shared requirement-handler tests.

@r-tome
r-tome requested a review from a team July 17, 2026 13:53
@r-tome r-tome added the t:feature Change Type - Feature Development label Jul 17, 2026
@r-tome
r-tome removed the request for review from a team July 17, 2026 14:46
JimmyVo16
JimmyVo16 previously approved these changes Jul 17, 2026
r-tome and others added 2 commits July 21, 2026 14:40
Removes the leftover FIXME/#nullable disable that was copy-pasted from
an older sibling model. This file is new on this branch, so it should
be nullable-clean from the start rather than carrying forward debt.
@r-tome
r-tome marked this pull request as ready for review July 22, 2026 13:11
@r-tome
r-tome requested a review from a team as a code owner July 22, 2026 13:11
@r-tome
r-tome requested review from JaredScar and JimmyVo16 July 22, 2026 13:11
@r-tome
r-tome merged commit a5829c5 into main Jul 22, 2026
34 checks passed
@r-tome
r-tome deleted the ac/pm-40523-private-key-endpoint branch July 22, 2026 17:16
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.47%. Comparing base (6f0b248) to head (3d397a5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ganizations/OrganizationPrivateKeyResponseModel.cs 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8006   +/-   ##
=======================================
  Coverage   62.46%   62.47%           
=======================================
  Files        2297     2298    +1     
  Lines       99981    99996   +15     
  Branches     9020     9022    +2     
=======================================
+ Hits        62456    62468   +12     
- Misses      35346    35348    +2     
- Partials     2179     2180    +1     

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

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