-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support customizing the redirect URL in OidcClientInitiatedServerLogoutSuccessHandler #14778
Comments
I agree. We needed to redirect to dynamic url when OIDC logout. @RequiredArgsConstructor
public class RedirectOidcServerLogoutSuccessHandler implements ServerLogoutSuccessHandler {
private final ReactiveClientRegistrationRepository clientRegistrationRepository;
@Override
public Mono<Void> onLogoutSuccess(WebFilterExchange exchange, Authentication authentication) {
OidcClientInitiatedServerLogoutSuccessHandler delegate =
new OidcClientInitiatedServerLogoutSuccessHandler(clientRegistrationRepository);
getLocation(exchange).ifPresent(delegate::setPostLogoutRedirectUri);
return delegate.onLogoutSuccess(exchange, authentication);
}
private Optional<String> getLocation(WebFilterExchange exchange) {
return Optional.ofNullable(exchange.getExchange().getRequest().getQueryParams().getFirst("location"));
}
} |
…InitiatedServerLogoutSuccessHandler Closes spring-projectsgh-14778
I can suggest a simple solution - add
Then:
|
While a clever solution, @CrazyParanoid, I think it would be preferable to align with the imperative component in this case and have a protected method. While it's rare in Spring Security to open a method up for overriding, delegation in this case requires a bit of gymnastics since it is not possible to operate on the return object. Instead of protected Mono<String> determineLogoutUri(WebFilterExchange exchange, Authentication authentication) {
return this.clientRegistrationRepository::findByRegistrationId).flatMap((clientRegistration) -> {
URI endSessionEndpoint = endSessionEndpoint(clientRegistration);
if (endSessionEndpoint == null) {
return Mono.empty();
}
String idToken = idToken(authentication);
String postLogoutRedirectUri = postLogoutRedirectUri(exchange.getExchange().getRequest(), clientRegistration);
return Mono.just(endpointUri(endSessionEndpoint, idToken, postLogoutRedirectUri));
});
} Then, an application can override this method to change the URI dynamically. |
@CrazyParanoid and @programista-zacny, I'm sorry. I should have looked at your proposal more carefully. Redirecting to a location specified in a query parameter is not advised as this is vulnerable to open redirect. You should not redirect to a location that a client can arbitrarily specify, say through a request parameter like Instead, the locations to which you can redirect should be something that the server controls. Before proceeding, then, I'd like to understand better what you are trying to do. This will ensure that we don't add a feature for an insecure reason. |
Hi @jzheaux this is a very interesting solution. I considered this solution, but I was confused by the fact that such an implementation is rarely found in the spring security API and developers are mostly accustomed to work with lambda-based factories. Maybe we should try to find such a solution? We can extract the logic for determining the URL to lambda-factory
Then we define:
Now you can easily determine the URL:
What do you think about this solution? |
@jzheaux I agree it looks unsafe. @stipx said: "In order to be able to work with some restrictive SSO implementations sometimes additional parameters are needed (like "state") in order that the logout request is succeeding." It seems that such implementation resolves this case, but at the same time it can add potential vulnerabilities if the developer is careless. |
@jzheaux In my case, Keycloak still controls valid redirect urls :) |
Nice idea, @CrazyParanoid, though, I'd prefer not to expose a So instead, could we do |
@jzheaux good solution! I have updated PR. |
Closing in favor of #14808 |
Expected Behavior
In order to be able to work with some restrictive SSO implementations sometimes additional parameters are needed (like "state") in order that the logout request is succeeding.
Current Behavior
In order to achieve this it was necessary to implement a custom logout handler which gets the logout/end_session URL from the client registration and sets the ID-Token hint, the redirect uri and the state and does a redirect then.
So a simple resolver/converter/customizer for the redirect URL would be much easier than implementing a whole new logout handler.
The text was updated successfully, but these errors were encountered: