feat(scim): Add SCIM /Users endpoints - #2747
Conversation
6e4416d to
edff202
Compare
f689fc8 to
fec7a7d
Compare
3538a0b to
77782ab
Compare
4709682 to
e114c8e
Compare
a9ac782 to
316aa74
Compare
eb969a8 to
86c6ef9
Compare
7294ee5 to
e6690b1
Compare
a96cfe7 to
fc3d2c3
Compare
432a3f6 to
56f3655
Compare
fc3d2c3 to
29c862c
Compare
56f3655 to
3e66f1d
Compare
annabkr
left a comment
There was a problem hiding this comment.
question: could we break this up for easier review? I find Claude is pretty good at doing that, if it feels tedious to do yourself
hf
left a comment
There was a problem hiding this comment.
Looks good, some minor clarifications not blocking from my POV.
| Name string `json:"name"` | ||
| Type AttributeType `json:"type"` | ||
| MultiValued bool `json:"multiValued"` | ||
| Description string `json:"description"` | ||
| Required bool `json:"required"` | ||
| CanonicalValues []string `json:"canonicalValues,omitempty"` | ||
| CaseExact bool `json:"caseExact"` | ||
| Mutability Mutability `json:"mutability"` | ||
| Returned Returned `json:"returned"` | ||
| Uniqueness Uniqueness `json:"uniqueness"` | ||
| ReferenceTypes []ReferenceType `json:"referenceTypes,omitempty"` | ||
| SubAttributes []*Attribute `json:"subAttributes,omitempty"` |
There was a problem hiding this comment.
Shouldn't all of these have omitempty?
There was a problem hiding this comment.
Shouldn't all of these have omitempty?
RFC 7643 says:
Unlike other core resources, the "Schema" resource MAY contain a complex object within a sub-attribute, and all attributes are REQUIRED unless otherwise specified.
So I opted to not add the omitempty so that they get the default zero values which would be false for all booleans.
| field | required |
|---|---|
| name | Y |
| name | Y |
| type | Y |
| multiValued | Y |
| description | Y |
| required | Y |
| caseExact | Y |
| mutability | Y |
| returned | Y |
| uniqueness | Y |
| canonicalValues | N |
| referenceTypes | N |
| subAttributes | N |
| if values.Get("sortBy") != "" { | ||
| return SortAscending, nil | ||
| } |
There was a problem hiding this comment.
How is ?sortBy (without =true) handled here?
There was a problem hiding this comment.
How is ?sortBy (without =true) handled here?
I think this will return as a default sort order and then the default sorting will kick in. I'll double check though.
9f216cd to
adc4c77
Compare
adc4c77 to
26eceaf
Compare
26eceaf to
ab06b11
Compare
| return nil, ErrNotFound | ||
| } | ||
|
|
||
| provider, err := models.FindSSOProviderBySCIMToken(srv.db.WithContext(ctx), bearerToken) |
There was a problem hiding this comment.
⚪ Severity: LOW
Each bearer token is resolved here, but this authentication path never records last_used_at. A compromised or leaked token can therefore be used without a last-use signal, preventing operators or automation from identifying active credentials or enforcing inactivity-based revocation.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Update last_used_at on the scim_tokens row each time a token is successfully authenticated. This requires two coordinated changes:
-
In
internal/models/sso.go: ModifyFindSSOProviderBySCIMTokento updatelast_used_aton the matched token row after a successful lookup, e.g. by callingtx.RawQuery("UPDATE scim_tokens SET last_used_at = now() WHERE token_hash = ?", ToSCIMHash(raw)).Exec()before returning. Alternatively, expose a new model helperUpdateSCIMTokenLastUsedAt(tx, tokenHash string) errorinscim_token.gothat performs this UPDATE. -
In
internal/api/scim/server.go: If a separate helper is introduced, call it from thelookupfunction immediately after the successfulFindSSOProviderBySCIMTokencall (line 307), and treat any update error as a warning/log rather than a hard failure so that a transient write error does not block valid requests.
No migration is needed since the last_used_at timestamptz column already exists in the scim_tokens table (migration 20260821010000_add_scim_tokens.up.sql) and the SCIMToken struct already maps it via db:"last_used_at".
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