Skip to content

Commit 0fb6cf5

Browse files
committed
fix(scim): make SCIM DELETE RFC 7644 compliant (WI-1310)
SCIM DELETE only flipped is_active=false while leaving scim_managed=true, so a deprovisioned user stayed fetchable via GET /Users/{id}, listed by /Users, switchable back to active via PUT/PATCH, and exposed through group member projections. RFC 7644 §3.6 permits retaining a deleted resource only when it returns 404 and is omitted from all query results. SCIM DELETE now tombstones the user in one transaction: is_active=false plus a new users.scim_deleted_at marker, and SCIM-managed group memberships removed. Every SCIM user query (list, count, get, and the group-member join and member-visibility guard) excludes tombstoned rows, so subsequent operations report 404. PATCH active=false remains the visible deactivation path. Re-provisioning a tombstoned email answers 409 uniqueness instead of silently resurrecting the account; the row is retained for historical references and stays distinct from the administrative offboarding state. Also carries the regenerated api/openapi-v2.json left stale by the theme logo commit. Black-box regression coverage in core-tests: delete-then-404 on every operation, list/filter omission, group membership cleanup, re-provision conflict, and PATCH-active deactivation staying visible. Tested against SQLite and PostgreSQL.
1 parent bcbab2a commit 0fb6cf5

4 files changed

Lines changed: 64 additions & 10 deletions

File tree

api/openapi-v2.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23676,6 +23676,10 @@
2367623676
"description": "The is default value.",
2367723677
"type": "boolean"
2367823678
},
23679+
"logo_url": {
23680+
"description": "The logo url value.",
23681+
"type": "string"
23682+
},
2367923683
"name": {
2368023684
"description": "The name value.",
2368123685
"type": "string"
@@ -44147,6 +44151,10 @@
4414744151
"description": "The is default value.",
4414844152
"type": "boolean"
4414944153
},
44154+
"logo_url": {
44155+
"description": "The logo url value.",
44156+
"type": "string"
44157+
},
4415044158
"name": {
4415144159
"description": "The name value.",
4415244160
"type": "string"

internal/database/schema/users.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
email_verification_expires DATETIME, -- Expiry time for verification token
1616
scim_external_id TEXT, -- SCIM externalId from identity provider
1717
scim_managed BOOLEAN DEFAULT false, -- If true, user is managed via SCIM
18+
scim_deleted_at DATETIME, -- Set when the IdP deprovisioned the user via SCIM DELETE; row retained but hidden from every SCIM query (RFC 7644 §3.6)
1819
offboarded_at DATETIME, -- Set when the account is administratively offboarded; never cleared, every activation path must reject it
1920
is_agent BOOLEAN DEFAULT FALSE, -- If true, user is a non-human agent (API-only; cannot log in)
2021
agent_owner_user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, -- NULL = service user (admin-provisioned); non-NULL = owned agent (inherits owner permissions)
@@ -165,6 +166,7 @@ CREATE INDEX IF NOT EXISTS idx_user_invitations_token ON user_invitations(token)
165166
CREATE INDEX IF NOT EXISTS idx_user_invitations_user_id ON user_invitations(user_id);
166167

167168

169+
-- migration: 20260911_users_scim_deleted_at
168170
-- migration: 20260911_users_offboarded_at
169171

170172
-- migration: 0014_users_is_agent

internal/handlers/scim.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,19 @@ func (h *SCIMHandler) CreateUser(w http.ResponseWriter, r *http.Request) {
364364

365365
existingUser, err := h.repo.GetUserByEmail(email)
366366
if err == nil {
367+
// A tombstoned account stays deleted: re-provisioning the same email
368+
// must conflict rather than resurrect it (RFC 7644 §3.6). Un-deleting
369+
// is an operator decision, not an IdP-driven one.
370+
if h.repo.IsUserSCIMDeleted(existingUser.ID) {
371+
h.logSCIMAuditEvent(r, logger.ActionSCIMUserCreate, logger.ResourceUser, &existingUser.ID, email,
372+
map[string]any{
373+
"username": existingUser.Username,
374+
"reason": "email_matches_tombstoned_user",
375+
}, false, "refused: email belongs to a SCIM-deleted user")
376+
respondSCIMErrorMsg(w, http.StatusConflict, "User with this email already exists", "uniqueness")
377+
return
378+
}
379+
367380
username := scimUser.UserName
368381
if username == "" {
369382
username = existingUser.Username
@@ -652,7 +665,7 @@ func (h *SCIMHandler) DeleteUser(w http.ResponseWriter, r *http.Request) {
652665
return
653666
}
654667

655-
err = h.repo.DeactivateUser(id)
668+
err = h.repo.TombstoneUser(id)
656669
if err != nil {
657670
respondSCIMErrorMsg(w, http.StatusInternalServerError, "Failed to delete user", "")
658671
return

internal/repository/scim_repository.go

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ type SCIMGroupMemberRow struct {
5656
func (r *SCIMRepository) ListUsersFiltered(whereClause string, filterArgs []any, count, offset int) ([]models.User, int, error) {
5757
baseQuery := `SELECT id, email, username, first_name, last_name, is_active,
5858
COALESCE(scim_external_id, '') as scim_external_id, created_at, updated_at
59-
FROM users WHERE is_agent = false AND scim_managed = true`
60-
countQuery := `SELECT COUNT(*) FROM users WHERE is_agent = false AND scim_managed = true`
59+
FROM users WHERE is_agent = false AND scim_managed = true AND scim_deleted_at IS NULL`
60+
countQuery := `SELECT COUNT(*) FROM users WHERE is_agent = false AND scim_managed = true AND scim_deleted_at IS NULL`
6161

6262
args := []any{}
6363
if whereClause != "" {
@@ -98,14 +98,16 @@ func (r *SCIMRepository) ListUsersFiltered(whereClause string, filterArgs []any,
9898
}
9999

100100
// GetUserByID loads a single user with the SCIM-relevant flags.
101+
// Deprovisioned (tombstoned) users are excluded so every SCIM operation on
102+
// them — GET, PUT, PATCH, DELETE — reports 404 per RFC 7644 §3.6.
101103
func (r *SCIMRepository) GetUserByID(id int) (*models.User, error) {
102104
var user models.User
103105
var scimExternalID sql.NullString
104106
err := r.db.QueryRow(`
105107
SELECT id, email, username, first_name, last_name, is_active,
106108
scim_external_id, COALESCE(scim_managed, false), COALESCE(is_agent, false),
107109
created_at, updated_at
108-
FROM users WHERE id = ?
110+
FROM users WHERE id = ? AND scim_deleted_at IS NULL
109111
`, id).Scan(&user.ID, &user.Email, &user.Username, &user.FirstName, &user.LastName,
110112
&user.IsActive, &scimExternalID, &user.SCIMManaged, &user.IsAgent,
111113
&user.CreatedAt, &user.UpdatedAt)
@@ -178,10 +180,39 @@ func (r *SCIMRepository) ReplaceUser(id int, email, username, firstName, lastNam
178180
return err
179181
}
180182

181-
// DeactivateUser flips a user inactive (SCIM DELETE deactivates, never deletes).
182-
func (r *SCIMRepository) DeactivateUser(id int) error {
183-
_, err := r.db.ExecWrite(`UPDATE users SET is_active = false, updated_at = CURRENT_TIMESTAMP WHERE id = ?`, id)
184-
return err
183+
// TombstoneUser applies an RFC 7644 §3.6 SCIM DELETE: the user is deactivated
184+
// and marked deprovisioned so every SCIM query hides the resource, while the
185+
// row itself is retained for historical references. SCIM-managed group
186+
// memberships are removed so group projections cannot expose the deleted
187+
// user. Runs in one transaction — a partial delete must never leave a visible
188+
// membership on a tombstoned user.
189+
func (r *SCIMRepository) TombstoneUser(id int) error {
190+
return database.WithTx(r.db, func(tx database.Tx) error {
191+
if _, err := tx.Exec(`
192+
UPDATE users
193+
SET is_active = false, scim_deleted_at = CURRENT_TIMESTAMP, updated_at = CURRENT_TIMESTAMP
194+
WHERE id = ? AND scim_deleted_at IS NULL
195+
`, id); err != nil {
196+
return fmt.Errorf("failed to tombstone user: %w", err)
197+
}
198+
if _, err := tx.Exec(`
199+
DELETE FROM group_members WHERE user_id = ? AND scim_managed = true
200+
`, id); err != nil {
201+
return fmt.Errorf("failed to remove SCIM group memberships: %w", err)
202+
}
203+
return nil
204+
})
205+
}
206+
207+
// IsUserSCIMDeleted reports whether the user row carries the SCIM
208+
// deprovisioning tombstone. Used by the create/adopt flow so a re-provisioning
209+
// request for a deleted account conflicts instead of resurrecting it.
210+
func (r *SCIMRepository) IsUserSCIMDeleted(id int) bool {
211+
var deleted bool
212+
err := r.db.QueryRow(
213+
`SELECT scim_deleted_at IS NOT NULL FROM users WHERE id = ?`, id,
214+
).Scan(&deleted)
215+
return err == nil && deleted
185216
}
186217

187218
// SetUserActive applies a SCIM PATCH to the active flag.
@@ -229,7 +260,7 @@ func (r *SCIMRepository) IsUserSCIMVisible(userID int) bool {
229260
var ok bool
230261
err := r.db.QueryRow(`
231262
SELECT COALESCE(scim_managed, false) = true AND COALESCE(is_agent, false) = false
232-
FROM users WHERE id = ?
263+
FROM users WHERE id = ? AND scim_deleted_at IS NULL
233264
`, userID).Scan(&ok)
234265
return err == nil && ok
235266
}
@@ -336,7 +367,7 @@ func (r *SCIMRepository) GetGroupMembers(groupID int) ([]SCIMGroupMemberRow, err
336367
SELECT u.id, u.first_name, u.last_name, u.username
337368
FROM group_members gm
338369
JOIN users u ON gm.user_id = u.id
339-
WHERE gm.group_id = ? AND gm.scim_managed = true
370+
WHERE gm.group_id = ? AND gm.scim_managed = true AND u.scim_deleted_at IS NULL
340371
`, groupID)
341372
if err != nil {
342373
return nil, err

0 commit comments

Comments
 (0)