-
Notifications
You must be signed in to change notification settings - Fork 326
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
Add Snapchat OAuth provider #449
Conversation
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.
hey @nwneisen, thanks for the contribution! could you kindly document the steps you took to set up the oauth2 app on snapchat? i've tried testing out your PR locally but am running into this issue on the callback / redirect
}) | ||
} else { | ||
data.Emails = append(data.Emails, Email{ | ||
Email: "Email not supported with Snapchat OAuth", |
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.
hmm this is pretty hacky and it wouldn't really work out because we have a unique constraint on the email column
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.
I agree. It doesn't look like there is a great solution ATM. I took a look at #414 and left a comment. Let me know if there's anything else that could help get it merged.
} | ||
|
||
func (g snapchatProvider) GetOAuthToken(code string) (*oauth2.Token, error) { | ||
return g.Exchange(oauth2.NoContext, code) |
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.
oauth2.NoContext
seems to be deprecated in favour of context.Background()
- lets use that instead
@kangmingtay This is how I have my snapchat configured. Let me know if you need additional info for any section.
|
Hey this PR has been outdated for a long while now. I'll close it, but if you still wish to contribute please re-open it or submit a new one! |
This was waiting for the email constraint to be removed. Otherwise, everything should've been addressed for it. |
@nwneisen Oh sorry, I may have rushed too much. |
What kind of change does this PR introduce?
Adds a feature for Snapchat OAuth. Resolves #436
What is the current behavior?
Snapchat is not available as an OAuth provided
What is the new behavior?
Snapchat can be used as an OAuth provider using the same process as the other OAuth providers found in GoTrue
Additional context
Snapchat OAuth does not use an email. This runs in to the same issue as #214. I put a hack in to get around the checks but I'm sure we want something better.