Skip to content

[PM-44796] feat: Require scoped policies on the Admin Console public API - #8564

Draft
withinfocus wants to merge 5 commits into
arch/pm-44795/org-api-scopesfrom
arch/pm-44796/admin-console-public-scopes
Draft

withinfocus wants to merge 5 commits into
arch/pm-44795/org-api-scopesfrom
arch/pm-44796/admin-console-public-scopes

Conversation

@withinfocus

@withinfocus withinfocus commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-44796, part of epic PM-28993. Third of nine stacked PRs.

📔 Objective

Moves the public Members, Groups, Collections, Policies and import endpoints to scoped policies, and limits scoped keys to members with the User role.

  • Reads take the read scope and writes the write scope. PUT public/policies/{type} stays on the legacy policy. Import needs both members write and groups write.
  • The threat model found that the public API acts as a system user, and system users skip role checks, so a members write key could have made anyone an Owner. With a scoped key, Core now rejects inviting or changing anyone to a role other than User and acting on Owner, Admin or Custom members. That covers update, invite, revoke, remove, restore and reinvite.
  • A scoped key also can't change the group membership of Owner, Admin or Custom members. PUT public/members/{id}/group-ids, group create and update, PUT public/groups/{id}/member-ids and deleting a group that contains one are rejected. A group can keep elevated members it already has while Users are added or removed. Group member-ids and delete write straight to the repository today, so the controller calls the Core validator before writing; routing them through the group commands would add events and checks the legacy key doesn't have.
  • Import with a scoped key skips members who aren't User when removing, linking or syncing groups, the same way it already skips Owners.
  • ICurrentContext.IsScopedOrganizationApiKey tells Core a scoped key is acting. It's true only for the scoped client ID shape, organization.{organizationId}.{keyId}, so legacy keys and the SCIM principal (organization.{organizationId} with api.scim) are unaffected. The commands read it directly, so enforcement doesn't depend on each controller passing the right actor.
  • The SCIM integration test factory now builds the same principal as the real SCIM authentication handler. Before, it built a different one, so no SCIM test could have caught a change like this. New tests deprovision an Admin through SCIM.
  • The legacy key behaves as before.

Replace the class-level Organization policy on the public Members,
Groups, Collections, Policies and Organization import controllers with
per-action policies: reads require the resource's read policy and
writes its write policy, each of which also accepts the legacy
api.organization scope. PUT public/policies/{type} keeps the legacy
Organization policy because there is no policies.write scope, and
POST public/organization/import requires both members.write and
groups.write.

Add tests that evaluate each action's combined authorization metadata
in the running Api host against legacy and scoped principals.
Public API commands run as a system user, and system users skip the
organization role hierarchy, so a members.write scope could otherwise
promote anyone to Owner or act on Owners, Admins and Custom members.

Add ICurrentContext.IsScopedOrganizationApiKey, true for an organization
client token without the legacy api.organization scope, and enforce in
Core that a scoped key only acts on members with the User role:
- role change validation (V2 update) rejects elevated current or new roles
- invite rejects any invite that isn't for the User role
- revoke, remove, restore and reinvite reject non-User targets
- import does not remove, or link an external ID to, non-User members

Legacy api.organization tokens keep their current behavior.
…affected

IsScopedOrganizationApiKey was true for any organization.* client
without the api.organization scope. The SCIM host builds exactly that
principal (organization.{orgId}, scope api.scim), so SCIM delete, PUT
deactivate and restore were rejected for Admin, Owner and Custom
members by the User-only rule.

Detect a scoped key by the client id Identity issues for it,
organization.{organizationId}.{keyId} with both parts GUIDs. Legacy
keys and the SCIM principal are not scoped whatever their scopes.

The SCIM integration test auth handler now emits the same claims as
ApiKeyAuthenticationHandler, and new tests cover deleting and
deactivating an Admin member.
@withinfocus
withinfocus added this pull request to stack #8571 October 9, 2026 21:23
@withinfocus withinfocus changed the title arch/pm 44796/admin console public scopes [PM-44796] feat: Require scoped policies on the Admin Console public API Oct 9, 2026
@withinfocus withinfocus added ai-review Request a Claude code review t:feature Change Type - Feature Development labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR replaces the class-level Organization policy on the public Members, Groups, Collections, Policies and import endpoints with per-action read and write scoped policies. It also uses ICurrentContext.IsScopedOrganizationApiKey to restrict scoped keys to members with the User role. I checked that every write path enforces that restriction in Core: invite, the role check in UpdateOrganizationUserValidator, revoke, remove, restore, reinvite and member group updates. ScopedApiKeyGroupMemberValidator is applied to group create, update, PUT member-ids and delete. The latest commit switches the import removal filter to TryGetValue. Scoped-key import still leaves elevated members alone when removing, overwriting, linking by email and syncing groups. No findings met the bar for an inline comment.

A scoped organization API key works at the User access level, but it
could still change which groups Owner, Admin and Custom members belong
to, which changes their collection access and their membership.

Extend the User-only rule to group membership for scoped keys:
- PUT public/members/{id}/group-ids rejects members who aren't User
  (UpdateOrganizationUserGroupsCommand)
- group create and update with a member list, PUT
  public/groups/{id}/member-ids and DELETE public/groups/{id} reject any
  change that would add or remove a non-User member; elevated members
  already in the group can stay (new ScopedApiKeyGroupMemberValidator)
- import group sync leaves non-User members' group memberships as they
  are instead of adding or removing them

The public member-ids and delete endpoints write through the group
repository directly, so they call the Core validator before writing.
Legacy api.organization keys and user tokens keep their behavior.
@codecov

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.10714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.96%. Comparing base (3bcd7e4) to head (9a458b6).

Files with missing lines Patch % Lines
...dminConsole/Public/Controllers/GroupsController.cs 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##           arch/pm-44795/org-api-scopes    #8564      +/-   ##
================================================================
+ Coverage                         66.86%   66.96%   +0.09%     
================================================================
  Files                              2591     2593       +2     
  Lines                            111158   111258     +100     
  Branches                          10123    10144      +21     
================================================================
+ Hits                              74328    74505     +177     
+ Misses                            34350    34271      -79     
- Partials                           2480     2482       +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.

This branch has not been deployed

No deployments
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.

1 participant