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

add follow redirect params flag in oauth2 final response #318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

enrico-neri-zerynth
Copy link

Without the follow redir param set to true, it's not so direct to test in the RedirectNonApi method of the redirector if the redir path contains the oauth2 provider authentication url or the user's redirect url typed in the url query.

In the redirect API method, the followRedirectParam field is used to checks the value of redir, in oauth2 it's not set after login.
I just added followRedirOptions = true in the redirect options built at the end of the "End" method after a oAuth2 login.

@aarondl
Copy link
Member

aarondl commented Dec 16, 2020

I'm not sure what this is for. There's already a mechanism for redirecting somewhere based on the redir url parameter that gets passed at the beginning of the OAuth2 flow.

@enrico-neri-zerynth
Copy link
Author

enrico-neri-zerynth commented Dec 17, 2020

I'm not sure what this is for. There's already a mechanism for redirecting somewhere based on the redir url parameter that gets passed at the beginning of the OAuth2 flow.

Hi Aaron, thanks for your reply :) The problem is that in the redirector module, there's a check about that parameter (followRedirParam), to know if the redirector has to check or not the redirect parameter (if it is allowed).
In Oauth2 login the first redirect is to the Google endpoint, for example. (https://accounts.google...)
So if I want to add a check in the Non API redirect as for the API one (that checks if the redir url is allowed if it doesn't contains "//"), i can't because i can't distinguish from a redirect to the google endpoint (or github, or facebook..) because there is not a param that tells me "the redirect url contains '//' but is the provider endpoint".
So, if the "followRedirect" param were set to true at the end of the operation, the redirector would know if the redirect path in the redirect options is the one chose by the user or the provider's one.
In the first case, it could be possible to enable the check on the url in the redirector module.

To solve the problem, i wrote a new custom redirector, that checks if the "success" param in redirect options struct is not "". In that case i Know that the login operation ended, so i can do custom checks on the redirect path.

@aarondl
Copy link
Member

aarondl commented Dec 23, 2020

There exists redirect mechanisms specific to Oauth2 that are separate from what you're talking about:

https://github.com/volatiletech/authboss/blob/master/oauth2/oauth2.go#L132
https://github.com/volatiletech/authboss/blob/master/oauth2/oauth2.go#L274

Pretty sure some combination of these things will suit your use case?

@enrico-neri-zerynth
Copy link
Author

enrico-neri-zerynth commented Dec 23, 2020

Thank you again aaron for your answer.
I saw that with oAuth2 login, is always called the redirect non Api method from the redirector.
The point is that the oauth2 login is split in 2 phases:

  1. redirect to the provider url
  2. redirect to the redir parameter of the url query

I missed to write that my problem born because i want to allow some redirect url containing https:// if they belong to a list of allowed url.

So, in my redirector version, the redirect API method from this

if strings.Contains(redir, "://") { // Guard against Open Redirect: https://cwe.mitre.org/data/definitions/601.html redir = "" } if len(redir) != 0 && ro.FollowRedirParam { path = redir }

it becames something like...

if !isAllowed(redir) { redir = "" } if len(redir) != 0 && ro.FollowRedirParam { path = redir }

But if i write something similiar in the non API redirect method, it won't work cause it is called also for provider redirect url, so the only know if i have to check the path parameter (in oauth2 "redir" is not used) is to check if the "success" message is not empty. So the user logged in, status message is not empty, i can check if the redirect url is allowed.

So, in the redirect non API method, i had to write something like:

if ro.Success != "" && r.RConverter.IsAllowed(path) { // go to the redirect url (the check ro.Success != "" doesn't catch the provider redirect (accounts.google..) } else if ro.Success != "" && !r.RConverter.IsAllowed(path) { path = r.Ab.Paths.AuthLoginOK } else if ro.Failure != "" { path = r.Ab.Paths.NotAuthorized }
Sorry if i didn't explain something well

@aarondl
Copy link
Member

aarondl commented Jan 8, 2021

Okay, so one of the key issues here is that your Redirector implementation has custom code in it that you want to activate, rather than using the secondary redirection mechanism built into the OAuth2 module. Is that correct?

@enrico-neri-zerynth
Copy link
Author

Yes, this is correct.

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.

None yet

2 participants