feat(auth): add Google OAuth authentication - #124
Conversation
|
@TejasNere99 is attempting to deploy a commit to the Akash Santra 's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughGoogle OAuth sign-in and sign-up are implemented across backend authentication, user persistence, and frontend flows, including token verification, account linking or creation, Google UI controls, and authenticated session storage. ChangesGoogle OAuth authentication
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Frontend
participant GoogleOAuth
participant googleAuth
participant UserModel
Frontend->>GoogleOAuth: Obtain Google ID token
Frontend->>googleAuth: POST /auth/google with token
googleAuth->>GoogleOAuth: Verify ID token
GoogleOAuth-->>googleAuth: Verified email and profile
googleAuth->>UserModel: Find, link, or create user
UserModel-->>googleAuth: User record
googleAuth-->>Frontend: JWT and user profile
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
backend/src/controllers/auth.controller.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. backend/src/models/user.model.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. backend/src/routes/auth.route.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/models/user.model.js (1)
35-39: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard password login for Google accounts
googleAuthcan create users without a local password, butloginstill callsbcrypt.compare(password, user.password)unconditionally. For those accounts this returns a 500 instead of a clear “use Google Sign-In” response. Add an early!user.passwordcheck inlogin.🤖 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 `@backend/src/models/user.model.js` around lines 35 - 39, Update the login method to check for a missing user.password before calling bcrypt.compare, and return a clear response directing Google-linked accounts to use Google Sign-In. Preserve the existing password comparison flow for users with local passwords.
🧹 Nitpick comments (2)
backend/src/models/user.model.js (1)
136-146: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider a unique/sparse index on
googleId.
googleIdhas no index constraints. Addingunique: true, sparse: trueguards against two user documents ever sharing the same Google account identifier, which is otherwise only implicitly prevented by the email-based lookup in the controller.🤖 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 `@backend/src/models/user.model.js` around lines 136 - 146, Update the googleId field in the user model schema to enforce a unique sparse index by adding the corresponding schema options. Preserve the existing String type and null default so users without Google accounts remain allowed while duplicate non-null Google identifiers are rejected.backend/src/controllers/auth.controller.js (1)
1066-1072: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winToken-verification failures return
500instead of401.Any failure inside the try block — including
verifyIdTokenrejecting an invalid/expired/tampered token — falls through to a generic500 "Internal Server Error". Distinguishing token-verification errors (401) from true server errors would give the frontend a more accurate signal and avoid masking client-side issues as backend failures.🤖 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 `@backend/src/controllers/auth.controller.js` around lines 1066 - 1072, Update the Google authentication error handling around the controller’s verifyIdToken flow to detect invalid, expired, or tampered token-verification failures and return HTTP 401 instead of the generic 500 response. Preserve the existing 500 response for unrelated server errors and keep the current error logging context.
🤖 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 `@backend/src/controllers/auth.controller.js`:
- Around line 1028-1052: Update the existing-user branch after User.findOne to
reject banned users and prevent Google linking or authentication for users whose
isVerified flag is false. Apply these checks before assigning googleId, saving
the user, or issuing the JWT, while preserving Google linking for verified,
non-banned users and the existing new-user creation flow.
- Around line 1-11: Add a startup validation immediately after the
`googleClient` initialization to require `process.env.GOOGLE_CLIENT_ID`,
matching the existing `JWT_SECRET` guard’s fail-fast behavior and error-handling
style. Ensure application startup stops when the value is missing, before any
token verification can occur.
---
Outside diff comments:
In `@backend/src/models/user.model.js`:
- Around line 35-39: Update the login method to check for a missing
user.password before calling bcrypt.compare, and return a clear response
directing Google-linked accounts to use Google Sign-In. Preserve the existing
password comparison flow for users with local passwords.
---
Nitpick comments:
In `@backend/src/controllers/auth.controller.js`:
- Around line 1066-1072: Update the Google authentication error handling around
the controller’s verifyIdToken flow to detect invalid, expired, or tampered
token-verification failures and return HTTP 401 instead of the generic 500
response. Preserve the existing 500 response for unrelated server errors and
keep the current error logging context.
In `@backend/src/models/user.model.js`:
- Around line 136-146: Update the googleId field in the user model schema to
enforce a unique sparse index by adding the corresponding schema options.
Preserve the existing String type and null default so users without Google
accounts remain allowed while duplicate non-null Google identifiers are
rejected.
🪄 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 Plus
Run ID: bbfcb0a6-fe39-40dd-86ff-ccde88070166
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
backend/.env.examplebackend/package.jsonbackend/src/controllers/auth.controller.jsbackend/src/models/user.model.jsbackend/src/routes/auth.route.jsfrontend/.env.examplefrontend/package.jsonfrontend/src/main.jsxfrontend/src/pages/LoginPage.jsxfrontend/src/pages/SignUpPage.jsxfrontend/src/store/useAuthStore.js
|
Thanks for the contribution! The implementation looks good overall. Before I review it further, please address the security issues highlighted by CodeRabbit, especially the |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Refactor Google OAuth client initialization to handle undefined GOOGLE_CLIENT_ID gracefully.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/controllers/auth.controller.js`:
- Around line 1048-1055: Update the Google-user lookup in the relevant auth
controller flow to explicitly select the password field before setting it
undefined, ensuring Mongoose persists its removal. In the local login handler,
guard against users without a password and return the existing
invalid-credentials response before calling bcrypt.compare.
🪄 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 Plus
Run ID: 824774c6-ec82-48ee-addb-f9b992714725
📒 Files selected for processing (1)
backend/src/controllers/auth.controller.js
Add validation for missing password in authentication.
|
Hi @Akash504-ai, I've addressed all the CodeRabbit review comments and pushed the required changes. The CI checks are now passing successfully. Could you please review the PR when you have a chance? If there's anything else that needs to be improved, I'd be happy to make the necessary changes. Thank you! |
|
Thanks for the updates! Looks good so far. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/SignUpPage.jsx (1)
104-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssociate every label with its input.
Add stable
idvalues to the full-name, email, password, and recovery-answer inputs, then reference them withhtmlForon the corresponding labels. This is required for accessible form navigation.Proposed fix
-<label className="label py-1"> +<label htmlFor="signup-email" className="label py-1"> -<input type="email" +<input id="signup-email" type="email" -<label className="label py-0.5"> +<label htmlFor={`security-question-${index}`} className="label py-0.5"> -<input type="text" +<input id={`security-question-${index}`} type="text"Also applies to: 120-131, 136-147, 200-210
🤖 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 `@frontend/src/pages/SignUpPage.jsx` around lines 104 - 115, Associate each form label with its input in SignUpPage by adding stable, unique id values to the full-name, email, password, and recovery-answer inputs and matching those ids through the corresponding labels’ htmlFor attributes. Update the relevant input and label blocks, including the sections around the full-name field and the additional fields noted in the review.
🤖 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 `@frontend/src/pages/SignUpPage.jsx`:
- Around line 148-154: Update the password visibility toggle button in
SignUpPage to include a conditional aria-label describing whether it will show
or hide the password, and add aria-pressed reflecting showPassword so
screen-reader users can identify and operate the control.
---
Outside diff comments:
In `@frontend/src/pages/SignUpPage.jsx`:
- Around line 104-115: Associate each form label with its input in SignUpPage by
adding stable, unique id values to the full-name, email, password, and
recovery-answer inputs and matching those ids through the corresponding labels’
htmlFor attributes. Update the relevant input and label blocks, including the
sections around the full-name field and the additional fields noted in the
review.
🪄 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 Plus
Run ID: 758402fe-9aa0-4894-b0dd-978d6fd65055
📒 Files selected for processing (3)
backend/src/controllers/auth.controller.jsfrontend/src/pages/LoginPage.jsxfrontend/src/pages/SignUpPage.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/controllers/auth.controller.js
- frontend/src/pages/LoginPage.jsx
Akash504-ai
added
enhancement
2 participants
Summary
fixes : #122
This PR adds Google OAuth authentication to the application, allowing users to securely sign in or sign up using their Google account.
The authentication pages were updated to include the Google Sign-In option while preserving the existing authentication flow.
Changes Made
Backend
google-auth-library./auth/googleendpoint.providerandgoogleIdin the User model.Frontend
@react-oauth/google.Testing
Screenshots
Summary by CodeRabbit
New Features
UI Improvements
Backend Updates
Configuration / Setup