Describe the bug When OAuth2 is configured, and in particular, when multiple OAuth2 endpoints are defined or in combination with a formLogin(), accessing the OAuth2 link directly (default of /oauth2/authorization/{client-id} ), after successful OAuth2 login, the browser is instructed to return to http(s)://{site}/ and not the referring page as expected.
To Reproduce
Configure multiple spring.security.oauth2.client and spring.security.ouath2.provider entries in the application.yaml
in the configure(HttpSecurity http):
http.authorizeRequests()
.antMatchers("/", "index", ....).permitAll()
.anyRequest().authenticated()
.and()
.oauth2Login();
Expected behavior After accessing the /oauth2/authorization/{client-id} URL (via browser GET), and after successful OAuth2 login, I would expect to return to the original referring URL, but end up at "http(s)://{site}/ (the default return URL)
The Code Issues
I've Tracked this down to a couple issues.
The trigger point is in org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestRedirectFilter
In doFilterInternal(), we only save our incoming request if a ClientAuthorizationRequiredException is thrown. If we are accessing the OAuth2 authorization endpoint directly, we are entering this filter not from an exception error and the referring request information is never saved.
This can be fixed by changing the first try/catch block to
try {
OAuth2AuthorizationRequest authorizationRequest = this.authorizationRequestResolver.resolve(request);
if (authorizationRequest != null) {
this.sendRedirectForAuthorization(request, response, authorizationRequest);
// If the OAuth2 login url is hit directly (say from a form login page)
// we need to make sure there is a saved session, so we can go back
// to the referring url
if (this.requestCache.getMatchingRequest(request, response) == null) {
this.requestCache.saveRequest(request, response);
}
return;
}
}
But this alone is not sufficient. And actually, only doing this will end up causing a redirect loop hitting the /oauth2/authorization/{client-id} over and over until the browser detects the loop.
This is where I'm not sure what the "proper" way to fix this is. One solution is to create a custom success handler and:
http.authrorizeRequests()
.oauth2Login()
.successHandler(new MyCustomHandler());
where MyCustomHandler overrides the determineTargetUrl(HttpServletRequest request, HttpServletResponse response) method. An example would be:
@Override
protected String determineTargetUrl(HttpServletRequest request, HttpServletResponse response){
SavedRequest savedRequest = requestCache.getRequest(request,response);
if (savedRequest == null) return super.determineTargetUrl(request, response);
String refer = request.getParameter("redirect");
if (refer == null)
{
List<String> headerValues = savedRequest.getHeaderValues("Referer");
if (headerValues != null && headerValues.size() > 0)
refer = headerValues.get(0);
}
if (refer == null) return super.determineTargetUrl(request, response);
return refer;
}
The slightly better fix is to just use the SavedRequestAwareAuthenticationSuccessHandler, that the OAuth2 code is already using, but set the useReferer (which is false by default) to true.
@Bean
public SavedRequestAwareAuthenticationSuccessHandler successHandler() {
SavedRequestAwareAuthenticationSuccessHandler handler = new SavedRequestAwareAuthenticationSuccessHandler();
handler.setUseReferer(true);
return handler;
}
and configure the HttpSecurity with:
http....
.oauth2Login().successHandler(successHandler());
But that isn't quite all as the SavedRequestAwareAuthenticationSuccessHandler ignores the useReferer variable and that class would also need a fix (along with AbstractAuthenticationTargetUrlRequestHandler to expose the useReferer).
Perhaps there is a better way to fix the 2nd part of the problem.
I can submit a pull request that updates:
AbstractAuthenticationTargetUrlRequestHandler
SavedRequestAwareAuthentiationSuccessHandler
OAuth2AuthroizationRequestRedirectFilter
But that still requires someone to turn on the useReferer (not ideal, but at least there would be an "in the box" solution). Or perhaps those that know the code much better than I have a better solution for the 2nd part.
Comment From: sjohnr
Hi @jdrews417, welcome to the project!
Sorry for the delay. I am taking a look at this issue now, and I'm unable to reproduce it. I have in the past seen issues with the RequestCache if things are not configured correctly, but in general the flow you're looking for seems to work for me.
Would you be able to provide a sample that reproduces the issue? If you provide a sample of a Spring Security-based login client with configuration that reproduces the issue for you, I think I can fill in the Authorization Server so you don't have to mock that part out.
Comment From: jdrews417
No worries. Here you go, a simple project to demonstrate the issue. https://github.com/jdrews417/spring-bug
Comment From: sjohnr
Thanks @jdrews417, that is very helpful!
Hopefully I'm understanding your sample and use case correctly. I believe you'd like to return to the page you were on previously once authentication is successful, and you're using the AuthenticationSuccessHandler to accomplish this. However, to use it effectively, it has to be combined with an AuthenticationEntryPoint that is redirect-based. The one you are supplying in your sample is simply returning a 401 Unauthorized response. This seems to be because you're also using an XHR request to GET /user-info that can fail and shouldn't result in redirects.
In this case, you would want to use the DelegatingAuthenticationEntryPoint so you can handle both scenarios depending on the request that comes in. For example, when we see an "Accept: text/html" request header, we know we clicked a link in the browser. Below is an example configuration that seems to produce the desired effect with only a customized AuthenticationEntryPoint.
@Configuration
public class ApplicationSecurityConfig {
@Bean
public SecurityFilterChain configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.authorizeRequests((authorize) -> authorize
.antMatchers("/", "/login.html", "/error", "/css/**", "/js/**", "/webjars/**").permitAll()
.anyRequest().authenticated()
)
.oauth2Login(Customizer.withDefaults())
.formLogin((formLogin) -> formLogin
.loginProcessingUrl("/login/form")
)
.exceptionHandling((exceptionHandling) -> exceptionHandling
.authenticationEntryPoint(authenticationEntryPoint())
)
.logout((logout) -> logout
.logoutSuccessUrl("/").permitAll()
)
.cors(Customizer.withDefaults())
.csrf(CsrfConfigurer::disable);
// @formatter:on
return http.build();
}
private AuthenticationEntryPoint authenticationEntryPoint() {
LoginUrlAuthenticationEntryPoint webAuthenticationEntryPoint =
new LoginUrlAuthenticationEntryPoint("/login.html");
MediaTypeRequestMatcher textHtmlMatcher =
new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
textHtmlMatcher.setUseEquals(true);
DelegatingAuthenticationEntryPoint delegatingAuthenticationEntryPoint = new DelegatingAuthenticationEntryPoint(new LinkedHashMap<>(
Map.of(textHtmlMatcher, webAuthenticationEntryPoint)));
delegatingAuthenticationEntryPoint.setDefaultEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED));
return delegatingAuthenticationEntryPoint;
}
@Bean
public UserDetailsService userDetailsService(PasswordEncoder passwordEncoder) {
UserDetails user = User.builder()
.username("demo")
.password(passwordEncoder.encode("password"))
.roles("somerole")
.build();
return new InMemoryUserDetailsManager(user);
}
}
The difference in this example is that the login page must be permitAll(), whereas the page that is the target of the post-authentication redirect must be authenticated(). So I created two pages, a static/login.html and a static/authenticated.html. They are both simply copies of your sample's static/demo/index.html.
To launch the login flow, the index page contains a link to the authenticated page, like so: <a href="authenticated.html">log in</a>.
Here's the PR that demonstrates the updates: https://github.com/jdrews417/spring-bug/pull/1
Does that help?
Comment From: jdrews417
Yes and no. The PR request essentially puts it back to the "known working use case". IE, if you hit a page that requires authentication, send the person to the login page (and thanks, the code makes it clear how to set up the "login page"), and after login goes back to where you came from. IE, a 401 Unauthorized exception was hit and down the code path that saves where you came from we go.
Perhaps describing the use case this way will help.
Let's say I have a public page "information" (not at the base of the site). The page is generated by code. If the code detects we are unauthenticated, it will return the "public" information and provide links to the various OAuth "/oauth2/authrorization/{registrationId}}" entries to login with. In the authenticated case, it shows the "public" as well as "private" information on the page and does not include the OAuth login links.
Now if we are not logged in, and click on one of the authorization links, do the OAuth login, I'll end up at the base of the site instead of coming back to my "information" page. In this case, we never triggered an "unauthenticated" exception, so the OAuth2 filter doesn't save where we came from.
Or put another way, I don't want to trigger an "unauthenticated" exception before the OAuth2 login is performed, I just want to hit the authorization link and return to the original location.
Comment From: sjohnr
Thanks, that does indeed clear it up. It was hard to tell at first whether that was the goal or just returning back to the page you were on using the authentication entry point + success handler.
In your case, you would need to take control of saving the desired request to the request cache, as it should not be the responsibility of the OAuth flow to know how to return to a public page.
You can do this adding a controller endpoint on the server that can handle this logic, as opposed to just a static html page. So your "information" page really should be a GetMapping("/some/information-page"). If you do that, you can use the RequestCache to directly add your current page, so that the success handler will automatically return you there. For example:
@Controller
public class PageController {
// Note: This is not exposed as a bean, as doing so would require us to re-define
// the requestMatcher to exclude XHR requests which is the default.
private final RequestCache requestCache = new HttpSessionRequestCache();
@GetMapping("/demo/index.html")
public String information(HttpServletRequest request, HttpServletResponse response, Authentication authentication) {
if (authentication == null) {
System.out.printf("Saving request: %s %s%n", request.getMethod(), request.getRequestURI());
requestCache.saveRequest(request, response);
}
return "information";
}
}
Then we can use the mechanism you were attempting to use for login success, but only for logout success.
@Bean
public SecurityFilterChain configure(HttpSecurity http) throws Exception {
http
...
.logout((logout) -> logout
.logoutSuccessHandler(logoutSuccessHandler())
)
...
return http.build();
}
private SimpleUrlLogoutSuccessHandler logoutSuccessHandler() {
SimpleUrlLogoutSuccessHandler logoutSuccessHandler = new SimpleUrlLogoutSuccessHandler();
logoutSuccessHandler.setUseReferer(true);
return logoutSuccessHandler;
}
~I updated the PR example.~ Oops, I didn't realize you merged it. https://github.com/jdrews417/spring-bug/pull/2
Comment From: jdrews417
Thanks! That clears it up and will work.