feat: Add OAuth profile completion flow #241
Conversation
- Modify OAuth callback to check profile completeness - Redirect incomplete OAuth profiles to /complete-profile page - Create new /complete-profile page with form for first_name, last_name, username - Add real-time username validation with debounced checking - Pre-fill form data from OAuth provider metadata (Google/GitHub) - Update middleware to allow access to /complete-profile route - Preserve existing email signup flow unchanged - Add username generation feature for convenience Fixes: OAuth users now complete their profiles instead of using random usernames Impact: Improved user experience for OAuth authentication flow
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughOAuth callback now fetches expanded profile fields, creates a profile if missing, and redirects incomplete profiles to /complete-profile. Adds a client-side /complete-profile page to collect names and username (with availability and generator RPCs). Middleware marks /complete-profile as public. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant NextJS as Next.js /auth/callback
participant Supabase as Supabase (Auth/DB)
Note over User,Browser: OAuth callback handling (high level)
User->>Browser: Return from OAuth provider
Browser->>NextJS: GET /auth/callback
NextJS->>Supabase: auth.getSession() / getUser()
NextJS->>Supabase: SELECT profiles(id, first_name, last_name, username, profile_complete)
alt Profile missing
NextJS->>Supabase: RPC create_oauth_profile(user_id)
NextJS-->>Browser: 302 Redirect to /complete-profile
else Profile exists
alt Incomplete (missing names/username or profile_complete=false)
NextJS-->>Browser: 302 Redirect to /complete-profile
else Complete
NextJS-->>Browser: 302 Redirect to returnUrl
end
end
sequenceDiagram
autonumber
actor User
participant Browser
participant Page as /complete-profile Page
participant Supabase as Supabase (Auth/DB)
Note over Page,Supabase: Profile completion flow (client)
Browser->>Page: Load /complete-profile
Page->>Supabase: auth.getUser()
alt No user
Page-->>Browser: Redirect to /auth/signin
else User present
Page->>Supabase: SELECT profiles by id
alt Profile complete
Page-->>Browser: Redirect to /protected/dashboard
else Incomplete
Note over Page: Prefill fields from profile/user_metadata
User->>Page: Edit fields / request generation
Page->>Supabase: RPC check_username_availability(username)
Supabase-->>Page: availability result
Page->>Supabase: RPC generate_safe_username() (on generate)
Supabase-->>Page: candidate username
User->>Page: Submit
Page->>Supabase: UPSERT profiles (profile_complete=true, username_set=true, username_editable=false)
alt Update OK
Page-->>Browser: Redirect to /protected/dashboard
else Error
Page-->>Browser: Show error toast
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
middleware.ts (1)
6-70: Daily login awarding is racy and can silently no-op; move to a single DB-side RPC/transaction.Current flow (check-then-insert, then select-then-update) is not atomic:
- Parallel requests can both see “no daily_login today” and double‑award.
.single()on user_activity_log will error if multiple rows exist.user_pointsupdate assumes a row exists; new users get no points.Minimal hardening plus better semantics:
- const { data: existing, error: checkError } = await supabase + const { data: existing, error: checkError } = await supabase .from('user_activity_log') .select('id') .eq('user_id', userId) .eq('activity_type', 'daily_login') .gte('created_at', `${today}T00:00:00Z`) .lt('created_at', `${today}T23:59:59Z`) - .single(); + .maybeSingle(); ... - const { data: userPoints, error: pointsError } = await supabase - .from('user_points') - .select('total_points') - .eq('user_id', userId) - .single(); + const { data: userPoints, error: pointsError } = await supabase + .from('user_points') + .select('total_points') + .eq('user_id', userId) + .maybeSingle(); ... - const currentPoints = userPoints?.total_points || 0; - const { error: updateError } = await supabase - .from('user_points') - .update({ - total_points: currentPoints + 5, - last_updated: new Date().toISOString() - }) - .eq('user_id', userId); + const now = new Date().toISOString(); + // Ensure row exists + const { error: upsertErr } = await supabase + .from('user_points') + .upsert({ user_id: userId, total_points: userPoints?.total_points ?? 0, last_updated: now }, { onConflict: 'user_id' }); + if (upsertErr) { console.error('Upsert user_points failed:', upsertErr); return; } + // Non-atomic increment; prefer RPC below + const { error: updateError } = await supabase + .from('user_points') + .update({ total_points: (userPoints?.total_points ?? 0) + 5, last_updated: now }) + .eq('user_id', userId);Recommended: replace the whole function with one RPC (e.g.,
award_daily_login) that:
- Inserts into
user_activity_logwith a unique constraint on(user_id, activity_type, date)and ON CONFLICT DO NOTHING.- In the same transaction, increments
user_points.total_points = COALESCE(total_points,0)+5when the insert succeeded.app/auth/callback/page.tsx (2)
11-11: Sanitize returnUrl to prevent open redirects.
router.replace(returnUrl)accepts arbitrary URLs. Restrict to same‑origin paths.Apply:
- const returnUrl = searchParams.get("returnUrl") || "/"; + const rawReturnUrl = searchParams.get("returnUrl") || "/"; + const safeReturnUrl = + typeof rawReturnUrl === 'string' && + rawReturnUrl.startsWith('/') && + !rawReturnUrl.startsWith('//') + ? rawReturnUrl + : '/'; ... - router.replace(`/auth/signin?returnUrl=${encodeURIComponent(returnUrl)}`); + router.replace(`/auth/signin?returnUrl=${encodeURIComponent(safeReturnUrl)}`); ... - router.replace(returnUrl); + router.replace(safeReturnUrl);Also applies to: 74-79, 151-154, 203-206, 27-28, 105-105, 209-209, 219-219, 223-223
31-79: Heavy duplication and unmanaged timeouts; extract a helper and clean up timers.Same profile‑check/create logic appears 3× and
setTimeouttimers aren’t cleared on unmount. ExtractensureProfileReady()and track timers with abort/cleanup inuseEffect.Also applies to: 108-154, 156-213
🧹 Nitpick comments (3)
app/auth/callback/page.tsx (1)
85-91: Don’t log OAuth hash fragments (may contain tokens).Remove
hashfrom logs to avoid leaking tokens in dev tools.- console.log('OAuth parameters check:', { - hasHash: !!hash, - hasCode: urlParams.has('code'), - hasError: urlParams.has('error'), - hash: hash.substring(0, 50) + '...', - search: window.location.search - }); + console.log('OAuth parameters check:', { + hasHash: !!hash, + hasCode: urlParams.has('code'), + hasError: urlParams.has('error') + });app/complete-profile/page.tsx (2)
36-87: Minor:checkUserre-runs due to deps and initial username availability isn’t checked.
- Remove
firstName/lastNamefrom deps to avoid extra fetch loops.- If you prefill
profile.username, check its availability immediately so the button isn’t disabled.- }, [router, firstName, lastName]); + }, [router]); ... - if (profile.username) setUsername(profile.username); + if (profile.username) { + setUsername(profile.username); + checkUsernameAvailability(profile.username); + }
312-317: Disable condition should use trimmed username.Prevents whitespace-only usernames from enabling the button.
- disabled={isLoading || !firstName.trim() || !lastName.trim() || !username || !usernameAvailable} + disabled={isLoading || !firstName.trim() || !lastName.trim() || !username.trim() || !usernameAvailable}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/auth/callback/page.tsx(6 hunks)app/complete-profile/page.tsx(1 hunks)middleware.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/complete-profile/page.tsx (1)
lib/services/unified-setup.ts (2)
setUsername(66-80)checkUsernameAvailability(129-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Suite
🔇 Additional comments (3)
middleware.ts (2)
112-112: Making /complete-profile public is fine; ensure the page self-enforces auth.Since middleware returns early for public routes, unified-setup checks won’t run. Confirm the page verifies
auth.getUser()and redirects unauthenticated users (it does). LGTM.
161-167: Verify 404 rewrite target.
NextResponse.rewrite(new URL('/_not-found', req.url))assumes a route at/_not-found. Ensure it exists and is matched byconfig.matcher; otherwise use Next’s built-in 404 handling or a known path.app/auth/callback/page.tsx (1)
40-48: Ensure create_oauth_profile ignores client-supplied user_id/email and RLS enforces auth.uid()create_oauth_profile must derive user_id via auth.uid() (ignore any client-sent user_id/email) and the profiles RLS must allow INSERT/UPDATE only when profiles.user_id = auth.uid() to prevent clients creating/modifying other users' profiles.
Call sites: app/auth/callback/page.tsx lines 40–48, 119–126, 171–178.
Repo search returned no SQL/policy definitions — verify the DB function and RLS policies implement the above.
| const checkUsernameAvailability = async (usernameToCheck: string) => { | ||
| if (!usernameToCheck || usernameToCheck.length < 3) { | ||
| setUsernameAvailable(null); | ||
| return; | ||
| } | ||
|
|
||
| setIsCheckingUsername(true); | ||
| try { | ||
| const { data: isAvailable } = await getSupabaseClient().rpc('check_username_availability', { | ||
| username_param: usernameToCheck | ||
| }); | ||
| setUsernameAvailable(isAvailable); | ||
| } catch (error) { | ||
| console.error('Error checking username:', error); | ||
| setUsernameAvailable(false); | ||
| } finally { | ||
| setIsCheckingUsername(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize and trim input for availability checks; surface network errors distinctly.
Trim (and, if your DB treats usernames case-insensitively, lowercase) before RPC; set null on transport errors so the UI doesn’t show “taken” incorrectly.
- const checkUsernameAvailability = async (usernameToCheck: string) => {
+ const checkUsernameAvailability = async (usernameToCheck: string) => {
if (!usernameToCheck || usernameToCheck.length < 3) {
setUsernameAvailable(null);
return;
}
setIsCheckingUsername(true);
try {
- const { data: isAvailable } = await getSupabaseClient().rpc('check_username_availability', {
- username_param: usernameToCheck
+ const clean = usernameToCheck.trim();
+ const { data: isAvailable } = await getSupabaseClient().rpc('check_username_availability', {
+ username_param: clean
});
setUsernameAvailable(isAvailable);
} catch (error) {
console.error('Error checking username:', error);
- setUsernameAvailable(false);
+ setUsernameAvailable(null);
} finally {
setIsCheckingUsername(false);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkUsernameAvailability = async (usernameToCheck: string) => { | |
| if (!usernameToCheck || usernameToCheck.length < 3) { | |
| setUsernameAvailable(null); | |
| return; | |
| } | |
| setIsCheckingUsername(true); | |
| try { | |
| const { data: isAvailable } = await getSupabaseClient().rpc('check_username_availability', { | |
| username_param: usernameToCheck | |
| }); | |
| setUsernameAvailable(isAvailable); | |
| } catch (error) { | |
| console.error('Error checking username:', error); | |
| setUsernameAvailable(false); | |
| } finally { | |
| setIsCheckingUsername(false); | |
| } | |
| }; | |
| const checkUsernameAvailability = async (usernameToCheck: string) => { | |
| if (!usernameToCheck || usernameToCheck.length < 3) { | |
| setUsernameAvailable(null); | |
| return; | |
| } | |
| setIsCheckingUsername(true); | |
| try { | |
| const clean = usernameToCheck.trim(); | |
| const { data: isAvailable } = await getSupabaseClient().rpc('check_username_availability', { | |
| username_param: clean | |
| }); | |
| setUsernameAvailable(isAvailable); | |
| } catch (error) { | |
| console.error('Error checking username:', error); | |
| setUsernameAvailable(null); | |
| } finally { | |
| setIsCheckingUsername(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/complete-profile/page.tsx around lines 93 to 111, the username
availability check should normalize input and treat transport errors separately:
trim the username (and lowercase it if your DB is case-insensitive) before
validating length and before calling the RPC; only call the RPC when the
normalized username passes validation; on RPC success setUsernameAvailable to
the returned boolean; on network/transport errors log the error and
setUsernameAvailable(null) so the UI doesn't show "taken" incorrectly; preserve
setting setIsCheckingUsername(true) before the RPC and
setIsCheckingUsername(false) in finally.
- Fix debounce implementation using useRef for proper timeout management - Normalize username input by trimming before availability checks - Set usernameAvailable to null on network errors instead of false - Use upsert instead of update to handle missing profile rows - Add proper error handling and data validation for profile updates - Add cleanup effect to clear timeout on component unmount Improves robustness and reliability of the profile completion flow.
🚀 Feature: OAuth Profile Completion Flow
📋 Overview
This PR implements a complete profile completion flow for OAuth users (Google/GitHub) to ensure they provide essential profile information instead of using random usernames.
✨ Changes Made
1. Modified OAuth Callback Handler ()
2. Created Complete Profile Page ()
3. Updated Middleware ()
🎯 Key Features
�� Flow Summary
OAuth Users (Google/GitHub):
Email Users:
🧪 Testing
📝 Files Changed
🎉 Impact
Summary by CodeRabbit
New Features
Bug Fixes
Chores