volatiletech / authboss

The boss of http auth.
MIT License
3.79k stars 204 forks source link

add follow redirect params flag in oauth2 final response #318

Open enrico-neri-zerynth opened 3 years ago

enrico-neri-zerynth commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Yes, this is correct.