Fix F-16: revoke old system token before issuing new one
- ui/handlers_accounts.go (handleIssueSystemToken): call GetSystemToken before issuing; if one exists, call RevokeToken(existing.JTI, "rotated") before TrackToken and SetSystemToken for the new token; mirrors the pattern in REST handleTokenIssue and gRPC IssueServiceToken - db/db_test.go: TestSystemTokenRotationRevokesOld verifies the full rotation flow: old JTI revoked with reason "rotated", new JTI tracked and active, GetSystemToken returns the new JTI - AUDIT.md: mark F-16 as fixed Security: without this fix an old system token remained valid after rotation until its natural expiry, giving a leaked or stolen old token extra lifetime. With the revocation the old JTI is immediately marked in token_revocation so any validator checking revocation status rejects it.
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user