Skip to content
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

feat-#429: Added Signin with Google button #454

Conversation

siddharthisrani
Copy link
Contributor

Summary
This PR introduces the 'Sign in with Google' button to the login page, enabling users to authenticate using their Google accounts.

Description
This PR addresses the need for Google authentication within the WanderLust application. The changes include:

Addition of 'Sign in with Google' Button:
Implemented a new button on the login page to initiate Google sign-in.

Google OAuth2 Strategy Integration:
Integrated Passport.js with the Google OAuth2 strategy for handling authentication.

Google OAuth2 Configuration:
Configured the application to use Google OAuth2 credentials (client ID and secret) via environment variables.

OAuth2 Callback Handling:
Implemented the logic to handle the Google OAuth2 callback, authenticate users, and create user sessions.

Frontend UI Update:
Updated the frontend to display 'Create Post' and 'Logout' buttons upon successful Google login.

Technical details:
The Google OAuth2 client ID and secret are securely managed through environment variables.
Passport.js is used to manage the Google authentication strategy.
The frontend dynamically updates to reflect the user's authenticated state.

Issue(s) Addressed
Closes #429

Prerequisites
Yes followed all the CONTRIBUTING GUIDELINES?

Copy link

vercel bot commented Jul 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
wanderlust ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 11:08am
wanderlust-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 11:08am

Copy link

Hey @siddharthisrani! Thanks for sticking to the guidelines! High five! 🙌🏻

@krishnaacharyaa
Copy link
Owner

Please attach a video of a working demo

@krishnaacharyaa
Copy link
Owner

@Sukomal07 can you please review

@krishnaacharyaa
Copy link
Owner

@Deblin798 feel free to review

@siddharthisrani
Copy link
Contributor Author

Please attach a video of a working demo

InShot_20240721_134133218.mp4

Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @siddharthisrani
Just couple of review comments, kindly address
And if any env variables are used, add it in the .env.sample

clientID: process.env.GOOGLE_CLIENT_ID,
clientSecret: process.env.GOOGLE_CLIENT_SECRET,
callbackURL: 'http://localhost:8080/api/auth/google/callback',
},
Copy link
Owner

Choose a reason for hiding this comment

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

As we are deploying in vercel, we can't hardcode this to 8080, kindly move this to evn variable or in this case i guess you need to use the backend_url env variable

sameSite: 'lax',
});
res.redirect('http://localhost:5173/signup?google-callback=true');
}
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, kindly use frontend url env variable


const handleGoogleLogin = () => {
window.location.href = 'http://localhost:8080/api/auth/google';
};
Copy link
Owner

Choose a reason for hiding this comment

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

Same here

const response = await axiosInstance.get('/api/auth/check');
const { token, user } = response.data;
localStorage.setItem('token', token);
if (user && user._id && user.role) {
Copy link
Owner

Choose a reason for hiding this comment

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

localstorage we are setting token not in the cookies? @Sukomal07 kindly confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

@siddharthisrani Is this a GoogleAuth token? In the /auth/check route, you collect token from cookies
const token = req.cookies.access_token;
Then why do you set Token in localstorage ?

@siddharthisrani
Copy link
Contributor Author

Hii @krishnaacharyaa the requested changes to use environment variables for URLs in Google OAuth and redirects

Changes Made:
Used environment variables for URLs in Google OAuth and redirects.
Updated .env.sample to include the new environment variables.

@krishnaacharyaa
Copy link
Owner

@Sukomal07 can you please have a look at this PR

@Sukomal07
Copy link
Contributor

@Sukomal07 can you please have a look at this PR

@siddharthisrani didn't reply to my question.

@krishnaacharyaa
Copy link
Owner

@siddharthisrani please sync up with @Sukomal07 , so that we can merge this

@siddharthisrani
Copy link
Contributor Author

@siddharthisrani didn't reply to my question.
Hi @Sukomal07 and @krishnaacharyaa ,

Regarding the question about token storage:

In the current implementation, the token is being set in local storage after a successful Google OAuth login. This was done to maintain consistency with other parts of the application that also use local storage for token management.

However, from the backend, we are also sending the token in cookies to enhance security and manage user sessions more effectively. Here’s the relevant code:

router.get(
'/google/callback',
passport.authenticate('google', { failureRedirect: '/' }),
(req, res) => {
const token = jwt.sign({ id: req.user._id }, process.env.JWT_SECRET, { expiresIn: '1h' });
res.cookie('access_token', token, {
httpOnly: true,

Cookie

This implementation ensures that the token is available in cookies for enhanced security, while local storage is used to maintain consistency in token management across the application.

Please let me know if my answer is according to your requirements. If any further changes needed i will be happy to assist you.

@Sukomal07
Copy link
Contributor

@siddharthisrani can we do without saving token in localstorage ?

@siddharthisrani
Copy link
Contributor Author

@siddharthisrani can we do without saving token in localstorage ?

Sure ,i will look in this part for solutions and get back to you .

@siddharthisrani
Copy link
Contributor Author

Hii @Sukomal07
Yes, it's working well without saving the token in local storage.
localStorage
removedLocalStorage

@Sukomal07
Copy link
Contributor

@siddharthisrani Nice , then remove localstorage part from all of your code.

@siddharthisrani
Copy link
Contributor Author

Hii @Sukomal07,

I've removed the token saving in local storage from all the relevant parts of the code. Please review the latest commits.

const response = await axiosInstance.get('/api/auth/check');
const { token, user } = response.data;
localStorage.setItem('token', token);
if (user && user._id && user.role) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@siddharthisrani Is this a GoogleAuth token? In the /auth/check route, you collect token from cookies
const token = req.cookies.access_token;
Then why do you set Token in localstorage ?

Copy link
Owner

@krishnaacharyaa krishnaacharyaa left a comment

Choose a reason for hiding this comment

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

Thank you @siddharthisrani for your active contribution

@krishnaacharyaa krishnaacharyaa merged commit f157e4a into krishnaacharyaa:main Jul 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] :Want to Add signin with google button
3 participants