Repository navigation
feat(auth): Firebase sign-in and ID token on API requests - #35
Conversation
- return repeated 401 errors after one token refresh attempt instead of unconditionally signing the user out - update HTTP client tests for failed refresh and empty responses - document verified Firebase authentication and backend user resolution - clarify that application-user IDs and ownership remain backend concerns
kctong529
left a comment
There was a problem hiding this comment.
Review — Approved
Thanks for implementing Firebase authentication! This is an important step toward our Increment 2 goal of completing a real end-to-end workflow, introducing Firebase authentication alongside the existing demo mode and enabling authenticated backend requests.
I reviewed the implementation and tested it locally against backend PR #41 (feat/firebase-user-resolution), with real Firebase accounts and PostgreSQL enabled.
Implementation and architecture
The separation between Firebase authentication (auth.ts), the shared HTTP client (api/http.ts), and the live/mock API adapters looks good. In particular, centralizing ID token attachment means individual screens don't need to handle Firebase tokens themselves.
Backend #41 introduces persistent application-user resolution: the backend verifies the Firebase ID token and maps its UID to an internal PostgreSQL user UUID. The frontend already maintains the appropriate separation of responsibilities by handling authentication and attaching tokens, while application-user resolution remains a backend concern.
Integration tests performed
| Area | Verification | Result |
|---|---|---|
| Registration | Created an account through Get started → Create your account and confirmed it in the Firebase Console | Passed |
| Login | Signed in successfully with existing and newly registered accounts | Passed |
| Authentication errors | Verified incorrect credentials, successful recovery, and repeated failed attempts without the form getting stuck | Passed |
| Session persistence | Refreshed the dashboard and confirmed the Firebase session and route were preserved | Passed |
| Frontend route protection | Verified signed-out redirects, direct navigation, return to the originally requested route, and browser history after logout | Passed |
| Token attachment | Confirmed that the shared HTTP client attaches Firebase ID tokens to outgoing API requests | Passed |
| Backend compatibility | Temporarily called backend #41's protected /api/users/me endpoint through the frontend HTTP client, confirming successful backend verification of the attached Firebase ID token |
Passed |
| Application-user resolution | Verified that two Firebase accounts resolve to distinct, persistent PostgreSQL user UUIDs, including after refresh | Passed |
The /api/users/me call was added temporarily for integration testing and removed afterward. It is not part of the frontend's normal authentication flow.
These tests verify both the frontend's implemented authentication features and their compatibility with the backend's Firebase token verification and user-resolution infrastructure. They do not establish authentication or ownership enforcement for the existing trip endpoints, which remains separate backend work.
Improvements and automated verification
I also incorporated a few follow-up improvements during review:
- Changed the HTTP client to return the backend error after a second 401 instead of unconditionally signing out. A repeated 401 can indicate a backend configuration problem rather than an invalid Firebase session.
- Extended the HTTP-client tests to cover this behavior, failed forced token refresh, and empty 204 responses.
- Updated the README to document Firebase integration, backend user resolution, and current limitations.
- Isolated Playwright acceptance tests from local Firebase/backend configuration so they consistently use the intended mock environment.
Final verification:
| Check | Result |
|---|---|
| Lint | Passed — 29 warnings, 0 errors |
| Production build | Passed |
| TypeScript typecheck | Passed |
| Focused API-client tests | 14/14 passed |
| Full Playwright acceptance suite | Passed |
| Manual Firebase/backend integration tests | Passed |
Scope and remaining work
The scope is appropriate for the authentication issues and our updated project plan. The frontend can now authenticate users and attach Firebase ID tokens to backend requests.
One important limitation remains: the current trip endpoints don't yet enforce ownership using the resolved application-user identity. Likewise, most photo and upload operations remain mocked. These are separate integration tasks, not blockers for this authentication PR.
Approved. The implementation has been verified, the review findings have been addressed, and this provides a solid foundation for the next backend integration work.
Explicitly disable Firebase and live backend integration in the Playwright test server so acceptance tests always run with mock data, regardless of the developer's .env.local configuration.
Closes #8, closes #22.
Not yet tested: the /api/auth/me smoke test against the backend.