Polish AuthoritiesGranter

Author: jzheaux

config/src/main/java/org/springframework/security/config/annotation/web/configurers/MfaConfigurer.java

@@ -20,7 +20,6 @@
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -44,6 +43,7 @@
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.AuthenticationResult;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.authority.ExpirableGrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -118,11 +118,9 @@ public void configure(B builder) throws Exception {
 
 	interface AuthoritiesGranter {
 
-		AuthenticationResult grantAuthorities(AuthenticationResult authentication);
+		Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest request);
 
-		default Collection<String> grantableAuthorities() {
-			return List.of();
-		}
+		Collection<GrantedAuthority> grantableAuthorities(Authentication result);
 
 	}
 
@@ -135,12 +133,17 @@ static final class PreAuthenticatedAuthoritiesGranter implements AuthoritiesGran
 		}
 
 		@Override
-		public AuthenticationResult grantAuthorities(AuthenticationResult authentication) {
+		public Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest request) {
+			return List.of();
+		}
+
+		@Override
+		public Collection<GrantedAuthority> grantableAuthorities(Authentication result) {
 			Authentication current = this.strategy.getContext().getAuthentication();
 			if (current == null || !current.isAuthenticated()) {
-				return authentication;
+				return List.of();
 			}
-			return authentication.withGrantedAuthorities((a) -> a.addAll(current.getAuthorities()));
+			return new HashSet<>(current.getAuthorities());
 		}
 
 	}
@@ -153,26 +156,22 @@ static final class CompositeAuthoritiesGranter implements AuthoritiesGranter {
 			this.authoritiesGranters = List.of(authorities);
 		}
 
-		CompositeAuthoritiesGranter(Collection<AuthoritiesGranter> authorities) {
-			this.authoritiesGranters = new ArrayList<>(authorities);
-		}
-
 		@Override
-		public Collection<String> grantableAuthorities() {
-			Collection<String> grantable = new ArrayList<>();
+		public Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest request) {
+			Collection<GrantedAuthority> authorities = new HashSet<>();
 			for (AuthoritiesGranter granter : this.authoritiesGranters) {
-				grantable.addAll(granter.grantableAuthorities());
+				authorities.addAll(granter.grantableAuthorities(request));
 			}
-			return grantable;
+			return authorities;
 		}
 
 		@Override
-		public AuthenticationResult grantAuthorities(AuthenticationResult authentication) {
-			AuthenticationResult granted = authentication;
+		public Collection<GrantedAuthority> grantableAuthorities(Authentication result) {
+			Collection<GrantedAuthority> authorities = new HashSet<>();
 			for (AuthoritiesGranter granter : this.authoritiesGranters) {
-				granted = granter.grantAuthorities(granted);
+				authorities.addAll(granter.grantableAuthorities(result));
 			}
-			return granted;
+			return authorities;
 		}
 
 	}
@@ -197,12 +196,15 @@ static final class SimpleAuthoritiesGranter implements AuthoritiesGranter {
 		}
 
 		@Override
-		public Collection<String> grantableAuthorities() {
-			return this.authorities;
+		public Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest request) {
+			Collection<GrantedAuthority> grantable = AuthorityUtils.createAuthorityList(this.authorities);
+			Collection<GrantedAuthority> requested = request.getAuthorities();
+			grantable.retainAll(requested);
+			return grantable;
 		}
 
 		@Override
-		public AuthenticationResult grantAuthorities(AuthenticationResult authentication) {
+		public Collection<GrantedAuthority> grantableAuthorities(Authentication result) {
 			Collection<GrantedAuthority> toGrant = new HashSet<>();
 			for (String authority : this.authorities) {
 				if (this.grantingTime == null) {
@@ -213,9 +215,8 @@ public AuthenticationResult grantAuthorities(AuthenticationResult authentication
 					toGrant.add(new ExpirableGrantedAuthority(authority, expiresAt));
 				}
 			}
-			Collection<GrantedAuthority> current = new HashSet<>(authentication.getAuthorities());
-			toGrant.addAll(current);
-			return authentication.withGrantedAuthorities(toGrant);
+			toGrant.removeAll(result.getAuthorities());
+			return toGrant;
 		}
 
 		void setClock(Clock clock) {
@@ -240,7 +241,8 @@ static final class AuthoritiesGranterAuthenticationManager implements Authentica
 		public Authentication authenticate(Authentication authentication) throws AuthenticationException {
 			Authentication result = this.authenticationManager.authenticate(authentication);
 			Assert.isInstanceOf(AuthenticationResult.class, result, "must be of type AuthenticationResult");
-			return this.authoritiesGranter.grantAuthorities((AuthenticationResult) result);
+			Collection<GrantedAuthority> authorities = this.authoritiesGranter.grantableAuthorities(result);
+			return ((AuthenticationResult) result).withGrantedAuthorities((a) -> a.addAll(authorities));
 		}
 
 	}
@@ -258,14 +260,8 @@ static final class SimpleAuthorizationEntryPoint implements AuthorizationEntryPo
 		}
 
 		@Override
-		public boolean authorizes(AuthorizationRequest authorizationRequest) {
-			Collection<String> grantable = this.authoritiesGranter.grantableAuthorities();
-			for (GrantedAuthority needed : authorizationRequest.getAuthorities()) {
-				if (grantable.contains(needed.getAuthority())) {
-					return true;
-				}
-			}
-			return false;
+		public Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest request) {
+			return this.authoritiesGranter.grantableAuthorities(request);
 		}
 
 		@Override

web/src/main/java/org/springframework/security/web/AuthorizationEntryPoint.java

@@ -16,10 +16,13 @@
 
 package org.springframework.security.web;
 
+import java.util.Collection;
+
 import org.springframework.security.authorization.AuthorizationRequest;
+import org.springframework.security.core.GrantedAuthority;
 
 public interface AuthorizationEntryPoint extends AuthenticationEntryPoint {
 
-	boolean authorizes(AuthorizationRequest authorizationRequest);
+	Collection<GrantedAuthority> grantableAuthorities(AuthorizationRequest authorizationRequest);
 
 }

web/src/main/java/org/springframework/security/web/AuthorizationRequestingAccessDeniedHandler.java

@@ -51,7 +51,7 @@ public void handle(HttpServletRequest request, HttpServletResponse response, Acc
 			return;
 		}
 		for (AuthorizationEntryPoint entry : this.entries) {
-			if (entry.authorizes(authorizationRequest)) {
+			if (!entry.grantableAuthorities(authorizationRequest).isEmpty()) {
 				AuthenticationException iae = new InsufficientAuthenticationException("access denied", access);
 				entry.commence(request, response, iae);
 				return;