diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 50bbcb3..3636326 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -1093,10 +1093,8 @@ func (f *fakeRepo) UpsertOwner(_ context.Context, id, username, email string) er return nil } func (f *fakeRepo) CreateSession(_ context.Context, ns NewSession) error { - // The API clock minted ExpiresAt, so this is the sign-in time on that clock. - now := ns.ExpiresAt.Add(-sessionTTL) f.sessions[ns.TokenHash] = &fakeSession{ - userID: ns.UserID, expiresAt: ns.ExpiresAt, createdAt: now, lastSeen: now, + userID: ns.UserID, expiresAt: ns.ExpiresAt, createdAt: ns.CreatedAt, lastSeen: ns.CreatedAt, userAgent: ns.UserAgent, clientIP: ns.ClientIP, reauthAt: ns.ReauthAt, } return nil diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 05fbe0b..205fe34 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -11,6 +11,13 @@ import ( // PGRepo is the production Repo backed by Postgres (spec ยง6). It owns only the // business projection the CRD cannot express. The SQL here is exercised by // integration tests against a live database, not the hermetic api_test.go suite. +// +// A method handed the API clock (now, or a NewSession minted on it) stamps the +// times it writes from that clock, column defaults included: a transaction on two +// clocks records one event at two instants, and the fake, which only has the API +// clock, stops describing it. now() is for methods given no clock. The updated_at +// triggers (0010) stay on now(): display bookkeeping no reader compares to the API +// clock. type PGRepo struct { db *sql.DB } @@ -136,9 +143,9 @@ func (p *PGRepo) VerifyLinkCode(ctx context.Context, userID, code string, now ti return "", "", ErrConflict } if _, err := tx.ExecContext(ctx, - `UPDATE account_links SET user_id = $1, auth_source = $2, verified_at = now() + `UPDATE account_links SET user_id = $1, auth_source = $2, verified_at = $4 WHERE mc_uuid = $3`, - userID, authSource, mcUUID); err != nil { + userID, authSource, mcUUID, now); err != nil { return "", "", fmt.Errorf("take over retired link: %w", err) } } @@ -149,9 +156,9 @@ func (p *PGRepo) VerifyLinkCode(ctx context.Context, userID, code string, now ti // Yggdrasil this time gets the latest source stored), keeping the persisted // value equal to the one returned to the caller. if _, err := tx.ExecContext(ctx, - `INSERT INTO account_links (user_id, mc_uuid, auth_source) VALUES ($1, $2, $3) + `INSERT INTO account_links (user_id, mc_uuid, auth_source, verified_at) VALUES ($1, $2, $3, $4) ON CONFLICT (user_id, mc_uuid) DO UPDATE SET auth_source = EXCLUDED.auth_source`, - userID, mcUUID, authSource); err != nil { + userID, mcUUID, authSource, now); err != nil { return "", "", fmt.Errorf("write account link: %w", err) } if _, err := tx.ExecContext(ctx, @@ -209,9 +216,9 @@ func (p *PGRepo) RedeemPlayerBindCode(ctx context.Context, newUserID, code strin mcUUID).Scan(&userID, &existingRole, &disabled, &deleted); { case errors.Is(err, sql.ErrNoRows): if _, err := tx.ExecContext(ctx, - `INSERT INTO users (id, username, role) VALUES ($1, $2, 'user') + `INSERT INTO users (id, username, role, created_at) VALUES ($1, $2, 'user', $3) ON CONFLICT (username) DO NOTHING`, - newUserID, mcUUID); err != nil { + newUserID, mcUUID, now); err != nil { return "", "", "", fmt.Errorf("create player: %w", err) } // Re-read by username so a cross-code race converges on the winner's row @@ -231,9 +238,9 @@ func (p *PGRepo) RedeemPlayerBindCode(ctx context.Context, newUserID, code strin return "", "", "", ErrPlayerAccountRetired } if _, err := tx.ExecContext(ctx, - `INSERT INTO account_links (user_id, mc_uuid, auth_source) VALUES ($1, $2, $3) + `INSERT INTO account_links (user_id, mc_uuid, auth_source, verified_at) VALUES ($1, $2, $3, $4) ON CONFLICT (mc_uuid) DO NOTHING`, - userID, mcUUID, authSource); err != nil { + userID, mcUUID, authSource, now); err != nil { return "", "", "", fmt.Errorf("write account link: %w", err) } case err != nil: @@ -298,13 +305,13 @@ func (p *PGRepo) CompleteOwnerSetup(ctx context.Context, newUserID, code string, `SELECT user_id FROM account_links WHERE mc_uuid = $1`, mcUUID).Scan(&userID); { case errors.Is(err, sql.ErrNoRows): if _, err := tx.ExecContext(ctx, - `INSERT INTO users (id, username, role) VALUES ($1, $2, 'owner')`, - newUserID, mcUUID); err != nil { + `INSERT INTO users (id, username, role, created_at) VALUES ($1, $2, 'owner', $3)`, + newUserID, mcUUID, now); err != nil { return "", "", "", fmt.Errorf("create owner: %w", err) } if _, err := tx.ExecContext(ctx, - `INSERT INTO account_links (user_id, mc_uuid, auth_source) VALUES ($1, $2, $3)`, - newUserID, mcUUID, authSource); err != nil { + `INSERT INTO account_links (user_id, mc_uuid, auth_source, verified_at) VALUES ($1, $2, $3, $4)`, + newUserID, mcUUID, authSource, now); err != nil { return "", "", "", fmt.Errorf("write account link: %w", err) } userID = newUserID @@ -318,14 +325,14 @@ func (p *PGRepo) CompleteOwnerSetup(ctx context.Context, newUserID, code string, } if _, err := tx.ExecContext(ctx, - `INSERT INTO platform_settings (key, value) VALUES ($1, $2::jsonb) - ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`, - LocalAuthEnabledKey, "true"); err != nil { + `INSERT INTO platform_settings (key, value, updated_at) VALUES ($1, $2::jsonb, $3) + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = EXCLUDED.updated_at`, + LocalAuthEnabledKey, "true", now); err != nil { return "", "", "", fmt.Errorf("enable local auth: %w", err) } if _, err := tx.ExecContext(ctx, - `INSERT INTO setup_tokens (token_hash, user_id, expires_at) VALUES ($1, $2, $3)`, - tokenHash, userID, tokenExpiresAt); err != nil { + `INSERT INTO setup_tokens (token_hash, user_id, expires_at, created_at) VALUES ($1, $2, $3, $4)`, + tokenHash, userID, tokenExpiresAt, now); err != nil { return "", "", "", fmt.Errorf("mint setup token: %w", err) } @@ -1427,9 +1434,9 @@ type sqlExecer interface { func insertSession(ctx context.Context, db sqlExecer, s NewSession) error { reauth := sql.NullTime{Time: s.ReauthAt, Valid: !s.ReauthAt.IsZero()} _, err := db.ExecContext(ctx, - `INSERT INTO sessions (token_hash, user_id, expires_at, user_agent, client_ip, reauth_at) - VALUES ($1, $2, $3, $4, $5, $6)`, - s.TokenHash, s.UserID, s.ExpiresAt, s.UserAgent, s.ClientIP, reauth) + `INSERT INTO sessions (token_hash, user_id, created_at, last_seen_at, expires_at, user_agent, client_ip, reauth_at) + VALUES ($1, $2, $3, $3, $4, $5, $6, $7)`, + s.TokenHash, s.UserID, s.CreatedAt, s.ExpiresAt, s.UserAgent, s.ClientIP, reauth) return err } @@ -2615,10 +2622,10 @@ func (p *PGRepo) RedeemMigration(ctx context.Context, targetUserID, codeHash str // Re-point every server the source owns to the target, collecting the names for // the audit trail. Server ownership is the only thing that moves. rows, err := tx.QueryContext(ctx, - `UPDATE servers SET owner_id = $2, claimed_at = now() + `UPDATE servers SET owner_id = $2, claimed_at = $3 WHERE owner_id = $1 AND deleted_at IS NULL RETURNING name`, - sourceUserID, targetUserID) + sourceUserID, targetUserID, now) if err != nil { return "", nil, err } @@ -2641,13 +2648,13 @@ func (p *PGRepo) RedeemMigration(ctx context.Context, targetUserID, codeHash str // log in nor start another migration (double-spend defense). The servers just moved // away, so there is nothing left to release. if _, err := tx.ExecContext(ctx, - `UPDATE sessions SET revoked_at = now() WHERE user_id = $1 AND revoked_at IS NULL`, - sourceUserID); err != nil { + `UPDATE sessions SET revoked_at = $2 WHERE user_id = $1 AND revoked_at IS NULL`, + sourceUserID, now); err != nil { return "", nil, err } if _, err := tx.ExecContext(ctx, - `UPDATE users SET disabled = true, deleted_at = now() WHERE id = $1 AND deleted_at IS NULL`, - sourceUserID); err != nil { + `UPDATE users SET disabled = true, deleted_at = $2 WHERE id = $1 AND deleted_at IS NULL`, + sourceUserID, now); err != nil { return "", nil, err } diff --git a/internal/api/repo.go b/internal/api/repo.go index 51e2234..ec6c92a 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -212,6 +212,9 @@ type SessionedUser struct { type NewSession struct { TokenHash string UserID string + // CreatedAt is the sign-in instant on the API clock: the session is created + // and last seen then, so the staff idle cutoff (also API clock) measures it. + CreatedAt time.Time ExpiresAt time.Time UserAgent string ClientIP string diff --git a/internal/api/session.go b/internal/api/session.go index 35bf52e..982d69a 100644 --- a/internal/api/session.go +++ b/internal/api/session.go @@ -114,6 +114,7 @@ func (a *API) mintSession(r *http.Request, userID string, proof signInProof) (st return token, NewSession{ TokenHash: hashCookie(token), UserID: userID, + CreatedAt: now, ExpiresAt: expires, UserAgent: truncateUTF8(r.UserAgent(), maxSessionUserAgent), ClientIP: ip, diff --git a/internal/pgint/accounts_test.go b/internal/pgint/accounts_test.go index 00dfaa5..cd5192d 100644 --- a/internal/pgint/accounts_test.go +++ b/internal/pgint/accounts_test.go @@ -614,7 +614,7 @@ func TestRedeemSetupTokenContract(t *testing.T) { redeem := func(hash string, at time.Time) (string, string, error) { mu.Lock() minted++ - s := api.NewSession{TokenHash: "st-sess-" + strconv.Itoa(minted) + "-" + suffix(t), UserID: "ignored", ExpiresAt: t0.Add(time.Hour)} + s := api.NewSession{TokenHash: "st-sess-" + strconv.Itoa(minted) + "-" + suffix(t), UserID: "ignored", CreatedAt: at, ExpiresAt: t0.Add(time.Hour)} mu.Unlock() id, err := repo.RedeemSetupToken(ctx, hash, at, s) return id, s.TokenHash, err @@ -660,7 +660,7 @@ func TestRedeemSetupTokenContract(t *testing.T) { kept := "st-kept-" + suffix(t) create(kept) taken := newSession(t, u.ID, "st-taken", t0.Add(time.Hour)) - if _, err := repo.RedeemSetupToken(ctx, kept, t0, api.NewSession{TokenHash: taken, ExpiresAt: t0.Add(time.Hour)}); err == nil || errors.Is(err, api.ErrNotFound) { + if _, err := repo.RedeemSetupToken(ctx, kept, t0, api.NewSession{TokenHash: taken, CreatedAt: t0, ExpiresAt: t0.Add(time.Hour)}); err == nil || errors.Is(err, api.ErrNotFound) { t.Fatalf("redeem into a taken session hash = %v, want the insert failure", err) } if got, _, err := redeem(kept, t0); err != nil || got != u.ID { diff --git a/internal/pgint/clock_test.go b/internal/pgint/clock_test.go new file mode 100644 index 0000000..c4b92c1 --- /dev/null +++ b/internal/pgint/clock_test.go @@ -0,0 +1,164 @@ +//go:build pgint + +package pgint + +import ( + "context" + "errors" + "testing" + "time" + + "felis.lolicon.best/internal/api" +) + +// A method handed the API clock writes only that clock (PGRepo's doc). Each case +// runs the API clock an hour behind the database's, so a time taken from now() +// instead lands an hour away from the one the method was given. + +func skewedClock() time.Time { return mustNow().Add(-time.Hour).Truncate(time.Microsecond) } + +func stampAt(t *testing.T, q string, args ...any) time.Time { + t.Helper() + var at time.Time + if err := db.QueryRow(q, args...).Scan(&at); err != nil { + t.Fatalf("%s: %v", q, err) + } + return at +} + +func wantStamp(t *testing.T, what string, got, want time.Time) { + t.Helper() + if !sameMicro(got, want) { + t.Errorf("%s = %v, want the API clock's %v", what, got, want) + } +} + +// A session is created and last seen at its sign-in on the API clock, which is +// also the clock the staff idle cutoff reads. +func TestSessionTimesComeFromTheSignIn(t *testing.T) { + ctx := context.Background() + now := skewedClock() + create := func(userID, tag string, signedIn time.Time) string { + t.Helper() + hash := tag + "-" + suffix(t) + if err := repo.CreateSession(ctx, api.NewSession{ + TokenHash: hash, UserID: userID, CreatedAt: signedIn, ExpiresAt: now.Add(time.Hour), + }); err != nil { + t.Fatalf("CreateSession(%s): %v", tag, err) + } + return hash + } + + admin := newUser(t, "admin", "clk-admin") + stale := create(admin.ID, "clk-stale", now.Add(-31*time.Minute)) + fresh := create(admin.ID, "clk-fresh", now.Add(-29*time.Minute)) + if _, err := repo.SessionUser(ctx, stale, now); !errors.Is(err, api.ErrNotFound) { + t.Fatalf("staff session signed in 31m ago = %v, want ErrNotFound (idled out)", err) + } + if _, err := repo.SessionUser(ctx, fresh, now); err != nil { + t.Fatalf("staff session signed in 29m ago: %v", err) + } + + player := newUser(t, "user", "clk-player") + signedIn := now.Add(-2 * time.Hour) + hash := create(player.ID, "clk-player", signedIn) + ss, err := repo.ListUserSessions(ctx, player.ID, now) + if err != nil || len(ss) != 1 || ss[0].TokenHash != hash { + t.Fatalf("ListUserSessions = %+v, %v; want only %s", ss, err, hash) + } + wantStamp(t, "created_at", ss[0].CreatedAt, signedIn) + wantStamp(t, "last_seen_at", ss[0].LastSeenAt, signedIn) +} + +// Every door that writes a link stamps it, and whatever else its transaction +// creates, at the time it was given. +func TestLinkWritesStampTheAPIClock(t *testing.T) { + ctx := context.Background() + now := skewedClock() + code := func(mc string) string { + t.Helper() + c := "clk-" + suffix(t) + if err := repo.CreateLinkCode(ctx, c, mc, "mojang", now.Add(10*time.Minute)); err != nil { + t.Fatalf("CreateLinkCode: %v", err) + } + return c + } + linkedAt := func(mc string) time.Time { + t.Helper() + return stampAt(t, `SELECT verified_at FROM account_links WHERE mc_uuid = $1`, mc) + } + createdAt := func(userID string) time.Time { + t.Helper() + return stampAt(t, `SELECT created_at FROM users WHERE id = $1`, userID) + } + + t.Run("verify", func(t *testing.T) { + u := newUser(t, "user", "clk-verify") + mc := testUUID(t) + if _, _, err := repo.VerifyLinkCode(ctx, u.ID, code(mc), now); err != nil { + t.Fatalf("VerifyLinkCode: %v", err) + } + wantStamp(t, "verified_at", linkedAt(mc), now) + }) + + t.Run("takeover of a retired link", func(t *testing.T) { + retired := newUser(t, "user", "clk-retired") + mc := testUUID(t) + if err := repo.LinkAccount(ctx, retired.ID, mc, "mojang"); err != nil { + t.Fatalf("LinkAccount: %v", err) + } + mustExec(t, `UPDATE users SET disabled = true, deleted_at = now() WHERE id = $1`, retired.ID) + taker := newUser(t, "user", "clk-taker") + if _, _, err := repo.VerifyLinkCode(ctx, taker.ID, code(mc), now); err != nil { + t.Fatalf("takeover VerifyLinkCode: %v", err) + } + wantStamp(t, "verified_at", linkedAt(mc), now) + }) + + t.Run("player bind", func(t *testing.T) { + mc := testUUID(t) + id := "usr-clk-player-" + suffix(t) + if got, _, _, err := repo.RedeemPlayerBindCode(ctx, id, code(mc), now); err != nil || got != id { + t.Fatalf("RedeemPlayerBindCode = %q, %v; want %s", got, err, id) + } + wantStamp(t, "user created_at", createdAt(id), now) + wantStamp(t, "verified_at", linkedAt(mc), now) + }) + + t.Run("owner setup", func(t *testing.T) { + mc := testUUID(t) + id := "usr-clk-owner-" + suffix(t) + tok := "tok-clk-" + suffix(t) + if got, _, _, err := repo.CompleteOwnerSetup(ctx, id, code(mc), now, tok, now.Add(time.Hour)); err != nil || got != id { + t.Fatalf("CompleteOwnerSetup = %q, %v; want %s", got, err, id) + } + wantStamp(t, "user created_at", createdAt(id), now) + wantStamp(t, "verified_at", linkedAt(mc), now) + wantStamp(t, "setup token created_at", + stampAt(t, `SELECT created_at FROM setup_tokens WHERE token_hash = $1`, tok), now) + wantStamp(t, "local auth updated_at", + stampAt(t, `SELECT updated_at FROM platform_settings WHERE key = $1`, api.LocalAuthEnabledKey), now) + }) +} + +// The redeem moves the servers, retires the source and ends its sessions at the +// instant it records the migration redeemed. +func TestMigrationRedeemStampsTheAPIClock(t *testing.T) { + ctx := context.Background() + now := skewedClock() + src, dst := newUser(t, "user", "clk-src"), newUser(t, "user", "clk-dst") + server := "clk-srv-" + suffix(t) + seedOwnedServer(t, server, src.ID, false) + sess := newSession(t, src.ID, "clk-src", now.Add(time.Hour)) + hash := "h-clk-" + suffix(t) + startToCode(t, src.ID, dst.ID, hash, now, now.Add(10*time.Minute)) + + if _, moved, err := repo.RedeemMigration(ctx, dst.ID, hash, now); err != nil || len(moved) != 1 { + t.Fatalf("RedeemMigration = %v, %v; want [%s]", moved, err, server) + } + wantStamp(t, "redeemed_at", + stampAt(t, `SELECT redeemed_at FROM account_migrations WHERE source_user_id = $1 AND state = 'redeemed'`, src.ID), now) + wantStamp(t, "server claimed_at", stampAt(t, `SELECT claimed_at FROM servers WHERE name = $1`, server), now) + wantStamp(t, "source deleted_at", stampAt(t, `SELECT deleted_at FROM users WHERE id = $1`, src.ID), now) + wantStamp(t, "source session revoked_at", stampAt(t, `SELECT revoked_at FROM sessions WHERE token_hash = $1`, sess), now) +} diff --git a/internal/pgint/pgint_test.go b/internal/pgint/pgint_test.go index 75bdbe7..87725ba 100644 --- a/internal/pgint/pgint_test.go +++ b/internal/pgint/pgint_test.go @@ -127,7 +127,7 @@ func TestSessionLifecycle(t *testing.T) { hash := "tok-" + suffix(t) expires := mustNow().Add(time.Hour) - if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, ExpiresAt: expires}); err != nil { + if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, CreatedAt: mustNow(), ExpiresAt: expires}); err != nil { t.Fatalf("CreateSession: %v", err) } su, err := repo.SessionUser(ctx, hash, mustNow()) @@ -371,7 +371,7 @@ func TestAuditAttributionContract(t *testing.T) { // The session principal carries the username the rows are signed with. hash := "audit-sess-" + suffix(t) - if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, ExpiresAt: mustNow().Add(time.Hour)}); err != nil { + if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, CreatedAt: mustNow(), ExpiresAt: mustNow().Add(time.Hour)}); err != nil { t.Fatalf("CreateSession: %v", err) } su, err := repo.SessionUser(ctx, hash, mustNow()) @@ -866,7 +866,7 @@ func TestDeadAccountsAreLockedOutInPG(t *testing.T) { t.Fatalf("alive UserByEmail: %v", err) } hash := "h-" + suffix(t) - if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, ExpiresAt: now.Add(time.Hour)}); err != nil { + if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, CreatedAt: now, ExpiresAt: now.Add(time.Hour)}); err != nil { t.Fatalf("CreateSession: %v", err) } if _, err := repo.SessionUser(ctx, hash, now); err != nil { @@ -919,7 +919,7 @@ func TestDeadAccountsAreLockedOutInPG(t *testing.T) { t.Fatalf("deleted UserByEmail = %v, want ErrNotFound", err) } hash2 := "h2-" + suffix(t) - if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash2, UserID: u.ID, ExpiresAt: now.Add(time.Hour)}); err != nil { + if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash2, UserID: u.ID, CreatedAt: now, ExpiresAt: now.Add(time.Hour)}); err != nil { t.Fatalf("CreateSession (deleted): %v", err) } if _, err := repo.SessionUser(ctx, hash2, now); !errors.Is(err, api.ErrNotFound) { diff --git a/internal/pgint/sessions_test.go b/internal/pgint/sessions_test.go index 24ceefc..4730089 100644 --- a/internal/pgint/sessions_test.go +++ b/internal/pgint/sessions_test.go @@ -15,7 +15,7 @@ func newSession(t *testing.T, userID, tag string, expires time.Time) string { t.Helper() hash := tag + "-" + suffix(t) if err := repo.CreateSession(context.Background(), api.NewSession{ - TokenHash: hash, UserID: userID, ExpiresAt: expires, + TokenHash: hash, UserID: userID, CreatedAt: mustNow(), ExpiresAt: expires, UserAgent: "agent " + tag, ClientIP: "192.0.2.1", }); err != nil { t.Fatalf("CreateSession(%s): %v", tag, err) @@ -207,7 +207,7 @@ func TestSessionReauthProof(t *testing.T) { proven := "proven-" + suffix(t) signedIn := now.Add(-time.Minute) if err := repo.CreateSession(ctx, api.NewSession{ - TokenHash: proven, UserID: u.ID, ExpiresAt: now.Add(time.Hour), ReauthAt: signedIn, + TokenHash: proven, UserID: u.ID, CreatedAt: signedIn, ExpiresAt: now.Add(time.Hour), ReauthAt: signedIn, }); err != nil { t.Fatalf("CreateSession: %v", err) } diff --git a/internal/pgint/store_test.go b/internal/pgint/store_test.go index 184e360..c405658 100644 --- a/internal/pgint/store_test.go +++ b/internal/pgint/store_test.go @@ -175,7 +175,7 @@ func TestSetUserDisabledIsAtomic(t *testing.T) { ctx := context.Background() u := newUser(t, "user", "disable") hash := "h-" + suffix(t) - if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, ExpiresAt: mustNow().Add(time.Hour)}); err != nil { + if err := repo.CreateSession(ctx, api.NewSession{TokenHash: hash, UserID: u.ID, CreatedAt: mustNow(), ExpiresAt: mustNow().Add(time.Hour)}); err != nil { t.Fatal(err) }