feat!: implement SCIM handler for SCIM 2.0 compliance by Emyrk · Pull Request #25572 · coder/coder

Emyrk · 2026-05-21T14:50:52Z

Rewrites the SCIM 2.0 user provisioning handler to be RFC 7644 compliant. Verified against an external SCIM compliance suite (16/16 checks).

The previous handler did not return queried users, did not support PATCH or filter expressions, omitted /Schemas, /ResourceTypes, and /ServiceProviderConfig discovery, and could only handle a narrow Okta-shaped happy path. The old handler is preserved behind the CODER_SCIM_USE_LEGACY flag (hidden, default false) so existing deployments can opt out if something regresses.

SCIM provisioning now runs under a dedicated SubjectSCIMProvisioner RBAC subject (dbauthz.AsSCIMProvisioner) scoped to user CRUD. CreateUser is additionally escalated to AsSystemRestricted because the implicit member role assignment is gated by the static assignRoles whitelist, which has no scim entry.

Refs PLAT-148, PLAT-225
Refs https://linear.app/codercom/project/refactor-and-fully-implement-scim-20-specification-d641c8c7ab92

Reviewer notes

What's new

enterprise/coderd/scim/ package replaces the old single-file handler under enterprise/coderd/.
/Schemas, /ResourceTypes, /ServiceProviderConfig discovery endpoints.
GET /Users supports filter, attributes, excludedAttributes, paging.
PATCH /Users/{id} (path/value/replace ops) and PUT /Users/{id}.
GET /Users/{id} and DELETE /Users/{id} consistent with the spec.
New ExactUsername SQL query for SCIM lookups; no migrations.

Authorization

subjectSCIM / AsSCIMProvisioner is a dedicated RBAC subject scoped to user CRUD.
AsSystemRestricted wraps CreateUser only, because the member role assignment is bypassed by the assignRoles whitelist.
enterprise/audit/table.go updated for the new resource interactions.

Caveats (Okta)

Okta requires FamilyName. Coder does not split names into GivenName / FamilyName.
Okta defaults the SCIM username to the email. Coder normalizes that to a valid username, and the transformation is not reversible.

Stack

scripts/scimverify (external compliance harness) ships on top in test: add script for unit testing scim 2.0 #25620.

Generated by @Emyrk in collaboration with Coder Agents.
Manual QA with okta by @Emyrk

Emyrk · 2026-05-22T14:53:44Z

@@ -0,0 +1,595 @@
+package scim


This is the main file to review. Most of the logic came from the legacy implementation

…ResourceUser - Replace: dbUser was discarded after updateUserStatus, so the SCIM response carried the stale pre-update row. Capture the updated row. - GetUserCount dbauthz test: authorization moved from ResourceSystem to ResourceUser; update the assertion to match. Generated by Coder Agents.

@success

…pidocs - legacyscim swag refs: rename coderd.SCIMUser to legacyscim.SCIMUser in @Param/@success comments. The type moved when the legacy handler was extracted into its own package, but the annotations were not updated, so swag init strict mode failed and make gen could not produce _gen/manifest-staging.json. - scim.go gocritic nolint: drop the stray space after //nolint:. The malformed directive was parsed as an unknown linter name and did not suppress the AsSCIMProvisioner rule, breaking lint/go. - Regenerate coderd/apidoc and docs/reference/api to pick up the legacyscim.SCIMUser rename and the new scim_use_legacy deployment option that was already added to codersdk. Generated by Coder Agents.

…ates users ResourceUser.Create called CreateUser with the SCIM provisioner context. CreateUser invokes InsertUser, which always implicitly assigns the member role, and may also call InsertOrganizationMember when org sync assigns the default org. Neither role can be assigned by the SCIM provisioner role, so creating a user through SCIM returned 500 with 'not authorized to assign role "member"'. The legacy SCIM handler wrapped the same call in dbauthz.AsSystemRestricted; the new handler had a comment promising the same wrapper but the wrapper itself was missing. Restore the legacy behavior so SCIM POST /Users succeeds and refresh the comment to describe what the code actually does. Verified with scimverify (https://verify.scim.dev/): 16/16 tests pass, up from 11/13 (2 user-create failures) before this fix. Generated by Coder Agents.

coder-tasks · 2026-05-22T19:04:56Z

Documentation Check

Updates Needed

docs/admin/users/oidc-auth/index.md - The SCIM section (lines 137-162) states the implementation is "not a fully certified or guaranteed implementation of the SCIM 2.0 specification" and references issue Refactor & fully implement SCIM 2.0 specification #15830 for "tracked gaps and ongoing work." This PR rewrites the handler to be RFC 7644 compliant (16/16 compliance checks). The disclaimer should be updated to reflect the improved compliance posture, and the Refactor & fully implement SCIM 2.0 specification #15830 reference should be re-evaluated.
docs/admin/users/oidc-auth/index.md - Document new SCIM capabilities added by this PR: GET /Users with filter/attributes/paging support, PATCH /Users/{id}, PUT /Users/{id}, and discovery endpoints (/Schemas, /ResourceTypes, /ServiceProviderConfig). The current docs only mention provisioning/deprovisioning; IdP administrators need to know which operations are now supported.
docs/admin/users/oidc-auth/index.md - Document the CODER_SCIM_USE_LEGACY environment variable. Although the flag is hidden and defaults to false, existing deployments upgrading from the old handler should know this rollback option exists during the transition period. A brief note in the SCIM section (e.g., "If you encounter issues after upgrading, set CODER_SCIM_USE_LEGACY=true to revert to the previous handler") would help operators.

Automated review via Coder Agents

zedkipp · 2026-05-22T22:04:50Z

+		return scim.Resource{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("changing the 'userName' field is not supported (current value: %q)", dbUser.Username))
+	}
+
+	// TODO: Check primary email


Did you mean to add this check?

zedkipp · 2026-05-22T22:09:13Z

+		switch op.Op {
+		case "add":
+		case "remove":
+			if op.Path.String() == "active" {


The replace case below has a nil check on op.Path. I suspect you want the same here

zedkipp · 2026-05-22T22:10:14Z

+	var activeSet *bool
+	for _, op := range operations {
+		switch op.Op {
+		case "add":


Did you mean to add handling for this add case?

zedkipp · 2026-05-22T22:14:09Z

+	})
+	if err == nil {
+		// User already exists. Update their status if needed.
+		status := scimUserStatus(dbUser, &active)


Did you mean to scope this inside the if active { branch below? The status only seems to be used inside that scope. I can't tell if that's intended.

zedkipp · 2026-05-22T22:19:57Z

+	totalCount := int64(len(users))
+	if len(users) == int(qry.LimitOpt) {
+		// If the limit is not reached, that is the count
+		// TODO: If there is a query and the limit is reached, this is inaccurate.


What are the implications of this? Could we just fix it now?

zedkipp · 2026-05-22T22:35:41Z

+
+// Patch implements scim.ResourceHandler. Updates user attributes based on
+// SCIM PatchOp operations. Currently, supports changing the active status.
+func (ru *ResourceUser) Patch(r *http.Request, idStr string, operations []scim.PatchOperation) (scim.Resource, error) {


Do we need a similar mutability check in Patch that's in Replace?

An operation that is not compatible with an attribute's mutability or schema SHALL return the appropriate HTTP response status code and a JSON detail error response as defined in Section 3.12.

zedkipp · 2026-05-22T22:38:42Z

+	err = ru.Delete(r, user.ID.String())
+	require.NoError(t, err)
+
+	// Step 7: Get → confirm inactive after delete.


I think we want a 404 here:

Service providers MAY choose not to permanently delete the resource, e.g., for audit purposes. However, in such cases, the service provider MUST return a 404 (Not Found) error code for all operations associated with the previously deleted resource.

zedkipp · 2026-05-22T22:41:01Z

+	qry.LimitOpt = int32(params.Count)           //nolint:gosec
+	qry.OffsetOpt = int32(params.StartIndex - 1) //nolint:gosec
+
+	if qry.LimitOpt <= 0 {


Are you sure on this default limit?

https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2.4

A value of "0" indicates that no resource results are to be returned except for "totalResults"

zedkipp · 2026-05-22T22:44:58Z

It would be nice to have some tests of the new HTTP handler.

zedkipp · 2026-05-22T22:47:04Z

nitpicky, but I would omit the scim prefix from these file names the scim package to reduce stutter.
enterprise/coderd/scim/scimexpression.go -> enterprise/coderd/scim/expression.go
enterprise/coderd/scim/scimusers.go -> enterprise/coderd/scim/users.go

Emyrk marked this pull request as draft May 21, 2026 14:50

github-actions Bot assigned Emyrk May 21, 2026

Emyrk changed the title ~~feat: Scim 2.0~~ feat!: Scim 2.0 May 21, 2026

Emyrk force-pushed the stevenmasley/scim2 branch from 13b61c3 to 701ef7e Compare May 21, 2026 21:19

Emyrk added 12 commits May 22, 2026 09:26

feat: work on scim 2.0 compliance

46eaffd

scim auth

a2d0e0d

working on primary auth mw

a277a6e

work on patch/put

3bd1b5a

no system ctx

d352775

work on scim create

db681d9

add some basic verify script

9882e25

add mocked tests

f4221fb

legacy scim

eb68c80

fixups

3a1817b

pull out verify

9c387d0

case insensitive matching

a0fd5b3

Emyrk force-pushed the stevenmasley/scim2 branch from 2d550a3 to a0fd5b3 Compare May 22, 2026 14:32

ad unit test for case insensistive

c041338

Emyrk commented May 22, 2026

View reviewed changes

Emyrk added 10 commits May 22, 2026 09:58

dry up some update user status

466d68b

dry up user fetch by uuid

130db62

1 pass for primary email

2c9aa7d

linting

630f1af

fmyt

67b6d6c

linting

874e27b

linting

329385f

Emyrk changed the title ~~feat!: Scim 2.0~~ feat!: implement SCIM handler for SCIM 2.0 compliance May 22, 2026

Emyrk marked this pull request as ready for review May 22, 2026 19:02

github-actions Bot added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label May 22, 2026

zedkipp reviewed May 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: implement SCIM handler for SCIM 2.0 compliance#25572

feat!: implement SCIM handler for SCIM 2.0 compliance#25572
Emyrk wants to merge 23 commits into
mainfrom
stevenmasley/scim2

Emyrk commented May 21, 2026 •

edited

Loading

Uh oh!

Emyrk May 22, 2026

Uh oh!

coder-tasks Bot commented May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

zedkipp May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Emyrk commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-tasks Bot commented May 22, 2026

Documentation Check

Updates Needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Emyrk commented May 21, 2026 •

edited

Loading