mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-09 19:28:58 +00:00
shankar0123
1697845493
Closes HIGH-1 + HIGH-2 of the 2026-05-10 audit.
HIGH-1: breakglass.Service.SetPassword and RemoveCredential now call
sessions.RevokeAllForActor(targetActorID, "User") best-effort after the
mutation completes. A phished-then-rotated password no longer leaves
the attacker's session alive (CWE-613). Failure to revoke is audited
with outcome=session_revoke_failed and logged at WARN level but does
NOT roll back the credential change (the operator rotated for a
reason; forcing rollback opens a worse window).
- breakglass.SessionMinter interface extended with RevokeAllForActor.
- cmd/server/main.go::breakglassSessionMinterAdapter gains the bridge
to session.Service.RevokeAllForActor.
- stubSessions in service_test.go tracks revokeAllIDs / revokeAllTypes
/ revokeAllErr.
- Three regression tests:
- TestService_SetPassword_RevokesExistingSessions
- TestService_RemoveCredential_RevokesExistingSessions
- TestService_SetPassword_RevokeFailureDoesNotRollback
HIGH-2: New session.Service.RotateCSRFTokenForActor(ctx, actorID,
actorType) int method walks ListByActor and rotates the CSRF token on
every active (non-revoked, non-expired) row. Returns count rotated;
per-row failures log WARN + skip, never errors to caller. New
handler.CSRFRotator interface + AuthHandler.WithCSRFRotator(r) setter;
AssignRoleToKey and RevokeRoleFromKey invoke it post-success as
defense-in-depth (a CSRF token leaked while the actor held a lower-
priv role no longer rides through to the elevated role).
- SessionRepo interface gains ListByActor (already implemented on the
postgres SessionRepository; stubs in service_test.go + bench_test.go
updated to match).
- cmd/server/main.go calls .WithCSRFRotator(sessionService) on the
AuthHandler.
- Two regression tests:
- TestRotateCSRFTokenForActor_RotatesAllActiveRows (asserts revoked /
expired / other-actor rows are skipped)
- TestRotateCSRFTokenForActor_NoSessionsReturnsZero
Verification gate green: gofmt clean, go vet clean, go test -short
-count=1 ./internal/auth/breakglass/ ./internal/auth/session/
./internal/api/handler/ ./internal/api/router/ ./cmd/server/
./internal/domain/auth/ — all pass.
CRIT-1..CRIT-5 + HIGH-1 + HIGH-2 of the 2026-05-10 audit now closed
on this branch. Spec at
cowork/auth-bundles-fixes-2026-05-10/06-high-1-2-revoke-and-rotate.md.
Refs: cowork/auth-bundles-audit-2026-05-10.md HIGH-1 HIGH-2
75 lines
2.9 KiB
Go
75 lines
2.9 KiB
Go
package breakglass
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"testing"
|
|
|
|
bgdomain "github.com/certctl-io/certctl/internal/auth/breakglass/domain"
|
|
)
|
|
|
|
// Audit 2026-05-10 HIGH-1 closure — regression tests pinning the
|
|
// wire from break-glass mutations to SessionMinter.RevokeAllForActor.
|
|
// Pre-fix, SetPassword and RemoveCredential rotated the password /
|
|
// removed the row but left active sessions for the target actor alive
|
|
// (CWE-613). The fix calls RevokeAllForActor(targetActorID, "User")
|
|
// best-effort after each mutation.
|
|
|
|
func TestService_SetPassword_RevokesExistingSessions(t *testing.T) {
|
|
svc, repo, _, sess := newSvc(t, true)
|
|
// Seed: target actor already has a break-glass credential.
|
|
repo.rows["u-target"] = &bgdomain.BreakglassCredential{
|
|
ID: "bg-target", TenantID: "t-default", ActorID: "u-target", PasswordHash: "$argon2id$old",
|
|
}
|
|
|
|
if _, err := svc.SetPassword(context.Background(), "u-admin", "u-target", "new-password-12345"); err != nil {
|
|
t.Fatalf("SetPassword: %v", err)
|
|
}
|
|
|
|
if len(sess.revokeAllIDs) != 1 || sess.revokeAllIDs[0] != "u-target" {
|
|
t.Errorf("expected RevokeAllForActor(u-target); got %v", sess.revokeAllIDs)
|
|
}
|
|
if len(sess.revokeAllTypes) != 1 || sess.revokeAllTypes[0] != "User" {
|
|
t.Errorf("expected actor_type=User; got %v", sess.revokeAllTypes)
|
|
}
|
|
}
|
|
|
|
func TestService_RemoveCredential_RevokesExistingSessions(t *testing.T) {
|
|
svc, repo, _, sess := newSvc(t, true)
|
|
repo.rows["u-target"] = &bgdomain.BreakglassCredential{
|
|
ID: "bg-target", TenantID: "t-default", ActorID: "u-target", PasswordHash: "$argon2id$x",
|
|
}
|
|
|
|
if err := svc.RemoveCredential(context.Background(), "u-admin", "u-target"); err != nil {
|
|
t.Fatalf("RemoveCredential: %v", err)
|
|
}
|
|
if len(sess.revokeAllIDs) != 1 || sess.revokeAllIDs[0] != "u-target" {
|
|
t.Errorf("expected RevokeAllForActor(u-target); got %v", sess.revokeAllIDs)
|
|
}
|
|
}
|
|
|
|
// TestService_SetPassword_RevokeFailureDoesNotRollback pins the
|
|
// best-effort contract: if RevokeAllForActor errors, the password
|
|
// rotation itself still SUCCEEDS (the operator rotated for a reason,
|
|
// forcing rollback opens a worse window). The failure is logged +
|
|
// audited but not surfaced to the caller.
|
|
func TestService_SetPassword_RevokeFailureDoesNotRollback(t *testing.T) {
|
|
svc, repo, _, sess := newSvc(t, true)
|
|
repo.rows["u-target"] = &bgdomain.BreakglassCredential{
|
|
ID: "bg-target", TenantID: "t-default", ActorID: "u-target", PasswordHash: "$argon2id$old",
|
|
}
|
|
sess.revokeAllErr = errors.New("transient db reset")
|
|
|
|
res, err := svc.SetPassword(context.Background(), "u-admin", "u-target", "new-password-12345")
|
|
if err != nil {
|
|
t.Fatalf("SetPassword should succeed even when revoke fails; got %v", err)
|
|
}
|
|
if res == nil || res.ActorID != "u-target" {
|
|
t.Fatalf("expected result with actor_id=u-target; got %+v", res)
|
|
}
|
|
// RevokeAllForActor WAS attempted.
|
|
if len(sess.revokeAllIDs) != 1 {
|
|
t.Errorf("expected RevokeAllForActor attempted; got %v", sess.revokeAllIDs)
|
|
}
|
|
}
|