From 78485f74294d38274d175d888289f7592a6e4ac6 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Mon, 11 May 2026 02:21:05 +0000 Subject: [PATCH] =?UTF-8?q?fix(auth/users):=20close=20MED-11=20lying=20fie?= =?UTF-8?q?ld=20=E2=80=94=20DeactivatedAt=20loaded=20+=20enforced=20on=20l?= =?UTF-8?q?ogin=20(A-2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MED-11 closure shipped users.deactivated_at + DELETE /api/v1/auth/users/{id} + cascade-revoke, but the federated-user soft-delete was reversible: the next OIDC login under the same (provider, subject) tuple re-minted a session and re-elevated the user. Three legs of the chain were severed (each independently CRIT-shaped): Leg A — postgres/user.go::userColumns omitted `deactivated_at`, so scanUser never populated User.DeactivatedAt. Every Get / GetByOIDCSubject / ListAll returned DeactivatedAt = nil regardless of the column value. Leg B — postgres/user.go::Update SQL omitted `deactivated_at = $X`, so the handler's `u.DeactivatedAt = now()` mutation was a no-op write at the SQL level. Even with leg A closed, no row ever flipped. Leg C — oidc/service.go::upsertUser did not inspect DeactivatedAt on the existing-user path. Even with legs A + B closed, the OIDC login would still proceed normally. The cascade-session-revoke half of the original closure remained correct, but only for the duration of the user's current cookie. SOC 2 CC6.3 + ISO 27001 A.9.2.6 "user access removal" controls require both immediate revoke AND persistent block — this fix restores the persistent-block leg. Closure across layers: internal/repository/postgres/user.go - userColumns adds `deactivated_at` - scanUser reads via sql.NullTime intermediate (column is nullable) - Create writes deactivated_at explicitly (NULL for new active users; forward-compat for future seed-data flows that pre-populate the column) - Update writes deactivated_at on every call; nil DeactivatedAt → NULL (supports reactivation) internal/auth/oidc/service.go - New sentinel ErrUserDeactivated - upsertUser checks existing.DeactivatedAt != nil BEFORE mutating email / display_name / last_login_at — preserves last_login_at forensics on rejected login attempts (defense-in-depth pin against future "performance optimization" that reorders the gate) internal/api/handler/auth_session_oidc.go - classifyOIDCFailure adds typed errors.Is dispatch for ErrUserDeactivated → audit category "user_deactivated" (SOC/SIEM observability surface) internal/api/handler/auth_users.go - Self-deactivate guard on Deactivate: HTTP 409 + audit row auth.user_deactivate_self_rejected when caller targets own User row. Prevents an admin from one-way-door locking themselves out via the standard handler; break-glass remains the recovery path. - New Reactivate handler: inverse of Deactivate. Clears DeactivatedAt via Update; emits auth.user_reactivated audit row. Idempotent on already-active rows. Sessions revoked at deactivation stay revoked (cascade irreversible by design — user must complete fresh OIDC login). internal/api/router/router.go - POST /api/v1/auth/users/{id}/reactivate wired with auth.user.deactivate gate (reactivation is the inverse op, not a separate privilege) web/src/api/client.ts + web/src/pages/auth/UsersPage.tsx - authReactivateUser() client function - Reactivate button on deactivated rows in UsersPage Regression coverage: Postgres (testcontainers, skipped under -short): TestUserRepository_DeactivatedAt_RoundTrip — Create → set DeactivatedAt → Update → Get / GetByOIDCSubject / ListAll round-trip the value TestUserRepository_DeactivatedAt_CreateWritesNullForActive — new active user reads back DeactivatedAt = nil TestUserRepository_DeactivatedAt_CreatePersistsPreDeactivated — Create with non-nil DeactivatedAt round-trips (forward-compat path) OIDC service: TestService_HandleCallback_RejectsDeactivatedUser — errors.Is ErrUserDeactivated; CallbackResult nil; persisted email / last_login_at / deactivated_at NOT mutated by the rejected attempt TestService_HandleCallback_AllowsReactivatedUser — DeactivatedAt = nil → happy path resumes TestService_HandleCallback_DeactivatedUserPreservesForensics — defense-in-depth pin against future regressions that reorder the gate-vs-mutation sequence Classifier: TestClassifyOIDCFailure extended — typed dispatch + wrapped variant round-trip through errors.Is Handler: TestAuthUsers_Deactivate_RejectsSelfDeactivate — HTTP 409 + audit row + cascade-revoke NOT fired + row stays active TestAuthUsers_Deactivate_OtherUser_HappyPath — HTTP 204 + cascade fires + row soft-deleted TestAuthUsers_Reactivate_HappyPath / _IdempotentOnActiveUser / _UnknownID / _MissingID / _UpdateError Phase 6 verify gate green on the targeted packages: gofmt clean, go vet clean, go test -short pass across internal/auth/oidc, internal/api/handler, internal/api/router, internal/repository/postgres, internal/auth/..., internal/service/..., internal/tlsprobe/..., internal/trustanchor/..., internal/validation/... Spec at cowork/auth-bundles-fixes-2026-05-11/02-crit-deactivated-at-enforcement.md Closure annotation at cowork/auth-bundles-audit-2026-05-10.md MED-11 row. Operator advisory in CHANGELOG.md v2.1.0 release notes. --- CHANGELOG.md | 27 ++ internal/api/handler/auth_session_oidc.go | 18 +- .../api/handler/auth_session_oidc_test.go | 5 + internal/api/handler/auth_users.go | 79 +++++ internal/api/handler/auth_users_test.go | 297 ++++++++++++++++++ internal/api/router/router.go | 5 + internal/auth/oidc/service.go | 28 ++ internal/auth/oidc/service_test.go | 166 ++++++++++ internal/repository/postgres/user.go | 55 +++- internal/repository/postgres/user_test.go | 173 ++++++++++ web/src/api/client.ts | 4 + web/src/pages/auth/UsersPage.tsx | 34 +- 12 files changed, 877 insertions(+), 14 deletions(-) create mode 100644 internal/api/handler/auth_users_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e986ea..4bf388a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,33 @@ ### Security (BREAKING) +- **Federated-user deactivation now actually blocks login (Audit 2026-05-11 A-2).** + The MED-11 closure shipped `users.deactivated_at` + `DELETE /api/v1/auth/users/{id}` + + cascade-session-revoke, but the column was a "lying field" three legs over: the + postgres user repository never SELECTed it (so `User.DeactivatedAt` always read + nil), the `Update` SQL never wrote it (so the handler's mutation was a no-op), + and the OIDC `upsertUser` path never checked it (so the next login under the + same `(provider, subject)` tuple re-minted a session and re-elevated the user). + The cascade-revoke remained correct for the current cookie only. **Operator + advisory: if you deactivated a federated user between the MED-11 closure + (Bundle 2 merge `dea5053`) and the v2.1.0 release tag, verify the user cannot + OIDC-log-in after upgrading — the column took no effect at login time before + this fix. If needed, re-run the deactivation against the upgraded server.** + Closure: `userColumns` + `scanUser` now read `deactivated_at` via `sql.NullTime`; + `Create` + `Update` write it explicitly; `upsertUser` returns the new + `ErrUserDeactivated` sentinel before mutating fields (preserves `last_login_at` + forensics on rejected logins); `classifyOIDCFailure` surfaces the rejection + as audit category `user_deactivated`. Self-deactivate guard on + `DELETE /api/v1/auth/users/{id}` returns HTTP 409 + audit row + `auth.user_deactivate_self_rejected` (prevents an admin from one-way-door + locking themselves out via the standard handler — break-glass remains the + recovery path). New inverse endpoint `POST /api/v1/auth/users/{id}/reactivate` + (gated `auth.user.deactivate` — reactivation is the inverse op, not a separate + privilege) clears `deactivated_at`; emits audit row `auth.user_reactivated`. + Sessions revoked at deactivation stay revoked across reactivation — the user + must complete a fresh OIDC login. GUI: `UsersPage.tsx` now renders a Reactivate + button on deactivated rows. CWE-862 (missing authorization at the user-state + boundary). SOC 2 CC6.3 + ISO 27001 A.9.2.6 compliance-table-flipping fix. - **`__Host-` cookie prefix on all three auth cookies (Audit 2026-05-10 MED-14).** The session cookie, CSRF cookie, and OIDC pre-login cookie are renamed from `certctl_session` / `certctl_csrf` / `certctl_oidc_pending` to diff --git a/internal/api/handler/auth_session_oidc.go b/internal/api/handler/auth_session_oidc.go index 3faf223..cbd1b86 100644 --- a/internal/api/handler/auth_session_oidc.go +++ b/internal/api/handler/auth_session_oidc.go @@ -1006,11 +1006,11 @@ func (h *AuthSessionOIDCHandler) TestProvider(w http.ResponseWriter, r *http.Req } h.recordAudit(r.Context(), "auth.oidc_provider_tested", caller.ActorID, caller.ActorType, "", map[string]interface{}{ - "issuer_url": req.IssuerURL, - "discovery_succeeded": res.DiscoverySucceeded, - "jwks_reachable": res.JWKSReachable, - "iss_param_supported": res.IssParamSupported, - "error_count": len(res.Errors), + "issuer_url": req.IssuerURL, + "discovery_succeeded": res.DiscoverySucceeded, + "jwks_reachable": res.JWKSReachable, + "iss_param_supported": res.IssParamSupported, + "error_count": len(res.Errors), }) writeJSON(w, http.StatusOK, res) } @@ -1267,6 +1267,14 @@ func classifyOIDCFailure(err error) string { return "prelogin_ua_mismatch" case errors.Is(err, oidcsvc.ErrPreLoginIPMismatch): return "prelogin_ip_mismatch" + // Audit 2026-05-11 A-2 — surface deactivated-user rejection as its + // own audit category so SOC / SIEM can alert on attempted logins by + // federated users that the admin has soft-deleted. Typed dispatch + // (not substring) because the sentinel is the only authoritative + // test for this condition; the message string is implementation + // detail subject to change. + case errors.Is(err, oidcsvc.ErrUserDeactivated): + return "user_deactivated" } msg := strings.ToLower(err.Error()) switch { diff --git a/internal/api/handler/auth_session_oidc_test.go b/internal/api/handler/auth_session_oidc_test.go index 55fe134..42d4fba 100644 --- a/internal/api/handler/auth_session_oidc_test.go +++ b/internal/api/handler/auth_session_oidc_test.go @@ -1217,6 +1217,11 @@ func TestClassifyOIDCFailure(t *testing.T) { // Wrapped variants must round-trip through errors.Is. {fmt.Errorf("upstream: %w", oidcsvc.ErrIssParamMissing), "iss_param_missing"}, {fmt.Errorf("upstream: %w", oidcsvc.ErrIssParamMismatch), "iss_param_mismatch"}, + // Audit 2026-05-11 A-2 — deactivated-user rejection is its own + // audit category (typed dispatch; wrapped variant must also + // round-trip). + {oidcsvc.ErrUserDeactivated, "user_deactivated"}, + {fmt.Errorf("upstream: %w", oidcsvc.ErrUserDeactivated), "user_deactivated"}, {errors.New("some other error"), "unspecified"}, } for _, tc := range cases { diff --git a/internal/api/handler/auth_users.go b/internal/api/handler/auth_users.go index 30aa1fa..93365bc 100644 --- a/internal/api/handler/auth_users.go +++ b/internal/api/handler/auth_users.go @@ -115,6 +115,24 @@ func (h *AuthUsersHandler) Deactivate(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusBadRequest, "missing user id") return } + // Audit 2026-05-11 A-2 — self-deactivate guard. An admin that + // deactivates their own User row immediately invalidates their next + // login (upsertUser at internal/auth/oidc/service.go rejects with + // ErrUserDeactivated); the cascade-revoke then kicks them out of the + // active session, leaving the tenant without an admin able to + // reactivate themselves. Break-glass credentials (Bundle 2 Phase 7.5) + // remain the recovery path, but the operator should not be able to + // trip the foot-gun through the standard handler. 409 (not 403) — + // the request is well-formed and authenticated; the conflict is + // between the action and the actor's own identity. Audit row records + // the rejection so an upstream SIEM can spot accidental triggers. + if caller.ActorType == domain.ActorTypeUser && caller.ActorID == id { + _ = h.audit.RecordEventWithCategory(r.Context(), caller.ActorID, caller.ActorType, "auth.user_deactivate_self_rejected", + domain.EventCategoryAuth, "user", id, + map[string]interface{}{"user_id": id, "reason": "self_deactivate_blocked"}) + Error(w, http.StatusConflict, "cannot deactivate your own account; use break-glass recovery or have another admin act") + return + } u, gerr := h.users.Get(r.Context(), id) if gerr != nil { if errors.Is(gerr, repository.ErrUserNotFound) { @@ -157,6 +175,67 @@ func (h *AuthUsersHandler) Deactivate(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// Reactivate clears users.deactivated_at, allowing the federated user +// to log in again via their OIDC provider. The next OIDC callback for +// the (provider_id, subject) tuple goes through upsertUser, which now +// passes the DeactivatedAt == nil gate, and the user's account +// information (email, display_name, last_login_at) updates normally. +// +// Audit 2026-05-11 A-2 — Reactivate is the inverse of Deactivate. The +// original MED-11 closure only shipped Deactivate; with A-2 closure the +// DeactivatedAt field now actually gates login, so the operator needs a +// supported way to undo a soft-delete without hand-editing the database. +// +// Gate: same auth.user.deactivate permission. Reactivation is the +// inverse op, not a separate privilege — anyone who can deactivate must +// be able to undo their own mistake. +// +// Idempotent: reactivating an already-active user returns 204 with no +// row write. +// +// No session-side-effect: reactivation does NOT mint a session. The +// user must complete a fresh OIDC login through their provider; sessions +// from before the deactivation stay revoked (the cascade-revoke in +// Deactivate is irreversible by design). +func (h *AuthUsersHandler) Reactivate(w http.ResponseWriter, r *http.Request) { + caller, err := callerFromRequest(r) + if err != nil { + writeAuthError(w, err) + return + } + id := r.PathValue("id") + if id == "" { + Error(w, http.StatusBadRequest, "missing user id") + return + } + u, gerr := h.users.Get(r.Context(), id) + if gerr != nil { + if errors.Is(gerr, repository.ErrUserNotFound) { + Error(w, http.StatusNotFound, "user not found") + return + } + Error(w, http.StatusInternalServerError, "could not load user") + return + } + // Idempotent: reactivating an already-active user is a no-op. + if u.DeactivatedAt == nil { + w.WriteHeader(http.StatusNoContent) + return + } + u.DeactivatedAt = nil + if uerr := h.users.Update(r.Context(), u); uerr != nil { + Error(w, http.StatusInternalServerError, "could not reactivate user") + return + } + _ = h.audit.RecordEventWithCategory(r.Context(), caller.ActorID, caller.ActorType, "auth.user_reactivated", + domain.EventCategoryAuth, "user", u.ID, + map[string]interface{}{ + "user_id": u.ID, + "oidc_provider_id": u.OIDCProviderID, + }) + w.WriteHeader(http.StatusNoContent) +} + // ============================================================================= // MED-12 — Auth runtime config read endpoint. // ============================================================================= diff --git a/internal/api/handler/auth_users_test.go b/internal/api/handler/auth_users_test.go new file mode 100644 index 0000000..3331b05 --- /dev/null +++ b/internal/api/handler/auth_users_test.go @@ -0,0 +1,297 @@ +package handler + +// Audit 2026-05-11 A-2 closure — federated-user admin handler test +// surface. Covers the self-deactivate guard, reactivate happy-path / +// idempotent / 404 branches, and the audit-event shape. + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + userdomain "github.com/certctl-io/certctl/internal/auth/user/domain" + "github.com/certctl-io/certctl/internal/domain" + "github.com/certctl-io/certctl/internal/repository" +) + +// stubFullUserRepo is a richer in-memory UserRepository than the one +// in auth_session_oidc_test.go (which always returns ErrUserNotFound +// from Get). The auth-users handler tests need round-trip semantics +// across Get / Update. +type stubFullUserRepo struct { + rows map[string]*userdomain.User + updateErr error + getErr error +} + +func newStubFullUserRepo() *stubFullUserRepo { + return &stubFullUserRepo{rows: make(map[string]*userdomain.User)} +} + +func (s *stubFullUserRepo) Get(_ context.Context, id string) (*userdomain.User, error) { + if s.getErr != nil { + return nil, s.getErr + } + if u, ok := s.rows[id]; ok { + // Defensive copy — Update path mutates the struct. + c := *u + if u.DeactivatedAt != nil { + t := *u.DeactivatedAt + c.DeactivatedAt = &t + } + return &c, nil + } + return nil, repository.ErrUserNotFound +} + +func (s *stubFullUserRepo) GetByOIDCSubject(_ context.Context, _, _ string) (*userdomain.User, error) { + return nil, repository.ErrUserNotFound +} + +func (s *stubFullUserRepo) Create(_ context.Context, u *userdomain.User) error { + s.rows[u.ID] = u + return nil +} + +func (s *stubFullUserRepo) Update(_ context.Context, u *userdomain.User) error { + if s.updateErr != nil { + return s.updateErr + } + if _, ok := s.rows[u.ID]; !ok { + return repository.ErrUserNotFound + } + // Persist the struct (defensive copy of nullable timestamp). + c := *u + if u.DeactivatedAt != nil { + t := *u.DeactivatedAt + c.DeactivatedAt = &t + } + s.rows[u.ID] = &c + return nil +} + +func (s *stubFullUserRepo) ListAll(_ context.Context, tenantID string) ([]*userdomain.User, error) { + out := make([]*userdomain.User, 0, len(s.rows)) + for _, u := range s.rows { + if tenantID == "" || u.TenantID == tenantID { + out = append(out, u) + } + } + return out, nil +} + +// stubRevoker records cascade-revoke calls. +type stubRevoker struct { + called bool + actorID string + actorType string + revokeErr error +} + +func (s *stubRevoker) RevokeAllForActor(_ context.Context, actorID, actorType string) error { + s.called = true + s.actorID = actorID + s.actorType = actorType + return s.revokeErr +} + +// stubAuditRecorder collects event actions for assertion. +type stubAuditRecorder struct { + events []string + last map[string]interface{} +} + +func (s *stubAuditRecorder) RecordEventWithCategory(_ context.Context, _ string, _ domain.ActorType, action, _, _, _ string, details map[string]interface{}) error { + s.events = append(s.events, action) + s.last = details + return nil +} + +func newSeededUser(id string, deactivatedAt *time.Time) *userdomain.User { + return &userdomain.User{ + ID: id, + TenantID: "t-default", + Email: id + "@example.test", + DisplayName: id, + OIDCSubject: "sub-" + id, + OIDCProviderID: "op-x", + LastLoginAt: time.Now().UTC(), + WebAuthnCredentials: []byte("[]"), + CreatedAt: time.Now().UTC(), + UpdatedAt: time.Now().UTC(), + DeactivatedAt: deactivatedAt, + } +} + +// ============================================================================= +// Self-deactivate guard (Audit 2026-05-11 A-2) +// ============================================================================= + +func TestAuthUsers_Deactivate_RejectsSelfDeactivate(t *testing.T) { + users := newStubFullUserRepo() + users.rows["u-admin"] = newSeededUser("u-admin", nil) + rev := &stubRevoker{} + audit := &stubAuditRecorder{} + h := NewAuthUsersHandler(users, rev, audit, "t-default") + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/auth/users/u-admin", nil) + req.SetPathValue("id", "u-admin") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Deactivate(w, req) + + if w.Code != http.StatusConflict { + t.Errorf("status = %d; want 409", w.Code) + } + // Cascade-revoke must NOT have fired. + if rev.called { + t.Error("RevokeAllForActor was called on a self-deactivate; the guard must short-circuit before cascade") + } + // Row must still be active. + row, _ := users.Get(context.Background(), "u-admin") + if row.DeactivatedAt != nil { + t.Error("user row was deactivated despite the self-deactivate guard") + } + // Audit row must record the rejection. + found := false + for _, e := range audit.events { + if e == "auth.user_deactivate_self_rejected" { + found = true + break + } + } + if !found { + t.Errorf("audit events missing self-reject marker: %v", audit.events) + } +} + +func TestAuthUsers_Deactivate_OtherUser_HappyPath(t *testing.T) { + users := newStubFullUserRepo() + users.rows["u-admin"] = newSeededUser("u-admin", nil) + users.rows["u-target"] = newSeededUser("u-target", nil) + rev := &stubRevoker{} + audit := &stubAuditRecorder{} + h := NewAuthUsersHandler(users, rev, audit, "t-default") + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/auth/users/u-target", nil) + req.SetPathValue("id", "u-target") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Deactivate(w, req) + + if w.Code != http.StatusNoContent { + t.Errorf("status = %d; want 204", w.Code) + } + if !rev.called || rev.actorID != "u-target" || rev.actorType != string(domain.ActorTypeUser) { + t.Errorf("cascade-revoke did not fire correctly: called=%v id=%q type=%q", + rev.called, rev.actorID, rev.actorType) + } + row, _ := users.Get(context.Background(), "u-target") + if row.DeactivatedAt == nil { + t.Error("user row was not soft-deleted") + } +} + +// ============================================================================= +// Reactivate (Audit 2026-05-11 A-2) +// ============================================================================= + +func TestAuthUsers_Reactivate_HappyPath(t *testing.T) { + now := time.Now().UTC() + users := newStubFullUserRepo() + users.rows["u-target"] = newSeededUser("u-target", &now) + audit := &stubAuditRecorder{} + h := NewAuthUsersHandler(users, &stubRevoker{}, audit, "t-default") + + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/users/u-target/reactivate", nil) + req.SetPathValue("id", "u-target") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Reactivate(w, req) + + if w.Code != http.StatusNoContent { + t.Errorf("status = %d; want 204", w.Code) + } + row, _ := users.Get(context.Background(), "u-target") + if row.DeactivatedAt != nil { + t.Errorf("user row still deactivated after reactivate: %v", row.DeactivatedAt) + } + // Audit row. + if len(audit.events) == 0 || audit.events[len(audit.events)-1] != "auth.user_reactivated" { + t.Errorf("audit events missing reactivate marker: %v", audit.events) + } +} + +func TestAuthUsers_Reactivate_IdempotentOnActiveUser(t *testing.T) { + users := newStubFullUserRepo() + users.rows["u-target"] = newSeededUser("u-target", nil) // already active + audit := &stubAuditRecorder{} + h := NewAuthUsersHandler(users, &stubRevoker{}, audit, "t-default") + + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/users/u-target/reactivate", nil) + req.SetPathValue("id", "u-target") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Reactivate(w, req) + + if w.Code != http.StatusNoContent { + t.Errorf("status = %d; want 204", w.Code) + } + // Idempotent — no audit event for the no-op. + for _, e := range audit.events { + if e == "auth.user_reactivated" { + t.Errorf("reactivate emitted audit row on an already-active user (no-op should be silent)") + } + } +} + +func TestAuthUsers_Reactivate_UnknownID(t *testing.T) { + users := newStubFullUserRepo() + audit := &stubAuditRecorder{} + h := NewAuthUsersHandler(users, &stubRevoker{}, audit, "t-default") + + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/users/u-missing/reactivate", nil) + req.SetPathValue("id", "u-missing") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Reactivate(w, req) + + if w.Code != http.StatusNotFound { + t.Errorf("status = %d; want 404", w.Code) + } +} + +func TestAuthUsers_Reactivate_MissingID(t *testing.T) { + h := NewAuthUsersHandler(newStubFullUserRepo(), &stubRevoker{}, &stubAuditRecorder{}, "t-default") + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/users//reactivate", nil) + // Intentionally do not SetPathValue — handler must reject the empty + // id with 400. + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Reactivate(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d; want 400", w.Code) + } +} + +func TestAuthUsers_Reactivate_UpdateError(t *testing.T) { + now := time.Now().UTC() + users := newStubFullUserRepo() + users.rows["u-target"] = newSeededUser("u-target", &now) + users.updateErr = errors.New("postgres exploded") + h := NewAuthUsersHandler(users, &stubRevoker{}, &stubAuditRecorder{}, "t-default") + + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/users/u-target/reactivate", nil) + req.SetPathValue("id", "u-target") + req = withActor(req, "u-admin", string(domain.ActorTypeUser)) + w := httptest.NewRecorder() + h.Reactivate(w, req) + + if w.Code != http.StatusInternalServerError { + t.Errorf("status = %d; want 500", w.Code) + } +} diff --git a/internal/api/router/router.go b/internal/api/router/router.go index 6c567a5..ddea6d8 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -486,11 +486,16 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { } // Audit 2026-05-10 MED-11 — federated-user admin surface. + // Audit 2026-05-11 A-2 — added reactivate route. Same permission + // gate as Deactivate (reactivation is the inverse op, not a + // separate privilege). if reg.AuthUsers != nil { r.Register("GET /api/v1/auth/users", rbacGate(reg.Checker, "auth.user.read", reg.AuthUsers.List)) r.Register("DELETE /api/v1/auth/users/{id}", rbacGate(reg.Checker, "auth.user.deactivate", reg.AuthUsers.Deactivate)) + r.Register("POST /api/v1/auth/users/{id}/reactivate", + rbacGate(reg.Checker, "auth.user.deactivate", reg.AuthUsers.Reactivate)) } // Audit 2026-05-10 MED-12 — auth runtime config read. diff --git a/internal/auth/oidc/service.go b/internal/auth/oidc/service.go index c5531db..6ee0d4c 100644 --- a/internal/auth/oidc/service.go +++ b/internal/auth/oidc/service.go @@ -317,6 +317,21 @@ var ( // Audit 2026-05-10 MED-9 closure. ErrProviderDisabled = errors.New("oidc: provider is disabled") + // ErrUserDeactivated signals the federated user row's + // `deactivated_at` is non-NULL. Audit 2026-05-11 A-2 closure — + // the deactivate flow at internal/api/handler/auth_users.go::Deactivate + // sets the column + cascade-revokes sessions, but pre-fix the OIDC + // login path never consulted it, so the very next login re-elevated + // the deactivated user. The check fires inside upsertUser before + // Update bumps last_login_at; an attempt to log in via OIDC after + // deactivation surfaces as audit `failure_category=user_deactivated` + // and the LoginPage's reason-aware error rendering shows the + // operator-friendly message. + // + // Reactivation surface: admin POSTs /api/v1/auth/users/{id}/reactivate + // (auth.user.deactivate perm) to clear the column. + ErrUserDeactivated = errors.New("oidc: user account is deactivated") + // ErrGroupsUnmapped: the user's groups don't match any of the // operator's group_role_mappings for this provider. No session // minted; audit row records auth.oidc_login_unmapped_groups. @@ -809,6 +824,19 @@ func (s *Service) upsertUser( existing, err := s.users.GetByOIDCSubject(ctx, provider.ID, subject) if err == nil { + // Audit 2026-05-11 A-2 — refuse login for deactivated users. + // The admin `DELETE /api/v1/auth/users/{id}` flow sets + // `users.deactivated_at` + cascade-revokes existing sessions. + // Without this gate the very next OIDC login mints a fresh + // session and re-elevates. Compliance-table-flipping bug. + // + // Defense order: the check runs BEFORE the email / display-name + // mutation + last_login_at bump so a deactivated user's + // last_login_at field isn't silently updated by a rejected + // login attempt (forensics value). + if existing.DeactivatedAt != nil { + return nil, ErrUserDeactivated + } // Update last_login_at, email, display_name (per the Phase 1 // mutable-field contract). existing.Email = email diff --git a/internal/auth/oidc/service_test.go b/internal/auth/oidc/service_test.go index ab77c74..1d4d843 100644 --- a/internal/auth/oidc/service_test.go +++ b/internal/auth/oidc/service_test.go @@ -934,6 +934,172 @@ func TestService_UpsertUser_UpdateExistingPath(t *testing.T) { } } +// TestService_HandleCallback_RejectsDeactivatedUser pins the A-2 +// CRIT closure. A federated user whose `users.deactivated_at` is +// non-nil must NOT be able to log in via OIDC; HandleCallback must +// return ErrUserDeactivated BEFORE the email/display-name mutation +// and last_login_at bump, and BEFORE the session mint. +// +// Audit 2026-05-11 A-2 — pre-fix, the deactivate handler set +// `users.deactivated_at` on the in-memory struct, but: (a) the SQL +// Update omitted the column so the write was a no-op; (b) the +// postgres SELECT didn't include the column so even if (a) were +// fixed scanUser returned DeactivatedAt = nil; (c) upsertUser never +// looked at DeactivatedAt. The lying-field chain meant the very +// next OIDC login re-elevated the user. This test pins the +// service-layer leg of the closure (the SQL legs are pinned by +// postgres/user_test.go). +func TestService_HandleCallback_RejectsDeactivatedUser(t *testing.T) { + idp := newMockIdP(t) + users := newStubUsers() + + // Pre-seed the user as deactivated. The default mockIdP subject + // is "test-subject"; the provider ID is "op-deact". + deactivatedAt := time.Now().UTC().Add(-1 * time.Hour) + prov := makeProvider(idp.URL(), "op-deact") + seeded := &userdomain.User{ + ID: "u-deactivated", + TenantID: prov.TenantID, + Email: "deactivated@example.com", + DisplayName: "Deactivated User", + OIDCSubject: "test-subject", + OIDCProviderID: "op-deact", + LastLoginAt: time.Now().UTC().Add(-2 * time.Hour), + WebAuthnCredentials: []byte("[]"), + CreatedAt: time.Now().UTC().Add(-24 * time.Hour), + UpdatedAt: time.Now().UTC().Add(-1 * time.Hour), + DeactivatedAt: &deactivatedAt, + } + users.byID[seeded.ID] = seeded + users.bySubject["op-deact:test-subject"] = seeded + originalLastLogin := seeded.LastLoginAt + + pl := newStubPreLogin() + mappings := &stubMappings{roleIDs: []string{"r-operator"}} + sessions := &stubSessions{} + svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") + + cookie, _, err := pl.CreatePreLogin(context.Background(), "op-deact", "deact-state", "test-nonce-fixed", + "v-deactiveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", "", "") + if err != nil { + t.Fatalf("CreatePreLogin: %v", err) + } + + res, err := svc.HandleCallback(context.Background(), cookie, "code", "deact-state", "", "10.0.0.1", "Mozilla/5.0") + if !errors.Is(err, ErrUserDeactivated) { + t.Fatalf("err = %v; want ErrUserDeactivated", err) + } + if res != nil { + t.Errorf("CallbackResult should be nil on rejection, got %+v", res) + } + + // Defense order pin — the rejected attempt must NOT have touched + // the persisted row's mutable fields. (Pre-fix the upsertUser + // path would update email + last_login_at first and only catch + // later; A-2 closure moves the check to the head of the function.) + row := users.byID["u-deactivated"] + if row.LastLoginAt != originalLastLogin { + t.Errorf("last_login_at advanced on rejected login: %v -> %v", originalLastLogin, row.LastLoginAt) + } + if row.Email != "deactivated@example.com" { + t.Errorf("email mutated on rejected login: %q", row.Email) + } + if row.DeactivatedAt == nil { + t.Error("deactivated_at was cleared on rejected login") + } +} + +// TestService_HandleCallback_AllowsReactivatedUser covers the +// Reactivate handler's wire end: after `users.deactivated_at` is +// cleared, the next OIDC login goes through the update path +// normally. Pins the inverse of TestService_HandleCallback_RejectsDeactivatedUser. +func TestService_HandleCallback_AllowsReactivatedUser(t *testing.T) { + idp := newMockIdP(t) + users := newStubUsers() + + prov := makeProvider(idp.URL(), "op-react") + seeded := &userdomain.User{ + ID: "u-reactivated", + TenantID: prov.TenantID, + Email: "reactivated@example.com", + DisplayName: "Reactivated User", + OIDCSubject: "test-subject", + OIDCProviderID: "op-react", + LastLoginAt: time.Now().UTC().Add(-2 * time.Hour), + WebAuthnCredentials: []byte("[]"), + CreatedAt: time.Now().UTC().Add(-24 * time.Hour), + UpdatedAt: time.Now().UTC().Add(-1 * time.Hour), + DeactivatedAt: nil, // active again + } + users.byID[seeded.ID] = seeded + users.bySubject["op-react:test-subject"] = seeded + + pl := newStubPreLogin() + mappings := &stubMappings{roleIDs: []string{"r-operator"}} + sessions := &stubSessions{} + svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") + + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-react", "react-state", "test-nonce-fixed", + "v-reactiveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", "", "") + res, err := svc.HandleCallback(context.Background(), cookie, "code", "react-state", "", "10.0.0.1", "Mozilla/5.0") + if err != nil { + t.Fatalf("HandleCallback after reactivation: %v", err) + } + if res == nil || res.User == nil { + t.Fatal("expected non-nil callback result after reactivation") + } + if res.User.ID != "u-reactivated" { + t.Errorf("CallbackResult.User.ID = %q; want u-reactivated", res.User.ID) + } +} + +// TestService_HandleCallback_DeactivatedUserPreservesForensics +// makes the defense-in-depth claim explicit: a rejected login does +// not bump last_login_at. This guards against a regression where +// someone "fixes" upsertUser by re-ordering the assignments to set +// LastLoginAt before checking DeactivatedAt. +func TestService_HandleCallback_DeactivatedUserPreservesForensics(t *testing.T) { + idp := newMockIdP(t) + users := newStubUsers() + + deactivatedAt := time.Now().UTC().Add(-30 * time.Minute) + prov := makeProvider(idp.URL(), "op-forensic") + seeded := &userdomain.User{ + ID: "u-forensic", + TenantID: prov.TenantID, + Email: "forensic@example.com", + DisplayName: "Forensic User", + OIDCSubject: "test-subject", + OIDCProviderID: "op-forensic", + LastLoginAt: time.Now().UTC().Add(-48 * time.Hour), + WebAuthnCredentials: []byte("[]"), + CreatedAt: time.Now().UTC().Add(-72 * time.Hour), + UpdatedAt: time.Now().UTC().Add(-30 * time.Minute), + DeactivatedAt: &deactivatedAt, + } + users.byID[seeded.ID] = seeded + users.bySubject["op-forensic:test-subject"] = seeded + frozenLastLogin := seeded.LastLoginAt + + pl := newStubPreLogin() + mappings := &stubMappings{roleIDs: []string{"r-operator"}} + sessions := &stubSessions{} + svc := NewService(&stubProviderLookup{provider: prov}, mappings, users, sessions, pl, "") + + cookie, _, _ := pl.CreatePreLogin(context.Background(), "op-forensic", "for-state", "test-nonce-fixed", + "v-forensiccccccccccccccccccccccccccccccccc", "", "") + _, err := svc.HandleCallback(context.Background(), cookie, "code", "for-state", "", "10.0.0.1", "Mozilla/5.0") + if !errors.Is(err, ErrUserDeactivated) { + t.Fatalf("err = %v; want ErrUserDeactivated", err) + } + + row := users.byID["u-forensic"] + if row.LastLoginAt != frozenLastLogin { + t.Errorf("last_login_at advanced on rejected login (forensics tainted): %v -> %v", + frozenLastLogin, row.LastLoginAt) + } +} + // TestService_ATHash_CoversAllAllowedAlgs pins the at_hash alg dispatch // for every algorithm in DefaultAllowedAlgs. func TestService_ATHash_CoversAllAllowedAlgs(t *testing.T) { diff --git a/internal/repository/postgres/user.go b/internal/repository/postgres/user.go index 95a9ad2..ec3358f 100644 --- a/internal/repository/postgres/user.go +++ b/internal/repository/postgres/user.go @@ -23,19 +23,33 @@ func NewUserRepository(db *sql.DB) *UserRepository { return &UserRepository{db: db} } +// Audit 2026-05-11 A-2 — deactivated_at column added in migration +// 000045 (MED-11 foundation) but pre-fix never read here. The +// federated-user soft-delete flow at +// internal/api/handler/auth_users.go::Deactivate set the column on +// Update, but Get / GetByOIDCSubject / ListAll all returned User +// with zero-value DeactivatedAt regardless. The OIDC login path +// trusts the returned struct, so a deactivated user's next login +// re-elevated them. Adding the column to userColumns + scanUser +// closes the read leg; service.go's upsertUser closes the enforce leg. const userColumns = `id, tenant_id, email, display_name, oidc_subject, oidc_provider_id, last_login_at, webauthn_credentials, - created_at, updated_at` + created_at, updated_at, deactivated_at` func scanUser(row interface{ Scan(...interface{}) error }) (*userdomain.User, error) { var u userdomain.User + var deactivatedAt sql.NullTime if err := row.Scan( &u.ID, &u.TenantID, &u.Email, &u.DisplayName, &u.OIDCSubject, &u.OIDCProviderID, &u.LastLoginAt, &u.WebAuthnCredentials, - &u.CreatedAt, &u.UpdatedAt, + &u.CreatedAt, &u.UpdatedAt, &deactivatedAt, ); err != nil { return nil, err } + if deactivatedAt.Valid { + t := deactivatedAt.Time + u.DeactivatedAt = &t + } return &u, nil } @@ -74,14 +88,26 @@ func (r *UserRepository) GetByOIDCSubject(ctx context.Context, providerID, subje // Create persists a new user. Translates SQLSTATE 23505 into // ErrUserDuplicateOIDCSubject (the unique constraint on // (oidc_provider_id, oidc_subject)). +// +// Audit 2026-05-11 A-2 — deactivated_at written explicitly. New rows +// pre-fix had deactivated_at NULL by schema default; the explicit +// write makes forward-compat with future seed-data paths that +// pre-populate the column (e.g. migration of an external user roster +// where some entries land deactivated). nil → NULL via sql.NullTime. func (r *UserRepository) Create(ctx context.Context, u *userdomain.User) error { + var deactivatedAt sql.NullTime + if u.DeactivatedAt != nil { + deactivatedAt = sql.NullTime{Time: *u.DeactivatedAt, Valid: true} + } _, err := r.db.ExecContext(ctx, ` INSERT INTO users ( id, tenant_id, email, display_name, oidc_subject, - oidc_provider_id, last_login_at, webauthn_credentials - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + oidc_provider_id, last_login_at, webauthn_credentials, + deactivated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, u.ID, u.TenantID, u.Email, u.DisplayName, u.OIDCSubject, - u.OIDCProviderID, u.LastLoginAt, u.WebAuthnCredentials) + u.OIDCProviderID, u.LastLoginAt, u.WebAuthnCredentials, + deactivatedAt) if err != nil { var pqErr *pq.Error if errors.As(err, &pqErr) && pqErr.Code == "23505" { @@ -93,18 +119,31 @@ func (r *UserRepository) Create(ctx context.Context, u *userdomain.User) error { } // Update writes the mutable fields (email, display_name, last_login_at, -// webauthn_credentials) back to the row. Immutable: id, tenant_id, -// oidc_subject, oidc_provider_id, created_at. updated_at = NOW(). +// webauthn_credentials, deactivated_at) back to the row. Immutable: +// id, tenant_id, oidc_subject, oidc_provider_id, created_at. +// updated_at = NOW(). +// +// Audit 2026-05-11 A-2 — deactivated_at is now in the mutable set so +// the federated-user soft-delete flow at +// internal/api/handler/auth_users.go::Deactivate persists. Pre-fix the +// Update SQL omitted it; the handler set u.DeactivatedAt = now on the +// in-memory struct, called Update, the SQL ignored the field, and the +// row was unchanged. nil DeactivatedAt → NULL (supports reactivation). func (r *UserRepository) Update(ctx context.Context, u *userdomain.User) error { + var deactivatedAt sql.NullTime + if u.DeactivatedAt != nil { + deactivatedAt = sql.NullTime{Time: *u.DeactivatedAt, Valid: true} + } res, err := r.db.ExecContext(ctx, ` UPDATE users SET email = $2, display_name = $3, last_login_at = $4, webauthn_credentials = $5, + deactivated_at = $6, updated_at = NOW() WHERE id = $1`, - u.ID, u.Email, u.DisplayName, u.LastLoginAt, u.WebAuthnCredentials) + u.ID, u.Email, u.DisplayName, u.LastLoginAt, u.WebAuthnCredentials, deactivatedAt) if err != nil { return fmt.Errorf("users update: %w", err) } diff --git a/internal/repository/postgres/user_test.go b/internal/repository/postgres/user_test.go index 3c6dcc0..5c5a22c 100644 --- a/internal/repository/postgres/user_test.go +++ b/internal/repository/postgres/user_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" userdomain "github.com/certctl-io/certctl/internal/auth/user/domain" "github.com/certctl-io/certctl/internal/repository" @@ -193,6 +194,178 @@ func TestUserRepository_ListAll(t *testing.T) { } } +// TestUserRepository_DeactivatedAt_RoundTrip pins the A-2 closure at +// the SQL layer. Pre-fix scanUser did not include deactivated_at in +// userColumns, Update did not write it, and Create did not write it. +// Result: a non-nil DeactivatedAt set in the in-memory User by the +// handler was lost on persist + always nil on read. This test +// exercises both legs. +// +// Audit 2026-05-11 A-2 — round-trip a non-nil DeactivatedAt through +// Update and verify Get + GetByOIDCSubject + ListAll all return it +// non-nil. Then clear it (reactivate path) and verify the nil +// round-trips back. +func TestUserRepository_DeactivatedAt_RoundTrip(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + providerRepo := postgres.NewOIDCProviderRepository(db) + userRepo := postgres.NewUserRepository(db) + ctx := context.Background() + + p := newValidProvider("deact-rt") + if err := providerRepo.Create(ctx, p); err != nil { + t.Fatalf("Create provider: %v", err) + } + u := newValidUser("deactivated-user", p.ID) + if err := userRepo.Create(ctx, u); err != nil { + t.Fatalf("Create user: %v", err) + } + + // Sanity: a freshly-created row reads back nil. + got, err := userRepo.Get(ctx, u.ID) + if err != nil { + t.Fatalf("Get (fresh): %v", err) + } + if got.DeactivatedAt != nil { + t.Errorf("freshly-created user has non-nil DeactivatedAt: %v", got.DeactivatedAt) + } + + // Soft-delete: set DeactivatedAt and Update. + now := time.Now().UTC().Truncate(time.Microsecond) // pg precision + got.DeactivatedAt = &now + if err := userRepo.Update(ctx, got); err != nil { + t.Fatalf("Update (deactivate): %v", err) + } + + // Read via Get. + rb, err := userRepo.Get(ctx, u.ID) + if err != nil { + t.Fatalf("Get (post-deactivate): %v", err) + } + if rb.DeactivatedAt == nil { + t.Fatal("Get returned nil DeactivatedAt after Update set it (A-2 regression)") + } + if !rb.DeactivatedAt.Equal(now) { + t.Errorf("Get round-trip DeactivatedAt mismatch: got %v want %v", *rb.DeactivatedAt, now) + } + + // Read via GetByOIDCSubject. + rs, err := userRepo.GetByOIDCSubject(ctx, p.ID, u.OIDCSubject) + if err != nil { + t.Fatalf("GetByOIDCSubject (post-deactivate): %v", err) + } + if rs.DeactivatedAt == nil { + t.Error("GetByOIDCSubject returned nil DeactivatedAt after Update set it (A-2 regression — OIDC login path leak)") + } + + // Read via ListAll. + rows, err := userRepo.ListAll(ctx, "t-default") + if err != nil { + t.Fatalf("ListAll: %v", err) + } + if len(rows) != 1 || rows[0].DeactivatedAt == nil { + t.Errorf("ListAll: expected 1 row with non-nil DeactivatedAt; got %d rows, first DeactivatedAt=%v", + len(rows), func() interface{} { + if len(rows) == 0 { + return "no rows" + } + return rows[0].DeactivatedAt + }()) + } + + // Reactivate: clear DeactivatedAt and verify the nil round-trips. + rb.DeactivatedAt = nil + if err := userRepo.Update(ctx, rb); err != nil { + t.Fatalf("Update (reactivate): %v", err) + } + rfin, err := userRepo.Get(ctx, u.ID) + if err != nil { + t.Fatalf("Get (post-reactivate): %v", err) + } + if rfin.DeactivatedAt != nil { + t.Errorf("Get returned non-nil DeactivatedAt after reactivate Update cleared it: %v", *rfin.DeactivatedAt) + } +} + +// TestUserRepository_DeactivatedAt_CreateWritesNullForActive pins +// the Create path's behavior for the common case (active user). +// Pre-fix Create omitted deactivated_at entirely so the column took +// the schema default (NULL). Now Create writes it explicitly; the +// observable behavior is unchanged for nil, but a regression would +// flip new users to deactivated. +// +// Audit 2026-05-11 A-2. +func TestUserRepository_DeactivatedAt_CreateWritesNullForActive(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + providerRepo := postgres.NewOIDCProviderRepository(db) + userRepo := postgres.NewUserRepository(db) + ctx := context.Background() + + p := newValidProvider("create-nil") + if err := providerRepo.Create(ctx, p); err != nil { + t.Fatalf("Create provider: %v", err) + } + u := newValidUser("active-user", p.ID) + u.DeactivatedAt = nil // explicit: new user is active + if err := userRepo.Create(ctx, u); err != nil { + t.Fatalf("Create: %v", err) + } + + got, err := userRepo.Get(ctx, u.ID) + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.DeactivatedAt != nil { + t.Errorf("active user has non-nil DeactivatedAt after Create: %v", *got.DeactivatedAt) + } +} + +// TestUserRepository_DeactivatedAt_CreatePersistsPreDeactivated +// covers the forward-compat path where a future seed-data flow +// (e.g. migration of an external user roster where some entries +// land deactivated) pre-populates the column on insert. Pre-fix +// Create omitted the column entirely, so this case wasn't +// representable; the A-2 closure makes the explicit write part of +// the Create contract. +// +// Audit 2026-05-11 A-2. +func TestUserRepository_DeactivatedAt_CreatePersistsPreDeactivated(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + providerRepo := postgres.NewOIDCProviderRepository(db) + userRepo := postgres.NewUserRepository(db) + ctx := context.Background() + + p := newValidProvider("create-deact") + if err := providerRepo.Create(ctx, p); err != nil { + t.Fatalf("Create provider: %v", err) + } + u := newValidUser("seed-deactivated", p.ID) + pre := time.Now().UTC().Add(-1 * time.Hour).Truncate(time.Microsecond) + u.DeactivatedAt = &pre + if err := userRepo.Create(ctx, u); err != nil { + t.Fatalf("Create: %v", err) + } + + got, err := userRepo.Get(ctx, u.ID) + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.DeactivatedAt == nil { + t.Fatal("DeactivatedAt nil after Create persisted a pre-deactivated user") + } + if !got.DeactivatedAt.Equal(pre) { + t.Errorf("Create round-trip DeactivatedAt: got %v want %v", *got.DeactivatedAt, pre) + } +} + // TestUserRepository_DeletingProviderRefusedWhenUsersReference complements // the OIDCProviderRepository test of the same shape; pinning both ends // of the FK ON DELETE RESTRICT contract. diff --git a/web/src/api/client.ts b/web/src/api/client.ts index e574573..db5df02 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -341,6 +341,10 @@ export const authListUsers = (providerID?: string) => { }; export const authDeactivateUser = (id: string) => fetchJSON(`${BASE}/auth/users/${id}`, { method: 'DELETE' }); +// Audit 2026-05-11 A-2 — inverse of authDeactivateUser. Clears +// users.deactivated_at; next OIDC login proceeds normally. +export const authReactivateUser = (id: string) => + fetchJSON(`${BASE}/auth/users/${id}/reactivate`, { method: 'POST' }); // MED-12 — runtime config. export const authRuntimeConfig = () => diff --git a/web/src/pages/auth/UsersPage.tsx b/web/src/pages/auth/UsersPage.tsx index 95549f2..cbead69 100644 --- a/web/src/pages/auth/UsersPage.tsx +++ b/web/src/pages/auth/UsersPage.tsx @@ -1,6 +1,6 @@ import { useState } from 'react'; import { useQuery, useQueryClient } from '@tanstack/react-query'; -import { authListUsers, authDeactivateUser, type AuthUser } from '../../api/client'; +import { authListUsers, authDeactivateUser, authReactivateUser, type AuthUser } from '../../api/client'; import PageHeader from '../../components/PageHeader'; import ErrorState from '../../components/ErrorState'; @@ -47,6 +47,29 @@ export default function UsersPage() { } } + // Audit 2026-05-11 A-2 — Reactivate inverse. Clears deactivated_at; + // the next OIDC login under the same (provider, subject) tuple + // proceeds normally. Sessions revoked at deactivation stay revoked + // (the cascade is irreversible by design — the user must complete + // a fresh login). + async function reactivate(u: AuthUser) { + if (!confirm(`Reactivate user ${u.email} (${u.id})?\n\n` + + `This clears deactivated_at. The user can OIDC-login again. ` + + `Previously-revoked sessions stay revoked.`)) { + return; + } + setPending(u.id); + setErr(null); + try { + await authReactivateUser(u.id); + await qc.invalidateQueries({ queryKey: ['auth', 'users'] }); + } catch (e) { + setErr(e instanceof Error ? e.message : String(e)); + } finally { + setPending(null); + } + } + return (
@@ -97,6 +120,15 @@ export default function UsersPage() { {pending === u.id ? 'Deactivating…' : 'Deactivate'} )} + {deactivated && ( + + )} );