adding query parameter to authorization_uri creates malformed url

[mlevkovsky]

Summary

When creating the authorization uri to login with google, there is the option to add a query parameter in order to get back the refresh token.
However, when the authorization_uri is set to:

https://accounts.google.com/o/oauth2/v2/auth?access_type=offline

The uri that I get redirect to is:

https://accounts.google.com/o/oauth2/v2/auth?access_type=offline?response_type=code&client_id=[my client id]&scope=[scopes]&state=[state]&redirect_uri=[redirect uri]

Note the ?access_type=offlince?response_type...
This url is malformed and google complains saying response_type and basic query params are not passed in.

Actual Behavior

1. User goes to /login
2. User sees an error from Google due to malformed URL

Expected Behavior

1. User goes to /login
2. User sees the google login page and the following URL in the address bar:
   https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&response_type=code&client_id=[my client id]&scope=[scopes]&state=[state]&redirect_uri=[redirect uri]
   The access_type query parameter is after the ? and following query parameters should have an & between them.
   The order of the query params does not matter.

Configuration

My application.yaml

spring:
  security:
    oauth2:
      client:
        registration:
          google:
            client-id: xxxxx
            client-secret: yyyyy
            scope: profile,email,https://www.googleapis.com/auth/analytics
        provider:
          google:
            authorization-uri: https://accounts.google.com/o/oauth2/v2/auth?access_type=offline

My WebSecurityConfigurationAdapter

@Configuration
public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        http.authorizeRequests()
                .antMatchers("/login").permitAll()
                .anyRequest().authenticated()
                .and()
                .oauth2Login()
                    //.loginPage("/login")
                        .defaultSuccessUrl("/dashboard")
                        .failureUrl("/loginFailure")
                    .authorizationEndpoint()
                        .authorizationRequestRepository(authorizationRequestRepository())
                    .and()
                        .tokenEndpoint().accessTokenResponseClient(accessTokenResponseClient());
    }

    @Bean
    public AuthorizationRequestRepository<OAuth2AuthorizationRequest> authorizationRequestRepository() {
        HttpSessionOAuth2AuthorizationRequestRepository request = new HttpSessionOAuth2AuthorizationRequestRepository();
        return request;
    }

    @Bean
    public OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> accessTokenResponseClient() {
        return new NimbusAuthorizationCodeTokenResponseClient();
    }
}

My pom.xml (only including security and oauth2 dependencies)

<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-security</artifactId>
            <version>2.1.0.M2</version>
        </dependency>
        <dependency>
            <groupId>org.springframework.security</groupId>
            <artifactId>spring-security-oauth2-client</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.security</groupId>
            <artifactId>spring-security-oauth2-jose</artifactId>
            <version>5.1.0.RC1</version>
        </dependency>

[traf]

+1

[mlevkovsky]

Hi @jgrandja I dug into the code and found the issue.
If you'd like I can submit a PR with the fix

[jgrandja]

@mlevkovsky Support for custom Authorization Request parameters has been added via #4911.
The authorization-uri property should only be configured with the Authorization Endpoint URI. And for adding custom/additional request parameters that are supported by the Provider, for example access_type=offline (Google), you need to configure an OAuth2AuthorizationRequestResolver.

For usage, see the tests in this comment.

Your custom OAuth2AuthorizationRequestResolver can be configured via oauth2Login().authorizationEndpoint().authorizationRequestResolver().

Makes sense?

[rwinch]

@jgrandja Is there a reason that we cannot support the user providing a custom parameter in the URL directly? It seems like a reasonable and fairly common thing to want that we would not want the user to need to provide a custom OAuth2AuthorizatioNRequestResolver

[jgrandja]

@rwinch We certainly can provide this additional support. However, when we started work on this feature the plan was to implement exactly this way but a couple of users found this to be limiting given that it doesn't support dynamic parameters. See comments #4911 and #5244.

If you still would like this additional support to be added, we can see if @mlevkovsky would like to send a PR for this?

[rwinch]

I think it is reasonable to have both layers of support. One is something we can easily provide out of the box. The other we simply provide a hook for something more advanced.

[jgrandja]

@rwinch Sounds good

@mlevkovsky Would you be interested in submitting a PR for this enhancement?

[mlevkovsky]

@jgrandja sure thing!
I agree with @rwinch as it makes sense for a user to be able to configure their oauth requirements from their application.yaml (or .xml)

I will provide a PR that will inspect the authorization_uri and if there is a query parameter it will create a proper url.

[rwinch]

Thanks @mlevkovsky! Please be sure to include a test too :)

[mlevkovsky]

@rwinch absolutely :) will try to get it done over the weekend 👍

[rwinch]

NOTE: Since we are releasing 5.1.0.RC2 on Friday and you won't get to it until this weekend I pushed this back to 5.1.0

[matthewbluezyoncom]

@jgrandja I have the same issue and was about to post something similar, so I'm pleased I found this issue first.
I'll take a look at using a OAuth2AuthorizatioNRequestResolver

[mlevkovsky]

@rwinch @jgrandja just created the PR.
Any feedback would be appreciated :)

[mlevkovsky]

@matthewbluezyoncom I was wondering if you had success configuring your own Oauth2AuthorizationRequestResolver and if you had an example.
While working on the PR I still want to make some progress with this method

@jgrandja when I try to configure my custom resolver I get stuck in an endless loop constantly going back to the google login page

I'm really not sure why. If you can give me a pointer where my configuration is wrong I would really appreciate it.

@Override
	public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
		OAuth2AuthorizationRequest.Builder builder;
		builder = OAuth2AuthorizationRequest.authorizationCode();

		Map<String, Object> additionalParameters = new HashMap<>();
		additionalParameters.put("access_type","offline");

		Set<String> scopes = new HashSet<>(Arrays.asList(SCOPES.split(",")));

		OAuth2AuthorizationRequest authorizationRequest = builder
				.clientId(CLIENT_ID)
				.authorizationUri(AUTHORIZATION_URI)
				.redirectUri(request.getScheme()+"://"+request.getServerName()+":"+request.getLocalPort()+"/login/oauth2/code/google")
				.scopes(scopes)
				.state(this.stateGenerator.generateKey())
				.additionalParameters(additionalParameters)
				.build();

		return authorizationRequest;
	}

Thanks in advance

[matthewbluezyoncom]

Don't use the additionalParameters property, the Spring code places registrationId in there.

[matthewbluezyoncom]

I got this working so I could add access_type=offline to get a refresh token.

See my code below, from the overidden configure method of my WebSecurityConfigurerAdapter configuration class its a bit rough around the edges but it works.

I decided it was safest to create a new redirect filter based on the default filter and append the extra request parameter on the end of the authorization request uri.

@Override
public void configure(HttpSecurity http) throws Exception {

    if (requireSsl) http.requiresChannel().anyRequest().requiresSecure();

    http
        .authorizeRequests()
            .antMatchers("/css/**","/images/**","/built/**","/error","/login").permitAll()
            .anyRequest().authenticated()
        .and()
            //.csrf().disable()
            .addFilter(new JWTAuthorizationFilter(authenticationManagerBean(), oauthClientService, clientRegistrationRepository))
            // this disables session creation on Spring Security
            .sessionManagement().sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)
        .and()
            .logout().permitAll()
        .and()
            .oauth2Login()
            .authorizationEndpoint().authorizationRequestResolver(request -> {
                OAuth2AuthorizationRequest authorizationRequest = new DefaultOAuth2AuthorizationRequestResolver(
                        clientRegistrationRepository,
                        OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI)
                        .resolve(request);
                if (authorizationRequest == null) return null;
                return OAuth2AuthorizationRequest
                        .from(authorizationRequest)
                        .authorizationRequestUri(authorizationRequest.getAuthorizationRequestUri() + "&access_type=offline")
                        .build();
    })
            .and()
            .loginPage("/login");
}

[mlevkovsky]

@matthewbluezyoncom thank you very much! It worked for me.
Now I get it and I will fix my PR accordingly

[jinqinghua]

@matthewbluezyoncom I use jdk8 and spring boot 2.1.0 M4 (spring security 5.1.0) it does not work! which your env and version?

[jinqinghua]

@mlevkovsky Your PR codes not in master branch, which spring security version will include your codes?

[mlevkovsky]

@jinqinghua i haven't had time to review the PR
for now you can use the solution above (creating a custom resolver) to get the the refresh token.

[mlevkovsky]

@jinqinghua here is an example

`
@component
public class GoogleOAuthResolver implements OAuth2AuthorizationRequestResolver {
@Autowired
private ClientRegistrationRepository clientRegistrationRepository;

@Override
public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
	OAuth2AuthorizationRequest authorizationRequest = new DefaultOAuth2AuthorizationRequestResolver(
			clientRegistrationRepository, OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI).resolve(request);
	if (authorizationRequest == null) return null;
	return OAuth2AuthorizationRequest
			.from(authorizationRequest)
			.authorizationRequestUri(authorizationRequest.getAuthorizationRequestUri() + "&access_type=offline")
			.build();
}

@Override
public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) {
	return null;
}

}
`

The security configuration will look like this
`
@configuration
public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {

@Autowired
private GoogleOAuthResolver resolver;

@Override
protected void configure(HttpSecurity http) throws Exception {
    http.authorizeRequests()
            .antMatchers("/login","/oauth2/authorization/google","/").permitAll()
            .anyRequest().authenticated()
			.and()
				.logout()
					.logoutRequestMatcher(new AntPathRequestMatcher("/logout"))
					.logoutSuccessUrl("/")
					.deleteCookies("JSESSIONID")
					.invalidateHttpSession(true)
			.and()
            .oauth2Login()
                    .defaultSuccessUrl("/register",true)
                    .failureUrl("/loginFailure")
			.authorizationEndpoint()
				.authorizationRequestResolver(resolver);
}

}
`

[jinqinghua]

@mlevkovsky thanks a lot. your code works. but still has a question: when my application support google, facebook, github ... uses login, parameter "access_type=offline" will post to facebook, github's api server, this is not necessary, and may occur potential issue (if the parameter "access_type=offline" if facebook, github... return unexpected result).

I have following solutions, but ...

1. I dubug the code, It always invoke OAuth2AuthorizationRequest resolve(HttpServletRequest request)but not OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) so i can't rewrite OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) to get the clientRegistrationId and add different parameter.

2. So I must resolve clientRegistrationId from request in method OAuth2AuthorizationRequest resolve(HttpServletRequest request) , DefaultOAuth2AuthorizationRequestResolver has a method

	private String resolveRegistrationId(HttpServletRequest request) {
		if (this.authorizationRequestMatcher.matches(request)) {
			return this.authorizationRequestMatcher
					.extractUriTemplateVariables(request).get(REGISTRATION_ID_URI_VARIABLE_NAME);
		}
		return null;
	}

but it is private, and I must transform the code to GoogleOAuthResolver, it is ugly.

3. Because DefaultOAuth2AuthorizationRequestResolver is final and i can't extend it, If I want to control the resolver I must implements my resolver though implements the interface OAuth2AuthorizationRequestResolver and copy the code from DefaultOAuth2AuthorizationRequestResolver, this is not make sense.

4. @jgrandja why not add a getter setter additionalParameters in DefaultOAuth2AuthorizationRequestResolver so we can add additional parameters easily?

[jgrandja]

@jinqinghua

why not add a getter setter additionalParameters in DefaultOAuth2AuthorizationRequestResolver so we can add additional parameters easily?

You can add additional parameters fairly easily using a delegation-based strategy for a custom OAuth2AuthorizationRequestResolver. The latest reference docs have been updated to demonstrate this - See OAuth2AuthorizationRequestResolver.

Are you having an issue with this strategy?

[jinqinghua]

@jgrandja Awesome! thanks a lot.

I share some enhancement:
My site support google, facebook, office365 account login, they share the CustomAuthorizationRequestResolver and send additional parameters to all the third-party server. this is not make sense.

        private OAuth2AuthorizationRequest customAuthorizationRequest(
                OAuth2AuthorizationRequest authorizationRequest) {

            String registrationId = resolveRegistrationId(authorizationRequest);

            Map<String, Object> additionalParameters = new LinkedHashMap<>(authorizationRequest.getAdditionalParameters());

            // Only sent access_type=offline to google
            if (registrationId.equalsIgnoreCase("google")) {
                additionalParameters.put("access_type", "offline");
            }

            return OAuth2AuthorizationRequest.from(authorizationRequest)
                    .additionalParameters(additionalParameters)
                    .build();
        }

[forgo]

@jgrandja

I'm still having issues with the method described in the OAuth2AuthorizationRequestResolver documentation you linked.

No matter what I do, I if I set my own resolver in order to add a custom parameter, the authorization endpoint is never even called.

If I test in Chrome, I can see the network tab in developer console keeps redirecting to "/oauth_login" a bunch of times before getting the ERR_TOO_MANY_REDIRECTS issue.

Why would adding a custom resolver itself cause this behavior? If I comment out this line in my security config:

.authorizationRequestResolver(LoginGovAuthorizationRequestResolver(clientRegistrationRepository))

Then I definitely hit the authorization endpoint with default parameters, but of course it doesn't have the custom parameters I need.

You can find my project on GitHub as login-gov. I am using Kotlin, but I was also struggling with the same issue in Java.

To run: ./gradlew bootrun

Do you know what I might be doing wrong here?

class LoginGovAuthorizationRequestResolver(clientRegistryRepository: ClientRegistrationRepository) : OAuth2AuthorizationRequestResolver {

    private val REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"
    private var defaultAuthorizationRequestResolver: OAuth2AuthorizationRequestResolver = DefaultOAuth2AuthorizationRequestResolver(
            clientRegistryRepository, OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI
    )
    private val authorizationRequestMatcher: AntPathRequestMatcher = AntPathRequestMatcher(
            OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI + "/{" + REGISTRATION_ID_URI_VARIABLE_NAME + "}")

    override fun resolve(request: HttpServletRequest?): OAuth2AuthorizationRequest {
        val authorizationRequest: OAuth2AuthorizationRequest = defaultAuthorizationRequestResolver.resolve(request)
        return customAuthorizationRequest(authorizationRequest)
    }

    override fun resolve(request: HttpServletRequest?, clientRegistrationId: String?): OAuth2AuthorizationRequest {
        val authorizationRequest: OAuth2AuthorizationRequest = defaultAuthorizationRequestResolver.resolve(request, clientRegistrationId)
        return customAuthorizationRequest(authorizationRequest)
    }

    private fun customAuthorizationRequest(authorizationRequest: OAuth2AuthorizationRequest): OAuth2AuthorizationRequest {

        val registrationId: String = this.resolveRegistrationId(authorizationRequest)
        val additionalParameters = LinkedHashMap(authorizationRequest.additionalParameters)

        // set login.gov specific params
        if(registrationId == "logingov") {
            additionalParameters["dude"] = "whatever"
        }

        return OAuth2AuthorizationRequest
            .from(authorizationRequest)
            .additionalParameters(additionalParameters)
            .build()
    }

    private fun resolveRegistrationId(authorizationRequest: OAuth2AuthorizationRequest): String {
        return authorizationRequest.additionalParameters[OAuth2ParameterNames.REGISTRATION_ID] as String
    }

}

@EnableWebSecurity
class SecurityConfig : WebSecurityConfigurerAdapter() {

    @Autowired
    lateinit var clientRegistrationRepository: ClientRegistrationRepository

    companion object {
        const val LOGIN_ENDPOINT = "/oauth_login"
        const val LOGIN_SUCCESS_ENDPOINT = "/login_success"
        const val LOGIN_FAILURE_ENDPOINT = "/login_failure"
        const val AUTHORIZATION_ENDPOINT = "/oauth2/authorize_client"
        const val LOGOUT_ENDPOINT = "/logout"
        const val LOGOUT_SUCCESS_ENDPOINT = "/"
    }

    override fun configure(http: HttpSecurity) {
        http.authorizeRequests()
            // login, login failure, and index are allowed by anyone
            .antMatchers(LOGIN_ENDPOINT, LOGIN_FAILURE_ENDPOINT, "/")
                .permitAll()
            // any other requests are allowed by an authenticated user
            .anyRequest()
                .authenticated()
            .and()
            // custom logout behavior
            .logout()
                .logoutRequestMatcher(AntPathRequestMatcher(LOGOUT_ENDPOINT))
                .logoutSuccessUrl(LOGOUT_SUCCESS_ENDPOINT)
                .deleteCookies("JSESSIONID")
                .invalidateHttpSession(true)
            .and()
            // configure authentication support using an OAuth 2.0 and/or OpenID Connect 1.0 Provider
            .oauth2Login()
                .loginPage(LOGIN_ENDPOINT)
                .authorizationEndpoint()
                .authorizationRequestResolver(LoginGovAuthorizationRequestResolver(clientRegistrationRepository))
                .baseUri(AUTHORIZATION_ENDPOINT)
                .authorizationRequestRepository(authorizationRequestRepository())
                .and()
                .tokenEndpoint()
                .accessTokenResponseClient(accessTokenResponseClient())
                .and()
                .defaultSuccessUrl(LOGIN_SUCCESS_ENDPOINT)
                .failureUrl(LOGIN_FAILURE_ENDPOINT)
    }

    @Bean
    fun authorizationRequestRepository(): AuthorizationRequestRepository<OAuth2AuthorizationRequest> {
        return HttpSessionOAuth2AuthorizationRequestRepository()
    }

    @Bean
    fun accessTokenResponseClient(): OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> {
        return DefaultAuthorizationCodeTokenResponseClient()
    }
}

[forgo]

@jgrandja

Just as a follow up, I if I run in the Intellij IDEA debugger, I catch a breakpoint in this function multiple times, but it never seems to run into the return statement that does the customization -- which doesn't really make any sense to me...

Called multiple times:

override fun resolve(request: HttpServletRequest?): OAuth2AuthorizationRequest {
        val authorizationRequest: OAuth2AuthorizationRequest = defaultAuthorizationRequestResolver.resolve(request)
        return customAuthorizationRequest(authorizationRequest)
    }

Never catches breakpoint:
private fun customAuthorizationRequest(authorizationRequest: OAuth2AuthorizationRequest): OAuth2AuthorizationRequest

UPDATE:
It appears that OAuth2AuthorizationRequestRedirectFilter is catching an exception in its doFilterInternal method and calling this.unsuccessfulRedirectForAuthorization(request, response, failed)

This appears to be the root cause of why my customization function is never called. Part of the issue seems to be that the registrationId is null, but I'm not exactly clear on what's going on.

How do I ensure the proper resolve method gets called so that registrationId is not null?

resolve(request: HttpServletRequest?, clientRegistrationId: String?)

instead of:

resolve(request: HttpServletRequest?)

[jgrandja]

@forgo Looking at the sample code for OAuth2AuthorizationRequestResolver in the reference:

    @Override
    public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
        OAuth2AuthorizationRequest authorizationRequest =
                this.defaultAuthorizationRequestResolver.resolve(request);

        return authorizationRequest != null ?
                customAuthorizationRequest(authorizationRequest) :
                null;
    }

If the authorizationRequest == null than you need to return null as well since the DefaultOAuth2AuthorizationRequestResolver was not able to resolve it. I looked at your repo and you're not doing this check hence the null error you're getting. You need to change your implementation to something like this:

    override fun resolve(request: HttpServletRequest?): OAuth2AuthorizationRequest? {
        val authorizationRequest: OAuth2AuthorizationRequest? = defaultAuthorizationRequestResolver.resolve(request)
        if (authorizationRequest != null) {
            return customAuthorizationRequest(authorizationRequest)
        }
        return null
    }

[forgo]

@jgrandja

Thanks for your guidance and quick response! That was definitely the problem. I am still getting familiar with how Kotlin handles or does not handle nulls. Basically I needed to add a bunch of ? and a !! in some of my type declarations and statements to get my IDE to stop bugging me about returning nulls.

For anyone who's interested in doing the same in Kotlin, this is what my resolver ended up looking like:

class LoginGovAuthorizationRequestResolver(clientRegistryRepository: ClientRegistrationRepository) : OAuth2AuthorizationRequestResolver {

    private val REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId"
    private var defaultAuthorizationRequestResolver: OAuth2AuthorizationRequestResolver = DefaultOAuth2AuthorizationRequestResolver(
            clientRegistryRepository, OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI
    )
    private val authorizationRequestMatcher: AntPathRequestMatcher = AntPathRequestMatcher(
            OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI + "/{" + REGISTRATION_ID_URI_VARIABLE_NAME + "}")

    override fun resolve(request: HttpServletRequest?): OAuth2AuthorizationRequest? {
        val authorizationRequest: OAuth2AuthorizationRequest? = defaultAuthorizationRequestResolver.resolve(request)
        return if(authorizationRequest == null)
        { null } else { customAuthorizationRequest(authorizationRequest) }
    }

    override fun resolve(request: HttpServletRequest?, clientRegistrationId: String?): OAuth2AuthorizationRequest? {
        val authorizationRequest: OAuth2AuthorizationRequest? = defaultAuthorizationRequestResolver.resolve(request, clientRegistrationId)
        return if(authorizationRequest == null)
        { null } else { customAuthorizationRequest(authorizationRequest) }
    }

    private fun customAuthorizationRequest(authorizationRequest: OAuth2AuthorizationRequest?): OAuth2AuthorizationRequest {

        val registrationId: String = this.resolveRegistrationId(authorizationRequest)
        val additionalParameters = LinkedHashMap(authorizationRequest?.additionalParameters)

        // set login.gov specific params
        if(registrationId == "logingov") {
            additionalParameters["dude"] = "whatever"
        }

        return OAuth2AuthorizationRequest
            .from(authorizationRequest)
            .additionalParameters(additionalParameters)
            .build()
    }

    private fun resolveRegistrationId(authorizationRequest: OAuth2AuthorizationRequest?): String {
        return authorizationRequest!!.additionalParameters[OAuth2ParameterNames.REGISTRATION_ID] as String
    }

}

[sl3w]

I add additional parameter in CustomOAuth2AuthorizationRequestResolver, but how and where I can get it after answer from OAuth?