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

fix: Make oAuth paths more consistent and improve authentication client #1377

Merged
merged 4 commits into from
May 31, 2019

Conversation

daffl
Copy link
Member

@daffl daffl commented May 29, 2019

This pull request makes the oAuth callback paths consistent. It uses the redirects discussed in simov/grant#116 to make things more straightforward and backwards compatible. Also changes the base path to /oauth as previously discussed with @KidkArolis (it can be made backwards compatible by setting authentication.oauth.defaults.path to /auth).

The relevant oAuth paths now are:

  • http(s)://<host>/oauth/<provider> to initiate the oAuth flow
  • http(s)://<host>/oauth/<provider>/callback as the callback path that should be set in the oAuth application settings
  • http(s)://<host>/oauth/<provider>/authenticate as the internal redirect

@KidkArolis
Copy link
Contributor

Looks good. Are you gonna be adding backwards compatible redirects?

@daffl
Copy link
Member Author

daffl commented May 29, 2019

I tried but couldn't get it to work. oAuth always errors when I try to redirect from a backwards compatible callback path. At the moment it looks like this is only possible if my PR in Grant gets accepted. I might submit it again to discuss more.

@daffl
Copy link
Member Author

daffl commented May 30, 2019

@KidkArolis Allright, got some help and managed to make the paths backwards compatible (updated in the description).

@daffl daffl merged commit adb2543 into master May 31, 2019
@daffl daffl deleted the oauth-final branch May 31, 2019 18:05
EliSadaka pushed a commit to yusernetwork/authentication-oauth that referenced this pull request Oct 20, 2020
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.

2 participants