breakfix: Login hint and Additional Scopes broken

[dbranco-sdc]

When using one of this features, the separator for the "Location" when redirecting remains as "?" instead "&". The configureRedirect function should be responsible to update the separator value after adding login_hint and/or additional_scopes.

Current behaviour:

/auth/oauth2/MyRealm?login_hint=dbranco+nothing@byom.de?redirect_url=http://localhost:8095/somewhere?login_hint=dbranco%2Bnothing%40byom.de

Expected:

/auth/oauth2/MyRealm?login_hint=dbranco+nothing@byom.de&redirect_url=http://localhost:8095/somewhere?login_hint=dbranco%2Bnothing%40byom.de

[nekrondev]

Yea.. run into this issue with redirecting to expired HTML pages (see https://github.com/authp/authp.github.io/issues/38):

File: pkg/authz/handlers/redirect.go

Just set the "&" separator after login_hint or additional_scopes had been added.

        if len(rr.Redirect.LoginHint) > 0 {
                loginHint := rr.Redirect.LoginHint
                escapedLoginHint := url.QueryEscape(loginHint)
                rr.Redirect.AuthURL = fmt.Sprintf("%s%slogin_hint=%s", rr.Redirect.AuthURL, rr.Redirect.Separator, escapedLoginHint)
                rr.Redirect.Separator = "&"
        }

        if len(rr.Redirect.AdditionalScopes) > 0 {
                additionalScopes := rr.Redirect.AdditionalScopes
                escapedAdditionalScopes := url.QueryEscape(additionalScopes)
                rr.Redirect.AuthURL = fmt.Sprintf("%s%sadditional_scopes=%s", rr.Redirect.AuthURL, rr.Redirect.Separator, escapedAdditionalScopes)
                rr.Redirect.Separator = "&"
        }

[dbranco-sdc]

Yea.. run into this issue with redirecting to expired HTML pages (see authp/authp.github.io#38):

File: pkg/authz/handlers/redirect.go

Just set the "&" separator after login_hint or additional_scopes had been added.

        if len(rr.Redirect.LoginHint) > 0 {
                loginHint := rr.Redirect.LoginHint
                escapedLoginHint := url.QueryEscape(loginHint)
                rr.Redirect.AuthURL = fmt.Sprintf("%s%slogin_hint=%s", rr.Redirect.AuthURL, rr.Redirect.Separator, escapedLoginHint)
                rr.Redirect.Separator = "&"
        }

        if len(rr.Redirect.AdditionalScopes) > 0 {
                additionalScopes := rr.Redirect.AdditionalScopes
                escapedAdditionalScopes := url.QueryEscape(additionalScopes)
                rr.Redirect.AuthURL = fmt.Sprintf("%s%sadditional_scopes=%s", rr.Redirect.AuthURL, rr.Redirect.Separator, escapedAdditionalScopes)
                rr.Redirect.Separator = "&"
        }

Thanks for your answer, this was my fault when added the Additional Scopes functionality. I will add the fix plus going to write some tests to avoid forget this kind of things.

[greenpau]

Merged.

[greenpau]

FYI, the fix is now a part of v1.1.15 release.