feat: deliver frontend auth flow#4
Conversation
Enable the configured web origin for auth calls and make runtime metadata explicit where the browser-driven E2E flow exposed it.
Add signup, signin, protected app routing, session storage, form validation, Tailwind styling, and focused browser-facing tests.
There was a problem hiding this comment.
Code Review
This pull request implements the frontend authentication flow, including sign-up, sign-in, and protected application pages, styled with Tailwind CSS and backed by Zod validation and React Router. On the backend, CORS is configured to allow requests from the frontend origin, and dependency injection is updated with explicit @Inject decorators. Feedback on these changes highlights several key areas for improvement: addressing a potential race condition and optimizing function stability with useCallback in AuthProvider.tsx, handling array-based validation error messages from NestJS in the API helper, preventing layout shifts in the sign-up page by unconditionally rendering the error slot, and securing the sign-in redirect logic against open redirect vulnerabilities.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
📝 WalkthroughWalkthroughThis PR implements a complete authentication system for a monorepo application. On the backend, the API gains explicit NestJS dependency injection decorators, CORS configuration pinned to a configurable web origin, refined User schema property annotations, and an e2e test validating CORS preflight handling. On the frontend, the web app adds npm dependencies for form handling, routing, and validation, migrates global CSS to a Tailwind-based system with design tokens, implements auth infrastructure (API client, session token storage, form validation schemas), provides an AuthProvider context managing login state and user data, protects routes with RequireAuth, and adds SignupPage, SigninPage, and ApplicationPage components. Integration tests verify the complete authentication flow across protected and public routes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/api/src/users/schemas/user.schema.ts (1)
5-5: 💤 Low valueMinor:
index: trueis redundant alongsideunique: true.Mongoose's
unique: trueimplicitly creates an index, so the explicitindex: trueis redundant. However, this is harmless and may improve clarity.♻️ Optional simplification
- `@Prop`({ index: true, lowercase: true, required: true, trim: true, type: String, unique: true }) + `@Prop`({ lowercase: true, required: true, trim: true, type: String, unique: true }) email!: string;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/users/schemas/user.schema.ts` at line 5, The `@Prop` decorator includes both index: true and unique: true which is redundant because Mongoose creates an index for unique fields; remove index: true from the options in the `@Prop`(...) call (the decorator that currently reads { index: true, lowercase: true, required: true, trim: true, type: String, unique: true }) so it becomes { lowercase: true, required: true, trim: true, type: String, unique: true }; keep unique: true and other flags unchanged and only re-add index: true if there is a specific reason documented.apps/web/src/auth/api.ts (1)
67-81: 💤 Low valueConsider adding more descriptive runtime validation errors.
The error messages "Unexpected authentication response." and "Unexpected current user response." are generic. Including details about which fields failed validation would improve debugging during development.
♻️ Example with more detail
function readAuthResponse(body: unknown): AuthResponse { if (!isObject(body) || typeof body.accessToken !== "string" || !isPublicUser(body.user)) { - throw new Error("Unexpected authentication response."); + throw new Error( + "Unexpected authentication response. Expected { accessToken: string, user: { id, email, name } }" + ); } return { accessToken: body.accessToken, user: body.user }; } function readCurrentUser(body: unknown): PublicUser { if (!isObject(body) || !isPublicUser(body.user)) { - throw new Error("Unexpected current user response."); + throw new Error( + "Unexpected current user response. Expected { user: { id, email, name } }" + ); } return body.user; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/auth/api.ts` around lines 67 - 81, Update readAuthResponse and readCurrentUser to throw more descriptive runtime validation errors that include which expected fields are missing or have the wrong type; for example, when readAuthResponse sees !isObject(body), missing/invalid accessToken, or invalid user, construct and throw an Error that indicates which of those checks failed (mention accessToken type and user validation via isPublicUser), and do the same in readCurrentUser to report whether body is not an object or body.user failed isPublicUser validation so logs show which field caused the failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/app.e2e.test.ts`:
- Around line 53-67: Update the CORS preflight test inside the "allows frontend
auth preflight requests from the configured web origin" it-block (the
request(...).options("/auth/signup") call) to assert that the response includes
both the Authorization header and POST method: check
response.headers["access-control-allow-headers"] (or its lowercase variant)
contains "Authorization" and response.headers["access-control-allow-methods"]
contains "POST" so the test fails if CORS removes required auth headers or
methods.
In `@apps/web/src/auth/AuthProvider.unit.test.tsx`:
- Around line 61-65: The synchronous assertions after clicking the "Sign in"
button are flaky because the click handler calls signin(...) without returning
its promise; change the test to wait for the post-signin state by replacing the
direct expects with an asynchronous wait (e.g., use waitFor to assert
localStorage has "easygen.accessToken" set to "token-123" and/or use
findByText("Person Name") or waitFor(() => expect(screen.getByText("Person
Name")).toBeInTheDocument())); update the test around the userEvent.click call
in AuthProvider.unit.test.tsx to await the appropriate waitFor/findBy* so
setAccessToken/setUser changes settle before asserting.
In `@apps/web/src/pages/authStyles.ts`:
- Around line 3-10: The focus outline utilities should use outline-hidden for
forced-colors accessibility: update the button class (the top-level button class
string that contains focus-visible:outline-none) to use
focus-visible:outline-hidden, and update the input class (the input key's string
that contains outline-none) to use outline-hidden instead; keep all other
classes and spacing identical.
---
Nitpick comments:
In `@apps/api/src/users/schemas/user.schema.ts`:
- Line 5: The `@Prop` decorator includes both index: true and unique: true which
is redundant because Mongoose creates an index for unique fields; remove index:
true from the options in the `@Prop`(...) call (the decorator that currently reads
{ index: true, lowercase: true, required: true, trim: true, type: String,
unique: true }) so it becomes { lowercase: true, required: true, trim: true,
type: String, unique: true }; keep unique: true and other flags unchanged and
only re-add index: true if there is a specific reason documented.
In `@apps/web/src/auth/api.ts`:
- Around line 67-81: Update readAuthResponse and readCurrentUser to throw more
descriptive runtime validation errors that include which expected fields are
missing or have the wrong type; for example, when readAuthResponse sees
!isObject(body), missing/invalid accessToken, or invalid user, construct and
throw an Error that indicates which of those checks failed (mention accessToken
type and user validation via isPublicUser), and do the same in readCurrentUser
to report whether body is not an object or body.user failed isPublicUser
validation so logs show which field caused the failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1d50f1f2-cbb3-47d4-a6fa-0de919920722
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
AI.mdREADME.mdapps/api/src/app.e2e.test.tsapps/api/src/auth/auth.controller.tsapps/api/src/auth/auth.service.tsapps/api/src/auth/jwt-auth.guard.tsapps/api/src/configure-app.tsapps/api/src/users/schemas/user.schema.tsapps/api/src/users/users.service.tsapps/web/package.jsonapps/web/src/App.tsxapps/web/src/auth/AuthProvider.tsxapps/web/src/auth/AuthProvider.unit.test.tsxapps/web/src/auth/RequireAuth.tsxapps/web/src/auth/api.tsapps/web/src/auth/api.unit.test.tsapps/web/src/auth/session.tsapps/web/src/auth/session.unit.test.tsapps/web/src/auth/validation.tsapps/web/src/auth/validation.unit.test.tsapps/web/src/index.cssapps/web/src/main.tsxapps/web/src/pages/ApplicationPage.tsxapps/web/src/pages/SigninPage.tsxapps/web/src/pages/SignupPage.tsxapps/web/src/pages/auth-pages.unit.test.tsxapps/web/src/pages/authStyles.tsapps/web/src/test/setup.tsapps/web/src/vite-env.d.tsapps/web/tsconfig.jsonapps/web/vite.config.ts
There was a problem hiding this comment.
3 issues found across 32 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Summary
Summary by cubic
Delivers a browser-ready auth flow with signup, signin, logout, and a protected app route. Updates API CORS, hardens redirects/session bootstrapping, and adds a Playwright browser smoke test to complete T003 by wiring the React app to
/auth/signup,/auth/signin, and/auth/me.New Features
/signup,/signin,/appwithRequireAuthand anAuthProviderthat persists tokens and boots the current user from/auth/me.VITE_API_URLsupport and localStorage session handling; forms usereact-hook-form+zod.@tailwindcss/viteandtailwindcss; added tests withvitest,@testing-library/react, and a Playwright browser smoke test via@playwright/testwithpnpm test:browserand README steps.Bug Fixes
http://127.0.0.1:${WEB_PORT}andhttp://localhost:${WEB_PORT}withAuthorizationandContent-Type; added preflight E2E coverage.@Injectand corrected user schema field types.Written for commit 20266cb. Summary will update on new commits.