mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
fix(server): enforce durable identity bans on session issuance and auth
banned_until and ban_reason have been in the schema since 0001, but no production query ever read them -- grepping the tree found no reference outside the migration itself. The only ban check was an in-memory map on domain.TicketVerifier used by domain tests. Once real Steam login is wired, a banned identity would keep full access through every existing session until expiry and could obtain new ones. Make the ban part of the durable authentication transaction rather than a policy each login adapter must remember to re-implement: - Session issuance inserts only when the identity exists and has no active ban, so a banned player cannot mint a session. - Authentication joins the identity and rejects an active ban on every request, so a ban takes effect immediately on every replica rather than at session expiry. - ApplyIdentityBan sets the ban and revokes that identity's sessions in one serializable transaction, closing the window where the ban is durable but another replica still accepts an issued session. Bans are time-bounded and clearing one does not resurrect sessions the ban revoked. Tests cover enforcement across two independently constructed stores standing in for two replicas, expiry/unban semantics, and -- separately, because revocation would otherwise mask it -- that a ban applied without revoking anything still blocks the next request.
This commit is contained in:
@@ -152,8 +152,13 @@ func (c *AuthCoordinator) Expire(now time.Time) []AuthAttempt {
|
||||
}
|
||||
|
||||
var (
|
||||
ErrTicketRejected = fmt.Errorf("steam ticket rejected")
|
||||
ErrTicketRejected = fmt.Errorf("steam ticket rejected")
|
||||
// ErrSessionRejected is deliberately opaque to the client: it must not
|
||||
// distinguish "no such session" from "wrong token".
|
||||
ErrSessionRejected = fmt.Errorf("session rejected")
|
||||
// ErrIdentityBanned is separate so the server can log and act on a ban
|
||||
// distinctly, even though the client sees the same rejection.
|
||||
ErrIdentityBanned = fmt.Errorf("identity is banned")
|
||||
)
|
||||
|
||||
func NewTicketVerifier(expectedApp uint64) (*TicketVerifier, error) {
|
||||
|
||||
@@ -1916,3 +1916,93 @@ func TestPostgreSQLQueuedCandidatesCarryAuthoritativeRatings(t *testing.T) {
|
||||
t.Fatalf("rating spread collapsed: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// banned_until and ban_reason existed in the schema from day one but no
|
||||
// production query ever read them: the only ban check was an in-memory map on
|
||||
// domain.TicketVerifier used by domain tests. A banned identity therefore kept
|
||||
// working through every already-issued session until expiry, and could obtain
|
||||
// new ones, defeating the server-authoritative anti-abuse boundary.
|
||||
func TestPostgreSQLIdentityBanIsEnforcedOnIssuanceAndAuthentication(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
applyIntegrationMigrations(t, db)
|
||||
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ('ban-player', 'ban-steam')`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Two independently constructed stores stand in for two control-plane
|
||||
// replicas: the ban must hold on the replica that did not apply it.
|
||||
replicaA := PostgresSessions{DB: db}
|
||||
replicaB := PostgresSessions{DB: db}
|
||||
|
||||
session, token, err := replicaA.Issue(ctx, "ban-player", time.Hour, now)
|
||||
if err != nil {
|
||||
t.Fatalf("issue before ban: %v", err)
|
||||
}
|
||||
if _, err := replicaB.Authenticate(ctx, session.SessionID, token, now); err != nil {
|
||||
t.Fatalf("authenticate before ban: %v", err)
|
||||
}
|
||||
|
||||
if err := ApplyIdentityBan(ctx, db, "ban-player", "cheating", now.Add(24*time.Hour), now); err != nil {
|
||||
t.Fatalf("apply ban: %v", err)
|
||||
}
|
||||
|
||||
// The pre-existing session must stop working immediately, on a replica
|
||||
// that never saw the ban being applied -- not at session expiry.
|
||||
if _, err := replicaB.Authenticate(ctx, session.SessionID, token, now.Add(time.Minute)); err == nil {
|
||||
t.Fatal("banned identity still authenticated with its existing session")
|
||||
}
|
||||
// ... and no new session may be minted.
|
||||
if _, _, err := replicaB.Issue(ctx, "ban-player", time.Hour, now.Add(time.Minute)); err == nil {
|
||||
t.Fatal("banned identity was issued a new session")
|
||||
}
|
||||
|
||||
// The ban is time-bounded: once it lapses, sign-in works again.
|
||||
afterBan := now.Add(25 * time.Hour)
|
||||
revived, revivedToken, err := replicaA.Issue(ctx, "ban-player", time.Hour, afterBan)
|
||||
if err != nil {
|
||||
t.Fatalf("issue after ban expiry: %v", err)
|
||||
}
|
||||
if _, err := replicaB.Authenticate(ctx, revived.SessionID, revivedToken, afterBan); err != nil {
|
||||
t.Fatalf("authenticate after ban expiry: %v", err)
|
||||
}
|
||||
|
||||
// An explicit unban clears the state without resurrecting revoked sessions.
|
||||
if err := ApplyIdentityBan(ctx, db, "ban-player", "", time.Time{}, now); err != nil {
|
||||
t.Fatalf("clear ban: %v", err)
|
||||
}
|
||||
if _, err := replicaB.Authenticate(ctx, session.SessionID, token, now.Add(time.Minute)); err == nil {
|
||||
t.Fatal("unban resurrected a session revoked by the ban")
|
||||
}
|
||||
}
|
||||
|
||||
// ApplyIdentityBan revokes sessions, so revocation alone would mask a missing
|
||||
// ban check in Authenticate. A ban applied by any other path -- an admin tool,
|
||||
// a future adapter, a direct operational UPDATE -- does not revoke anything,
|
||||
// and must still take effect on the very next authenticated request.
|
||||
func TestPostgreSQLBanWithoutRevocationStillBlocksExistingSessions(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
applyIntegrationMigrations(t, db)
|
||||
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ('raw-ban-player', 'raw-ban-steam')`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sessions := PostgresSessions{DB: db}
|
||||
session, token, err := sessions.Issue(ctx, "raw-ban-player", time.Hour, now)
|
||||
if err != nil {
|
||||
t.Fatalf("issue: %v", err)
|
||||
}
|
||||
if _, err := sessions.Authenticate(ctx, session.SessionID, token, now); err != nil {
|
||||
t.Fatalf("authenticate before ban: %v", err)
|
||||
}
|
||||
// Set the ban only; deliberately leave every session unrevoked.
|
||||
if _, err := db.ExecContext(ctx, `UPDATE identities SET banned_until = $2, ban_reason = 'manual' WHERE player_id = $1`, "raw-ban-player", now.Add(time.Hour)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := sessions.Authenticate(ctx, session.SessionID, token, now.Add(time.Minute)); err == nil {
|
||||
t.Fatal("a ban applied without revocation left the existing session usable")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,13 +13,35 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
// SessionInsertSQL refuses to mint a session for an actively banned
|
||||
// identity. Enforcing it here rather than in the login adapter makes the
|
||||
// ban part of the durable authentication transaction, so it holds for any
|
||||
// present or future adapter rather than depending on each one to
|
||||
// re-implement the policy.
|
||||
SessionInsertSQL = `INSERT INTO sessions (session_id, player_id, token_digest, expires_at, created_at)
|
||||
VALUES ($1, $2, $3, $4, $5)`
|
||||
SessionSelectSQL = `SELECT session_id, player_id, token_digest, expires_at, revoked_at
|
||||
FROM sessions
|
||||
WHERE session_id = $1`
|
||||
SELECT $1, $2, $3, $4, $5
|
||||
FROM identities i
|
||||
WHERE i.player_id = $2
|
||||
AND (i.banned_until IS NULL OR i.banned_until <= $5)`
|
||||
// SessionSelectSQL joins the identity so an existing session stops working
|
||||
// the moment a ban lands. Without this a banned player kept full access
|
||||
// through every already-issued session until it expired.
|
||||
SessionSelectSQL = `SELECT s.session_id, s.player_id, s.token_digest, s.expires_at, s.revoked_at,
|
||||
i.banned_until
|
||||
FROM sessions s
|
||||
JOIN identities i ON i.player_id = s.player_id
|
||||
WHERE s.session_id = $1`
|
||||
SessionRevokeSQL = `UPDATE sessions SET revoked_at = COALESCE(revoked_at, $2)
|
||||
WHERE session_id = $1`
|
||||
// SessionRevokeAllForPlayerSQL is applied in the same transaction as a ban
|
||||
// so there is no window in which the ban is durable but the player's
|
||||
// existing sessions still authenticate on another replica.
|
||||
SessionRevokeAllForPlayerSQL = `UPDATE sessions SET revoked_at = COALESCE(revoked_at, $2)
|
||||
WHERE player_id = $1 AND revoked_at IS NULL`
|
||||
IdentityBanSQL = `UPDATE identities SET banned_until = $2, ban_reason = $3
|
||||
WHERE player_id = $1`
|
||||
IdentityBanClearSQL = `UPDATE identities SET banned_until = NULL, ban_reason = NULL
|
||||
WHERE player_id = $1`
|
||||
)
|
||||
|
||||
// PostgresSessions persists only a SHA-256 token digest. The plaintext token
|
||||
@@ -40,9 +62,18 @@ func (s PostgresSessions) Issue(ctx context.Context, playerID string, lifetime t
|
||||
}
|
||||
session := domain.Session{SessionID: sessionID, PlayerID: playerID, ExpiresAt: now.Add(lifetime)}
|
||||
digest := sha256.Sum256([]byte(token))
|
||||
if _, err := s.DB.ExecContext(ctx, SessionInsertSQL, session.SessionID, session.PlayerID, digest[:], session.ExpiresAt, now); err != nil {
|
||||
result, err := s.DB.ExecContext(ctx, SessionInsertSQL, session.SessionID, session.PlayerID, digest[:], session.ExpiresAt, now)
|
||||
if err != nil {
|
||||
return domain.Session{}, "", err
|
||||
}
|
||||
inserted, err := result.RowsAffected()
|
||||
if err != nil {
|
||||
return domain.Session{}, "", err
|
||||
}
|
||||
if inserted != 1 {
|
||||
// Either no such identity or an active ban; both refuse issuance.
|
||||
return domain.Session{}, "", domain.ErrSessionRejected
|
||||
}
|
||||
return session, token, nil
|
||||
}
|
||||
|
||||
@@ -53,13 +84,19 @@ func (s PostgresSessions) Authenticate(ctx context.Context, sessionID, token str
|
||||
var session domain.Session
|
||||
var digestBytes []byte
|
||||
var revokedAt sql.NullTime
|
||||
if err := s.DB.QueryRowContext(ctx, SessionSelectSQL, sessionID).Scan(&session.SessionID, &session.PlayerID, &digestBytes, &session.ExpiresAt, &revokedAt); err != nil {
|
||||
var bannedUntil sql.NullTime
|
||||
if err := s.DB.QueryRowContext(ctx, SessionSelectSQL, sessionID).Scan(&session.SessionID, &session.PlayerID, &digestBytes, &session.ExpiresAt, &revokedAt, &bannedUntil); err != nil {
|
||||
return domain.Session{}, domain.ErrSessionRejected
|
||||
}
|
||||
provided := sha256.Sum256([]byte(token))
|
||||
if len(digestBytes) != sha256.Size || subtle.ConstantTimeCompare(digestBytes, provided[:]) != 1 || (revokedAt.Valid && !revokedAt.Time.IsZero()) || !now.Before(session.ExpiresAt) {
|
||||
return domain.Session{}, domain.ErrSessionRejected
|
||||
}
|
||||
// Checked on every authenticated request, not only at issuance, so a ban
|
||||
// takes effect immediately across every replica rather than at expiry.
|
||||
if bannedUntil.Valid && now.Before(bannedUntil.Time) {
|
||||
return domain.Session{}, domain.ErrIdentityBanned
|
||||
}
|
||||
return session, nil
|
||||
}
|
||||
|
||||
@@ -85,3 +122,44 @@ func opaqueSessionValue() (string, error) {
|
||||
}
|
||||
return hex.EncodeToString(value), nil
|
||||
}
|
||||
|
||||
// ApplyIdentityBan makes a ban and the revocation of that identity's existing
|
||||
// sessions one atomic change. Applying the ban alone would leave a window in
|
||||
// which another control-plane replica still authenticates an already-issued
|
||||
// session, which is exactly the gap that made banned_until dead schema.
|
||||
//
|
||||
// bannedUntil is the instant the ban lifts; a zero value clears the ban.
|
||||
func ApplyIdentityBan(ctx context.Context, db *sql.DB, playerID, reason string, bannedUntil, now time.Time) error {
|
||||
if db == nil || playerID == "" || now.IsZero() {
|
||||
return domain.ErrSessionRejected
|
||||
}
|
||||
if !bannedUntil.IsZero() && !bannedUntil.After(now) {
|
||||
return domain.ErrSessionRejected
|
||||
}
|
||||
return RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error {
|
||||
var result sql.Result
|
||||
var err error
|
||||
if bannedUntil.IsZero() {
|
||||
result, err = tx.ExecContext(ctx, IdentityBanClearSQL, playerID)
|
||||
} else {
|
||||
result, err = tx.ExecContext(ctx, IdentityBanSQL, playerID, bannedUntil, reason)
|
||||
}
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
changed, err := result.RowsAffected()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if changed != 1 {
|
||||
return domain.ErrSessionRejected
|
||||
}
|
||||
if bannedUntil.IsZero() {
|
||||
// Unbanning does not resurrect revoked sessions; the player signs
|
||||
// in again and receives a fresh one.
|
||||
return nil
|
||||
}
|
||||
_, err = tx.ExecContext(ctx, SessionRevokeAllForPlayerSQL, playerID, now)
|
||||
return err
|
||||
})
|
||||
}
|
||||
|
||||
@@ -7,9 +7,13 @@ import (
|
||||
|
||||
func TestSessionSQLStoresDigestAndEnforcesRevocationBoundary(t *testing.T) {
|
||||
for query, fragments := range map[string][]string{
|
||||
SessionInsertSQL: {"token_digest", "expires_at", "created_at"},
|
||||
SessionSelectSQL: {"token_digest", "revoked_at", "WHERE session_id = $1"},
|
||||
SessionRevokeSQL: {"COALESCE(revoked_at", "WHERE session_id = $1"},
|
||||
// Issuance and authentication must both consult the identity's ban
|
||||
// state; these fragments are the durable enforcement points.
|
||||
SessionInsertSQL: {"token_digest", "expires_at", "created_at", "banned_until", "FROM identities"},
|
||||
SessionSelectSQL: {"token_digest", "revoked_at", "banned_until", "JOIN identities", "WHERE s.session_id = $1"},
|
||||
SessionRevokeSQL: {"COALESCE(revoked_at", "WHERE session_id = $1"},
|
||||
SessionRevokeAllForPlayerSQL: {"COALESCE(revoked_at", "WHERE player_id = $1"},
|
||||
IdentityBanSQL: {"banned_until", "ban_reason", "WHERE player_id = $1"},
|
||||
} {
|
||||
for _, fragment := range fragments {
|
||||
if !contains(query, fragment) {
|
||||
|
||||
Reference in New Issue
Block a user