Repository navigation
feat(scim): Add SCIM 2.0 discovery endpoints - #119884
michelletran-sentry merged 8 commits into
Conversation
|
Clean and minimal — appreciate that it stays scoped to the actual fix. |
Validation evidence (no IGA tenant required)The discovery endpoints were validated with three independent layers against a local devserver with SCIM enabled through the production path ( Layer 1 — Live HTTP matrix (23/23 pass)curl-based checks against the running devserver covering: exact RFC 7643 §5 ServiceProviderConfig shape (no Layer 2 —
|
The /Schemas index returned the raw stored schema dicts, whose hardcoded relative meta.location (/v2/Schemas/...) does not resolve and which lacked the schemas attribute the new details endpoint adds — so an entry listed at the index disagreed with the resource at its own location. Both handlers now share build_schema_representation(), the stale meta blocks are dropped from the stored dicts, and a test locks index entries to be byte-identical to their details responses. Flagged by Cursor Bugbot on getsentry#119884.
6416edf to
fee0ce2
Compare
The /Schemas index returned the raw stored schema dicts, whose hardcoded relative meta.location (/v2/Schemas/...) does not resolve and which lacked the schemas attribute the new details endpoint adds — so an entry listed at the index disagreed with the resource at its own location. Both handlers now share build_schema_representation(), the stale meta blocks are dropped from the stored dicts, and a test locks index entries to be byte-identical to their details responses. Flagged by Cursor Bugbot on getsentry#119884.
Testing summary — end to end in real environments, plus every guideline tierEverything below is against the current head ( Working end to end, in a real environmentReal environment. A live devserver with SCIM enabled the production way (real Against a real IdP client. Microsoft's Entra ID SCIM Validator — a production Microsoft service making requests from Microsoft infrastructure — run against the getsentry-layer environment above. It discovered the schema through this PR's endpoints and populated its attribute model from them:
Provisioning test results — 16/21 passed, and the banner's "not compatible" verdict refers entirely to the 5 failures in the pre-existing
The discovery goal — the thing this PR adds — is the part Microsoft's validator confirms: schema discovery works against a running Sentry over the public internet. (Two Entra-stricter-than-RFC behaviors were found via this harness during development, fixed, and locked by tests.) Independent RFC harness. Full scim2-tester transcriptThe harness is reproducibleNo bespoke rig — standard develop.sentry.dev tooling plus two public validators. Any reviewer can rerun the whole matrix in ~15 minutes: devservices up && sentry devserver --ngrok=<your-tunnel-domain>
# enable SCIM + mint a token on a dev org (script in the validation comment)
BASE=https://<your-tunnel-domain>/api/0/organizations/<org>/scim/v2
curl -H "Authorization: Bearer $TOKEN" "$BASE/ServiceProviderConfig"
uv tool install scim2-cli && scim2 --url "$BASE" --header "Authorization: Bearer $TOKEN" testI'm also happy to demo this live against a running instance for whoever picks up the review — the environment is two commands to bring up. Standard suites — all green on this head
Why 4 checks show red (fork-PR artifacts, not failures)
|
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 472b83e. Configure here.
michelletran-sentry
left a comment
There was a problem hiding this comment.
Generally LGTM
Implement the RFC 7644 section 4 discovery endpoints under the per-org SCIM v2 prefix: GET
/ServiceProviderConfig (single resource describing supported features), GET /ResourceTypes and
GET /ResourceTypes/{name} (User and Group), and GET /Schemas/{schemaUri} (per-URI lookup with a
SCIM-format 404 for unknown URIs). IGA platforms run SCIM compliance pre-flights that require
these endpoints and currently mark Sentry as non-compliant when they 404.
All discovery endpoints require the SCIM bearer token like the rest of the SCIM API, are
GET-only (SCIM-format 405 bodies otherwise), reject filter params with 403, and ignore
pagination params per RFC 7644 section 4. They are published to the API docs.
Also normalizes the invalid 'mutability: read' values in the stored schema definitions to
'readOnly' (the RFC 7643 keyword) and aligns the private /Schemas index with the discovery
behavior (filter now 403s, startIndex is no longer echoed). Okta and Entra ID call neither
path with parameters, so existing provisioning is unaffected; members and teams endpoints are
untouched.
Fixes getsentry#114950
Validated the discovery endpoints against the Entra ID SCIM Validator and scim2-tester. Two schema fixes fell out: - The User schema's complex name attribute omitted mutability entirely; RFC 7643 defaults it when absent, but Entra's schema parser rejects attributes without an explicit value. - userName and emails were declared readOnly (a mechanical normalization of the previously invalid "read" keyword); Entra excludes non-writable attributes from provisioning payloads entirely and cannot create users without sending userName, so they are now readWrite per the RFC 7643 core schema. emails.display stays readOnly (service-generated). With these fixes the Entra validator discovers the schema cleanly and runs its full provisioning suite from the discovered attributes.
The /Schemas index returned the raw stored schema dicts, whose hardcoded relative meta.location (/v2/Schemas/...) does not resolve and which lacked the schemas attribute the new details endpoint adds — so an entry listed at the index disagreed with the resource at its own location. Both handlers now share build_schema_representation(), the stale meta blocks are dropped from the stored dicts, and a test locks index entries to be byte-identical to their details responses. Flagged by Cursor Bugbot on getsentry#119884.
Review feedback: the discovery schema advertised readWrite for attributes Sentry never consumes. userName is now immutable (consumed only at create as the member email; renames are unsupported), and emails/name plus their sub-attributes are readOnly (derived server-side, values sent by clients are ignored). active stays readWrite since PATCH active=false is the deprovisioning path. immutable rather than readOnly for userName because Entra excludes readOnly attributes from provisioning payloads entirely; verified against the Entra SCIM Validator that immutable attributes are still sent on create (17/21, one better than the previous 16/21 baseline) and scim2-tester discovery remains 11/11.
09a6e4f to
14b5afc
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Pull request was closed
The agent-token matrix (getsentry#121840) enumerates every PUBLIC GET endpoint and checks each login method against it. The four SCIM discovery endpoints use OrganizationSCIMMemberPermission, so like the existing SCIM routes they reject non-SCIM credentials with 403 at the permission boundary. List them in OUTER_BOUNDARY_GET_STATUSES and give the matrix real path values for resource_type_name and schema_uri: the User resource type and the core User schema URI, as in the RFC 7644 section 4 examples. Refs getsentry#114950
The Group schema declared members sub-attributes value and $ref, both returned by default, but /Groups returns value and display. So $ref was never returned despite returned:default, and display was never declared (RFC 7643 sections 2.4 and 7). Replace $ref with display. It is readOnly: Sentry derives it from the member's email and ignores client-sent values, which RFC 7644 sections 3.3 and 3.5.1 define as readOnly handling. RFC 7643 is inconsistent about display mutability (erratum 8358), so the schema declares what Sentry actually does, as it already does for the User schema. A new test checks the declared members sub-attributes against a real /Groups response. Refs getsentry#114950
Sentry Snapshot Testing
|
tests/AGENTS.md requires fixtures over Model.objects.create. The create_auth_provider fixture already runs in the control silo, so the explicit assume_test_silo_mode wrapper and its imports go too. Refs getsentry#114950
|
@michelletran-sentry, could you take another look? Your July approval is still on the PR, but product code has changed since then, and auto-merge was turned off when the stale bot closed the PR. Here's what changed after 1. Master merged in ( 2. Agent-token auth matrix (
3. Group
4. Test fixture cleanup ( Validation on the current head. I ran it against a live devserver, with SCIM enabled through the real
CI is green apart from the three jobs that always fail on fork PRs: the api-url-typescript auto-commit, Vercel, and openapi-diff. The API publishing guide says openapi-diff is expected to fail and isn't required to merge; here it shows only the four new paths being added. Seer flagged the leading slash in the ResourceType I have two
If it still looks good, could you approve the current head and turn auto-merge back on? |








Completes the three SCIM 2.0 discovery endpoints defined in RFC 7644 §4 under the per-organization SCIM prefix — read-only GETs returning static, protocol-defined documents — plus RFC-compliance fixes to the responses of the existing private
/Schemasindex (own section below). The new endpoints reuse the existing SCIM auth gate: no new token types, scopes, or permission classes, and nothing tenant-specific in the responses except the org slug insidemeta.locationURLs.Identity Governance & Administration (IGA) platforms (SailPoint, Saviynt, One Identity, among others) run compliance pre-flights that probe these endpoints and refuse to integrate when they fail, marking Sentry "not SCIM 2.0 compliant". Before this PR, two of the three endpoints 404'd; the third (
/Schemas) existed as a private index whose content failed the same validators.SCIM background (if you haven't worked with it)
/Users,/Groups, and three discovery endpoints that let a client ask the server what it supports./ServiceProviderConfig("which protocol features do you support?"),/ResourceTypes("which resource kinds exist here?" — each entry binds a URL path like/Usersto a schema), and/Schemas("what attributes does each resource have?"). Their content is derived from the implementation, not from tenant data.urn:ietf:params:scim:schemas:core:2.0:Useris a well-known identifier defined by the RFC, not something Sentry invents.How SCIM auth works at Sentry, and what this PR does with it
AuthProvider(AuthProvider.enable_scim()) auto-creates an internal integration ("SCIM Internal Integration") whose installation API token is the credential IdPs send asAuthorization: Bearer …. The token is valid only for that org and carriesmember:read|write|admin+team:write|adminscopes. This PR does not touch token minting.SCIMEndpoint(OrganizationEndpoint). Authorization isOrganizationSCIMPermission(OrganizationPermission): the standard org permission check runs first, then it additionally requires the org's auth provider to haveflags.scim_enabled. The new endpoints useOrganizationSCIMMemberPermission(GET → member:read|write|admin), the same class that already guards/Usersand the existing/Schemasindex.filterparam with a SCIM-format 403 (an RFC 7644 §4 SHOULD; rationale under Deviations below).reject_filter_paramis called at the top of each GET handler, i.e. after DRF's permission checks (it can't live ininitial(), which runs before the org is resolved). A caller who fails authorization therefore gets the normal authorization failure and can't use the filter rejection as an oracle. Each endpoint's filter-403 has its own test; the ordering property is asserted bytest_filter_param_does_not_bypass_permission_check.tests/sentry/seer/endpoints/test_organization_agent_token.pyexercises every PUBLIC GET endpoint with session, user-token, ViewerContext and agent-token credentials. The four new endpoints join the existing SCIM routes in itsOUTER_BOUNDARY_GET_STATUSESlist — they accept only the SCIM credential, so every other credential gets 403 — and the matrix gets real path values forresource_type_nameandschema_uri. Test-only.What this PR adds
GET /scim/v2/ServiceProviderConfig— single resource describing Sentry's actual capabilities (patchsupported,filtersupported witheqonly andmaxResults: 100— describing/Users–/Groupsfiltering, no bulk/sort/etag/changePassword, bearer-token auth scheme).GET /scim/v2/ResourceTypesandGET /scim/v2/ResourceTypes/{name}—User(organization members) andGroup(teams) entries mapped to the existing core schema URNs; unknown names get a SCIM-format 404.GET /scim/v2/Schemas/{schemaUri}— per-URI lookup of the existing User/Group schema definitions, with a SCIM-format 404 for unknown URIs. The route pattern is deliberately slash-permissive so malformed URIs still reach the endpoint and get the SCIM 404 rather than a generic Django 404.The endpoints reject
filterwith 403 (an RFC 7644 §4 SHOULD) and, as §4 permits, always return everything and ignore pagination parameters. They are GET-only; write methods (POST/PUT/PATCH/DELETE) get 405s in the RFC 7644 §3.12 error format (the envelope shown under Deviations below). The new endpoints arepublish_status = PUBLICwith full drf-spectacular docs — PUBLIC from day one because the response shape is fixed by the RFC and the point of the feature is that external platforms can rely on it; the pre-existing/Schemasindex stays PRIVATE. No new models or migrations. The handlers perform no database queries; the only DB access is the standard org-endpoint plumbing (token auth, org resolution, the permission check).Members and teams endpoints are untouched.
knownSentryApiUrls.generated.tsis committed and verified drift-free locally (theapi-url-typescriptauto-commit job doesn't run on fork PRs). The docs.sentry.io SSO prose page (sentry-docs repo) is a follow-up.API house-style deviations (all RFC-driven)
Three places this intentionally differs from the develop.sentry.dev API guidelines, matching how the existing SCIM tree already behaves:
/ServiceProviderConfig,/ResourceTypes) — the fixed endpoint names RFC 7644 §4 defines.{"schemas": [...], "status": "403", "detail": …}) rather than DRF's bare{"detail": …}— required (MUST) by RFC 7644 §3.12.403(not 400) for afilterparam — RFC 7644 §4 recommends (SHOULD) rejection so clients can't assume their filter was applied; the compliance validators check for exactly this.Changes to the existing (PRIVATE)
/Schemasindex responsesLow-risk: neither Okta nor Entra is known to probe discovery or send query parameters to
/Schemas(both hardcode the schema client-side, per the background above), no request handler reads these schema definitions (they are documentation only), and the previous values were RFC-invalid or didn't match what Sentry returns. One index entry before and after:"read"is not a legal RFC 7643 §7 keyword and would fail the very validators this PR targets):userName→immutable: consumed only at create (it becomes the member's email), and Sentry has no rename. NotreadOnly, because Entra excludesreadOnlyattributes from provisioning payloads entirely and cannot create users withoutuserName.emailsandname, with their sub-attributes →readOnly: both are derived server-side (emailsfromuserName;name.givenName/familyNamealways returnN/A), and client-sent values are ignored, which is how RFC 7644 §3.3/§3.5.1 definereadOnlyhandling.name.givenName/familyNamewere previouslyreadWrite.activestaysreadWrite:PATCH active=falseis the deprovisioning path.nameattribute also gains explicitmutability/returned/uniqueness: RFC 7643 §2.2 supplies defaults when they're omitted, but Entra's schema parser rejects attributes without an explicitmutability.membersnow matches the/Groupspayload: the schema declared sub-attributesvalueand$ref, bothreturned: default, but/Groupsreturnsvalueanddisplay. So$refwas never returned despitereturned: default(RFC 7643 §7), anddisplaywas never declared.$ref(optional in the RFC 7643 §8.7.1 Group schema) is replaced withdisplay, declaredreadOnlybecause Sentry derives it from the member's email. RFC 7643 is internally inconsistent ondisplaymutability (§2.4 saysimmutable; the §8.7.1 User schema'sgroups.displayisreadOnly; see erratum 8358), so the schema follows what Sentry does.build_schema_representation): each resource gains its RFC-required per-resourceschemasdeclaration, andmeta.locationchanged from a broken relative path that matched no real route to the absolute per-org URL. Locked bytest_index_entries_match_details_representation.?filter=returns a SCIM-format 403 (previously 400 on an invalid filter, silently ignored on a valid one), pagination params are ignored rather than validated (startIndexis now always1where it previously echoed the requested value; all schemas were returned in both old and new versions), and the 405 body is SCIM-format.The mutability values and the Group
memberssub-attributes are locked by tests, including one that compares the declared sub-attributes with a real/Groupsresponse. Attribute definitions are otherwise unchanged.Review focus and validation
Start with
utils.py— theSCIMDiscoveryEndpointbase and the permission wiring linked above hold all the behavior. The judgment calls in this PR are the/Schemaschanges above and thepublish_status = PUBLICdecision. The payload shapes (the three endpoint files, mirrored insrc/sentry/apidocs/examples/scim_examples.py) are machine-validated against RFC 7643 §5/§6 by the two independent checkers below; the slash-permissive.+route regexes inurls.pydeserve a glance.Validated on the current head against a live devserver with SCIM enabled via the real
AuthProvider.enable_scim()flow, over a public tunnel:scim2-testerpasses all 11 discovery checks, and Microsoft's Entra ID SCIM Validator discovers the schema and passes 17/21 provisioning tests, including all 12 Group tests (and its 3 Group preview tests). The 4 failures are pre-existing/Usersbehaviors inmembers.py(a duplicate of a pending invite returns 400,PATCHsupports onlyactive: false, no rename, and deactivation deletes the member), unchanged by this PR and queued as follow-up issues. Method and earlier validator screenshots are in this comment; layer-by-layer evidence is in the validation comment.Issues