spring-projects / spring-projects/spring-session

Highly confident there is a casting / deserialisation bug in SpringSessionBackedReactiveSessionRegistry

Open
#3,182 3 comments 0 reactions 1 assignee View on GitHub

@marcusdacoregio is already working on this.

Since Sep 4, 2024.

status: feedback-provided type: bug
Dominant language
Java
Stars
1.9k
Forks
1.2k
Avg merge
4h 27m
Merged PRs (30d)
55

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.