-
Notifications
You must be signed in to change notification settings - Fork 3
feat(backend, frontend) google login #192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5408e5a
7622d25
6c8adb4
dc8f0b7
1bbbdc0
e90c9b4
1e0dcda
fe0ce46
4c66b8f
d2c94ea
93abb69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { Controller, Get, Logger, Req, Res, UseGuards } from '@nestjs/common'; | ||
| import { AuthGuard } from '@nestjs/passport'; | ||
| import { ConfigService } from '@nestjs/config'; | ||
| import { AuthService } from './auth.service'; | ||
|
|
||
| @Controller('auth') | ||
| export class GoogleController { | ||
| constructor( | ||
| private configService: ConfigService, | ||
| private authService: AuthService, | ||
| ) {} | ||
|
|
||
| @Get('google') | ||
| @UseGuards(AuthGuard('google')) | ||
| async googleAuth() { | ||
| // This route initiates the Google OAuth flow | ||
| // The guard redirects to Google | ||
| } | ||
|
|
||
| @Get('google/callback') | ||
| @UseGuards(AuthGuard('google')) | ||
| async googleAuthCallback(@Req() req, @Res() res) { | ||
| Logger.log('Google callback'); | ||
| const googleProfile = req.user as { | ||
| googleId: string; | ||
| email: string; | ||
| firstName?: string; | ||
| lastName?: string; | ||
| }; | ||
|
|
||
| // Call the AuthService method | ||
| const { accessToken, refreshToken } = | ||
| await this.authService.handleGoogleCallback(googleProfile); | ||
|
|
||
| const frontendUrl = | ||
| this.configService.get<string>('FRONTEND_URL') || 'http://localhost:3000'; | ||
|
|
||
| // TO DO IS UNSAFE | ||
| // Redirect to frontend, pass tokens in query params | ||
| return res.redirect( | ||
| `${frontendUrl}/auth/oauth-callback?accessToken=${accessToken}&refreshToken=${refreshToken}`, | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||||||||||||||||||||||
| import { Injectable, Logger } from '@nestjs/common'; | ||||||||||||||||||||||||||||||||||
| import { PassportStrategy } from '@nestjs/passport'; | ||||||||||||||||||||||||||||||||||
| import { Strategy, VerifyCallback } from 'passport-google-oauth20'; | ||||||||||||||||||||||||||||||||||
| import { ConfigService } from '@nestjs/config'; | ||||||||||||||||||||||||||||||||||
| import { AuthService } from '../auth.service'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @Injectable() | ||||||||||||||||||||||||||||||||||
| export class GoogleStrategy extends PassportStrategy(Strategy, 'google') { | ||||||||||||||||||||||||||||||||||
| constructor( | ||||||||||||||||||||||||||||||||||
| private configService: ConfigService, | ||||||||||||||||||||||||||||||||||
| private authService: AuthService, | ||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||
| super({ | ||||||||||||||||||||||||||||||||||
| 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', | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
- 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
Suggested change
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling and type safety to the validate method The validate method has several potential improvements:
- 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'; |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,11 +21,14 @@ export class User extends SystemBaseModel { | |
| @PrimaryGeneratedColumn('uuid') | ||
| id: string; | ||
|
|
||
| @Column({ nullable: true }) | ||
| googleId: string; | ||
|
|
||
| @Field() | ||
| @Column() | ||
| username: string; | ||
|
|
||
| @Column() | ||
| @Column({ nullable: true }) // Made nullable for OAuth users | ||
| password: string; | ||
|
Comment on lines
+24
to
32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify 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
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
🏁 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.,
Please create or update the migration accordingly. |
||
|
|
||
| @Field({ nullable: true }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| 'use client'; | ||
|
|
||
| import { useEffect } from 'react'; | ||
| import { useRouter, useSearchParams } from 'next/navigation'; | ||
| import { useAuthContext } from '@/providers/AuthProvider'; | ||
| import { toast } from 'sonner'; | ||
|
|
||
| export default function OAuthCallbackPage() { | ||
| const router = useRouter(); | ||
| const searchParams = useSearchParams(); | ||
| const { login } = useAuthContext(); | ||
|
|
||
| useEffect(() => { | ||
| const handleAuth = async () => { | ||
| try { | ||
| const accessToken = searchParams.get('accessToken'); | ||
| const refreshToken = searchParams.get('refreshToken'); | ||
| const error = searchParams.get('error'); | ||
|
Comment on lines
+16
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security risk: Tokens exposed in URL parameters Passing tokens as URL parameters poses security risks:
Consider using a more secure approach like exchanging a temporary code for tokens. |
||
|
|
||
| // Handle error cases | ||
| if (error) { | ||
| console.error('Authentication error:', error); | ||
| toast.error('Authentication failed'); | ||
| router.push('/login'); | ||
| return; | ||
| } | ||
|
|
||
| // Check if tokens exist | ||
| if (!accessToken || !refreshToken) { | ||
| console.error('Missing tokens in callback'); | ||
| toast.error('Authentication failed: Missing tokens'); | ||
| router.push('/login'); | ||
| return; | ||
| } | ||
|
|
||
| // Store tokens using the context | ||
| login(accessToken, refreshToken); | ||
|
|
||
| // Show success message | ||
| toast.success('Logged in successfully!'); | ||
|
|
||
| // Redirect to home or dashboard | ||
| router.push('/'); | ||
| } catch (error) { | ||
| console.error('Error processing authentication:', error); | ||
| toast.error('Authentication processing failed'); | ||
| router.push('/login'); | ||
| } | ||
| }; | ||
|
|
||
| handleAuth(); | ||
| }, [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> | ||
| <p className="text-neutral-600 dark:text-neutral-400 mb-4"> | ||
| Please wait while we sign you in. | ||
| </p> | ||
| <div className="flex justify-center"> | ||
| <div className="w-8 h-8 border-4 border-t-primary rounded-full animate-spin"></div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
📝 Committable suggestion