Skip to content
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

Clarify the behavior of Concurrent Session Management when an IdP is involved #15071

Closed
jsantana3c opened this issue May 14, 2024 · 8 comments
Closed
Assignees
Labels
in: docs An issue in Documentation or samples type: enhancement A general enhancement
Milestone

Comments

@jsantana3c
Copy link

jsantana3c commented May 14, 2024

Expected Behavior

the current tutorial for spring concurrent session management says it works out of the box with OAuth:
https://docs.spring.io/spring-security/reference/6.3-SNAPSHOT/reactive/authentication/concurrent-sessions-control.html

but it seems to not be clear about what happens for logout, the code isn't proceeding with an RP-Initiated Logout / Back Channel logout, which seems that needs to be done manually somehow?

Current Behavior
when using the InvalidateLeastUsedServerMaximumSessionsExceededHandler, the logout on the application works properly, but when logging in back, the redirect to the IDP (Spring Authorization Server in my case) is still logged in (even though the IDP implements the same 1 session concurrency).

Context
How has this issue affected you?
isn't possible to logout from IDP, even when the IDP has already the concurrent session to 1, because the filter isn't applied on OAuth server matchers?
What are you trying to accomplish?
logout in both application & IDP
What other alternatives have you considered?
writing myself an OidcBackChannelLogoutWebFilter / ConcurrentSessionControlServerAuthenticationSuccessHandler

Are you aware of any workarounds?
not as of now

@jsantana3c jsantana3c added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels May 14, 2024
@jsantana3c jsantana3c changed the title OAuth with Concurrent Session & Spring Reactive OAuth with Concurrent Session Management With Spring Webflux May 14, 2024
@jsantana3c jsantana3c changed the title OAuth with Concurrent Session Management With Spring Webflux OAuth with Concurrent Session Management on Spring Webflux May 14, 2024
@marcusdacoregio marcusdacoregio self-assigned this May 16, 2024
@marcusdacoregio marcusdacoregio added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels May 16, 2024
@marcusdacoregio
Copy link
Contributor

Hi, @jsantana3c. Have you tried creating a ServerAuthenticationSuccessHandler that invokes OidcClientInitiatedServerLogoutSuccessHandler? I think that would solve the issue for you.

I wonder if we should configure it automatically if the application uses concurrent sessions and OIDC Logout. What do you think @sjohnr?

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label May 17, 2024
@joaquinjsb
Copy link
Contributor

Hi, @jsantana3c. Have you tried creating a ServerAuthenticationSuccessHandler that invokes OidcClientInitiatedServerLogoutSuccessHandler? I think that would solve the issue for you.

I wonder if we should configure it automatically if the application uses concurrent sessions and OIDC Logout. What do you think @sjohnr?

Where should I implement it? In my IDP or on the Application ? I tried adding to my filter chain of the Auth server a session management of 1 but no luck

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 18, 2024
@marcusdacoregio
Copy link
Contributor

You will add it to your OAuth2 Client configuration since it's where the OIDC Logout will be initiated.

@marcusdacoregio marcusdacoregio added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels May 20, 2024
@jsantana3c
Copy link
Author

jsantana3c commented May 20, 2024

that's the workaround I wanted to do, take the code of the session concurrency control, when the concurrent sessions have reached my parameter on the ServerAuthenticationSuccessHandler, i logout one, but that is like duplicating the session management in my code, one for session, and one for OAuth, the InvalidateLeastUsedServerMaximumSessionsExceededHandler, doesn't fire a "logout event", so the handler doesn't trigger the logout on the OAuth side

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 20, 2024
@sjohnr
Copy link
Member

sjohnr commented May 20, 2024

I wonder if we should configure it automatically if the application uses concurrent sessions and OIDC Logout. What do you think @sjohnr?

I don't think I'm able to fully understand without more context, so I may not have a very good answer. But I don't think that I prefer to automatically configure things based on detecting other things (in I assume other unrelated configurers). Any time that sort of thing happens in the configurers, it gets pretty unwieldy very quickly. It's also harder to re-use when trying to take control of the configuration yourself, as the "default settings" are very hard to determine. It's true however that Spring Security already does that in a number of places. Having said that,

but it seems to not be clear about what happens for logout, the code isn't proceeding with an RP-Initiated Logout / Back Channel logout, which seems to be done manually?

I agree that RP-Initiated logout isn't enabled by default when oauth2Login() is enabled, nor does it have a simple mechanism for being enabled via a configurer. This is obviously not ideal.

However, I don't think the current configuration model of Spring Security intends to link login mechanisms and logout mechanisms together such that they are both configured simultaneously/automatically. If it did, I would expect them to be under a single configurer/DSL method. So I don't know that we should start trying to do that now.

Since RP-initiated logout is clearly documented, I don't think there's much to change. Perhaps we can provide links from the Concurrent Sessions Control section of the docs to the various logout mechansmism? Or provide examples?

@jsantana3c
Copy link
Author

jsantana3c commented May 20, 2024

sorry, probably my english was the fault in the beginning, let me fix your quote real quick

but it seems to not be clear about what happens for logout, the code isn't proceeding with an RP-Initiated Logout / Back Channel logout, which seems to be done manually? that needs to be done manually somehow?

having this DSL:

@Bean
    SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) {
        http
                .authorizeExchange(authorizeRequests -> authorizeRequests
                        .anyExchange()
                        .authenticated()
                )
                .sessionManagement(sessions -> sessions
                        .concurrentSessions(concurrency -> concurrency
                                .maximumSessions(SessionLimit.of(1))
                        )
                )
                .csrf(ServerHttpSecurity.CsrfSpec::disable)
                .oauth2Login(spec -> spec
                        .oidcSessionRegistry(sessionRegistry)
                )
                .logout(logout -> logout
                        .logoutSuccessHandler(oidcLogoutSuccessHandler())
                )
                .oauth2Client(withDefaults());

        return http.build();
    }

when the max is reached, the standard session is logged out on the APP, when logging in back with the IDP (Spring Authorization Server), the session is still up(within the OAuth context).

so my question originally was: can you guys document how to do the logout on the IDP the same way of the RP Logout / Backchannel when the limit of the concurrent sessions is reached, I didn't find any way to do it "cleanly"

EDIT:
for cleanly I mean, this is what I had to do:
defined a ServerAuthenticationSuccessHandler, which implements the following:

@Override
    public Mono<Void> onAuthenticationSuccess(WebFilterExchange webFilterExchange, Authentication authentication) {
        return this.sessionLimit.apply(authentication)
                .flatMap(maxSessions -> handleConcurrency(webFilterExchange, authentication, maxSessions));
    }

    private Mono<Void> handleConcurrency(WebFilterExchange exchange, Authentication authentication,
                                         Integer maximumSessions) {
        return this.sessionRegistry.getAllSessions(authentication.getPrincipal())
                .collectList()
                .flatMap((registeredSessions) -> exchange.getExchange()
                        .getSession()
                        .map((currentSession) -> Tuples.of(currentSession, registeredSessions)))
                .flatMap((sessionTuple) -> {
                    WebSession currentSession = sessionTuple.getT1();
                    List<ReactiveSessionInformation> registeredSessions = sessionTuple.getT2();
                    int registeredSessionsCount = registeredSessions.size();
                    if (registeredSessionsCount < maximumSessions) {
                        return Mono.empty();
                    }
                    if (registeredSessionsCount == maximumSessions) {
                        for (ReactiveSessionInformation registeredSession : registeredSessions) {
                            if (registeredSession.getSessionId().equals(currentSession.getId())) {
                                return Mono.empty();
                            }
                        }
                    }

                    return oidcLogoutSuccessHandler.onLogoutSuccess(exchange, authentication)
                             .then(this.maximumSessionsExceededHandler.handle(new MaximumSessionsContext(authentication,
                                 registeredSessions, maximumSessions, currentSession)));
                });
    }

as you can see, that's the code of the ConcurrentSessionControlServerAuthenticationSuccessHandler and i had to add the oidcLogoutSuccessHandler, but at that point, the dsl for the session management, is nearly doing nothing?

@marcusdacoregio
Copy link
Contributor

when the max is reached, the standard session is logged out on the APP, when logging in back with the IDP (Spring Authorization Server), the session is still up(within the OAuth context).

That is the expected behavior, the concurrent session management is intended to invalidate sessions current registered in the ReactiveSessionRegistry, it has no way to know whether there is a session in the IdP. If you need to add such behavior, you must, as I mentioned, configure a ServerMaximumSessionsExceededHandler that does that for you where you could probably reuse the OidcClientInitiatedServerLogoutSuccessHandler.

With that said, I agree that the documentation could benefit from some improvement where such scenarios happen, I'll repurpose this ticket to cover that if you are ok with it.

@joaquinjsb
Copy link
Contributor

Yes, that's what I tried, but the OidcClientInitiatedServerLogoutSuccessHandler needs an exchange for some the uri components etc, so I ended up duplicating the ConcurrentSessionControlServerAuthenticationSuccessHandler to do the proper processing, so I don't know if maybe this is the proper way of doing it, or, if the MaxiumSessionsContext/handle method, could be enriched with the exchange to improve "deeper" processing?

but if the documentation in that sense is improved, maybe that's not a lot of an issue for other people

@marcusdacoregio marcusdacoregio changed the title OAuth with Concurrent Session Management on Spring Webflux Clarify the behavior of Concurrent Session Management when an IdP is involved May 21, 2024
@marcusdacoregio marcusdacoregio added in: docs An issue in Documentation or samples and removed in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided labels May 21, 2024
@marcusdacoregio marcusdacoregio added this to the 6.3.1 milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: docs An issue in Documentation or samples type: enhancement A general enhancement
Projects
Status: No status
Development

No branches or pull requests

5 participants