diff --git a/server/domain/auth.go b/server/domain/auth.go index 67fbf569..a8fe84b7 100644 --- a/server/domain/auth.go +++ b/server/domain/auth.go @@ -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) { diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index 291753e1..bf42563b 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -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") + } +} diff --git a/server/store/session_sql.go b/server/store/session_sql.go index a6fccb0a..d4f539f4 100644 --- a/server/store/session_sql.go +++ b/server/store/session_sql.go @@ -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 + }) +} diff --git a/server/store/session_sql_test.go b/server/store/session_sql_test.go index fd00cb68..a09aa611 100644 --- a/server/store/session_sql_test.go +++ b/server/store/session_sql_test.go @@ -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) {