Adjust createNewSessionIfAllowed to prevent NPE

Ensure that isTransientAuthentication reuses the same authentication object from saveContext

Closes gh-8947

Author: marcusdacoregio

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 @@ protected void saveContext(SecurityContext context) {
 				}
 				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 @@ private boolean contextChanged(SecurityContext context) {
 					|| context.getAuthentication() != authBeforeExecution;
 		}
 
-		private HttpSession createNewSessionIfAllowed(SecurityContext context) {
-			if (isTransientAuthentication(context.getAuthentication())) {
+		private HttpSession createNewSessionIfAllowed(SecurityContext context, Authentication authentication) {
+			if (isTransientAuthentication(authentication)) {
 				return null;
 			}
 

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 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 void saveContextWhenTransientAuthenticationWithCustomAnnotationThenSkippe
 		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());