feat(web): authentication with Firebase, NextAuth v5, and sidebar dashboard#42
Merged
Conversation
…hboard (closes #37) - NextAuth v5 (Auth.js) with a single Credentials provider that accepts Firebase ID tokens — Google OAuth and email/password both flow through Firebase Auth SDK on the client, then exchange the ID token for a NextAuth session - proxy.ts (Next.js 16 convention) protects /dashboard and /settings, redirecting unauthenticated users to /login - tRPC protectedProcedure updated to use NextAuth auth() for session validation; TRPCContext now carries Session | null - Login, Register, and Google Sign-In pages with shadcn Card + Form + react-hook-form + Zod validation; errors surfaced via Sonner toasts - Dashboard layout with shadcn Sidebar (AppSidebar), collapsible trigger, and UserMenu (avatar + dropdown) in the sidebar footer - Settings page added so sidebar nav link resolves without 404 - Header component wired into home page; "App" logo links home - Manrope replaces Geist Sans as the global font - next.config.ts allows lh3.googleusercontent.com for Google profile pics - Shared lib/firebase.ts initialises the Firebase app and exports getFirebaseAuth() for use across client components - .env.example updated with all required vars; both backend and web examples verified complete - 79 Vitest tests pass (19 test files)
github-actions
Bot
added
area: web
|
Warning Review limit reached
More reviews will be available in 39 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds a complete authentication layer to the web app: NextAuth v5 credentials provider backed by Firebase Auth, Zod-validated login/register forms with Google OAuth support, proxy middleware and server-component redirects for route protection, a session-aware tRPC context replacing header/cookie auth, a sidebar-based dashboard layout, and supporting UI primitives (Form, Sheet, Tooltip, Sidebar). ChangesAuthentication Feature — NextAuth v5 + Firebase + Protected Routes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginForm
participant FirebaseAuth as Firebase Auth
participant NextAuth as NextAuth (credentials)
participant proxy_ts as proxy.ts middleware
participant DashboardPage as Dashboard Server Component
User->>LoginForm: submit email + password
LoginForm->>FirebaseAuth: signInWithEmailAndPassword
FirebaseAuth-->>LoginForm: Firebase User
LoginForm->>FirebaseAuth: user.getIdToken()
FirebaseAuth-->>LoginForm: idToken (JWT)
LoginForm->>NextAuth: signIn('credentials', { idToken, redirect: false })
NextAuth->>NextAuth: authorize — decode JWT payload, validate sub
NextAuth-->>LoginForm: { ok: true }
LoginForm->>User: router.push('/dashboard')
User->>proxy_ts: GET /dashboard
proxy_ts->>NextAuth: auth(req)
NextAuth-->>proxy_ts: session | null
alt no session
proxy_ts-->>User: redirect /login?callbackUrl=/dashboard
else authenticated
proxy_ts->>DashboardPage: pass through
DashboardPage->>NextAuth: await auth()
NextAuth-->>DashboardPage: session
DashboardPage-->>User: render "Welcome back, {name}"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/features/auth/components/UserMenu.tsx (1)
1-56: 📐 Maintainability & Code Quality | 🟡 MinorAdd a render test for
UserMenu
web/features/auth/components/UserMenu.tsxneeds its own Vitest +@testing-library/reactrender test. The currentNavAuthtest mocksUserMenu, so it doesn’t cover this client component directly.🤖 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 `@web/features/auth/components/UserMenu.tsx` around lines 1 - 56, Add a dedicated Vitest render test for UserMenu so this client component is covered directly instead of only through NavAuth mocks. Create a test that renders UserMenu with mocked useSession and useSignOut, verifies the authenticated state shows the avatar/menu trigger and user details, and confirms the sign-out action is wired through DropdownMenuItem. Use the UserMenu, useSession, and useSignOut symbols to locate the component and keep the test focused on its own rendering behavior.Source: Coding guidelines
🧹 Nitpick comments (2)
web/components/layout/AppSidebar.tsx (1)
35-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo active-route indication in the sidebar nav.
SidebarMenuButtonexposes anisActiveprop (drivingdata-activestyling insidebar.tsx), but it's never set, so the current route is not highlighted. Consider deriving the active item fromusePathname()(requires"use client").♻️ Sketch
- <SidebarMenuButton asChild> + <SidebarMenuButton asChild isActive={pathname === item.url}> <Link href={item.url}>🤖 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 `@web/components/layout/AppSidebar.tsx` around lines 35 - 44, The sidebar nav currently never marks the active route, so `SidebarMenuButton`’s `data-active` styling is not applied. Update `AppSidebar` to be a client component, derive the current path with `usePathname()`, and pass `isActive` to each `SidebarMenuButton` by comparing the pathname to each `navItems` entry (using the existing `item.url` and `navItems.map` render).web/components/ui/sidebar.tsx (1)
181-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpreading
...propsontoSheetinstead ofSheetContent.On the mobile branch,
{...props}(typed asReact.ComponentProps<"div">) is forwarded to theSheetroot rather thanSheetContent. AnyclassName/HTML div attributes a caller passes toSidebarwon't reach the rendered content element and may not be valid on the Radix Dialog root. Forwarding them toSheetContentmatches the desktop branch behavior.🤖 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 `@web/components/ui/sidebar.tsx` around lines 181 - 205, The mobile branch in Sidebar is forwarding { ...props } to Sheet instead of the rendered content container, so div-style props like className never reach the actual sidebar element. Update the mobile path in sidebar.tsx to pass the spread props through SheetContent, matching the desktop branch behavior and keeping Sheet as only the dialog root. Use the existing Sidebar, Sheet, and SheetContent structure as the reference point when moving the props.
🤖 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 `@web/auth.ts`:
- Around line 9-32: The authorize flow in auth.ts is only decoding the JWT
payload and must be changed to verify the Firebase ID token before creating a
session. Replace the manual Buffer/JSON.parse logic in authorize with
firebase-admin/auth verifyIdToken() so signature and standard claims are
checked, then build the returned user from the verified decoded claims (for
example sub, email, name, picture) after successful verification. Keep the
existing zod credential validation and null returns for invalid tokens.
In `@web/components/layout/__tests__/NavAuth.test.tsx`:
- Around line 22-34: The `NavAuth` test mocks are using the wrong `useSession`
return shape, which makes them incompatible with `UseSessionReturn`. Update the
`mockUseSession.mockReturnValue` calls in `NavAuth.test.tsx` to pass `user`,
`isAuthenticated`, and `isLoading` instead of `session`, keeping the
authenticated case’s user email data under `user` so the `NavAuth` and
`useSession` types line up.
In `@web/components/ui/sidebar.tsx`:
- Around line 55-88: The sidebar persistence is only writing to the cookie in
SidebarProvider but never initializing from it, so reloads always fall back to
the hard-coded defaultOpen value. Update the component flow around
SidebarProvider so the saved sidebar_state cookie is read before rendering (for
example in DashboardLayout using next/headers) and passed into SidebarProvider
as defaultOpen, ensuring the initial open state matches the persisted value.
In `@web/docs/_index.md`:
- Line 18: The docs index entry for the Authentication page points to the wrong
source file, so update the sources list in the `_index.md` row for the NextAuth
v5 documentation to reference `proxy.ts` instead of `middleware.ts`; keep the
rest of the row aligned with the actual implementation symbols used by the auth
flow, such as `auth.ts` and `features/auth/`.
In `@web/docs/auth.md`:
- Around line 176-186: Update the auth docs to match the Firebase-backed client
flow: revise LoginForm to describe `signInWithEmailAndPassword`, ID token
retrieval, then `signIn('credentials', { redirect: false })`, and replace the
inline error note with Sonner toast handling; revise RegisterForm to describe
`createUserWithEmailAndPassword`, `updateProfile` for `displayName`, then the
same credentials sign-in instead of posting to
`${NEXT_PUBLIC_API_URL}/auth/register`; revise GoogleSignInButton to mention
Firebase `signInWithPopup` followed by credentials sign-in rather than
`signIn('google', ...)`. Keep the changes anchored to the `LoginForm`,
`RegisterForm`, and `GoogleSignInButton` sections.
- Around line 26-53: The auth docs are out of sync with the actual `auth.ts`
flow, so update the “Providers” section to match the implementation. Remove the
claim that NextAuth uses a Google provider with `AUTH_GOOGLE_ID` /
`AUTH_GOOGLE_SECRET`, and describe that Google sign-in happens via Firebase on
the client before reaching the single Credentials provider. Also rewrite the
Credentials description to reflect `authorize` validating an `idToken`,
base64url-decoding the Firebase JWT payload, and returning `{ id, email, name,
image }` or `null`, instead of email/password Zod validation plus a backend
POST.
- Around line 91-96: The auth docs example is incomplete because it only shows
the matcher for dashboard routes even though proxy.ts also protects settings
routes. Update the config snippet referenced by export const config to include
both /dashboard/:path* and /settings/:path* so the documented example matches
the actual protected paths.
In `@web/features/auth/components/GoogleSignInButton.tsx`:
- Around line 13-23: Update handleClick in GoogleSignInButton to treat popup
cancellation as a non-error by catching the signInWithPopup flow and suppressing
toast.error when the Firebase auth error code is auth/popup-closed-by-user or
auth/cancelled-popup-request. Also add an in-flight loading/disabled state
around the button and set it while handleClick is running so repeated clicks
cannot open multiple popups.
In `@web/features/auth/hooks/useSignOut.ts`:
- Around line 9-11: The sign-out flow in useSignOut only calls nextAuthSignOut
and then navigates to /login, so Firebase remains authenticated. Update the
signOut function to also call firebaseSignOut(getFirebaseAuth()) in the same
logout path, keeping the existing router.push('/login') behavior and using the
Firebase auth helper alongside nextAuthSignOut.
In `@web/features/auth/validation.ts`:
- Line 4: The email validation schemas in the auth validation module are using
the deprecated string-based email helper. Update both schema definitions in
validation.ts to use the top-level z.email with the same message, and keep the
existing validation behavior unchanged while replacing the deprecated
z.string().email usage.
In `@web/hooks/use-mobile.ts`:
- Around line 5-19: The useIsMobile hook currently sets React state
synchronously inside the mounting useEffect, which triggers the
react-hooks/set-state-in-effect warning. Refactor useIsMobile to read the
media-query value through a subscription-based snapshot approach such as
useSyncExternalStore instead of calling setIsMobile in the effect, and keep the
window.matchMedia listener logic encapsulated in the hook so the mobile state
stays in sync without effect-time state updates.
In `@web/proxy.ts`:
- Around line 7-9: The login redirect in the proxy drops the original query
string because it uses req.nextUrl.pathname for callbackUrl. Update the redirect
logic in the proxy handler to build callbackUrl from req.nextUrl.pathname plus
req.nextUrl.search, so users return to the same route with its params after
login.
---
Outside diff comments:
In `@web/features/auth/components/UserMenu.tsx`:
- Around line 1-56: Add a dedicated Vitest render test for UserMenu so this
client component is covered directly instead of only through NavAuth mocks.
Create a test that renders UserMenu with mocked useSession and useSignOut,
verifies the authenticated state shows the avatar/menu trigger and user details,
and confirms the sign-out action is wired through DropdownMenuItem. Use the
UserMenu, useSession, and useSignOut symbols to locate the component and keep
the test focused on its own rendering behavior.
---
Nitpick comments:
In `@web/components/layout/AppSidebar.tsx`:
- Around line 35-44: The sidebar nav currently never marks the active route, so
`SidebarMenuButton`’s `data-active` styling is not applied. Update `AppSidebar`
to be a client component, derive the current path with `usePathname()`, and pass
`isActive` to each `SidebarMenuButton` by comparing the pathname to each
`navItems` entry (using the existing `item.url` and `navItems.map` render).
In `@web/components/ui/sidebar.tsx`:
- Around line 181-205: The mobile branch in Sidebar is forwarding { ...props }
to Sheet instead of the rendered content container, so div-style props like
className never reach the actual sidebar element. Update the mobile path in
sidebar.tsx to pass the spread props through SheetContent, matching the desktop
branch behavior and keeping Sheet as only the dialog root. Use the existing
Sidebar, Sheet, and SheetContent structure as the reference point when moving
the props.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fdc2732-d01b-4db2-83b6-dacf6b0113b4
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
web/.env.exampleweb/__tests__/page.test.tsxweb/app/(auth)/login/page.tsxweb/app/(auth)/register/page.tsxweb/app/(dashboard)/dashboard/page.tsxweb/app/(dashboard)/layout.tsxweb/app/(dashboard)/settings/page.tsxweb/app/api/auth/[...nextauth]/route.tsweb/app/layout.tsxweb/app/page.tsxweb/app/providers.tsxweb/auth.tsweb/components/layout/AppSidebar.tsxweb/components/layout/NavAuth.tsxweb/components/layout/__tests__/NavAuth.test.tsxweb/components/layout/footer.tsxweb/components/layout/header.tsxweb/components/ui/form.tsxweb/components/ui/sheet.tsxweb/components/ui/sidebar.tsxweb/components/ui/tooltip.tsxweb/docs/_index.mdweb/docs/auth.mdweb/docs/data-fetching.mdweb/docs/trpc.mdweb/features/auth/__tests__/GoogleSignInButton.test.tsxweb/features/auth/__tests__/LoginForm.test.tsxweb/features/auth/__tests__/RegisterForm.test.tsxweb/features/auth/__tests__/useSession.test.tsweb/features/auth/__tests__/validation.test.tsweb/features/auth/components/GoogleSignInButton.tsxweb/features/auth/components/LoginForm.tsxweb/features/auth/components/RegisterForm.tsxweb/features/auth/components/UserMenu.tsxweb/features/auth/hooks/useSession.tsweb/features/auth/hooks/useSignOut.tsweb/features/auth/types.tsweb/features/auth/validation.tsweb/hooks/use-mobile.tsweb/lib/firebase.tsweb/lib/trpc/__tests__/server.test.tsweb/next.config.tsweb/package.jsonweb/proxy.tsweb/server/routers/__tests__/auth.test.tsweb/server/routers/__tests__/health.test.tsweb/server/routers/auth.tsweb/server/trpc.ts
- Move initial isMobile read into useState initializer to avoid calling setState synchronously inside useEffect body - Suppress @next/next/no-img-element warning in test mock (intentional) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Security: - Verify Firebase ID tokens with firebase-admin verifyIdToken() instead of decoding the JWT payload without signature verification; add lib/firebase-admin.ts with singleton Admin app init - Sign out of Firebase client alongside NextAuth in useSignOut hook to prevent stale Firebase auth state after logout Correctness: - Preserve query string in callbackUrl (proxy.ts) so users are returned to their original URL with params after login - Restore sidebar open/close state from cookie in DashboardLayout by reading the sidebar_state cookie server-side and passing as defaultOpen - Fix NavAuth test mock shape: use user instead of session to match the UseSessionReturn type - Handle popup-dismissed Firebase errors silently in GoogleSignInButton; disable button while sign-in is in flight Quality: - Switch to z.email() (Zod v4.4.3+) from deprecated z.string().email() - Update docs/auth.md to accurately describe the Firebase idToken flow - Fix stale middleware.ts reference in docs/_index.md to proxy.ts - Add FIREBASE_PROJECT_ID and FIREBASE_SERVICE_ACCOUNT_JSON to web/.env.example Co-Authored-By: Claude Sonnet 4.6 <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
1 participant
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.
Closes #37
Summary
proxy.ts(Next.js 16 convention) guards/dashboard/*and/settings/*, redirecting unauthenticated users to/loginprotectedProcedurenow validates the NextAuth session viaauth()instead of the previous Bearer/cookie stubUserMenu(Google avatar + sign-out dropdown) in the footer/login,/register,/dashboard,/settings— all wired up and protectedlh3.googleusercontent.comadded tonext.config.tsimage remote patterns for Google profile pictures; both.env.examplefiles verified completeTest plan
web/.envFirebase values, runpnpm dev, verify Google Sign-In popup works end-to-end/dashboardwhile logged out — confirm redirect to/login/settingswhile logged out — confirm redirect to/login/loginpnpm lint && pnpm build && pnpm test— 79 tests, 0 errors🤖 Generated with Claude Code
Summary by CodeRabbit