feat: add Google and GitHub OAuth providers (AGE-67)#61
feat: add Google and GitHub OAuth providers (AGE-67)#61
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR integrates Google and GitHub OAuth providers into the authentication system. Changes include adding environment variables for OAuth credentials, configuring NextAuth with Google and GitHub providers, adding social login buttons to sign-in and sign-up pages, making the User password field optional in the schema, and adjusting middleware to permit /api/auth routes. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend as Login/Signup Page
participant NextAuth
participant OAuthProvider as OAuth Provider<br/>(Google/GitHub)
participant Backend as Backend API
User->>Frontend: Click "Sign in with Google/GitHub"
Frontend->>NextAuth: signIn(provider, {callbackUrl})
NextAuth->>OAuthProvider: Redirect to OAuth consent screen
OAuthProvider->>User: Show consent prompt
User->>OAuthProvider: Approve
OAuthProvider->>NextAuth: Return authorization code
NextAuth->>OAuthProvider: Exchange code for token
OAuthProvider->>NextAuth: Return access token & user info
NextAuth->>Backend: Create/update user session
Backend->>NextAuth: Session established
NextAuth->>Frontend: Redirect to callbackUrl
Frontend->>User: Authenticated, navigated to dashboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@web/src/lib/auth.ts`:
- Around line 12-21: Add an inline comment next to the GoogleProvider and
GitHubProvider configurations explaining that allowDangerousEmailAccountLinking:
true is an intentional security trade-off: document that this setting auto-links
OAuth accounts by email, why it's enabled here (e.g., trusting provider email
verification or acceptable for this app's threat model), and warn maintainers
about the potential account takeover risk so they know to review before changing
it; reference the GoogleProvider and GitHubProvider blocks and the
allowDangerousEmailAccountLinking option when adding the comment.
🧹 Nitpick comments (4)
web/src/lib/auth.ts (1)
13-14: Non-null assertions on environment variables may cause cryptic runtime crashes.If OAuth credentials are not configured, the app will crash with an unhelpful error. Consider either:
- Conditionally adding providers only when credentials exist
- Adding startup validation with descriptive error messages
♻️ Option 1: Conditionally add providers
+const providers: NextAuthConfig["providers"] = [ + CredentialsProvider({ + // ... existing config + }), +]; + +if (process.env.GOOGLE_CLIENT_ID && process.env.GOOGLE_CLIENT_SECRET) { + providers.unshift( + GoogleProvider({ + clientId: process.env.GOOGLE_CLIENT_ID, + clientSecret: process.env.GOOGLE_CLIENT_SECRET, + allowDangerousEmailAccountLinking: true, + }) + ); +} + +if (process.env.GITHUB_CLIENT_ID && process.env.GITHUB_CLIENT_SECRET) { + providers.unshift( + GitHubProvider({ + clientId: process.env.GITHUB_CLIENT_ID, + clientSecret: process.env.GITHUB_CLIENT_SECRET, + allowDangerousEmailAccountLinking: true, + }) + ); +}Also applies to: 18-19
web/src/middleware.ts (1)
7-9: Stale comment - update to reflect actual public routes.The comment says "Only the home page and health check are publicly accessible" but the code now allows
/,/api/health,/blog/*, and/api/auth/*.📝 Suggested fix
- // Only the home page and health check are publicly accessible - // Everything else redirects to home + // Public routes: home, health check, blog, and auth callbacks + // Everything else redirects to home (unauthenticated users) if (pathname === "/" || pathname === "/api/health" || pathname.startsWith("/blog") || pathname.startsWith("/api/auth")) {web/src/app/signup/page.tsx (2)
142-142: Inconsistent callbackUrl handling between login and signup.The signup page hardcodes
/dashboardas the callbackUrl, while the login page reads it fromsearchParams. This could lead to unexpected behavior if a user is redirected to signup with a specific destination in mind.Consider extracting the callbackUrl from searchParams for consistency:
♻️ Suggested fix
+"use client"; + +import { signIn } from "next-auth/react"; +import { useRouter, useSearchParams } from "next/navigation"; +import { useState } from "react"; + +export default function SignupPage() { + const router = useRouter(); + const searchParams = useSearchParams(); + const callbackUrl = searchParams.get("callbackUrl") || "/dashboard"; // ... rest of component - onClick={() => signIn("google", { callbackUrl: "/dashboard" })} + onClick={() => signIn("google", { callbackUrl })} // ... - onClick={() => signIn("github", { callbackUrl: "/dashboard" })} + onClick={() => signIn("github", { callbackUrl })}Note: You'll also need to wrap the component in
<Suspense>like the login page does when usinguseSearchParams().Also applies to: 157-157
141-169: Consider disabling social buttons during form submission.The social login buttons remain clickable even when
isLoadingis true (during credential signup). While this may be intentional, it could lead to confusing UX if a user clicks a social button while a signup request is in flight.
stepandel
left a comment
stepandel
left a comment
There was a problem hiding this comment.
🔍 QA Review — APPROVED — All acceptance criteria for AGE-67 verified:
- ✅ Google OAuth provider added to
auth.tswith env vars - ✅ GitHub OAuth provider added with env vars
- ✅
hashedPasswordmade nullable (String?) for OAuth-only users - ✅ Login page has Google/GitHub sign-in buttons
- ✅ Signup page has matching OAuth buttons
- ✅ Middleware allows
/api/authcallback paths - ✅
.env.exampleupdated with placeholder OAuth vars - ✅ Existing credentials login code unchanged
- ✅ Security comment documents
allowDangerousEmailAccountLinkingrationale - ✅ Build passes clean
No automated tests to run (project has none yet).
stepandel
left a comment
stepandel
left a comment
There was a problem hiding this comment.
🔍 QA Review — APPROVED (posted as comment since bot shares owner token)
All acceptance criteria for AGE-67 verified against the diff:
- ✅ Google OAuth provider added to
auth.tswith env vars - ✅ GitHub OAuth provider added to
auth.tswith env vars - ✅
hashedPasswordchanged toString?for OAuth-only users - ✅ Login page has Google/GitHub sign-in buttons
- ✅ Signup page has matching OAuth buttons
- ✅ Middleware allows
/api/authpaths through - ✅
.env.exampleupdated with placeholders - ✅ Existing credentials flow unchanged
- ✅ Security comment documents
allowDangerousEmailAccountLinking
Build: Root tsc passes. Web next build fails on pre-existing dashboard/new/page.tsx issue (same on main).
Tests: No automated tests yet.
Ship it 🚢
1 participant
Summary
Adds Google and GitHub OAuth sign-in alongside existing email/password credentials.
Changes
hashedPasswordnow optional (String?) for OAuth-only usersallowDangerousEmailAccountLinking/api/authcallback routes throughSetup
Set these env vars to enable OAuth:
Closes AGE-67
Summary by CodeRabbit
Release Notes