feat(scim): Add SCIM /Users endpoints - #2747
Conversation
6e4416d to
edff202
Compare
3538a0b to
77782ab
Compare
4709682 to
e114c8e
Compare
eb969a8 to
86c6ef9
Compare
| } | ||
|
|
||
| var rows []scimUser | ||
| if err := r.db.WithContext(ctx).RawQuery("INSERT INTO scim_users (id, sso_provider_id, resource) VALUES (?, ?, ?) RETURNING id, resource, active, created_at, updated_at", uuid.Must(uuid.NewV4()), r.tenant(ctx), resource).All(&rows); err != nil { |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
An authenticated SCIM client can POST a userName, but this insert never populates the user_id link to the corresponding auth.users account. Provisioning therefore creates only a shadow scim_users record, so later SCIM lifecycle operations cannot disable or revoke access for the account users actually authenticate with.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The Create function must be updated to link the new scim_users row to the corresponding auth.users account by populating the user_id column. The fix requires three coordinated changes:
-
In
Create(line 99): Before the INSERT, queryauth.users(or theidentitiestable) to find an existing user whose email matches the SCIMuserName. If found, includeuser_idin the INSERT:INSERT INTO scim_users (id, sso_provider_id, user_id, resource) VALUES (?, ?, ?, ?). Pass the resolved UUID (ornil/NULL if no match yet). -
In
Delete(line 123): After soft-deleting thescim_usersrow, also disable or delete the linkedauth.usersaccount. Use theuser_idreturned from the DELETE (add it to the RETURNING clause) and issue anUPDATE auth.users SET ... WHERE id = ?or call the relevant storage layer helper to ban/disable the account. -
In
Replace(line 112): When the SCIMactiveattribute is set tofalsevia a PUT, propagate that deactivation to the linkedauth.usersaccount using the sameuser_idlookup.
Without these three changes, the scim_users shadow record is fully decoupled from the real authentication account, so SCIM lifecycle operations (deprovision, deactivate) have no effect on actual user access.
|
|
||
| func (r *userRepository) Delete(ctx context.Context, id string) error { | ||
| var ids []string | ||
| if err := r.db.WithContext(ctx).RawQuery("UPDATE scim_users SET deleted_at = now() WHERE sso_provider_id = ? AND deleted_at IS NULL AND id = ? RETURNING id", r.tenant(ctx), id).All(&ids); err != nil { |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
An authenticated SCIM client can DELETE a User, but this operation only timestamps scim_users.deleted_at. It does not disable the associated Auth account or revoke its sessions, allowing a deprovisioned user to continue logging in or using already-issued credentials despite a successful SCIM deletion.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The Delete method must be extended to also deprovision the linked auth account when a SCIM user is deleted. The scim_users table already has a user_id foreign key column referencing auth.users. The fix requires several coordinated steps:
- Add
user_id(nullableuuid.UUID) to thescimUserstruct inmodels.gowithdb:"user_id". - Update the
DeleteSQL query toRETURNING id, user_idso the linked auth user ID is retrieved. - Wrap the entire delete operation in a database transaction (
r.db.Transaction(...)). - After soft-deleting the
scim_usersrow, ifuser_idis non-null, callmodels.Logout(tx, userID)to revoke all active sessions for that auth user. - Also ban the auth user by setting
banned_untilto a permanent/far-future timestamp (e.g.,time.Date(9999, 12, 31, ...)) and persisting that viamodels.UpdateUserBannedUntil(tx, user)to prevent new logins even if sessions are somehow reused.
All database mutations (soft-delete + session revocation + ban) must be atomic within a single transaction to avoid a partial-deprovision state.
| } | ||
| users = append(users, user) | ||
| } | ||
| return users, total, nil |
There was a problem hiding this comment.
⚪ Severity: LOW
An authenticated SCIM client can request excludedAttributes=emails (or a restrictive attributes list), but the parsed selectors are ignored before this method returns every stored core.User, including emails and names. Integrations relying on SCIM attribute filtering therefore receive PII they explicitly excluded.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The List method ignores query.Attributes and query.ExcludedAttributes, causing full core.User objects (including PII such as emails and names) to be returned even when the SCIM client explicitly excludes them.
There are two approaches to fix this:
Option 1 – Reject unsupported attribute selection (short-term, consistent with existing behaviour for filter):
At the top of the List function (after the existing query.Filter check around line 61), add a guard that rejects requests that use attributes or excludedAttributes:
if len(query.Attributes) > 0 || len(query.ExcludedAttributes) > 0 {
return nil, 0, protocol.ErrNotImplemented("attribute selection is not supported")
}This mirrors the existing treatment of filter and signals clearly to clients that the feature is not yet available, rather than silently leaking data.
Option 2 – Implement proper attribute projection (complete fix per RFC 7644):
Add a helper function (e.g., applyAttributeSelection(user *core.User, include, exclude []string) *core.User) that, given the Attributes inclusion list and ExcludedAttributes exclusion list, returns a copy of the user with non-requested fields set to their zero values. The function must respect RFC 7644 §3.9 rules: id, schemas, meta, and userName must always be included. Call this helper inside the loop in List after r.mapFrom, and also in UserByID in server.go (which has the same gap). This approach requires careful case-insensitive string matching of SCIM path names to struct fields and comprehensive tests covering edge cases.
What kind of change does this PR introduce?
Feature. Add SCIM endpoints for core User schema.
What is the current behavior?
These endpoints do not exist yet.
What is the new behavior?
Adds:
/scim/v2/Users/scim/v2/Users/{id}/scim/v2/Users/scim/v2/Users/{id}/scim/v2/Users/{id}/scim/v2/ResourceTypes/{id}/scim/v2/Schemas/{id}Additional context
/Usersendpoints authenticate with a bearer SCIM token. Tokens are stored hashed in the newscim_tokenstable, are revocable (revoked_at) and expirable (expires_at), and resolve to ansso_provider_idthat scopes every query to a single tenant./ResourceTypes/{id}and/Schemas/{id}routes are discovery metadata and are unauthenticated, consistent with the existing/ServiceProviderConfig,/ResourceTypes, and/Schemasendpoints.requireScimServerEnabledgate.scim_usersandscim_tokenstables. A user's SCIM payload is stored as a JSONBresourcecolumn and deletes are soft usingdeleted_at.Listrejects anyfilterparameter.Extracted from #2731