Skip to content

feat: add GitHub OAuth login and registration#44

Merged
HarshYadav152 merged 6 commits into
HarshYadav152:mainfrom
SakethSumanBathini:feat/github-oauth
Jun 8, 2026
Merged

feat: add GitHub OAuth login and registration#44
HarshYadav152 merged 6 commits into
HarshYadav152:mainfrom
SakethSumanBathini:feat/github-oauth

Conversation

@SakethSumanBathini

@SakethSumanBathini SakethSumanBathini commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

Adds "Continue with GitHub" OAuth to both the login and register pages, so
users can authenticate without a username/password.

The flow is fully server-side:

  1. GET /api/v1/auth/github sets a short-lived CSRF state cookie (sameSite
    lax so it survives GitHub's redirect) and redirects to GitHub's authorize
    screen with read:user user:email scope.
  2. GET /api/v1/auth/github/callback validates the state, exchanges the code
    for an access token, fetches the GitHub profile, and resolves a verified
    primary email server-side (with a deterministic GitHub no-reply fallback so
    the required email field is always satisfied).
  3. It then links or creates the account and issues the same authToken cookie
    the password login uses, redirecting to /vault.

Account linking prevents duplicates:

  • returning GitHub user matched by githubId
  • existing local account with same email gets githubId linked onto it
  • otherwise a new GitHub-authenticated account is created (username
    de-duplicated against the unique index)

Because the existing AuthContext already hydrates from /api/v1/auth/me on
mount, no client-side auth logic needed to change.

Fixes #13

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • npx eslint . — no new warnings or errors vs the pre-existing baseline
  • Schema validation (mongoose validateSync): GitHub account without
    password/keyword validates; local account missing either is still rejected
  • Control-flow tests: CSRF state validation, verified-email selection + fallback,
    username de-duplication, link-vs-create branch (17/17 pass)

To run locally: register a GitHub OAuth App, copy the three GITHUB_OAUTH_*
values from .env.local.example into .env.local, set callback to
http://localhost:3000/api/v1/auth/github/callback, then click
"Continue with GitHub" on the login or register page.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Added GitHub OAuth authentication option on login and signup pages for streamlined account access
    • Implemented automatic session verification upon successful OAuth sign-in to ensure seamless login completion
  • Chores

    • Updated environment configuration template to include required GitHub OAuth credentials and callback URL setup

@vercel

vercel Bot commented Jun 5, 2026

Copy link
Copy Markdown

@SakethSumanBathini is attempting to deploy a commit to the Harsh Yadav's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Thank you @SakethSumanBathini for your first PR to Gsecure!

We really appreciate your contribution 🙌

What happens next:

  • 🔍 Maintainers will review your PR
  • 🧪 Automated checks will run
  • ✨ Feedback may be shared if needed

Please confirm your PR includes:

  • ✔️ Clear summary of changes
  • ✔️ Linked issue (e.g., Fixes #123)
  • ✔️ Steps to test
  • ✔️ Screenshots (for UI changes)

📘 Contribution Standards:
👉 https://github.com/HarshYadav152/Gsecure/blob/main/CONTRIBUTING.md


💬 Stay Connected with Our Community

💚 WhatsApp (Friendly Community)
For informal chats, quick help, and building friendships with contributors:
👉 https://chat.whatsapp.com/I8GYXd3mHlDCC2iXhNGeqV

🌟 Our Philosophy: We value both professional collaboration (Discord) and personal connections (WhatsApp). Join both to get the complete SS-Capture community experience!


Thanks for helping improve Gsecure 🚀
Let's build something amazing together! 💪

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@SakethSumanBathini, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 18 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 219882cd-93c1-4d0d-a21d-bc48fcd1abd0

📥 Commits

Reviewing files that changed from the base of the PR and between 55484a4 and e4f498e.

📒 Files selected for processing (1)
  • gsecure/app/api/v1/auth/github/callback/route.js
📝 Walkthrough

Walkthrough

This PR implements GitHub OAuth 2.0 authentication, adding social login as an alternative to credential-based signup and login. The feature spans configuration, database schema updates, OAuth authorization and callback endpoints, account linking/creation logic, and client-side UI integration with a post-auth success interstitial.

Changes

GitHub OAuth Authentication

Layer / File(s) Summary
Configuration and User schema
gsecure/.env.local.example, gsecure/lib/models/User.js
Adds GitHub OAuth environment variables (GITHUB_OAUTH_CLIENT_ID, GITHUB_OAUTH_CLIENT_SECRET, GITHUB_OAUTH_CALLBACK_URL) to config template. User schema now includes authProvider (enum: local/github, default local) and githubId (unique, sparse string), with password and keyword fields required only for local authentication.
OAuth authorization initiation
gsecure/app/api/v1/auth/github/route.js
GET /api/v1/auth/github validates required env vars and redirects to GitHub's authorize endpoint after generating a cryptographically strong UUID state and setting a short-lived HTTP-only CSRF cookie (github_oauth_state, 10-minute max age, sameSite: lax).
Callback: request handling and upstream fetches
gsecure/app/api/v1/auth/github/callback/route.js (parts 1-2)
Callback handler extracts and validates code/state against the CSRF cookie, exchanges the code for a GitHub access token, fetches the GitHub user profile (githubId, login), and resolves a verified email with deterministic no-reply fallback, then lowercases the result.
Callback: account resolution and session issuance
gsecure/app/api/v1/auth/github/callback/route.js (parts 3-4)
Connects to MongoDB and resolves user by preferred githubId, fallback email link (guarded against conflicts), or new-user creation with unique username deduplication. Issues authToken cookie (HTTP-only, secure in production, sameSite: strict, 7-day max age), redirects to /auth/success interstitial, clears CSRF state cookie, and logs errors with fallback redirect to /login?error=github_oauth_failed.
UI integration and success interstitial
gsecure/app/(auth)/login/page.js, gsecure/app/(auth)/register/page.js, gsecure/app/(auth)/success/page.js
Adds "Continue with GitHub" call-to-action buttons (with GitHub icon) on login and register pages, linking to /api/v1/auth/github. Implements /auth/success as a client-side interstitial that retries session confirmation via /api/v1/auth/me (with abort controller timeout and exponential delay), updates auth context on success, and redirects to /vault or back to /login on failure.

Sequence Diagram

sequenceDiagram
  participant Browser
  participant LoginPage
  participant GitHubAuth
  participant GitHubAPI
  participant CallbackHandler
  participant MongoDB
  participant SuccessPage
  participant VaultPage
  
  Browser->>LoginPage: User clicks "Continue with GitHub"
  LoginPage->>GitHubAuth: GET /api/v1/auth/github
  GitHubAuth->>GitHubAuth: Generate state UUID, set CSRF cookie
  GitHubAuth->>Browser: Redirect to GitHub authorize URL
  
  Browser->>GitHubAPI: User authorizes app
  GitHubAPI->>Browser: Redirect to callback with code+state
  
  Browser->>CallbackHandler: GET /api/v1/auth/github/callback?code=...&state=...
  CallbackHandler->>CallbackHandler: Validate CSRF state cookie
  CallbackHandler->>GitHubAPI: Exchange code for access token
  CallbackHandler->>GitHubAPI: Fetch user profile (githubId, login)
  CallbackHandler->>GitHubAPI: Fetch verified email
  
  CallbackHandler->>MongoDB: Find/link/create user by githubId or email
  MongoDB->>CallbackHandler: User record
  CallbackHandler->>CallbackHandler: Generate authToken, set HTTP-only cookie
  CallbackHandler->>Browser: Redirect to /success
  
  Browser->>SuccessPage: Load /auth/success interstitial
  SuccessPage->>SuccessPage: Retry /api/v1/auth/me with credentials
  SuccessPage->>Browser: Update auth context (setUser, setAuthenticated)
  SuccessPage->>VaultPage: Redirect to /vault
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

ADVENTURER-L2

Suggested reviewers

  • HarshYadav152

Poem

🐰 A rabbit hops through GitHub's gates,
OAuth flows at rapid rates!
No passwords needed, just a click,
GitHub login's here—how slick!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add GitHub OAuth login and registration' accurately and concisely summarizes the main change—adding GitHub OAuth support to login and registration flows.
Linked Issues check ✅ Passed The PR fulfills all coding requirements from issue #13: adds OAuth social login buttons to Sign In/Up pages, implements secure backend token verification with account linking/creation logic, stores user profile data (email, GitHub ID), prevents duplicate accounts, maintains JWT authentication, and provides dark-mode compatible accessible UI.
Out of Scope Changes check ✅ Passed All changes are directly related to GitHub OAuth integration: environment configuration, UI buttons, OAuth flow handlers, account linking logic, and session verification. No unrelated or extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@gsecure/app/api/v1/auth/github/callback/route.js`:
- Around line 82-105: The code currently falls back to emails[0] and
unconditionally assigns githubId to an account found by email; restrict the
email selection to verified addresses only (use primaryVerified || anyVerified
and do not use emails[0] as fallback) and when handling the email-match branch
in the account-linking logic (the User.findOne/email path) only attach githubId
if byEmail.githubId is null/undefined or already equals the current githubId—do
not overwrite an existing, different byEmail.githubId; if a conflict exists,
skip linking (or create a new user / return an error) instead of reassigning and
ensure you still lowercase email and call byEmail.save() only after setting
githubId safely.
- Around line 42-79: The three outbound fetches (tokenRes ->
fetch(GITHUB_TOKEN_URL), userRes -> fetch(GITHUB_USER_URL), emailsRes ->
fetch(GITHUB_EMAILS_URL)) need explicit timeouts: create an AbortController for
each call, start a setTimeout that calls controller.abort() after a reasonable
timeout (e.g. 5s or configurable), pass controller.signal into the fetch
options, and clear the timeout on success; also catch AbortError/failed fetches
and map them to the existing loginRedirect error paths (e.g. treat as token
exchange / profile fetch / emails fetch failure) so a stalled upstream cannot
hang the auth flow.
🪄 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: c05d11a2-59e2-4c62-826c-349d901f3997

📥 Commits

Reviewing files that changed from the base of the PR and between dcfb7ee and 2e7e313.

📒 Files selected for processing (6)
  • gsecure/.env.local.example
  • gsecure/app/(auth)/login/page.js
  • gsecure/app/(auth)/register/page.js
  • gsecure/app/api/v1/auth/github/callback/route.js
  • gsecure/app/api/v1/auth/github/route.js
  • gsecure/lib/models/User.js

Comment thread gsecure/app/api/v1/auth/github/callback/route.js Outdated
Comment thread gsecure/app/api/v1/auth/github/callback/route.js Outdated
@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Addressed in the latest commit:

Security (verified-email linking) — the callback now only ever trusts a
GitHub-verified email for account creation/linking. The emails[0] fallback
is removed, so an unverified address can never be used; if no verified email is
available it uses the deterministic …@users.noreply.github.com fallback. I
also added a guard so an email already bound to a different githubId is
refused (github_account_conflict) rather than silently re-linked — no
overwrite, no takeover path.

Request timeouts — all three GitHub calls (token, /user, /user/emails) now
go through an 8s AbortController timeout, so a stalled upstream can't hang the
auth flow. The emails fetch fails gracefully to the no-reply fallback.

Docstrings — added JSDoc to the exported handler and helpers.

On the linked-issue note: this implements GitHub OAuth per your guidance
earlier in #13 ("for now we can go with GitHub OAuth … Google OAuth requires
managing cloud and GCP APIs"), which you approved before assigning. The issue
title still says "Google" only because it was never renamed.

@HarshYadav152 HarshYadav152 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SakethSumanBathini All good.
Just some minor changes

  1. Currently you redirect to /vault after oauth authentication but there is an issue in this when you redirect in some browsers or in some cases cookies not be read instantly it require a page refresh or a manual /vault visiting.

So for the solution of this add a page /auth/success/ so instead of redirect to /vault redirect user first to /auth/success/ then make a request to /me route for confirming cookies exist and then redirect to /vault.

DO these change and let me know.

@HarshYadav152 HarshYadav152 added ADVENTURER Beginner-friendly tasks involving small documentation updates, typo fixes, or minor UI improvements. ELUSOC Project Issue registered under EduLinkUp Summer of Code labels Jun 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@gsecure/app/auth/success/page.js`:
- Around line 35-63: The confirmSession retry loop can hang because fetch() has
no timeout; modify confirmSession to create an AbortController for each attempt,
pass controller.signal into fetch(`${apiHost}/api/v1/auth/me`, ...), and start a
timeout (e.g., via setTimeout) that calls controller.abort() after a defined
REQUEST_TIMEOUT_MS; ensure you clear the timeout on success/failure, catch the
abort error (treat as a failed attempt) so the loop continues to the next
attempt or falls back to /login, and keep the existing active checks, setStatus
updates, and router.replace('/vault') behavior intact.
- Around line 24-30: Remove the useRef-based "startedRef" run-once guard and its
import usage (drop useRef from the import) so the effect in useEffect that calls
confirmSession() can run on the second StrictMode pass; instead rely on the
existing active flag/cleanup inside confirmSession() to prevent duplicate state
updates. In addition, add a per-attempt timeout/AbortController to the fetch()
calls targeting "/api/v1/auth/me" inside confirmSession() (and any retry loop)
so each fetch is aborted after a reasonable timeout and the retry logic can
proceed; ensure the AbortController is cleaned up in the effect cleanup so
in-flight requests are cancelled on unmount.
🪄 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: 6df6e5a5-6853-4f07-84c5-1139f6504657

📥 Commits

Reviewing files that changed from the base of the PR and between 2e7e313 and 7eacd1c.

📒 Files selected for processing (2)
  • gsecure/app/api/v1/auth/github/callback/route.js
  • gsecure/app/auth/success/page.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • gsecure/app/api/v1/auth/github/callback/route.js

Comment thread gsecure/app/auth/success/page.js Outdated
Comment thread gsecure/app/(auth)/success/page.js
@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

@SakethSumanBathini All good. Just some minor changes

  1. Currently you redirect to /vault after oauth authentication but there is an issue in this when you redirect in some browsers or in some cases cookies not be read instantly it require a page refresh or a manual /vault visiting.

So for the solution of this add a page /auth/success/ so instead of redirect to /vault redirect user first to /auth/success/ then make a request to /me route for confirming cookies exist and then redirect to /vault.

DO these change and let me know.

@HarshYadav152 Done — pushed.
After OAuth the callback now redirects to a new /auth/success page instead of straight to /vault. That page calls /api/v1/auth/me to confirm the authToken cookie is readable, sets the auth state, and only then forwards to /vault — so no more manual refresh in browsers that lag on the freshly-set cookie. If /me doesn't come back authenticated it falls back to /login.
One small addition: since the cookie can lag by a beat, the /me check retries a few times (5× / 400ms) before giving up, so the confirmation doesn't itself race the cookie. Styled to match the login page. Tested locally end-to-end. Let me know if you'd like any tweaks.

@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

@HarshYadav152 Everything's addressed — the /auth/success page is pushed and both CodeRabbit issues (StrictMode guard + per-fetch timeout) are resolved and marked closed by CodeRabbit. Ready for your approval whenever you get a chance!

@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

Hi @HarshYadav152 — all your requested changes are addressed in the latest commit 95254d8. The /auth/success page is live, both CodeRabbit issues (StrictMode guard + per-fetch timeout) are resolved. Could you take a re-look whenever you get a chance? Thanks!

@HarshYadav152

Copy link
Copy Markdown
Owner

@SakethSumanBathini All good just a bit tweek.
Move /auth/success/ page inside (auth)/ for consistency

@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

@SakethSumanBathini All good just a bit tweek. Move /auth/success/ page inside (auth)/ for consistency

Done — /auth/success is now at app/(auth)/success/page.js, consistent with login/register/forgot-password. Since (auth) is a route group the URL becomes /success, so I've updated the OAuth callback redirect from /auth/success to /success in the same commit. No other references to /auth/success remain.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
gsecure/app/api/v1/auth/github/callback/route.js (2)

157-163: 💤 Low value

Consider capping username deduplication iterations.

The while loop has no upper bound. While collisions are statistically improbable due to the random suffix, a defensive cap (e.g., 10 iterations) would prevent theoretical runaway loops and make intent clearer.

🛡️ Suggested defensive cap
      let username = login.toLowerCase();
      let attempt = 0;
-     while (await User.findOne({ username })) {
+     const MAX_USERNAME_ATTEMPTS = 10;
+     while (await User.findOne({ username })) {
+       if (attempt >= MAX_USERNAME_ATTEMPTS) {
+         return loginRedirect('github_username_conflict');
+       }
        attempt += 1;
        username =
          attempt === 1
            ? `${login.toLowerCase()}-${githubId.slice(-4)}`
            : `${login.toLowerCase()}-${Math.random().toString(36).slice(2, 8)}`;
      }
🤖 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 `@gsecure/app/api/v1/auth/github/callback/route.js` around lines 157 - 163, The
username deduplication while loop using User.findOne has no iteration cap; add a
MAX_USERNAME_ATTEMPTS constant (e.g., 10) and increment attempt on each
iteration, breaking/throwing a clear error (or returning a 500 response) if
attempt >= MAX_USERNAME_ATTEMPTS to avoid a runaway loop; keep the existing
username-generation logic (using githubId.slice and random suffix) but ensure
you check attempt against the cap before continuing the loop and surface a
descriptive failure from the surrounding handler so callers can handle the
error.

182-187: ⚡ Quick win

Add explicit path: '/' to the authToken cookie for consistency.

The state cookie (line 194) explicitly sets path: '/', and the kickoff route's state cookie also uses explicit path. While Next.js likely defaults to /, being explicit ensures the cookie is readable by /api/v1/auth/me and the middleware across all paths, and maintains consistency with the other cookie in this file.

🔧 Suggested fix
    response.cookies.set('authToken', authToken, {
      httpOnly: true,
      secure: process.env.NODE_ENV === 'production',
      sameSite: 'strict',
      maxAge: 60 * 60 * 24 * 7, // 7 days
+     path: '/',
    });
🤖 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 `@gsecure/app/api/v1/auth/github/callback/route.js` around lines 182 - 187, The
authToken cookie set via response.cookies.set('authToken', authToken, {...}) is
missing an explicit path option; update the cookie options passed to
response.cookies.set in the callback route to include path: '/' (matching the
state cookie and kickoff route) so the token is accessible to /api/v1/auth/me
and middleware across all paths and to keep behavior consistent.
🤖 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.

Nitpick comments:
In `@gsecure/app/api/v1/auth/github/callback/route.js`:
- Around line 157-163: The username deduplication while loop using User.findOne
has no iteration cap; add a MAX_USERNAME_ATTEMPTS constant (e.g., 10) and
increment attempt on each iteration, breaking/throwing a clear error (or
returning a 500 response) if attempt >= MAX_USERNAME_ATTEMPTS to avoid a runaway
loop; keep the existing username-generation logic (using githubId.slice and
random suffix) but ensure you check attempt against the cap before continuing
the loop and surface a descriptive failure from the surrounding handler so
callers can handle the error.
- Around line 182-187: The authToken cookie set via
response.cookies.set('authToken', authToken, {...}) is missing an explicit path
option; update the cookie options passed to response.cookies.set in the callback
route to include path: '/' (matching the state cookie and kickoff route) so the
token is accessible to /api/v1/auth/me and middleware across all paths and to
keep behavior consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b2facab-8b8e-435e-aa4b-a07874cd4a90

📥 Commits

Reviewing files that changed from the base of the PR and between 7eacd1c and 55484a4.

📒 Files selected for processing (2)
  • gsecure/app/(auth)/success/page.js
  • gsecure/app/api/v1/auth/github/callback/route.js

@SakethSumanBathini

Copy link
Copy Markdown
Contributor Author

🧹 Nitpick comments (2)

gsecure/app/api/v1/auth/github/callback/route.js (2)> 157-163: 💤 Low value

Consider capping username deduplication iterations.
The while loop has no upper bound. While collisions are statistically improbable due to the random suffix, a defensive cap (e.g., 10 iterations) would prevent theoretical runaway loops and make intent clearer.

🛡️ Suggested defensive cap

      let username = login.toLowerCase();
      let attempt = 0;
-     while (await User.findOne({ username })) {
+     const MAX_USERNAME_ATTEMPTS = 10;
+     while (await User.findOne({ username })) {
+       if (attempt >= MAX_USERNAME_ATTEMPTS) {
+         return loginRedirect('github_username_conflict');
+       }
        attempt += 1;
        username =
          attempt === 1
            ? `${login.toLowerCase()}-${githubId.slice(-4)}`
            : `${login.toLowerCase()}-${Math.random().toString(36).slice(2, 8)}`;
      }

🤖 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 `@gsecure/app/api/v1/auth/github/callback/route.js` around lines 157 - 163, The
username deduplication while loop using User.findOne has no iteration cap; add a
MAX_USERNAME_ATTEMPTS constant (e.g., 10) and increment attempt on each
iteration, breaking/throwing a clear error (or returning a 500 response) if
attempt >= MAX_USERNAME_ATTEMPTS to avoid a runaway loop; keep the existing
username-generation logic (using githubId.slice and random suffix) but ensure
you check attempt against the cap before continuing the loop and surface a
descriptive failure from the surrounding handler so callers can handle the
error.

182-187: ⚡ Quick win
Add explicit path: '/' to the authToken cookie for consistency.
The state cookie (line 194) explicitly sets path: '/', and the kickoff route's state cookie also uses explicit path. While Next.js likely defaults to /, being explicit ensures the cookie is readable by /api/v1/auth/me and the middleware across all paths, and maintains consistency with the other cookie in this file.

🔧 Suggested fix

    response.cookies.set('authToken', authToken, {
      httpOnly: true,
      secure: process.env.NODE_ENV === 'production',
      sameSite: 'strict',
      maxAge: 60 * 60 * 24 * 7, // 7 days
+     path: '/',
    });

🤖 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 `@gsecure/app/api/v1/auth/github/callback/route.js` around lines 182 - 187, The
authToken cookie set via response.cookies.set('authToken', authToken, {...}) is
missing an explicit path option; update the cookie options passed to
response.cookies.set in the callback route to include path: '/' (matching the
state cookie and kickoff route) so the token is accessible to /api/v1/auth/me
and middleware across all paths and to keep behavior consistent.

🤖 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.

Nitpick comments:
In `@gsecure/app/api/v1/auth/github/callback/route.js`:
- Around line 157-163: The username deduplication while loop using User.findOne
has no iteration cap; add a MAX_USERNAME_ATTEMPTS constant (e.g., 10) and
increment attempt on each iteration, breaking/throwing a clear error (or
returning a 500 response) if attempt >= MAX_USERNAME_ATTEMPTS to avoid a runaway
loop; keep the existing username-generation logic (using githubId.slice and
random suffix) but ensure you check attempt against the cap before continuing
the loop and surface a descriptive failure from the surrounding handler so
callers can handle the error.
- Around line 182-187: The authToken cookie set via
response.cookies.set('authToken', authToken, {...}) is missing an explicit path
option; update the cookie options passed to response.cookies.set in the callback
route to include path: '/' (matching the state cookie and kickoff route) so the
token is accessible to /api/v1/auth/me and middleware across all paths and to
keep behavior consistent.

ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b2facab-8b8e-435e-aa4b-a07874cd4a90

📥 Commits
Reviewing files that changed from the base of the PR and between 7eacd1c and 55484a4.

📒 Files selected for processing (2)

  • gsecure/app/(auth)/success/page.js
  • gsecure/app/api/v1/auth/github/callback/route.js

Addressed both CodeRabbit points — added a MAX_USERNAME_ATTEMPTS = 10 cap to the username deduplication loop so it returns a clear username_generation_failed error rather than running indefinitely, and added path: '/' to the authToken cookie for consistency with the state cookie. Both are valid defensive improvements.

@HarshYadav152 HarshYadav152 merged commit af01d22 into HarshYadav152:main Jun 8, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ADVENTURER Beginner-friendly tasks involving small documentation updates, typo fixes, or minor UI improvements. ELUSOC Project Issue registered under EduLinkUp Summer of Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Add Google Authentication (OAuth) Support

2 participants