From 02285708ebc4049ec8c6579038f373a82535b6be Mon Sep 17 00:00:00 2001 From: Marcus Hert da Coregio Date: Wed, 26 May 2021 09:03:38 -0300 Subject: [PATCH] Adjust createNewSessionIfAllowed to prevent NPE Ensure that isTransientAuthentication reuses the same authentication object from saveContext Closes gh-8947 --- .../HttpSessionSecurityContextRepository.java | 12 ++++-------- ...SessionSecurityContextRepositoryTests.java | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java index 21719089fd..135ffe3fef 100644 --- a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java +++ b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -354,11 +354,7 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo } return; } - - if (httpSession == null) { - httpSession = createNewSessionIfAllowed(context); - } - + httpSession = (httpSession != null) ? httpSession : createNewSessionIfAllowed(context, authentication); // If HttpSession exists, store current SecurityContext but only if it has // actually changed in this thread (see SEC-37, SEC-1307, SEC-1528) if (httpSession != null) { @@ -381,8 +377,8 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo || context.getAuthentication() != authBeforeExecution; } - private HttpSession createNewSessionIfAllowed(SecurityContext context) { - if (isTransientAuthentication(context.getAuthentication())) { + private HttpSession createNewSessionIfAllowed(SecurityContext context, Authentication authentication) { + if (isTransientAuthentication(authentication)) { return null; } diff --git a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java index 60b2a475df..1c0937b998 100644 --- a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,6 +53,7 @@ import org.springframework.security.core.userdetails.UserDetails; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @@ -719,6 +720,22 @@ public class HttpSessionSecurityContextRepositoryTests { assertThat(session).isNull(); } + // gh-8947 + @Test + public void saveContextWhenSecurityContextAuthenticationUpdatedToNullThenSkipped() { + HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository(); + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, response); + SomeOtherTransientAuthentication authentication = new SomeOtherTransientAuthentication(); + repo.loadContext(holder); + SecurityContext context = mock(SecurityContext.class); + given(context.getAuthentication()).willReturn(authentication).willReturn(null); + repo.saveContext(context, holder.getRequest(), holder.getResponse()); + MockHttpSession session = (MockHttpSession) request.getSession(false); + assertThat(session).isNull(); + } + private SecurityContext createSecurityContext(UserDetails userDetails) { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(userDetails, userDetails.getPassword(), userDetails.getAuthorities());