mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:51:33 +00:00
78485f7429
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.
298 lines
9.3 KiB
Go
298 lines
9.3 KiB
Go
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)
|
|
}
|
|
}
|