Fix SEC-08: make system token issuance atomic
- Add IssueSystemToken() method in internal/db/accounts.go that wraps revoke-old, track-new, and upsert-system_tokens in a single SQLite transaction - Update handleTokenIssue in internal/server/server.go to use the new atomic method instead of three separate DB calls - Update IssueServiceToken in internal/grpcserver/tokenservice.go with the same fix - Add TestIssueSystemTokenAtomic test covering first issue and rotation Security: token issuance now uses a single transaction to prevent inconsistent state (e.g., old token revoked but new token not tracked) if a crash occurs between operations. Follows the same pattern as RenewToken which was already correctly transactional. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -692,6 +692,70 @@ func (db *DB) RenewToken(oldJTI, reason, newJTI string, accountID int64, issuedA
|
||||
return nil
|
||||
}
|
||||
|
||||
// IssueSystemToken atomically revokes an existing system token (if oldJTI is
|
||||
// non-empty), tracks the new token in token_revocation, and upserts the
|
||||
// system_tokens table — all within a single SQLite transaction.
|
||||
//
|
||||
// Security: these three operations must be atomic so that a crash between them
|
||||
// cannot leave the database in an inconsistent state (e.g., old token revoked
|
||||
// but new token not tracked, or token tracked but system_tokens not updated).
|
||||
// With MaxOpenConns(1) and SQLite's serialised write path, BEGIN IMMEDIATE
|
||||
// acquires the write lock immediately and prevents any other writer from
|
||||
// interleaving.
|
||||
func (db *DB) IssueSystemToken(oldJTI, newJTI string, accountID int64, issuedAt, expiresAt time.Time) error {
|
||||
tx, err := db.sql.Begin()
|
||||
if err != nil {
|
||||
return fmt.Errorf("db: issue system token begin tx: %w", err)
|
||||
}
|
||||
defer func() { _ = tx.Rollback() }()
|
||||
|
||||
n := now()
|
||||
|
||||
// If there is an existing token, revoke it.
|
||||
if oldJTI != "" {
|
||||
_, err := tx.Exec(`
|
||||
UPDATE token_revocation
|
||||
SET revoked_at = ?, revoke_reason = ?
|
||||
WHERE jti = ? AND revoked_at IS NULL
|
||||
`, n, nullString("rotated"), oldJTI)
|
||||
if err != nil {
|
||||
return fmt.Errorf("db: issue system token revoke old %q: %w", oldJTI, err)
|
||||
}
|
||||
// We do not require rows affected > 0 because the old token may
|
||||
// already be revoked or expired; the important thing is that we
|
||||
// proceed to track the new token regardless.
|
||||
}
|
||||
|
||||
// Track the new token in token_revocation.
|
||||
_, err = tx.Exec(`
|
||||
INSERT INTO token_revocation (jti, account_id, issued_at, expires_at)
|
||||
VALUES (?, ?, ?, ?)
|
||||
`, newJTI, accountID,
|
||||
issuedAt.UTC().Format(time.RFC3339),
|
||||
expiresAt.UTC().Format(time.RFC3339))
|
||||
if err != nil {
|
||||
return fmt.Errorf("db: issue system token track new %q: %w", newJTI, err)
|
||||
}
|
||||
|
||||
// Upsert the system_tokens table so GetSystemToken returns the new JTI.
|
||||
_, err = tx.Exec(`
|
||||
INSERT INTO system_tokens (account_id, jti, expires_at, created_at)
|
||||
VALUES (?, ?, ?, ?)
|
||||
ON CONFLICT(account_id) DO UPDATE SET
|
||||
jti = excluded.jti,
|
||||
expires_at = excluded.expires_at,
|
||||
created_at = excluded.created_at
|
||||
`, accountID, newJTI, expiresAt.UTC().Format(time.RFC3339), n)
|
||||
if err != nil {
|
||||
return fmt.Errorf("db: issue system token set system token for account %d: %w", accountID, err)
|
||||
}
|
||||
|
||||
if err := tx.Commit(); err != nil {
|
||||
return fmt.Errorf("db: issue system token commit: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// RevokeAllUserTokens revokes all non-expired, non-revoked tokens for an account.
|
||||
func (db *DB) RevokeAllUserTokens(accountID int64, reason string) error {
|
||||
n := now()
|
||||
|
||||
Reference in New Issue
Block a user