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

Allow users to authenticate with Google OAuth #414

Merged
merged 16 commits into from
Feb 16, 2024
Merged

Allow users to authenticate with Google OAuth #414

merged 16 commits into from
Feb 16, 2024

Conversation

WangGithub0
Copy link
Collaborator

@WangGithub0 WangGithub0 commented Feb 7, 2024

Add google login according to OAuth 2.0 for server-side Web Applications

Step 1: Redirect to Google's OAuth 2.0 server
Step 2: Google prompts user for consent
Step 3: Handle the OAuth 2.0 server response - get code
Step 4: Calling Google APIs - get user info using access_token

I provide my google client_id, which can be used in testing on http://localhost:5173/ :
add these in .dev.vars
GOOGLE_CLIENT_ID=70478082635-iaa28pt6bg1h06ooeic3vo8fgtu90trh.apps.googleusercontent.com
GOOGLE_CLIENT_SECRET=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
GOOGLE_REDIRECT_URI=http://localhost:9339/api/login/
GOOGLE_RESPONSE_TYPE=code
GOOGLE_SCOPE=profile email

if need test handleGoogleProdLogin in local, please directly return handleGoogleProdLogin function for onRequestGet: PagesFunction in file login.ts

image image image

resolve #330

@WangGithub0 WangGithub0 self-assigned this Feb 7, 2024
Copy link

cloudflare-pages bot commented Feb 7, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5a825df
Status: ✅  Deploy successful!
Preview URL: https://617f8299.console-overthinker-dev.pages.dev
Branch Preview URL: https://issue-330.console-overthinker-dev.pages.dev

View logs

Copy link
Collaborator

@humphd humphd left a comment

Choose a reason for hiding this comment

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

We should probably set a time to meet and go over this approach. Doing this in CloudFlare will require some alternate approaches.

I also notice that other people have solved this, for example: https://github.com/Schachte/cloudflare-google-auth and the blog post at https://ryan-schachte.com/blog/oauth_cloudflare_workers/

functions/google.ts Outdated Show resolved Hide resolved
functions/google.ts Outdated Show resolved Hide resolved
functions/google.ts Outdated Show resolved Hide resolved
functions/google.ts Outdated Show resolved Hide resolved
@WangGithub0 WangGithub0 changed the title Allow users to authenticate with Google OAuth - step1 get google info Allow users to authenticate with Google OAuth Feb 14, 2024
Copy link
Collaborator

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This is an amazing piece of work, well done @WangGithub0! I'm really excited to see this land for 1.3.

I've left some feedback. You can fix this now, or in follow-ups, but if you do it in follow-ups, please file issues.

functions/api/login.ts Outdated Show resolved Hide resolved
functions/api/login.ts Outdated Show resolved Hide resolved
functions/api/login.ts Outdated Show resolved Hide resolved
functions/api/login.ts Outdated Show resolved Hide resolved
src/components/Header.tsx Outdated Show resolved Hide resolved
src/router.tsx Outdated Show resolved Hide resolved
@humphd
Copy link
Collaborator

humphd commented Feb 15, 2024

@WangGithub0 please click "Resolve" for each of the review comments above that you've fixed, to indicate that they are no longer an issue we need to track/discuss.

functions/api/login.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@Amnish04 Amnish04 left a comment

Choose a reason for hiding this comment

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

I tried logging in with google on Cloudfare deployed url and was a success, but I guess it was a dev login.
image

I've also gone through the code, and even though I don't fully understand each and everything, I've gained a basic understanding of the flow and how we're managing different auth providers.

As @humphd said, this is a great piece of work! I can understand the pain of implementing Auth since I recently had to do it for the capstone project.

I can't suggest any big changes at this point as lots of improvements have already been made going through the conversations, but I've suggested some minor changes and commented some questions for my clarity.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its nice that you are also adding tests as this won't be changing much in the future, and we'll always know if it breaks at any point!

functions/api/login.ts Show resolved Hide resolved
functions/google.ts Outdated Show resolved Hide resolved
@@ -13,7 +13,7 @@ import { isProd } from "../lib/utils";

type UserContextType = {
user?: User;
login: (chatId?: string) => void;
login: (provider: string, chatId?: string) => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we have an enum for supported auth providers somewhere in functions/lib?

@WangGithub0
Copy link
Collaborator Author

I tried logging in with google on Cloudfare deployed url and was a success, but I guess it was a dev login. image

I've also gone through the code, and even though I don't fully understand each and everything, I've gained a basic understanding of the flow and how we're managing different auth providers.

As @humphd said, this is a great piece of work! I can understand the pain of implementing Auth since I recently had to do it for the capstone project.

I can't suggest any big changes at this point as lots of improvements have already been made going through the conversations, but I've suggested some minor changes and commented some questions for my clarity.

Yes, it's dev. You can set isDev:false at line58 in login.ts to test product environment
if (provider === "google") {
return handleGoogleLogin({
isDev: true,

@humphd
Copy link
Collaborator

humphd commented Feb 16, 2024

@WangGithub0 can you "Squash and merge" this when you have a minute? I'd love to see this land in 1.3! Such a cool new feature.

@WangGithub0
Copy link
Collaborator Author

WangGithub0 commented Feb 16, 2024

@WangGithub0 can you "Squash and merge" this when you have a minute? I'd love to see this land in 1.3! Such a cool new feature.

Do we have a .dev.vars for product environment? could you add ChatCraft GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET in it?

@humphd
Copy link
Collaborator

humphd commented Feb 16, 2024

@WangGithub0 can you "Squash and merge" this when you have a minute? I'd love to see this land in 1.3! Such a cool new feature.

Do we have a .dev.vars for product environment? could you add ChatCraft GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET in it?

I've updated the CloudFlare environment variables for production.

@WangGithub0 WangGithub0 merged commit 5b028cc into main Feb 16, 2024
4 checks passed
@WangGithub0 WangGithub0 deleted the issue-330 branch February 16, 2024 15:43
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.

Allow users to authenticate with Google OAuth
3 participants