Conversation
|
@RafaDSan is attempting to deploy a commit to the Heron Lancellot's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements GitHub OAuth authentication using Supabase, replacing the previous simulated authentication flow with a real OAuth integration.
Key Changes:
- Integrated Supabase SSR for server-side authentication handling with PKCE flow
- Implemented real GitHub OAuth login flow with proper session management
- Added middleware for authentication state management and route protection
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/middleware.ts |
Added Next.js middleware to handle session updates on all routes |
frontend/src/integrations/supabase/server.ts |
Created server-side Supabase client for authentication operations |
frontend/src/integrations/supabase/middleware.ts |
Implemented session update logic and unauthenticated user redirection |
frontend/src/integrations/supabase/client.ts |
Migrated from createClient to createBrowserClient with PKCE flow |
frontend/src/components/login-form.tsx |
Updated login form to use real GitHub OAuth with loading states |
frontend/src/app/auth/callback/page.tsx |
Converted to server component to handle OAuth code exchange |
frontend/package.json |
Added @supabase/ssr dependency |
Files not reviewed (1)
- frontend/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,10 @@ | |||
| import { type NextRequest } from 'next/server'; | |||
| import { updateSession } from '@/integrations/supabase/middleware' | |||
There was a problem hiding this comment.
Missing semicolon at the end of the import statement.
| import { updateSession } from '@/integrations/supabase/middleware' | |
| import { updateSession } from '@/integrations/supabase/middleware'; |
| cookiesToSet.forEach(({ name, value, options }) => | ||
| cookieStore.set(name, value, options) | ||
| ); | ||
| } catch { |
There was a problem hiding this comment.
Empty catch block silently swallows all errors. At minimum, log the error or add a comment explaining why errors can be safely ignored in this context.
| } catch { | |
| } catch (error) { | |
| console.error("Error setting cookies in setAll:", error); |
| autoRefreshToken: true, | ||
| persistSession: true, | ||
| detectSessionInUrl: true, | ||
| // import { supabase } from ":/integrations/supabase/client"; |
There was a problem hiding this comment.
Corrected malformed import path prefix from ':' to '@'.
| // import { supabase } from ":/integrations/supabase/client"; | |
| // import { supabase } from "@/integrations/supabase/client"; |
| const { error } = await supabase.auth.signInWithOAuth({ | ||
| provider: "github", | ||
| options: { | ||
| redirectTo: "http://localhost:3000/auth/callback", |
There was a problem hiding this comment.
Hardcoded localhost URL should use the getURL() helper function defined above (lines 19-27) to support different environments.
| redirectTo: "http://localhost:3000/auth/callback", | |
| redirectTo: getURL(), |
| redirect("/login?error=Invalid_callback_request"); | ||
|
|
||
| useEffect(() => { | ||
| // Simulate auth processing | ||
| const timer = setTimeout(() => { | ||
| router.push("/onboarding"); | ||
| }, 2000); | ||
| return null; |
There was a problem hiding this comment.
Unreachable code: the return null; statement on line 20 will never execute because redirect() throws and transfers control.
1 participant
No description provided.