fix: complete GitHub OAuth flow for non-GitHub-App setups#65
Open
dmartinezh97 wants to merge 3 commits intolak7:mainfrom
Open
fix: complete GitHub OAuth flow for non-GitHub-App setups#65dmartinezh97 wants to merge 3 commits intolak7:mainfrom
dmartinezh97 wants to merge 3 commits intolak7:mainfrom
Conversation
When GITHUB_APP_NEW_USERS is false (common in local dev setups without GitHub App), the /api/github/auth route ended without returning any response. Add standard GitHub OAuth redirect as fallback so users can still connect their GitHub account.
When the Clerk webhook is not configured (common in local development), users are never created in PostgreSQL. This causes the OAuth callback to fail with "Failed to save GitHub connection" because db.user.update() expects the user to already exist. Add ensureUserExists() helper that checks the DB and creates the user from Clerk API data if missing. Call it in the OAuth callback and in getCurrentUserProfile.
The ImportGitRepository component only showed repos when an installationId (GitHub App) was present, even though /api/github/repos already supports OAuth fallback. Users who connected via OAuth saw no repos to import. Add isOAuthConnected state so the component also fetches repos for OAuth-connected users. Also condition the "OAuth is Deprecated" notice to only show when GITHUB_APP_FLOW_ENABLED=true, since it makes no sense when GitHub App is not configured.
|
@dmartinezh97 is attempting to deploy a commit to the lak7's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR addresses three bugs by implementing GitHub OAuth fallback when GitHub App flow is disabled, ensuring users exist in the database before operations (with Clerk fallback), and enabling the import repository component to support OAuth-connected users alongside GitHub App installations. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Client as Web Client
participant AuthEndpoint as /api/github/auth
participant GitHub as GitHub OAuth Server
participant CallbackEndpoint as /api/github/callback
participant Clerk as Clerk API
participant DB as Database
User->>Client: Click "Connect GitHub"
Client->>AuthEndpoint: GET /api/github/auth
rect rgba(100, 200, 255, 0.5)
Note over AuthEndpoint: Check GITHUB_APP_FLOW_ENABLED
alt GitHub App Enabled
AuthEndpoint->>GitHub: Redirect to GitHub App Installation
else OAuth Fallback
AuthEndpoint->>GitHub: Redirect to OAuth Authorize<br/>(client_id + state with userId)
end
end
AuthEndpoint-->>Client: 302 Redirect
Client->>GitHub: Follow redirect
GitHub-->>User: GitHub authorization prompt
User->>GitHub: Authorize
GitHub->>CallbackEndpoint: Redirect with code & state
rect rgba(150, 200, 100, 0.5)
CallbackEndpoint->>CallbackEndpoint: Extract userId from state
CallbackEndpoint->>DB: Check if user exists
alt User not in DB
CallbackEndpoint->>Clerk: Fetch user info
Clerk-->>CallbackEndpoint: User details
CallbackEndpoint->>DB: Create user record
end
CallbackEndpoint->>DB: Update user with GitHub token
end
CallbackEndpoint-->>Client: Redirect to app
Client->>User: Show confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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. ✨ 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.
🧹 Nitpick comments (3)
src/lib/ensureUser.ts (1)
10-36: Consider handling race conditions on user creation.If two concurrent requests call
ensureUserExistsfor the sameuserIdsimultaneously, both may pass thefindUniquecheck and attempt to create the user, resulting in a unique constraint violation ondb.user.create.This is a minor edge case, but you could handle it gracefully by catching the Prisma unique constraint error (
P2002) and returning the existing user:♻️ Suggested improvement
export async function ensureUserExists(userId: string) { const existing = await db.user.findUnique({ where: { id: userId } }); if (existing) return existing; const clerk = await clerkClient(); const clerkUser = await clerk.users.getUser(userId); const primaryEmail = clerkUser.emailAddresses.find( (e) => e.id === clerkUser.primaryEmailAddressId, ); if (!primaryEmail) { throw new Error(`No primary email found for Clerk user ${userId}`); } - return db.user.create({ - data: { - id: userId, - email: primaryEmail.emailAddress, - name: - clerkUser.firstName && clerkUser.lastName - ? `${clerkUser.firstName} ${clerkUser.lastName}` - : clerkUser.firstName || clerkUser.lastName || null, - username: clerkUser.username || null, - credits: signUpInitialSouls, - }, - }); + try { + return await db.user.create({ + data: { + id: userId, + email: primaryEmail.emailAddress, + name: + clerkUser.firstName && clerkUser.lastName + ? `${clerkUser.firstName} ${clerkUser.lastName}` + : clerkUser.firstName || clerkUser.lastName || null, + username: clerkUser.username || null, + credits: signUpInitialSouls, + }, + }); + } catch (error: any) { + // Handle race condition: user was created by concurrent request + if (error?.code === 'P2002') { + const user = await db.user.findUnique({ where: { id: userId } }); + if (user) return user; + } + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ensureUser.ts` around lines 10 - 36, ensureUserExists currently races: two requests can both pass db.user.findUnique and call db.user.create causing a Prisma P2002 unique constraint error; wrap the create call in a try/catch and on catching Prisma's P2002 error (or checking error.code === 'P2002') re-query db.user.findUnique and return the existing record instead of throwing; keep existing logic for other errors and still use clerkClient() / clerk.users.getUser to build create payload when needed.src/app/api/github/auth/route.ts (1)
8-8: Remove stray empty statements.There are several empty statements (
;on their own lines) that appear to be debug artifacts or accidental code. These should be removed for code cleanliness.♻️ Lines to remove
try { - ; // Check if user is authenticated with Clerk const { userId } = await auth(); if (!userId) { return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); } - ; // ... select: { isGithubConnected: true, githubAccessToken: true, isGithubAppConnected: true }, }); - ; const state = crypto.randomUUID(); - ; - ; // Store the state mapping in database for webhook lookup // ... - ; - // ; const installUrl = new URL(`https://github.com/apps/${appSlug}/installations/new`);Also applies to: 15-15, 26-26, 29-30, 42-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/github/auth/route.ts` at line 8, Remove the stray standalone semicolon statements in src/app/api/github/auth/route.ts (they appear as lone ";" on their own lines—e.g., the instances called out at lines 8, 15, 26, 29-30, 42-43 in the review); simply delete those empty statements so the file contains only meaningful expressions/exports (verify surrounding functions/exports like the route handler(s) are unaffected) and run the linter to confirm no extra semicolons remain.src/components/GithubOAuthDeprecatedNotice.tsx (1)
22-30: Stale localStorage cache may cause incorrect notice display.The current logic returns early if
localStorage.getItem('githubOAuthConnected')has any value ('true'or'false'), never refreshing from the API. This could cause issues:
- User disconnects GitHub OAuth → server returns
oauthDeprecated: false, but localStorage still has'true'- Admin disables
GITHUB_APP_FLOW_ENABLED→ server returnsoauthDeprecated: false, but localStorage still has'true'Consider either removing the localStorage cache, using a time-based expiry, or clearing it when relevant user actions occur (e.g., after disconnecting GitHub).
♻️ Suggested approach - add expiry or always fetch
One option is to always fetch and update the cache:
const checkGithubOAuthStatus = async () => { - if(localStorage.getItem('githubOAuthConnected') === 'true'){ - setGithubOAuthConnected(true); - setIsGithubStatusLoading(false); - return; - }else if(localStorage.getItem('githubOAuthConnected') === 'false'){ - setGithubOAuthConnected(false); - setIsGithubStatusLoading(false); - return; - } setIsGithubStatusLoading(true); try { const response = await fetch('/api/github/status');Or use session storage instead of local storage so it refreshes each session.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GithubOAuthDeprecatedNotice.tsx` around lines 22 - 30, The localStorage short-circuits in the GithubOAuthDeprecatedNotice component (checks via localStorage.getItem('githubOAuthConnected') and returns early) so stale values can prevent re-fetching the server status; remove the early return and instead always call the API to determine oauthDeprecated then call setGithubOAuthConnected(...) and setIsGithubStatusLoading(false), and update or clear the local cache (localStorage.setItem(...) or removeItem) based on that fresh response — alternatively replace localStorage usage with sessionStorage or add a timestamp-based expiry when reading/writing the 'githubOAuthConnected' key so the component (and functions like the GitHub disconnect handler) refresh the cached value after relevant user actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/api/github/auth/route.ts`:
- Line 8: Remove the stray standalone semicolon statements in
src/app/api/github/auth/route.ts (they appear as lone ";" on their own
lines—e.g., the instances called out at lines 8, 15, 26, 29-30, 42-43 in the
review); simply delete those empty statements so the file contains only
meaningful expressions/exports (verify surrounding functions/exports like the
route handler(s) are unaffected) and run the linter to confirm no extra
semicolons remain.
In `@src/components/GithubOAuthDeprecatedNotice.tsx`:
- Around line 22-30: The localStorage short-circuits in the
GithubOAuthDeprecatedNotice component (checks via
localStorage.getItem('githubOAuthConnected') and returns early) so stale values
can prevent re-fetching the server status; remove the early return and instead
always call the API to determine oauthDeprecated then call
setGithubOAuthConnected(...) and setIsGithubStatusLoading(false), and update or
clear the local cache (localStorage.setItem(...) or removeItem) based on that
fresh response — alternatively replace localStorage usage with sessionStorage or
add a timestamp-based expiry when reading/writing the 'githubOAuthConnected' key
so the component (and functions like the GitHub disconnect handler) refresh the
cached value after relevant user actions.
In `@src/lib/ensureUser.ts`:
- Around line 10-36: ensureUserExists currently races: two requests can both
pass db.user.findUnique and call db.user.create causing a Prisma P2002 unique
constraint error; wrap the create call in a try/catch and on catching Prisma's
P2002 error (or checking error.code === 'P2002') re-query db.user.findUnique and
return the existing record instead of throwing; keep existing logic for other
errors and still use clerkClient() / clerk.users.getUser to build create payload
when needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
actions/user.tssrc/app/api/github/auth/route.tssrc/app/api/github/callback/route.tssrc/app/api/github/status/route.tssrc/components/GithubOAuthDeprecatedNotice.tsxsrc/components/ImportGitRepository.tsxsrc/lib/ensureUser.ts
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.
Summary
Fixes the GitHub OAuth integration flow that was broken for setups not using GitHub App (e.g., local development with standard OAuth credentials). Three independent bugs prevented users from completing the "Connect GitHub Account" → "Import Repository" flow.
/api/github/authreturned no response whenGITHUB_APP_NEW_USERS=false. Now redirects to GitHub OAuth authorize endpoint as fallback.db.user.update()when the user didn't exist in PostgreSQL (no Clerk webhook). AddedensureUserExists()that creates the user from Clerk API if missing.ImportGitRepositoryonly worked with GitHub AppinstallationId. AddedisOAuthConnectedstate so OAuth-connected users can also browse and import repos. Conditioned the "OAuth Deprecated" notice to only show whenGITHUB_APP_FLOW_ENABLED=true.Closes #62
Closes #63
Closes #64
Changes
Commit 1: Auth route OAuth fallback (
#62)src/app/api/github/auth/route.ts— Add redirect togithub.com/login/oauth/authorizewhen GitHub App flow is not activeCommit 2: ensureUserExists (
#63)src/lib/ensureUser.ts— New utility that checks DB and creates user from Clerk API if missingsrc/app/api/github/callback/route.ts— CallensureUserExists()beforedb.user.update()actions/user.ts— CallensureUserExists()ingetCurrentUserProfileCommit 3: ImportGitRepository OAuth support (
#64)src/components/ImportGitRepository.tsx— AddisOAuthConnectedstate, update guards to allow OAuth userssrc/app/api/github/status/route.ts— AddoauthDeprecatedfield based onGITHUB_APP_FLOW_ENABLEDsrc/components/GithubOAuthDeprecatedNotice.tsx— UseoauthDeprecatedinstead ofisConnectedTest plan
GITHUB_APP_NEW_USERS=falseand validGITHUB_OAUTH_CLIENT_ID/SECRET, verify/api/github/authredirects to GitHub OAuthImportGitRepositoryshows the user's repositoriesGITHUB_APP_FLOW_ENABLED=true, verify the deprecation notice appears for OAuth-connected usersGITHUB_APP_FLOW_ENABLED=false(or unset), verify the deprecation notice does NOT appearSummary by CodeRabbit
New Features
Improvements