diff --git a/AUDIT.md b/AUDIT.md index 2df3ecc..71bfa52 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -234,7 +234,7 @@ The REST `handleTokenIssue` and gRPC `IssueServiceToken` both revoke the existin | No | F-12 | LOW | No username length/charset validation | Small | | No | F-13 | LOW | No minimum password length enforcement | Small | | No | F-14 | LOW | Passphrase string not zeroed after KDF | Small | -| No | F-16 | LOW | UI system token issuance skips old token revocation | Small | +| Yes | F-16 | LOW | UI system token issuance skips old token revocation | Small | | No | F-15 | INFO | Bearer prefix check inconsistency | Trivial | --- diff --git a/internal/db/db_test.go b/internal/db/db_test.go index 95e19fa..8067055 100644 --- a/internal/db/db_test.go +++ b/internal/db/db_test.go @@ -375,6 +375,76 @@ func TestRenewTokenAtomic(t *testing.T) { } } +// TestSystemTokenRotationRevokesOld verifies the rotation pattern used by +// handleIssueSystemToken (F-16): issuing a second system token revokes the first. +func TestSystemTokenRotationRevokesOld(t *testing.T) { + db := openTestDB(t) + acct, err := db.CreateAccount("svc", model.AccountTypeSystem, "hash") + if err != nil { + t.Fatalf("CreateAccount: %v", err) + } + + now := time.Now().UTC() + exp := now.Add(time.Hour) + + // Issue first token. + jti1 := "sys-tok-1" + if err := db.TrackToken(jti1, acct.ID, now, exp); err != nil { + t.Fatalf("TrackToken jti1: %v", err) + } + if err := db.SetSystemToken(acct.ID, jti1, exp); err != nil { + t.Fatalf("SetSystemToken jti1: %v", err) + } + + // Simulate token rotation: look up existing, revoke it, issue second. + existing, err := db.GetSystemToken(acct.ID) + if err != nil { + t.Fatalf("GetSystemToken: %v", err) + } + if existing.JTI != jti1 { + t.Errorf("expected JTI %q, got %q", jti1, existing.JTI) + } + _ = db.RevokeToken(existing.JTI, "rotated") + + jti2 := "sys-tok-2" + if err := db.TrackToken(jti2, acct.ID, now, exp); err != nil { + t.Fatalf("TrackToken jti2: %v", err) + } + if err := db.SetSystemToken(acct.ID, jti2, exp); err != nil { + t.Fatalf("SetSystemToken jti2: %v", err) + } + + // Old token must be revoked. + old, err := db.GetTokenRecord(jti1) + if err != nil { + t.Fatalf("GetTokenRecord jti1: %v", err) + } + if !old.IsRevoked() { + t.Error("old system token should be revoked after rotation") + } + if old.RevokeReason != "rotated" { + t.Errorf("revoke reason = %q, want %q", old.RevokeReason, "rotated") + } + + // New token must be active. + newRec, err := db.GetTokenRecord(jti2) + if err != nil { + t.Fatalf("GetTokenRecord jti2: %v", err) + } + if newRec.IsRevoked() { + t.Error("new system token should not be revoked after rotation") + } + + // GetSystemToken must return the new JTI. + cur, err := db.GetSystemToken(acct.ID) + if err != nil { + t.Fatalf("GetSystemToken after rotation: %v", err) + } + if cur.JTI != jti2 { + t.Errorf("current system token JTI = %q, want %q", cur.JTI, jti2) + } +} + func TestRevokeAllUserTokens(t *testing.T) { db := openTestDB(t) acct, err := db.CreateAccount("ivan", model.AccountTypeHuman, "hash") diff --git a/internal/ui/handlers_accounts.go b/internal/ui/handlers_accounts.go index a96f771..0b04869 100644 --- a/internal/ui/handlers_accounts.go +++ b/internal/ui/handlers_accounts.go @@ -349,6 +349,14 @@ func (u *UIServer) handleIssueSystemToken(w http.ResponseWriter, r *http.Request return } + // Security: revoke the previous system token before issuing a new one (F-16). + // This matches the pattern in the REST handleTokenIssue and gRPC IssueServiceToken + // so that old tokens do not remain valid after rotation. + existing, err := u.db.GetSystemToken(acct.ID) + if err == nil { + _ = u.db.RevokeToken(existing.JTI, "rotated") + } + expiry := u.cfg.ServiceExpiry() tokenStr, claims, err := u.issueToken(acct.UUID, roles, expiry) if err != nil {