spring-projects / spring-session

Spring Session
https://spring.io/projects/spring-session
Apache License 2.0
1.86k stars 1.11k forks source link

Highly confident there is a casting / deserialisation bug in SpringSessionBackedReactiveSessionRegistry #3182

Open dreamstar-enterprises opened 2 weeks ago

dreamstar-enterprises commented 2 weeks ago

Describe the bug

When my logout handler calls this

fun invalidateSession(sessionId: String): Mono<Void> {
        logger.info("Invalidating sessionId: ${sessionId}")
        // handle the session invalidation process
        return reactiveSessionRegistry.getSessionInformation(sessionId)
            .flatMap { session ->
                // invalidate session
                session.invalidate()
                    .then(
                        // delete session
                        webSessionStore.removeSession(sessionId)
                    )
                    .doOnSuccess {
                        logger.info("Session invalidated and removed: ${sessionId}")
                    }
                    .doOnError { error ->
                        logger.error("Error invalidating session: ${sessionId}", error)
                    }
            }
    }

The following function inside SpringSessionBackedReactiveSessionRegistry gets called:

    @Override
    public Mono<ReactiveSessionInformation> getSessionInformation(String sessionId) {
        return this.sessionRepository.findById(sessionId).map(SpringSessionBackedReactiveSessionInformation::new);
    }

The inner class is implemented as follows:

class SpringSessionBackedReactiveSessionInformation extends ReactiveSessionInformation {

        SpringSessionBackedReactiveSessionInformation(S session) {
            super(resolvePrincipalName(session), session.getId(), session.getLastAccessedTime());
        }

        private static String resolvePrincipalName(Session session) {
            String principalName = session
                .getAttribute(ReactiveFindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME);
            if (principalName != null) {
                return principalName;
            }
            SecurityContext securityContext = session.getAttribute(SPRING_SECURITY_CONTEXT);
            if (securityContext != null && securityContext.getAuthentication() != null) {
                return securityContext.getAuthentication().getName();
            }
            return "";
        }

        @Override
        public Mono<Void> invalidate() {
            return super.invalidate()
                .then(Mono.defer(() -> SpringSessionBackedReactiveSessionRegistry.this.sessionRepository
                    .deleteById(getSessionId())));
        }

    }

But, here on this line:

SecurityContext securityContext = session.getAttribute(SPRING_SECURITY_CONTEXT);

SPRING_SECURITY_CONTEXT is received from Redis as a HashMap or LinkedHashMap, so cannot be cast to SecurityContext (w/o proper de-serialisation)

This is EXACTLY the error I see:

Screenshot 2024-08-30 at 00 01 35

Two calls to get session?

Also, I'm not sure if this is calling Redis again to get the security context, but is it necessary?, given just before calling /logout endpoint, the session / security context is retrieved anyway, (see below.)

SessionId would come from the session, here, when this is called at the very start fun invalidateSession(sessionId: String): Mono ) so calling getSessionInformation(String sessionId) and with it, this.sessionRepository.findById(sessionId), again, seems a bit wasteful...?

Screenshot 2024-08-30 at 00 02 24

To Reproduce See above, just try the above, with sessions stored to redis, then try to invalidate a session calling the above functions

Expected behavior The casting should be properly deserialised. A linkedHashmap cannot be cast to a SecurityContext object directly

Sample

See above. Github code can be found here:

My implementations https://github.com/dreamstar-enterprises/docs/blob/master/Spring%20BFF/BFF/src/main/kotlin/com/frontiers/bff/auth/sessions/SessionControl.kt https://github.com/dreamstar-enterprises/docs/blob/master/Spring%20BFF/BFF/src/main/kotlin/com/frontiers/bff/auth/sessions/SessionRegistryConfig.kt

Spring implementation (where error is I believe) https://github.com/spring-projects/spring-session/blob/main/spring-session-core/src/main/java/org/springframework/session/security/SpringSessionBackedReactiveSessionRegistry.java

dreamstar-enterprises commented 2 weeks ago
Screenshot 2024-08-31 at 21 01 32
marcusdacoregio commented 1 week ago

Hi @dreamstar-enterprises, thanks for the report.

Your links are returning a 404.

I tried it with a sample from the repository and it worked fine. I might be missing something. Can you please provide a minimal, reproducible sample with only Spring Session Data Redis?

dreamstar-enterprises commented 1 week ago

Hi Marcus,

I fixed the links

The root cause of the problem is this

image

When i use session.getAttributes, objects are returned from Redis as HashedMaps

So this line fails in your implementation does not work. I've tested it several times:

https://github.com/spring-projects/spring-session/blob/main/spring-session-core/src/main/java/org/springframework/session/security/SpringSessionBackedReactiveSessionRegistry.java#L119