From 3a11ca4d3a804e817981b3484f7e472bd152ada6 Mon Sep 17 00:00:00 2001 From: Ronni Skansing Date: Thu, 11 Jun 2026 20:53:15 +0200 Subject: [PATCH] scim fixes Signed-off-by: Ronni Skansing --- backend/controller/scim.go | 4 +++- backend/repository/companyScimConfig.go | 24 ------------------------ backend/repository/recipient.go | 6 ++++-- backend/service/scim.go | 25 +++++++++++++------------ 4 files changed, 20 insertions(+), 39 deletions(-) diff --git a/backend/controller/scim.go b/backend/controller/scim.go index 1971df3..3dae49d 100644 --- a/backend/controller/scim.go +++ b/backend/controller/scim.go @@ -64,8 +64,10 @@ func (c *Scim) authenticate(g *gin.Context, companyID *uuid.UUID) (*service.Scim config, authed, err := c.ScimService.VerifyAndLoadConfig(g.Request.Context(), companyID, token) if err != nil { + // a missing config is reported with the same generic 401 as a bad token so + // the response does not reveal whether a company has SCIM configured if isNotFound(err) { - scimError(g, http.StatusUnauthorized, "no SCIM configuration found for this company") + scimError(g, http.StatusUnauthorized, "invalid bearer token or SCIM provisioning is disabled") return nil, false } c.Logger.Errorw("scim auth: error verifying token", "error", err) diff --git a/backend/repository/companyScimConfig.go b/backend/repository/companyScimConfig.go index ef356e0..61c058a 100644 --- a/backend/repository/companyScimConfig.go +++ b/backend/repository/companyScimConfig.go @@ -183,30 +183,6 @@ func (r *CompanyScimConfig) UpdateLastSyncAt( return nil } -// GetTokenHashByCompanyID fetches only the token hash for the given company, -// used during bearer-token verification so the hash never leaks into the model layer -func (r *CompanyScimConfig) GetTokenHashByCompanyID( - ctx context.Context, - companyID *uuid.UUID, -) (string, error) { - var row database.CompanyScimConfig - res := r.DB. - Select("token_hash"). - Where( - fmt.Sprintf( - "%s = ?", - TableColumn(database.COMPANY_SCIM_CONFIG_TABLE, "company_id"), - ), - companyID.String(), - ). - First(&row) - - if res.Error != nil { - return "", errs.Wrap(res.Error) - } - return row.TokenHash, nil -} - // GetWithTokenHashByCompanyID fetches the full config row and the token hash in a // single query, used during bearer-token verification. func (r *CompanyScimConfig) GetWithTokenHashByCompanyID( diff --git a/backend/repository/recipient.go b/backend/repository/recipient.go index ae32d89..fecffc7 100644 --- a/backend/repository/recipient.go +++ b/backend/repository/recipient.go @@ -877,7 +877,9 @@ func (r *Recipient) DeleteByID( } // MarkScimSoftDeleted sets scim_soft_deleted_at, marking a SCIM-deprovisioned -// recipient as pending prune without deleting it. +// recipient as pending prune without deleting it. The scim_soft_deleted_at IS NULL +// guard preserves the original disable time when an IdP re-sends active=false for +// an already-disabled recipient, so the retention window is not reset each sync. func (r *Recipient) MarkScimSoftDeleted( ctx context.Context, id *uuid.UUID, @@ -885,7 +887,7 @@ func (r *Recipient) MarkScimSoftDeleted( ) error { res := r.DB. Model(&database.Recipient{}). - Where("id = ?", id). + Where("id = ? AND scim_soft_deleted_at IS NULL", id). Updates(map[string]any{ "scim_soft_deleted_at": at.UTC(), "updated_at": time.Now().UTC(), diff --git a/backend/service/scim.go b/backend/service/scim.go index f8eb479..f215b60 100644 --- a/backend/service/scim.go +++ b/backend/service/scim.go @@ -1088,11 +1088,11 @@ func (s *Scim) CreateUser( } // deprovisionedUserResponse builds the SCIM body returned after a user has been -// deprovisioned via active=false. Microsoft Entra sends a soft-delete (PATCH +// deprovisioned via active=false. Microsoft Entra sends a disable (PATCH // active=false) and expects a 200 response with the resource showing // active=false. Returning 404 makes Entra log the disable as a failure and retry -// it every sync cycle, so the recipient is hard-deleted but a success body with -// active=false is returned. +// it every sync cycle, so the recipient is marked disabled (soft-deleted) and a +// success body with active=false is returned. func deprovisionedUserResponse(existing *model.Recipient, baseURL string) *ScimUser { u := recipientToScimUser(existing, baseURL) u.Active = false @@ -1121,7 +1121,7 @@ func (s *Scim) ReplaceUser( if compErr != nil || rCompanyID != *companyID { return nil, errs.Wrap(gorm.ErrRecordNotFound) } - // a PUT with active=false is a deprovision request — hard-delete the recipient + // a PUT with active=false is a deprovision request — mark the recipient disabled // but return 200 with active=false so the IdP records the disable as a success if !scimUser.Active { if err := s.deprovisionRecipient(ctx, recipientID); err != nil { @@ -1185,14 +1185,14 @@ func (s *Scim) PatchUser( if err != nil { return nil, err } - // active=false triggers a hard-delete; return 200 with active=false so - // the IdP records the soft-delete as a success instead of retrying + // active=false marks the recipient disabled; return 200 with active=false + // so the IdP records the disable as a success instead of retrying if deactivated { s.auditScim("Scim.DeprovisionUser", config, map[string]any{"recipientID": recipientID.String(), "via": "patch"}) return deprovisionedUserResponse(existing, baseURL), nil } case "remove": - // remove op on "active" means deactivate — hard-delete the recipient + // remove op on "active" means deactivate — mark the recipient disabled if strings.EqualFold(op.Path, "active") { if err := s.deprovisionRecipient(ctx, recipientID); err != nil { return nil, errs.Wrap(err) @@ -1212,9 +1212,10 @@ func (s *Scim) PatchUser( return &u, nil } -// DeprovisionUser hard-deletes a recipient provisioned via SCIM. -// a subsequent GET returns 404, satisfying the validator expectation that -// a deleted user is no longer accessible. +// DeprovisionUser marks a SCIM-provisioned recipient as disabled (soft-deleted) +// during the retention grace period rather than deleting it outright. The +// recipient is excluded from sending and targeting but stays retrievable; a +// subsequent GET returns 200 with active=false until the prune window elapses. func (s *Scim) DeprovisionUser( ctx context.Context, companyID *uuid.UUID, @@ -1274,8 +1275,8 @@ func (s *Scim) UpdateLastSync(ctx context.Context, config *model.CompanyScimConf // ── helpers ─────────────────────────────────────────────────────────────────── -// deprovisionRecipient removes a recipient from all groups and hard-deletes it. -// shared by DELETE, PUT active=false and PATCH active=false. +// deprovisionRecipient marks a recipient disabled (soft-deleted) and cancels its +// pending sends. shared by DELETE, PUT active=false and PATCH active=false. func (s *Scim) deprovisionRecipient(ctx context.Context, recipientID *uuid.UUID) error { // mark the recipient as SCIM soft-deleted; the anonymizing delete runs only // after the retention grace period (scheduled job or on-demand prune)