feat(scim): Manage privileges via SCIM Groups #107709

michelletran-sentry · 2026-02-05T20:36:47Z

The main purpose of this PR is to allow our SaaS instance to manage elevated privileges in a secondary IdP system. It is done by specific team names (i.e. snty-staff and snty-superuser) created on the default org.

When members are added/removed from "staff" or "superuser" teams in the default SaaS organization, automatically grant or revoke the corresponding is_staff/is_superuser flags.

Key changes:

Grant is_staff when adding to "staff" team, is_superuser to "superuser" team
Revoke only the matching privilege when removing from privileged teams
Security-first two-pass architecture: revoke before delete, grant after add
Handle transaction constraints (RPC calls outside transactions)
Add error handling and logging for privilege update failures
Add comprehensive test coverage (8 new tests)

michelletran-sentry · 2026-02-10T02:12:28Z

src/sentry/core/endpoints/scim/teams.py

+        return (
+            settings.SENTRY_MODE == SentryMode.SAAS
+            and team.organization.id == settings.SENTRY_DEFAULT_ORGANIZATION_ID
+            and team.slug in ("snty-staff", "snty-superuser")


@Jeffreyhung Are these team names OK?

can we break superuser down to read and write? or is this just prefix and we can append read write at the end?

Ah good point about superuser read and write! Hmm... So users who are superuser.write would be a subset of users who are superuser.read. How would you like to model this in the Idp? We could either have 2 distinct groups, which permissions read and write separately (i.e. User Y would need to be both in read and write groups), or we can have 2 distinct groups that are mutually exclusive (i.e. User Y who has superuser write, will be given both read and write permissions, but will only be in the write group in Okta). @Jeffreyhung What do you think?

I think this will make things easier to manage

we can have 2 distinct groups that are mutually exclusive (i.e. User Y who has superuser write, will be given both read and write permissions, but will only be in the write group in Okta

src/sentry/core/endpoints/scim/teams.py

cursor · 2026-02-10T17:35:59Z

src/sentry/core/endpoints/scim/teams.py

+                    )
+                    members_to_revoke.append((member, team))
+                except OrganizationMember.DoesNotExist:
+                    pass


REMOVE pre-scan revokes privileges without verifying team membership

High Severity

The REMOVE pre-scan only checks that the OrganizationMember exists in the organization, but never verifies the member is actually in the team before scheduling privilege revocation. In contrast, the REPLACE pre-scan correctly queries OrganizationMemberTeam.objects.filter(team_id=team.id) to only collect actual team members. Additionally, _remove_members_operation itself correctly handles the "not in team" case by catching OrganizationMemberTeam.DoesNotExist and returning — but by then, the privilege has already been irrevocably revoked via the RPC call. This means a SCIM REMOVE operation targeting any org member (even one never in snty-staff or snty-superuser) would incorrectly strip their is_staff or is_superuser flag.

Hmm... this might be OK because it lets us remove the flags from users who might not be added to the team yet.

cursor · 2026-02-10T17:35:59Z

src/sentry/core/endpoints/scim/teams.py

+                        "member_id": member.id,
+                    },
+                )
+                failed_grants.append(member.id)


Grant phase doesn't verify final team membership state

Low Severity

members_to_grant_privileges accumulates members from all ADD and REPLACE operations across the entire PATCH, but the post-transaction grant phase never re-verifies that those members are still in the team after all operations complete. If a later operation (e.g., REPLACE or REMOVE) undoes an earlier ADD within the same PATCH, the member ends up with is_staff or is_superuser granted despite no longer being in the privileged team. The grant phase at line 588 iterates all accumulated members without checking OrganizationMemberTeam existence.

Additional Locations (1)

src/sentry/core/endpoints/scim/teams.py#L532-L567

src/sentry/core/endpoints/scim/teams.py

When members are added/removed from "staff" or "superuser" teams in the default SaaS organization, automatically grant or revoke the corresponding is_staff/is_superuser flags. Key changes: - Grant is_staff when adding to "staff" team, is_superuser to "superuser" team - Revoke only the matching privilege when removing from privileged teams - Security-first two-pass architecture: revoke before delete, grant after add - Handle transaction constraints (RPC calls outside transactions) - Add error handling and logging for privilege update failures - Add comprehensive test coverage (8 new tests)

When a snty-staff or snty-superuser team is deleted, automatically revoke the corresponding privileges from all team members. Key changes: - Revoke is_staff when deleting snty-staff team - Revoke is_superuser when deleting snty-superuser team - Fail with 500 error if privilege revocation fails during deletion - Add 4 new tests for team deletion privilege management Co-Authored-By: Claude <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-10T18:12:57Z

src/sentry/core/endpoints/scim/teams.py

-    def _remove_members_operation(self, request: Request, member_id, team):
+            added_members.append(member)
+
+        return added_members


ADD retry after grant failure silently skips privilege

Medium Severity

If _grant_privilege fails (RPC error) and returns a 500, the team membership transaction has already committed. When the SCIM provider retries the same ADD operation, _add_members_operation sees the member already exists in the team (line 411) and continues, returning an empty added_members list. The grant loop has nothing to process, and the endpoint returns 204 success — but the privilege was never granted. The operation is not idempotent for privileged teams, leaving a member in the team without the corresponding is_staff/is_superuser flag.

Additional Locations (1)

src/sentry/core/endpoints/scim/teams.py#L586-L602

If this happens, then we can just remove then re-add the user. There's going to be inconsistencies here because it's over 2 different silos and there's no real "transaction" we can enforce on this.

Add RPC service methods to manage UserPermission records across silos: - `add_permission(user_id, permission)` - Creates a permission for a user - `remove_permission(user_id, permission)` - Removes a permission from a user These methods enable region silo endpoints to manage control silo UserPermission records through proper RPC calls instead of direct database access. Both methods use proper transaction handling with `router.db_for_write(UserPermission)` and return boolean values indicating whether the operation created/removed a record. Co-Authored-By: Claude <noreply@anthropic.com>

Rename snty-superuser to snty-superuser-read and add new snty-superuser-write team for granular superuser privilege management: - **snty-superuser-read**: Grants `is_superuser` flag only - **snty-superuser-write**: Grants `is_superuser` flag + "superuser.write" permission Changes: - Update `_should_manage_privileges()` to recognize three privileged teams - Update `_grant_privilege()` to use `user_service.add_permission()` RPC for managing UserPermission records across silos - Update `_revoke_privileges()` to use `user_service.remove_permission()` RPC - Fix tests to wrap UserPermission access with `assume_test_silo_mode(SiloMode.CONTROL)` since UserPermission is a control silo model This enables proper cross-silo permission management where region silo SCIM endpoints can manage control silo UserPermission records through RPC instead of direct database access. Tests: 34 passing Co-Authored-By: Claude <noreply@anthropic.com>

sentry · 2026-02-10T21:09:00Z

src/sentry/core/endpoints/scim/teams.py

+                    "scim.privilege_grant_failed",
+                    extra={
+                        "organization_id": team.organization.id,
+                        "team_id": team.id,
+                        "member_id": member.id,
+                    },
+                )
+                return Response(
+                    {"detail": "Failed to grant user privileges"},
+                    status=status.HTTP_500_INTERNAL_SERVER_ERROR,
+                )


Bug: A failure during privilege granting, after team membership is committed, leads to a permanent inconsistent state because idempotent retries will not re-attempt the grant.
_{Severity: HIGH}

Suggested Fix

To ensure atomicity, the privilege granting logic should be moved inside the main database transaction. Alternatively, implement a compensation mechanism, such as rolling back the team membership if the privilege grant fails, or create a background job to reconcile users who are in this inconsistent state.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/sentry/core/endpoints/scim/teams.py#L608-L623 Potential issue: The SCIM team update logic separates team membership changes from privilege granting. Team membership is saved in a database transaction, and then privilege-granting RPC calls are made afterward. If a privilege grant fails, the user is left in an inconsistent state: they are a member of the team but lack the necessary privileges. Because the operation is idempotent, subsequent retries by the SCIM client will detect that the user is already on the team and will not re-attempt the failed privilege grant, making the inconsistent state permanent.

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 5, 2026

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from 5817726 to 53727cd Compare February 6, 2026 01:12

vercel bot deployed to Preview February 6, 2026 01:14 View deployment

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from 53727cd to 94b13e2 Compare February 6, 2026 03:32

vercel bot deployed to Preview February 6, 2026 03:35 View deployment

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from 94b13e2 to ae266ed Compare February 10, 2026 02:05

vercel bot deployed to Preview February 10, 2026 02:08 View deployment

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from ae266ed to f2b5d5a Compare February 10, 2026 02:10

michelletran-sentry commented Feb 10, 2026

View reviewed changes

vercel bot deployed to Preview February 10, 2026 02:13 View deployment

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from f2b5d5a to 60b2f7c Compare February 10, 2026 17:00

vercel bot deployed to Preview February 10, 2026 17:03 View deployment

michelletran-sentry requested a review from a team February 10, 2026 17:27

michelletran-sentry marked this pull request as ready for review February 10, 2026 17:27

sentry bot reviewed Feb 10, 2026

View reviewed changes

src/sentry/core/endpoints/scim/teams.py Show resolved Hide resolved

cursor bot reviewed Feb 10, 2026

View reviewed changes

michelletran-sentry and others added 2 commits February 10, 2026 12:47

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from 60b2f7c to 38fcbb4 Compare February 10, 2026 18:00

vercel bot deployed to Preview February 10, 2026 18:03 View deployment

cursor bot reviewed Feb 10, 2026

View reviewed changes

vercel bot deployed to Preview February 10, 2026 21:00 View deployment

michelletran-sentry force-pushed the feat/scim-groups-privilege-management branch from 92c132d to 19452e9 Compare February 10, 2026 21:05

vercel bot deployed to Preview February 10, 2026 21:07 View deployment

sentry bot reviewed Feb 10, 2026

View reviewed changes

Uh oh!

feat(scim): Manage privileges via SCIM Groups #107709

Are you sure you want to change the base?

feat(scim): Manage privileges via SCIM Groups #107709

Conversation

michelletran-sentry commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michelletran-sentry Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Jeffreyhung Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Jeffreyhung Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michelletran-sentry Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Jeffreyhung Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot Feb 10, 2026

Choose a reason for hiding this comment

REMOVE pre-scan revokes privileges without verifying team membership

Uh oh!

michelletran-sentry Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 10, 2026

Choose a reason for hiding this comment

Grant phase doesn't verify final team membership state

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 10, 2026

Choose a reason for hiding this comment

ADD retry after grant failure silently skips privilege

Uh oh!

michelletran-sentry Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sentry bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

michelletran-sentry commented Feb 5, 2026 •

edited

Loading

Jeffreyhung Feb 10, 2026 •

edited

Loading

michelletran-sentry Feb 10, 2026 •

edited

Loading