mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-12 15:48:56 +00:00
Merge Fix 02 (CRIT A-2): close MED-11 lying field — DeactivatedAt loaded + enforced on login
This commit is contained in:
@@ -24,6 +24,33 @@
|
|||||||
|
|
||||||
### Security (BREAKING)
|
### 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).**
|
- **`__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
|
The session cookie, CSRF cookie, and OIDC pre-login cookie are renamed from
|
||||||
`certctl_session` / `certctl_csrf` / `certctl_oidc_pending` to
|
`certctl_session` / `certctl_csrf` / `certctl_oidc_pending` to
|
||||||
|
|||||||
@@ -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, "",
|
h.recordAudit(r.Context(), "auth.oidc_provider_tested", caller.ActorID, caller.ActorType, "",
|
||||||
map[string]interface{}{
|
map[string]interface{}{
|
||||||
"issuer_url": req.IssuerURL,
|
"issuer_url": req.IssuerURL,
|
||||||
"discovery_succeeded": res.DiscoverySucceeded,
|
"discovery_succeeded": res.DiscoverySucceeded,
|
||||||
"jwks_reachable": res.JWKSReachable,
|
"jwks_reachable": res.JWKSReachable,
|
||||||
"iss_param_supported": res.IssParamSupported,
|
"iss_param_supported": res.IssParamSupported,
|
||||||
"error_count": len(res.Errors),
|
"error_count": len(res.Errors),
|
||||||
})
|
})
|
||||||
writeJSON(w, http.StatusOK, res)
|
writeJSON(w, http.StatusOK, res)
|
||||||
}
|
}
|
||||||
@@ -1267,6 +1267,14 @@ func classifyOIDCFailure(err error) string {
|
|||||||
return "prelogin_ua_mismatch"
|
return "prelogin_ua_mismatch"
|
||||||
case errors.Is(err, oidcsvc.ErrPreLoginIPMismatch):
|
case errors.Is(err, oidcsvc.ErrPreLoginIPMismatch):
|
||||||
return "prelogin_ip_mismatch"
|
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())
|
msg := strings.ToLower(err.Error())
|
||||||
switch {
|
switch {
|
||||||
|
|||||||
@@ -1217,6 +1217,11 @@ func TestClassifyOIDCFailure(t *testing.T) {
|
|||||||
// Wrapped variants must round-trip through errors.Is.
|
// Wrapped variants must round-trip through errors.Is.
|
||||||
{fmt.Errorf("upstream: %w", oidcsvc.ErrIssParamMissing), "iss_param_missing"},
|
{fmt.Errorf("upstream: %w", oidcsvc.ErrIssParamMissing), "iss_param_missing"},
|
||||||
{fmt.Errorf("upstream: %w", oidcsvc.ErrIssParamMismatch), "iss_param_mismatch"},
|
{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"},
|
{errors.New("some other error"), "unspecified"},
|
||||||
}
|
}
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
|
|||||||
@@ -115,6 +115,24 @@ func (h *AuthUsersHandler) Deactivate(w http.ResponseWriter, r *http.Request) {
|
|||||||
Error(w, http.StatusBadRequest, "missing user id")
|
Error(w, http.StatusBadRequest, "missing user id")
|
||||||
return
|
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)
|
u, gerr := h.users.Get(r.Context(), id)
|
||||||
if gerr != nil {
|
if gerr != nil {
|
||||||
if errors.Is(gerr, repository.ErrUserNotFound) {
|
if errors.Is(gerr, repository.ErrUserNotFound) {
|
||||||
@@ -157,6 +175,67 @@ func (h *AuthUsersHandler) Deactivate(w http.ResponseWriter, r *http.Request) {
|
|||||||
w.WriteHeader(http.StatusNoContent)
|
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.
|
// MED-12 — Auth runtime config read endpoint.
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -486,11 +486,16 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Audit 2026-05-10 MED-11 — federated-user admin surface.
|
// 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 {
|
if reg.AuthUsers != nil {
|
||||||
r.Register("GET /api/v1/auth/users",
|
r.Register("GET /api/v1/auth/users",
|
||||||
rbacGate(reg.Checker, "auth.user.read", reg.AuthUsers.List))
|
rbacGate(reg.Checker, "auth.user.read", reg.AuthUsers.List))
|
||||||
r.Register("DELETE /api/v1/auth/users/{id}",
|
r.Register("DELETE /api/v1/auth/users/{id}",
|
||||||
rbacGate(reg.Checker, "auth.user.deactivate", reg.AuthUsers.Deactivate))
|
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.
|
// Audit 2026-05-10 MED-12 — auth runtime config read.
|
||||||
|
|||||||
@@ -317,6 +317,21 @@ var (
|
|||||||
// Audit 2026-05-10 MED-9 closure.
|
// Audit 2026-05-10 MED-9 closure.
|
||||||
ErrProviderDisabled = errors.New("oidc: provider is disabled")
|
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
|
// ErrGroupsUnmapped: the user's groups don't match any of the
|
||||||
// operator's group_role_mappings for this provider. No session
|
// operator's group_role_mappings for this provider. No session
|
||||||
// minted; audit row records auth.oidc_login_unmapped_groups.
|
// 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)
|
existing, err := s.users.GetByOIDCSubject(ctx, provider.ID, subject)
|
||||||
if err == nil {
|
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
|
// Update last_login_at, email, display_name (per the Phase 1
|
||||||
// mutable-field contract).
|
// mutable-field contract).
|
||||||
existing.Email = email
|
existing.Email = email
|
||||||
|
|||||||
@@ -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
|
// TestService_ATHash_CoversAllAllowedAlgs pins the at_hash alg dispatch
|
||||||
// for every algorithm in DefaultAllowedAlgs.
|
// for every algorithm in DefaultAllowedAlgs.
|
||||||
func TestService_ATHash_CoversAllAllowedAlgs(t *testing.T) {
|
func TestService_ATHash_CoversAllAllowedAlgs(t *testing.T) {
|
||||||
|
|||||||
@@ -23,19 +23,33 @@ func NewUserRepository(db *sql.DB) *UserRepository {
|
|||||||
return &UserRepository{db: db}
|
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,
|
const userColumns = `id, tenant_id, email, display_name, oidc_subject,
|
||||||
oidc_provider_id, last_login_at, webauthn_credentials,
|
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) {
|
func scanUser(row interface{ Scan(...interface{}) error }) (*userdomain.User, error) {
|
||||||
var u userdomain.User
|
var u userdomain.User
|
||||||
|
var deactivatedAt sql.NullTime
|
||||||
if err := row.Scan(
|
if err := row.Scan(
|
||||||
&u.ID, &u.TenantID, &u.Email, &u.DisplayName, &u.OIDCSubject,
|
&u.ID, &u.TenantID, &u.Email, &u.DisplayName, &u.OIDCSubject,
|
||||||
&u.OIDCProviderID, &u.LastLoginAt, &u.WebAuthnCredentials,
|
&u.OIDCProviderID, &u.LastLoginAt, &u.WebAuthnCredentials,
|
||||||
&u.CreatedAt, &u.UpdatedAt,
|
&u.CreatedAt, &u.UpdatedAt, &deactivatedAt,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
if deactivatedAt.Valid {
|
||||||
|
t := deactivatedAt.Time
|
||||||
|
u.DeactivatedAt = &t
|
||||||
|
}
|
||||||
return &u, nil
|
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
|
// Create persists a new user. Translates SQLSTATE 23505 into
|
||||||
// ErrUserDuplicateOIDCSubject (the unique constraint on
|
// ErrUserDuplicateOIDCSubject (the unique constraint on
|
||||||
// (oidc_provider_id, oidc_subject)).
|
// (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 {
|
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, `
|
_, err := r.db.ExecContext(ctx, `
|
||||||
INSERT INTO users (
|
INSERT INTO users (
|
||||||
id, tenant_id, email, display_name, oidc_subject,
|
id, tenant_id, email, display_name, oidc_subject,
|
||||||
oidc_provider_id, last_login_at, webauthn_credentials
|
oidc_provider_id, last_login_at, webauthn_credentials,
|
||||||
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`,
|
deactivated_at
|
||||||
|
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||||
u.ID, u.TenantID, u.Email, u.DisplayName, u.OIDCSubject,
|
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 {
|
if err != nil {
|
||||||
var pqErr *pq.Error
|
var pqErr *pq.Error
|
||||||
if errors.As(err, &pqErr) && pqErr.Code == "23505" {
|
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,
|
// Update writes the mutable fields (email, display_name, last_login_at,
|
||||||
// webauthn_credentials) back to the row. Immutable: id, tenant_id,
|
// webauthn_credentials, deactivated_at) back to the row. Immutable:
|
||||||
// oidc_subject, oidc_provider_id, created_at. updated_at = NOW().
|
// 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 {
|
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, `
|
res, err := r.db.ExecContext(ctx, `
|
||||||
UPDATE users SET
|
UPDATE users SET
|
||||||
email = $2,
|
email = $2,
|
||||||
display_name = $3,
|
display_name = $3,
|
||||||
last_login_at = $4,
|
last_login_at = $4,
|
||||||
webauthn_credentials = $5,
|
webauthn_credentials = $5,
|
||||||
|
deactivated_at = $6,
|
||||||
updated_at = NOW()
|
updated_at = NOW()
|
||||||
WHERE id = $1`,
|
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 {
|
if err != nil {
|
||||||
return fmt.Errorf("users update: %w", err)
|
return fmt.Errorf("users update: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
userdomain "github.com/certctl-io/certctl/internal/auth/user/domain"
|
userdomain "github.com/certctl-io/certctl/internal/auth/user/domain"
|
||||||
"github.com/certctl-io/certctl/internal/repository"
|
"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
|
// TestUserRepository_DeletingProviderRefusedWhenUsersReference complements
|
||||||
// the OIDCProviderRepository test of the same shape; pinning both ends
|
// the OIDCProviderRepository test of the same shape; pinning both ends
|
||||||
// of the FK ON DELETE RESTRICT contract.
|
// of the FK ON DELETE RESTRICT contract.
|
||||||
|
|||||||
@@ -341,6 +341,10 @@ export const authListUsers = (providerID?: string) => {
|
|||||||
};
|
};
|
||||||
export const authDeactivateUser = (id: string) =>
|
export const authDeactivateUser = (id: string) =>
|
||||||
fetchJSON<unknown>(`${BASE}/auth/users/${id}`, { method: 'DELETE' });
|
fetchJSON<unknown>(`${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<unknown>(`${BASE}/auth/users/${id}/reactivate`, { method: 'POST' });
|
||||||
|
|
||||||
// MED-12 — runtime config.
|
// MED-12 — runtime config.
|
||||||
export const authRuntimeConfig = () =>
|
export const authRuntimeConfig = () =>
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { useState } from 'react';
|
import { useState } from 'react';
|
||||||
import { useQuery, useQueryClient } from '@tanstack/react-query';
|
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 PageHeader from '../../components/PageHeader';
|
||||||
import ErrorState from '../../components/ErrorState';
|
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 (
|
return (
|
||||||
<div>
|
<div>
|
||||||
<PageHeader title="Federated Users" subtitle="One row per (oidc_provider_id, oidc_subject) tuple." />
|
<PageHeader title="Federated Users" subtitle="One row per (oidc_provider_id, oidc_subject) tuple." />
|
||||||
@@ -97,6 +120,15 @@ export default function UsersPage() {
|
|||||||
{pending === u.id ? 'Deactivating…' : 'Deactivate'}
|
{pending === u.id ? 'Deactivating…' : 'Deactivate'}
|
||||||
</button>
|
</button>
|
||||||
)}
|
)}
|
||||||
|
{deactivated && (
|
||||||
|
<button
|
||||||
|
onClick={() => reactivate(u)}
|
||||||
|
disabled={pending === u.id}
|
||||||
|
style={{ padding: '4px 12px' }}
|
||||||
|
>
|
||||||
|
{pending === u.id ? 'Reactivating…' : 'Reactivate'}
|
||||||
|
</button>
|
||||||
|
)}
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in New Issue
Block a user