Google auth bearer - #177
Open
tsg21 wants to merge 13 commits into
Open
Conversation
Add infra/firebase.tf, infra/firestore.rules, firebase/ and scripts/ to the repo manifest, plus a Firebase section explaining the split between production Terraform resources and the local emulator config. Records four gotchas that have already cost us commits: the emulator not enforcing composite indexes (37b76e0), google_identity_platform_config being required to avoid CONFIGURATION_NOT_FOUND (481a9fe), the one-way nature of google_firebase_project, and VITE_FIREBASE_* surfacing as auth/invalid-api-key when wrong (bf86b81). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replace the trusted X-Player header with server-verified Google ID tokens on every player-scoped endpoint, and gate the UI behind Google sign-in via Firebase Auth. Supersedes the unmerged codex/implement-google-auth-in-ui branch, which predates PR #169 and specified the standalone GIS SDK — that would have stood up a second identity stack alongside the firebase/auth session #169 introduced. Key points: - Sign-in uses GoogleAuthProvider on the existing firebaseAuth instance. The resulting Firebase ID token serves both Firestore reads and our own API, so the custom-token mint is removed entirely. - POST /auth/session survives, but only to maintain the `games` custom claim. Firestore rules run inside Firestore and cannot call the API, so bearer auth does not replace that claim. - X-Player is narrowed to an override header, honoured only when a game carries allow_player_override and the target is a participant. - allow_player_override is per-game rather than per-deployment, so real and test games coexist with one explicit, enforced bypass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New backend/openstars/server/auth.py, the verification machinery the rest of the sign-in gate builds on. Routes are not migrated yet — that is step 4. - get_verified_token parses the Authorization header and calls verify_id_token, mapping every failure to a 401 with a distinct error code. Expired and revoked are matched before invalid, since both subclass InvalidIdTokenError in firebase_admin and would otherwise be swallowed by the broad clause. check_revoked is left off: it costs a round-trip to the user record on every request. - get_current_identity extracts the verified email — the player identity for the whole API. A token with no email, or an unverified one, is 401 rather than a usable session. - get_game_player resolves the effective player for a game-scoped route and returns (summary, player) so routes need not re-read the game. X-Player naming a *different* player is honoured only on games with allow_player_override and only for participants; naming the caller's own identity requests nothing and is ignored. That distinction is load bearing — step 4's test fixture makes identity and override the same string at every existing call site. - GameSummary.allow_player_override is pulled forward from step 4, because get_game_player cannot be written or tested without it. The model defaults to True so Firestore documents predating the field stay reachable; new() defaults to False so new games are strict. _firebase_app moves from routes/auth.py to server/auth.py as firebase_app — step 2 needs it, and importing it the other way would be circular. routes/auth.py aliases it back on import, so existing callers and tests are unaffected. Tests mount the real dependencies on a throwaway FastAPI app rather than the shared one, so they keep exercising the genuine auth path after step 4 installs its dependency_overrides fixture on the shared app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Under the sign-in gate a username is the authenticated Google email (PRD 04), but _USERNAME_RE was ^[a-zA-Z0-9][a-zA-Z0-9._-]*$ — which rejects every address with an @ in it. POST /games would have refused the identities the whole feature makes primary, and the lobby's "Players (comma-separated emails)" form would have failed on submit. The character class now also permits @ and +, the latter because plus-addressing is how test accounts are usually spelled. The leading-alphanumeric anchor and the 64-character limit are unchanged, so path traversal, leading-dot names, slashes and whitespace are still rejected. The error message listed the old character set and now lists the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
POST /auth/firebase-token minted a Firebase custom token with uid set to the client-supplied X-Player header — it manufactured an identity from an unverified string (decision #3). Google sign-in already yields a verified token, so nothing needs minting and the endpoint is gone. POST /api/v1/auth/session replaces it. It takes no body, authorises via the bearer token like every other endpoint, and exists solely to maintain the Firestore `games` custom claim (decision #4): rules execute inside Firestore and cannot call the API, so the claim is the only way to gate document reads. The route depends on both get_verified_token and get_current_identity — the uid comes from the raw claims while the identity rules belong in get_current_identity, and FastAPI's per-request dependency cache means the token is still verified exactly once. Claims are written against the verified uid, never the email. _fit_game_ids and its truncation warning are unchanged apart from the log prefix. display_name is the token's `name` claim, left as None when the token carries none rather than inventing a fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All 47 x_player header parameters across games.py, play.py, designs.py and race.py now resolve identity through the step 2 dependencies. The four duplicated participant checks (_validate_player in play.py and designs.py, inline checks in games.py and race.py) are deleted. Two behaviour changes fall out of that. race.py's check reported its own NOT_PLAYER code; the shared dependency reports NOT_PARTICIPANT like every other route, so NOT_PLAYER is gone. And GET /games is now identity-scoped via list_games_for_player_or_override — the unfiltered "no header lists everything" fall-back no longer exists. create_game built a GameSummary directly and never called new(), so it would have bypassed the strict default that decision #8 sets up. It now goes through new() and honours allow_player_override from the request. FirestoreGameDirectory.create_game writes an explicit field list, so allow_player_override had to be added there too — otherwise new documents would omit it and the model default of True would make every new game silently permissive. Covered by a write/read round-trip test plus a negative control that fails if the field is ever dropped again. The new composite index orders by created_at DESC rather than the updated_at the task file specified. The two halves of the query are merged and re-sorted in Python, so ordering them by different fields would make "most-recent first" meaningless, and updated_at would reshuffle the lobby on every turn submission. Test fixtures: the five identical local client fixtures are replaced by one in tests/server/conftest.py that overrides get_current_identity with an X-Player reader, keeping the existing call sites working. It raises a real 401 when the header is missing rather than a 422. test_auth_routes.py shadows it deliberately, and test_route_auth_migration.py exercises the real dependency across every route family so the override cannot hide a broken auth path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign-in is now Google via signInWithPopup on the existing firebaseAuth
instance, and useAuth owns the whole session: status, user, games claim,
and the sign-in/sign-out/refresh actions. onAuthStateChanged restores the
session on reload instead of forcing a fresh popup.
The session call runs before the forced ID-token refresh, because claims
only appear in tokens minted after the write. Refreshing first yields a
stale token and Firestore then denies the listener, so there is a test
asserting that ordering specifically.
The API client no longer takes a player argument as identity. request()
gets a bearer token from an injected async getter that useAuth installs,
so the client never reaches into Firebase itself. The third argument
survives as playerOverride, setting X-Player only when the caller actually
wants to act as somebody else. A 401 throws a new AuthError subclass so
the UI can drop to the sign-in screen rather than show a generic failure.
useGameState previously called useFirebaseAuth(player) itself, giving one
session per mounted game. App owns useAuth() now and passes it down;
useGameNotifications consumes { games, refreshSession }.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SignInScreen is the unauthenticated entry point: OpenStars! branding reusing the lobby's cover-art treatment, one primary Google action, a loading state, and an ErrorBox whose action becomes a retry. App gates ahead of the lobby branch — loading shows a spinner, signed-out and error show the sign-in screen, and only signed-in reaches the lobby. ?player= stops being identity. App now holds it as playerOverride and the effective player is the override or the signed-in email. Once the loaded game reports allowPlayerOverride: false the override is discarded and the parameter stripped from the address bar, so a stale deep link cannot keep advertising an identity the app no longer takes from it. That discard adjusts state during render rather than in an effect. An effect would commit one frame with the wrong player, and the repo's react-hooks/set-state-in-effect rule rejects it anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lobby now shows who is signed in and offers a sign-out action, and the "Join as:" picker appears only for games that carry allowPlayerOverride. A strict game containing the signed-in email is joined directly with no override; a strict game that does not contain them says so plainly rather than quietly joining as somebody else. The create form prefills the creator's email, is relabelled for emails rather than usernames, and gained an "Allow play-as-any-player (testing)" checkbox wired to the request field. A successful create refreshes the session so the new game reaches the `games` claim before its Firestore listener tries to attach; a failed create deliberately does not. Sign-out clears the game id, override, selection and mode before ending the session, so nothing loaded under the old identity survives into the signed-out render. There is a test asserting the post-sign-out render contains no previously loaded game data at all. The lobby takes its identity as props rather than reading the auth hook, which keeps it a presentation component and keeps the session in one place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Frontend: src/authFlow.test.tsx drives the real App, gate, useAuth and lobby through sign-in, join and sign-out with only the Firebase SDK and API client mocked, asserting the claim is written before the token refresh and that the Firestore listener attaches afterwards. Backend: test_auth_integration.py covers authenticated GET /games returning own plus override games, the override rules end to end, and games written before allow_player_override existed staying listed, joinable and override-capable — the legacy path decision #8 protects. AGENTS.md documents signing in against the auth emulator, including that the emulator serves a fake account picker and that FIREBASE_AUTH_EMULATOR_HOST makes verify_id_token skip signature checks. frontend/AGENTS.md records that App is the only caller of useAuth and that feature components take identity as props. The backend integration tests live in tests/server/ rather than int_tests/, against the usual convention. int_tests/client.py sends only X-Player and now 401s everywhere; fixing it requires minting emulator tokens, which only works under run_docker.sh, and no docker daemon was available to verify that. The blocker and the steps to clear it are recorded in the task file rather than being papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
int_tests sent only X-Player, so every request 401'd once identity moved to the bearer token. Clients now sign in through the auth emulator as the email they play, which keeps the suite on the same code path as real players and leaves X-Player free to be exercised as what it now is — an override request, covered by its own TestPlayerOverride class. run.sh starts the auth emulator in docker for the same reason: there is no way to produce a token verify_id_token accepts without it, short of adding a test-mode bypass to the backend. The backend still runs as a bare local process, which is what that script is for. This uncovered a real break in the auth change: SAFE_SEGMENT_RE never followed _USERNAME_RE in widening to allow @ and +, so creating a game with email usernames 500'd on the first save_player_state. Every storage unit test used "tim", so nothing caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements steps 3–8 of
tasks/2026-08-02-google-auth-ui-gate.md.Google sign-in gates the lobby and every game route, the verified email is the
player identity, and
X-Playeris demoted from identity to an override requesthonoured only on games carrying
allow_player_override.POST /auth/firebase-token(which minted a token from aclient-supplied uid) removed;
POST /auth/sessionmaintains the Firestoregamesclaim.x_playerheader params migrated to the bearerdependencies; four duplicated participant checks deleted;
allow_player_overrideadded through the directory, schemas and Firestore index.
useAuthowns the session,SignInScreengates the app, andthe lobby shows identity, a conditional override picker and sign-out.
AGENTS.md/frontend/AGENTS.md.npm run lint,npm run typecheck,npm test(391 passed) anduv run pytest(753 passed) all pass.Reviewer notes
player-scoped endpoint now 401s on
X-Player-only requests, and/auth/firebase-tokenis gone while the deployed frontend still calls it.backend/int_tests/is knowingly broken and not fixed here. Its clientsends only
X-Player, so the suite 401s throughout. Fixing it needsemulator-minted tokens, which only works under
run_docker.sh; no dockerdaemon was available to verify a fix, so the blocker and remediation steps are
recorded in the task file rather than papered over.
created_at DESC, not theupdated_atthetask file specified — the two query halves are merged and re-sorted in
Python, so mixed ordering would make "most-recent first" meaningless.
race.py'sNOT_PLAYERerror code no longer exists; the shared dependencyreports
NOT_PARTICIPANTlike every other route.