Skip to content

Fix: Retrieve Twitter and GitHub usernames via OAuth#127

Merged
kangmingtay merged 4 commits into
supabase:masterfrom
dsumer:master
Jul 8, 2021
Merged

Fix: Retrieve Twitter and GitHub usernames via OAuth#127
kangmingtay merged 4 commits into
supabase:masterfrom
dsumer:master

Conversation

@dsumer
Copy link
Copy Markdown
Contributor

@dsumer dsumer commented Jul 1, 2021

What kind of change does this PR introduce?

During the OAuth login the usernames from Twitter and GitHub are retrieved and saved as meta_data of the user.

Additionally I've changed the authorization URL of Twitter from https://api.twitter.com/oauth/authorize to https://api.twitter.com/oauth/authenticate so that the user only has to authorize the application on the first login, instead of at every login.

What is the current behavior?

Usernames from Twitter and GitHub are not retrieved.

The user has to authorize the Twitter App on every login again.

What is the new behavior?

Usernames from Twitter and GitHub are retrieved and saved as meta_data.

The user only has to authorize the Twitter App on the first login.

Additional context

Description for the /authenticate URL from Twitter:
https://developer.twitter.com/en/docs/authentication/api-reference/authenticate

Comment thread api/provider/twitter.go Outdated
Comment thread api/provider/provider.go
Comment thread api/provider/github.go
Comment thread api/provider/twitter.go
Copy link
Copy Markdown
Member

@kangmingtay kangmingtay left a comment

Choose a reason for hiding this comment

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

Hey @dsumer, thanks for contributing! Yeah, initially, I decided to use the /authorize endpoint instead of /authenticate because the twitter docs kinda recommended it. But, in terms of user-friendliness, i can see why it can be a pain to ask your users if they want to share their data every time they sign in.

@kangmingtay kangmingtay changed the title Feat: Retrieve Twitter and GitHub usernames via OAuth Fix: Retrieve Twitter and GitHub usernames via OAuth Jul 2, 2021
@dsumer
Copy link
Copy Markdown
Contributor Author

dsumer commented Jul 8, 2021

@kangmingtay @awalias how should we proceed with this? :) .. I can also change the user_name field to 'login' and 'screen_name' respectively

this change is a bit of a blocker for me as I need the username in order to integrate supabase auth into my product :D

@kangmingtay
Copy link
Copy Markdown
Member

@kangmingtay @awalias how should we proceed with this? :) .. I can also change the user_name field to 'login' and 'screen_name' respectively

sorry for the delay, yeah we had a discussion today about this and we agreed that gotrue should take care of abstracting the definition of user_name for the OAuth providers. thanks for the contribution! :)

@kangmingtay kangmingtay merged commit 8a49414 into supabase:master Jul 8, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 8, 2021

🎉 This PR is included in version 1.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@dsumer
Copy link
Copy Markdown
Contributor Author

dsumer commented Jul 11, 2021

@kangmingtay thanks for the fast merge! when will those changes be available in production?

@kangmingtay
Copy link
Copy Markdown
Member

@dsumer we're currently testing it out with a few other changes (e.g. twitch integration & custom smtp sender name) in our staging environment and will be looking to roll it out in the next few weeks!

@Ehesp
Copy link
Copy Markdown

Ehesp commented Jul 28, 2021

Hey @kangmingtay, any eta on this? At the moment I'm having to query the github api like this: #83 (comment)

@Ehesp
Copy link
Copy Markdown

Ehesp commented Jul 28, 2021

Oh wait, for some reason it now exists. Think I needed additional scope... doh!

uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
Fix: Retrieve Twitter and GitHub usernames via OAuth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants