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: OAuth #35

Merged
merged 8 commits into from
Apr 6, 2023
Merged

feat: OAuth #35

merged 8 commits into from
Apr 6, 2023

Conversation

iuioiua
Copy link
Contributor

@iuioiua iuioiua commented Apr 3, 2023

This change adds OAuth functionality for authentication. So far, GitHub is used as an example.

I'm aware that this approach is suboptimal compared to the incoming PKCE approach but would rather enable OAuth sooner rather than later. We'll switch to PKCE once available.

Towards #13.

@thorwebdev and @kt3k , WDYT?

@iuioiua iuioiua mentioned this pull request Apr 3, 2023
4 tasks
@thorwebdev
Copy link
Contributor

Thanks @iuioiua , and sorry for the delay on PCKE. We needed to fix some issues in gotrue-js but those have now been released. Hoping we'll land PCKE support in the auth helpers by end of week!

@iuioiua
Copy link
Contributor Author

iuioiua commented Apr 3, 2023

Thanks @iuioiua , and sorry for the delay on PCKE. We needed to fix some issues in gotrue-js but those have now been released. Hoping we'll land PCKE support in the auth helpers by end of week!

Not a problem! One of the reasons I made this PR was so that we can directly see the improvement once PKCE has landed.

@deno-deploy deno-deploy bot requested a deployment to Preview April 4, 2023 06:18 Abandoned
@iuioiua iuioiua requested review from kt3k and hashrock April 5, 2023 01:59
@iuioiua
Copy link
Contributor Author

iuioiua commented Apr 5, 2023

@thorwebdev, does the implementation look sound to you? It was taken from https://github.com/thorwebdev/everydaylytics.

Copy link
Contributor

@thorwebdev thorwebdev left a comment

Choose a reason for hiding this comment

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

Yes, LGTM 👍

@iuioiua iuioiua removed the request for review from hashrock April 5, 2023 08:05
routes/signup.tsx Outdated Show resolved Hide resolved
routes/api/oauth.ts Outdated Show resolved Hide resolved
@kt3k
Copy link
Member

kt3k commented Apr 5, 2023

Other part looks good to me.

@iuioiua
Copy link
Contributor Author

iuioiua commented Apr 5, 2023

Changes made:

  1. Login/signup pages look prettier.
  2. Changed /login-success route file to /login/success and /login to /login/index.tsx.
  3. If provider isn't a string, return HTTP code 400.

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.

3 participants