Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,26 @@ public SkillHubOAuth2AuthorizationRequestResolver(ClientRegistrationRepository c

@Override
public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request);
oauthLoginFlowService.rememberReturnTo(request);
return authorizationRequest;
return rememberIfAuthorizationRequest(request, delegate.resolve(request));
}

@Override
public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) {
OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request, clientRegistrationId);
oauthLoginFlowService.rememberReturnTo(request);
return rememberIfAuthorizationRequest(request, delegate.resolve(request, clientRegistrationId));
}

/**
* {@code OAuth2AuthorizationRequestRedirectFilter} calls the resolver on every request in the
* chain, not only on authorization requests; the delegate simply answers null for the rest.
* Recording the return target on those calls would clear it again on the very next request —
* including the provider callback, which carries no {@code returnTo} and is processed by this
* filter before authentication succeeds. Only an actual authorization request may touch it.
*/
private OAuth2AuthorizationRequest rememberIfAuthorizationRequest(
HttpServletRequest request, OAuth2AuthorizationRequest authorizationRequest) {
if (authorizationRequest != null) {
oauthLoginFlowService.rememberReturnTo(request);
}
return authorizationRequest;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,25 @@ void resolve_storesSanitizedReturnToInSession() {
.isEqualTo("/dashboard/publish?draft=1");
}

@Test
void resolve_keepsReturnToOnNonAuthorizationRequests() {
// The redirect filter runs the resolver on every request in the chain, the provider
// callback included. That request carries no returnTo, so treating it as an
// authorization request would clear the target before the success handler reads it.
MockHttpServletRequest authorization = new MockHttpServletRequest("GET", "/oauth2/authorization/github");
authorization.setParameter("returnTo", "/device");
resolver.resolve(authorization, "github");
HttpSession session = authorization.getSession(false);

MockHttpServletRequest callback = new MockHttpServletRequest("GET", "/login/oauth2/code/github");
callback.setParameter("code", "auth-code");
callback.setSession(session);

assertThat(resolver.resolve(callback)).isNull();
assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE))
.isEqualTo("/device");
}

@Test
void resolve_ignoresUnsafeReturnTo() {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/github");
Expand Down
Loading