소스 검색

Prevent Duplicate Authorities

Issue gh-17981
Josh Cummings 1 주 전
부모
커밋
4281f6b00b

+ 19 - 4
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java

@@ -18,6 +18,8 @@ package org.springframework.security.oauth2.server.resource.web.authentication;
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
@@ -30,6 +32,7 @@ import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationManagerResolver;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.context.SecurityContextHolderStrategy;
@@ -53,6 +56,7 @@ import org.springframework.security.web.context.RequestAttributeSecurityContextR
 import org.springframework.security.web.context.SecurityContextRepository;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.filter.OncePerRequestFilter;
 
@@ -181,10 +185,17 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter {
 				throw new OAuth2AuthenticationException(error);
 			}
 			Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
-			if (current != null && current.isAuthenticated()) {
-				authenticationResult = authenticationResult.toBuilder()
-					.authorities((a) -> a.addAll(current.getAuthorities()))
-					.build();
+			if (current != null && current.isAuthenticated() && declaresToBuilder(authenticationResult)) {
+				authenticationResult = authenticationResult.toBuilder().authorities((a) -> {
+					Set<String> newAuthorities = a.stream()
+						.map(GrantedAuthority::getAuthority)
+						.collect(Collectors.toUnmodifiableSet());
+					for (GrantedAuthority currentAuthority : current.getAuthorities()) {
+						if (!newAuthorities.contains(currentAuthority.getAuthority())) {
+							a.add(currentAuthority);
+						}
+					}
+				}).build();
 			}
 			SecurityContext context = this.securityContextHolderStrategy.createEmptyContext();
 			context.setAuthentication(authenticationResult);
@@ -202,6 +213,10 @@ public class BearerTokenAuthenticationFilter extends OncePerRequestFilter {
 		}
 	}
 
+	private static boolean declaresToBuilder(Authentication authentication) {
+		return ReflectionUtils.findMethod(authentication.getClass(), "toBuilder") != null;
+	}
+
 	/**
 	 * Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use
 	 * the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}.

+ 24 - 1
oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java

@@ -17,7 +17,6 @@
 package org.springframework.security.oauth2.server.resource.web.authentication;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Set;
 
@@ -293,6 +292,30 @@ public class BearerTokenAuthenticationFilterTests {
 			.isThrownBy(() -> filter.setBearerTokenResolver(this.bearerTokenResolver));
 	}
 
+	/**
+	 * This is critical to avoid adding duplicate GrantedAuthority instances with the same
+	 * authority when the issuedAt is too old and a new instance is requested.
+	 * @throws Exception
+	 */
+	@Test
+	void doFilterWhenDefaultEqualsGrantedAuthorityThenNoDuplicates() throws Exception {
+		TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password",
+				new DefaultEqualsGrantedAuthority());
+		SecurityContextHolder.setContext(new SecurityContextImpl(existingAuthn));
+		given(this.authenticationManager.authenticate(any()))
+			.willReturn(new TestingAuthenticationToken("username", "password", new DefaultEqualsGrantedAuthority()));
+		given(this.bearerTokenResolver.resolve(any())).willReturn("token");
+		BearerTokenAuthenticationFilter filter = addMocks(
+				new BearerTokenAuthenticationFilter(this.authenticationManager));
+		filter.doFilter(this.request, this.response, this.filterChain);
+		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+		// @formatter:off
+		SecurityAssertions.assertThat(authentication).authorities()
+				.extracting(GrantedAuthority::getAuthority)
+				.containsExactly(DefaultEqualsGrantedAuthority.AUTHORITY);
+		// @formatter:on
+	}
+
 	@Test
 	public void setAuthenticationEntryPointWhenNullThenThrowsException() {
 		BearerTokenAuthenticationFilter filter = new BearerTokenAuthenticationFilter(this.authenticationManager);