feat(backend, frontend) google login #192
Conversation
WalkthroughThis update integrates Google OAuth authentication into both the backend and frontend. The backend enhancements include new Passport.js dependencies, the addition of a Google authentication strategy and controller, and modifications to the user model and authentication service. On the frontend, new components and environment configurations handle the OAuth callback and redirect flows. Together, these changes enable users to authenticate via their Google accounts. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as Frontend (UI)
participant GCtrl as Google Controller
participant G as Google OAuth Provider
participant Auth as AuthService
U->>F: Click "Continue with Google"
F->>GCtrl: Redirect to Google OAuth endpoint
GCtrl->>G: Initiate Google OAuth flow
G-->>GCtrl: Return Google profile upon successful auth
GCtrl->>Auth: Call handleGoogleCallback(profile)
Auth-->>GCtrl: Return access & refresh tokens
GCtrl->>F: Redirect to frontend with tokens
F->>F: OAuthCallbackPage processes tokens
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ZHallen122
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
frontend/src/components/sign-in-modal.tsx (1)
157-162: Google OAuth redirects implemented correctly.The implementation properly redirects to the Google OAuth endpoint using the environment variable with a fallback URL.
Consider extracting this Google OAuth redirection logic into a shared utility function since it's duplicated in both sign-in and sign-up modals:
// Create a new file: src/utils/auth.ts +export const redirectToGoogleAuth = () => { + window.location.href = + process.env.NEXT_PUBLIC_BACKEND_GOOGLE_OAUTH || + 'http://localhost:8080/auth/google'; +}; // Then in both modals: -onClick={() => { - // Redirect to your NestJS backend's Google OAuth endpoint - window.location.href = - process.env.NEXT_PUBLIC_BACKEND_GOOGLE_OAUTH || - 'http://localhost:8080/auth/google'; -}} +onClick={redirectToGoogleAuth}backend/src/user/user.model.ts (1)
24-25: Consider adding an index to the googleId fieldFor better query performance when looking up users by their Google ID, consider adding an index to this column.
- @Column({ nullable: true }) + @Column({ nullable: true, index: true }) googleId: string;frontend/src/app/auth/oauth-callback/page.tsx (3)
36-38: Add token validation before storingYou should validate tokens before storing them, for example by checking JWT structure or making a test API call to verify they work.
44-48: Improve error handling specificityThe catch-all error handling doesn't distinguish between different error types. Consider adding more specific error handling for network issues, token validation failures, etc.
54-67: Add dynamic loading stateThe loading UI is shown regardless of the authentication state. Consider adding a state variable to show different UI based on the authentication progress (loading, success, error).
+ const [authState, setAuthState] = useState('loading'); // 'loading', 'success', 'error' useEffect(() => { const handleAuth = async () => { try { + setAuthState('loading'); // ...authentication logic... + setAuthState('success'); } catch (error) { + setAuthState('error'); // ...error handling... } }; // ... }, [searchParams, login, router]); return ( <div className="flex h-screen w-full items-center justify-center"> <div className="text-center p-8 max-w-md rounded-xl bg-white dark:bg-zinc-900 shadow-lg"> - <h1 className="text-2xl font-bold mb-4"> - Completing authentication... - </h1> + <h1 className="text-2xl font-bold mb-4"> + {authState === 'loading' ? 'Completing authentication...' : + authState === 'success' ? 'Authentication successful!' : + 'Authentication failed'} + </h1> {/* Rest of the UI with conditional rendering based on authState */}backend/src/auth/google.controller.ts (1)
20-37: Consider handling possible errors from the AuthService.While
AuthGuard('google')handles errors relating to the authentication flow, any internal exceptions fromhandleGoogleCallbackcould surface here. You may want to wrap the service call in a try-catch or similar to provide a user-friendly error message or redirection upon failure.backend/src/auth/auth.service.ts (1)
540-620: Streamline user creation logic and consider concurrency checks.The approach to either link an existing user or create a new one works well. However, if multiple concurrent requests arrive for the same Google profile, there's a small risk of race conditions. Also, user creation logic appears in multiple places (e.g., in the
registermethod). Consolidating this into a dedicated helper function or repository method could eliminate duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
backend/package.json(2 hunks)backend/src/auth/auth.module.ts(2 hunks)backend/src/auth/auth.service.ts(1 hunks)backend/src/auth/google.controller.ts(1 hunks)backend/src/auth/oauth/GoogleStrategy.ts(1 hunks)backend/src/user/user.model.ts(1 hunks)frontend/.env.example(1 hunks)frontend/src/app/auth/oauth-callback/page.tsx(1 hunks)frontend/src/components/sign-in-modal.tsx(1 hunks)frontend/src/components/sign-up-modal.tsx(1 hunks)
🔇 Additional comments (9)
frontend/.env.example (1)
2-2: Good addition of Google OAuth configuration.The Google OAuth endpoint environment variable is correctly defined following Next.js naming conventions with the
NEXT_PUBLIC_prefix for client-side access.frontend/src/components/sign-up-modal.tsx (1)
279-284: Google OAuth redirects implemented correctly.The implementation properly redirects to the Google OAuth endpoint using the environment variable with a fallback URL.
This code is duplicated from the sign-in modal. Consider extracting this logic into a shared utility function as suggested in the sign-in modal review.
backend/src/auth/auth.module.ts (2)
13-14: Google OAuth strategy and controller correctly imported.The necessary Google authentication components are properly imported.
31-32:Details
✅ Verification successful
Google OAuth components correctly registered in the module.
The
GoogleControllerandGoogleStrategyare properly added to the module configuration.Verify that all required dependencies for Google OAuth are installed in the project:
🏁 Script executed:
#!/bin/bash # Check for Passport.js and Google OAuth dependencies grep -E "passport|passport-google-oauth20" backend/package.jsonLength of output: 167
Google OAuth Module Integration Verified
The auth module in
backend/src/auth/auth.module.ts(lines 31-32) is correctly configured, withGoogleControllerandGoogleStrategyproperly registered. The required dependencies (@nestjs/passport,passport, andpassport-google-oauth20) are also present inbackend/package.json.backend/package.json (1)
42-42: Dependencies correctly added for Google OAuth integration!The addition of
@nestjs/passport,passport, andpassport-google-oauth20packages with appropriate versions provides all the necessary dependencies for implementing Google OAuth authentication in the NestJS backend.Also applies to: 68-69
backend/src/user/user.model.ts (2)
24-25: Consider adding @field decorator for GraphQL schema exposureThe
googleIdfield isn't marked with a@Field()decorator. If this field needs to be accessible through GraphQL queries, you should add the decorator; if this is intentional for security reasons, please add a comment explaining why.
31-32: Password field correctly made nullable for OAuth usersGood job making the password field nullable to accommodate OAuth users who authenticate without passwords.
backend/src/auth/google.controller.ts (2)
1-11: Constructor and imports look fine.The injection of both
ConfigServiceandAuthServiceis straightforward. No issues found here.
13-18: Google OAuth initiation method is concise and clear.Using the
AuthGuard('google')here is a standard approach to trigger the Google OAuth flow. Nice job!
| @Column({ nullable: true }) | ||
| googleId: string; | ||
|
|
||
| @Field() | ||
| @Column() | ||
| username: string; | ||
|
|
||
| @Column() | ||
| @Column({ nullable: true }) // Made nullable for OAuth users | ||
| password: string; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify database migration for schema changes
These model changes will require a database migration. Ensure you've created a migration file to update the schema for existing databases.
🏁 Script executed:
#!/bin/bash
# Check if there are migration files related to this change
fd -e ts -d 3 "migration|user.*google" srcLength of output: 135
Action: Verify and Confirm Database Migration Implementation
The changes in backend/src/user/user.model.ts—adding the googleId field and making the password column nullable—require a corresponding database migration. The previous script failed due to an incorrect search path (src is not a directory). Please verify that a migration file addressing these schema changes exists in the correct directory (e.g., within the backend directory).
-
The model changes affecting
googleIdandpasswordneed an accompanying migration for existing databases. -
Next Step: Manually confirm or run the following shell script to search for migration files in the
backendfolder:#!/bin/bash # Search for migration files in the 'backend' directory fd -e ts "migration" backend
Once you confirm that an appropriate migration file is present (or create one if missing), please update the migration as necessary so that the database schema matches these model changes.
Action Required: Verify Database Migration for User Model Changes
The model change in backend/src/user/user.model.ts—adding the googleId field and marking the password field as nullable—requires a corresponding database migration. Since the initial search script used an invalid path, please run the following shell script to check for migration files in the common directories. If no migration file is found that addresses these schema changes, please create the necessary migration file.
- Ensure a migration file exists (typically in
backend/migrationsordatabase/migrations) to update existing databases. - Use the provided script to search for migration files:
🏁 Script executed:
#!/bin/bash
# Search for migration files in common migration directories
echo "Searching for migration files in backend/migrations:"
find backend/migrations -type f -name "*.ts" 2>/dev/null
echo "Searching for migration files in database/migrations:"
find database/migrations -type f -name "*.ts" 2>/dev/nullLength of output: 448
Action: Create the Required Migration File for User Model Schema Changes
It appears that no migration files were found in the common migration directories (i.e., backend/migrations and database/migrations). Since the model changes—adding the googleId field and marking the password field as nullable—affect the database schema, a corresponding migration file is required to update existing databases.
- User Model Location:
backend/src/user/user.model.ts(Lines 24-32) - Required Changes:
- Add migration steps for adding the new
googleIdcolumn. - Update the
passwordcolumn to be nullable.
- Add migration steps for adding the new
- Next Steps:
- Verify if a migration file exists in a non-standard directory.
- If not, create a migration file reflecting these model changes.
Please create or update the migration accordingly.
| async validate( | ||
| accessToken: string, | ||
| refreshToken: string, | ||
| profile: any, | ||
| done: VerifyCallback, | ||
| ): Promise<any> { | ||
| const { name, emails, photos, id } = profile; | ||
| Logger.log(`Google profile ID: ${id}`); | ||
|
|
||
| const user = { | ||
| id: id, // Include the profile ID | ||
| googleId: id, // Also map to googleId | ||
| email: emails[0].value, | ||
| firstName: name.givenName, | ||
| lastName: name.familyName, | ||
| picture: photos[0].value, | ||
| accessToken, | ||
| refreshToken, | ||
| }; | ||
|
|
||
| done(null, user); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and type safety to the validate method
The validate method has several potential improvements:
- Use proper TypeScript types for the Google profile
- Add error handling for missing profile data
- Consider privacy implications of logging Google profile IDs
- async validate(
- accessToken: string,
- refreshToken: string,
- profile: any,
- done: VerifyCallback,
- ): Promise<any> {
+ async validate(
+ accessToken: string,
+ refreshToken: string,
+ profile: Profile,
+ done: VerifyCallback,
+ ): Promise<void> {
const { name, emails, photos, id } = profile;
- Logger.log(`Google profile ID: ${id}`);
+ Logger.debug('Google authentication successful');
+ if (!emails || emails.length === 0) {
+ return done(new Error('No email provided from Google'), null);
+ }
const user = {
id: id, // Include the profile ID
googleId: id, // Also map to googleId
email: emails[0].value,
- firstName: name.givenName,
- lastName: name.familyName,
- picture: photos[0].value,
+ firstName: name?.givenName || '',
+ lastName: name?.familyName || '',
+ picture: photos && photos.length > 0 ? photos[0].value : null,
accessToken,
refreshToken,
};
done(null, user);
}Don't forget to add the Profile type import:
import { Profile, Strategy, VerifyCallback } from 'passport-google-oauth20';| clientID: | ||
| configService.get<string>('GOOGLE_CLIENT_ID') || | ||
| 'Just_a_placeholder_GOOGLE_CLIENT_ID', | ||
| clientSecret: | ||
| configService.get<string>('GOOGLE_SECRET') || | ||
| 'Just_a_placeholder_GOOGLE_SECRET', | ||
| callbackURL: configService.get<string>('GOOGLE_CALLBACK_URL'), | ||
| scope: ['email', 'profile'], | ||
| prompt: 'select_account', | ||
| }); |
There was a problem hiding this comment.
Remove placeholder secrets in production code
Using placeholder values for sensitive credentials is risky and could lead to security issues if deployed to production. Instead of hardcoded fallbacks:
- Fail fast by throwing clear configuration errors
- Use environment validation at application startup
- clientID:
- configService.get<string>('GOOGLE_CLIENT_ID') ||
- 'Just_a_placeholder_GOOGLE_CLIENT_ID',
- clientSecret:
- configService.get<string>('GOOGLE_SECRET') ||
- 'Just_a_placeholder_GOOGLE_SECRET',
+ clientID: configService.get<string>('GOOGLE_CLIENT_ID'),
+ clientSecret: configService.get<string>('GOOGLE_SECRET'),📝 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.
| clientID: | |
| configService.get<string>('GOOGLE_CLIENT_ID') || | |
| 'Just_a_placeholder_GOOGLE_CLIENT_ID', | |
| clientSecret: | |
| configService.get<string>('GOOGLE_SECRET') || | |
| 'Just_a_placeholder_GOOGLE_SECRET', | |
| callbackURL: configService.get<string>('GOOGLE_CALLBACK_URL'), | |
| scope: ['email', 'profile'], | |
| prompt: 'select_account', | |
| }); | |
| clientID: configService.get<string>('GOOGLE_CLIENT_ID'), | |
| clientSecret: configService.get<string>('GOOGLE_SECRET'), | |
| callbackURL: configService.get<string>('GOOGLE_CALLBACK_URL'), | |
| scope: ['email', 'profile'], | |
| prompt: 'select_account', | |
| }); |
| const accessToken = searchParams.get('accessToken'); | ||
| const refreshToken = searchParams.get('refreshToken'); | ||
| const error = searchParams.get('error'); |
There was a problem hiding this comment.
Security risk: Tokens exposed in URL parameters
Passing tokens as URL parameters poses security risks:
- Tokens can be captured in browser history
- They may appear in server logs and referrer headers
- They're visible in bookmarks
Consider using a more secure approach like exchanging a temporary code for tokens.
| // TO DO IS UNSAFE | ||
| // Redirect to frontend, pass tokens in query params | ||
| return res.redirect( | ||
| `${frontendUrl}/auth/oauth-callback?accessToken=${accessToken}&refreshToken=${refreshToken}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Avoid passing tokens in the query string to prevent potential token leakage.
Embedding tokens in the URL query parameters can lead to security issues (e.g., tokens in logs, browser history, and referrer headers). As an alternative, you could set an HTTP-only cookie or use a secure POST form submission. For instance:
- return res.redirect(
- `${frontendUrl}/auth/oauth-callback?accessToken=${accessToken}&refreshToken=${refreshToken}`
- );
+ res.cookie('accessToken', accessToken, {
+ httpOnly: true,
+ secure: true,
+ sameSite: 'strict',
+ });
+ res.cookie('refreshToken', refreshToken, {
+ httpOnly: true,
+ secure: true,
+ sameSite: 'strict',
+ });
+ return res.redirect(`${frontendUrl}/auth/oauth-callback`);📝 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.
| // TO DO IS UNSAFE | |
| // Redirect to frontend, pass tokens in query params | |
| return res.redirect( | |
| `${frontendUrl}/auth/oauth-callback?accessToken=${accessToken}&refreshToken=${refreshToken}`, | |
| ); | |
| } | |
| // TO DO IS UNSAFE | |
| // Redirect to frontend, pass tokens in query params | |
| res.cookie('accessToken', accessToken, { | |
| httpOnly: true, | |
| secure: true, | |
| sameSite: 'strict', | |
| }); | |
| res.cookie('refreshToken', refreshToken, { | |
| httpOnly: true, | |
| secure: true, | |
| sameSite: 'strict', | |
| }); | |
| return res.redirect(`${frontendUrl}/auth/oauth-callback`); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/.env.example (1)
35-38: Google OAuth Configuration AddedThe new block for Google OAuth environment variables is clear and concise. It correctly introduces the required values:
GOOGLE_CLIENT_ID=YOUR_CLIENT_IDGOOGLE_SECRET=Your_SECRETGOOGLE_CALLBACK_URL=http://localhost:8080/auth/google/callbackSuggestions:
- Variable Naming Consistency: Consider whether the variable
GOOGLE_SECRETshould be renamed toGOOGLE_CLIENT_SECRETfor clarity and consistency with common naming conventions. If the rest of the code expectsGOOGLE_SECRET, this is fine; otherwise, a consistent naming approach may help avoid confusion.- Placeholder Casing: The placeholder for the secret is written as
Your_SECRETwith mixed case. You might standardize it (e.g.,YOUR_SECRET) to align with the client ID placeholder for clarity.Overall, these additions meet the intended objectives for enabling Google OAuth integration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/.env.example(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Install and Build Frontend
- GitHub Check: Install and Build Backend
Summary by CodeRabbit
New Features
Bug Fixes