AllFactorsAuthorizationManager->AllRequiredFactorsAuthorizationManager

This allows the authorization logic to be relaxed so that if RequiredFactor
only has an authority specified, then the GrantedAuthority can be of any
type.

Closes gh-18031

core/src/main/java/org/springframework/security/authorization/AllFactorsAuthorizationManager.java → core/src/main/java/org/springframework/security/authorization/AllRequiredFactorsAuthorizationManager.java

@@ -30,6 +30,7 @@ import java.util.stream.Collectors;
 import org.jspecify.annotations.Nullable;
 
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.FactorGrantedAuthority;
 import org.springframework.util.Assert;
 
@@ -42,7 +43,7 @@ import org.springframework.util.Assert;
  * @since 7.0
  * @see AuthorityAuthorizationManager
  */
-public final class AllFactorsAuthorizationManager<T> implements AuthorizationManager<T> {
+public final class AllRequiredFactorsAuthorizationManager<T> implements AuthorizationManager<T> {
 
 	private Clock clock = Clock.systemUTC();
 
@@ -52,7 +53,7 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 	 * Creates a new instance.
 	 * @param requiredFactors the authorities that are required.
 	 */
-	private AllFactorsAuthorizationManager(List<RequiredFactor> requiredFactors) {
+	private AllRequiredFactorsAuthorizationManager(List<RequiredFactor> requiredFactors) {
 		Assert.notEmpty(requiredFactors, "requiredFactors cannot be empty");
 		Assert.noNullElements(requiredFactors, "requiredFactors must not contain null elements");
 		this.requiredFactors = Collections.unmodifiableList(requiredFactors);
@@ -80,7 +81,7 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 	@Override
 	public FactorAuthorizationDecision authorize(Supplier<? extends @Nullable Authentication> authentication,
 			T object) {
-		List<FactorGrantedAuthority> currentFactorAuthorities = getFactorGrantedAuthorities(authentication.get());
+		List<GrantedAuthority> currentFactorAuthorities = getFactorGrantedAuthorities(authentication.get());
 		List<RequiredFactorError> factorErrors = this.requiredFactors.stream()
 			.map((factor) -> requiredFactorError(factor, currentFactorAuthorities))
 			.filter(Objects::nonNull)
@@ -96,8 +97,8 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 	 * @return the {@link RequiredFactor} or null if granted.
 	 */
 	private @Nullable RequiredFactorError requiredFactorError(RequiredFactor requiredFactor,
-			List<FactorGrantedAuthority> currentFactors) {
-		Optional<FactorGrantedAuthority> matchingAuthority = currentFactors.stream()
+			List<GrantedAuthority> currentFactors) {
+		Optional<GrantedAuthority> matchingAuthority = currentFactors.stream()
 			.filter((authority) -> authority.getAuthority().equals(requiredFactor.getAuthority()))
 			.findFirst();
 		if (!matchingAuthority.isPresent()) {
@@ -108,13 +109,16 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 				// granted (only requires authority to match)
 				return null;
 			}
-			Instant now = this.clock.instant();
-			Instant expiresAt = authority.getIssuedAt().plus(requiredFactor.getValidDuration());
-			if (now.isBefore(expiresAt)) {
-				// granted
-				return null;
+			else if (authority instanceof FactorGrantedAuthority factorAuthority) {
+				Instant now = this.clock.instant();
+				Instant expiresAt = factorAuthority.getIssuedAt().plus(requiredFactor.getValidDuration());
+				if (now.isBefore(expiresAt)) {
+					// granted
+					return null;
+				}
 			}
-			// denied (expired)
+
+			// denied (expired or no issuedAt to compare)
 			return RequiredFactorError.createExpired(requiredFactor);
 		}).orElse(null);
 	}
@@ -128,14 +132,12 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 	 * @return all of the {@link FactorGrantedAuthority} instances from
 	 * {@link Authentication#getAuthorities()}.
 	 */
-	private List<FactorGrantedAuthority> getFactorGrantedAuthorities(@Nullable Authentication authentication) {
+	private List<GrantedAuthority> getFactorGrantedAuthorities(@Nullable Authentication authentication) {
 		if (authentication == null || !authentication.isAuthenticated()) {
 			return Collections.emptyList();
 		}
 		// @formatter:off
 		return authentication.getAuthorities().stream()
-			.filter(FactorGrantedAuthority.class::isInstance)
-			.map(FactorGrantedAuthority.class::cast)
 			.collect(Collectors.toList());
 		// @formatter:on
 	}
@@ -149,7 +151,7 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 	}
 
 	/**
-	 * A builder for {@link AllFactorsAuthorizationManager}.
+	 * A builder for {@link AllRequiredFactorsAuthorizationManager}.
 	 *
 	 * @author Rob Winch
 	 * @since 7.0
@@ -160,15 +162,15 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 
 		/**
 		 * Allows the user to consume the {@link RequiredFactor.Builder} that is passed in
-		 * and then adds the result to the {@link #requiredFactor(RequiredFactor)}.
+		 * and then adds the result to the {@link #requireFactor(RequiredFactor)}.
 		 * @param requiredFactor the {@link Consumer} to invoke.
 		 * @return the builder.
 		 */
-		public Builder requiredFactor(Consumer<RequiredFactor.Builder> requiredFactor) {
+		public Builder requireFactor(Consumer<RequiredFactor.Builder> requiredFactor) {
 			Assert.notNull(requiredFactor, "requiredFactor cannot be null");
 			RequiredFactor.Builder builder = RequiredFactor.builder();
 			requiredFactor.accept(builder);
-			return requiredFactor(builder.build());
+			return requireFactor(builder.build());
 		}
 
 		/**
@@ -176,19 +178,20 @@ public final class AllFactorsAuthorizationManager<T> implements AuthorizationMan
 		 * @param requiredFactor the requiredFactor to add. Cannot be null.
 		 * @return the builder.
 		 */
-		public Builder requiredFactor(RequiredFactor requiredFactor) {
+		public Builder requireFactor(RequiredFactor requiredFactor) {
 			Assert.notNull(requiredFactor, "requiredFactor cannot be null");
 			this.requiredFactors.add(requiredFactor);
 			return this;
 		}
 
 		/**
-		 * Builds the {@link AllFactorsAuthorizationManager}.
+		 * Builds the {@link AllRequiredFactorsAuthorizationManager}.
 		 * @param <T> the type.
-		 * @return the {@link AllFactorsAuthorizationManager}
+		 * @return the {@link AllRequiredFactorsAuthorizationManager}
 		 */
-		public <T> AllFactorsAuthorizationManager<T> build() {
-			return new AllFactorsAuthorizationManager<T>(this.requiredFactors);
+		public <T> AllRequiredFactorsAuthorizationManager<T> build() {
+			Assert.state(!this.requiredFactors.isEmpty(), "requiredFactors cannot be empty");
+			return new AllRequiredFactorsAuthorizationManager<T>(this.requiredFactors);
 		}
 
 	}

core/src/main/java/org/springframework/security/authorization/RequiredFactor.java

@@ -21,11 +21,21 @@ import java.util.Objects;
 
 import org.jspecify.annotations.Nullable;
 
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.FactorGrantedAuthority;
 import org.springframework.util.Assert;
 
 /**
- * The requirements for an {@link FactorGrantedAuthority} to be considered valid.
+ * The requirements for an {@link GrantedAuthority} to be considered a valid factor.
+ *
+ * <ul>
+ * <li>If the {@link #getAuthority()} is specified, then it must match
+ * {@link GrantedAuthority#getAuthority()}</li>
+ * <li>If {@link #getValidDuration()} is specified, the matching {@link GrantedAuthority}
+ * must be of type {@link FactorGrantedAuthority} and
+ * {@link FactorGrantedAuthority#getIssuedAt()} must be such that it is not considered
+ * expired when compared to {@link #getValidDuration()}.</li>
+ * </ul>
  *
  * @author Rob Winch
  * @since 7.0
@@ -43,7 +53,7 @@ public final class RequiredFactor {
 	}
 
 	/**
-	 * The {@link FactorGrantedAuthority#getAuthority()}.
+	 * The expected {@link GrantedAuthority#getAuthority()}.
 	 * @return the authority.
 	 */
 	public String getAuthority() {

core/src/test/java/org/springframework/security/authorization/AllFactorsAuthorizationManagerTests.java → core/src/test/java/org/springframework/security/authorization/AllRequiredFactorsAuthorizationManagerTests.java

@@ -32,12 +32,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 /**
- * Test {@link AllFactorsAuthorizationManager}.
+ * Test {@link AllRequiredFactorsAuthorizationManager}.
  *
  * @author Rob Winch
  * @since 7.0
  */
-class AllFactorsAuthorizationManagerTests {
+class AllRequiredFactorsAuthorizationManagerTests {
 
 	private static final Object DOES_NOT_MATTER = new Object();
 
@@ -52,8 +52,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenGranted() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(REQUIRED_PASSWORD.getAuthority())
 			.issuedAt(Instant.now())
@@ -65,8 +65,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenConsumerGranted() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor((required) -> required.authority(FactorGrantedAuthority.PASSWORD_AUTHORITY))
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor((required) -> required.authority(FactorGrantedAuthority.PASSWORD_AUTHORITY))
 			.build();
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority
 			.withAuthority(FactorGrantedAuthority.PASSWORD_AUTHORITY)
@@ -79,8 +79,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenUnauthenticated() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(REQUIRED_PASSWORD.getAuthority())
 			.issuedAt(Instant.now())
@@ -94,8 +94,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenNullAuthentication() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(EXPIRING_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(EXPIRING_PASSWORD)
 			.build();
 		Authentication authentication = null;
 		FactorAuthorizationDecision result = allFactors.authorize(() -> authentication, DOES_NOT_MATTER);
@@ -105,8 +105,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenRequiredFactorHasNullDurationThenNullIssuedAtGranted() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(REQUIRED_PASSWORD.getAuthority())
 			.build();
@@ -116,21 +116,21 @@ class AllFactorsAuthorizationManagerTests {
 	}
 
 	@Test
-	void authorizeWhenRequiredFactorHasDurationAndNotFactorGrantedAuthorityThenMissing() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(EXPIRING_PASSWORD)
+	void authorizeWhenRequiredFactorHasDurationAndNotFactorGrantedAuthorityThenExpired() {
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(EXPIRING_PASSWORD)
 			.build();
 		Authentication authentication = new TestingAuthenticationToken("user", "password",
 				EXPIRING_PASSWORD.getAuthority());
 		FactorAuthorizationDecision result = allFactors.authorize(() -> authentication, DOES_NOT_MATTER);
 		assertThat(result.isGranted()).isFalse();
-		assertThat(result.getFactorErrors()).containsExactly(RequiredFactorError.createMissing(EXPIRING_PASSWORD));
+		assertThat(result.getFactorErrors()).containsExactly(RequiredFactorError.createExpired(EXPIRING_PASSWORD));
 	}
 
 	@Test
 	void authorizeWhenFactorAuthorityMissingThenMissing() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		Authentication authentication = new TestingAuthenticationToken("user", "password", "ROLE_USER");
 		FactorAuthorizationDecision result = allFactors.authorize(() -> authentication, DOES_NOT_MATTER);
@@ -139,21 +139,20 @@ class AllFactorsAuthorizationManagerTests {
 	}
 
 	@Test
-	void authorizeWhenFactorGrantedAuthorityMissingThenMissing() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+	void authorizeWhenGrantedAuthorityThenGranted() {
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		Authentication authentication = new TestingAuthenticationToken("user", "password",
 				REQUIRED_PASSWORD.getAuthority());
 		FactorAuthorizationDecision result = allFactors.authorize(() -> authentication, DOES_NOT_MATTER);
-		assertThat(result.isGranted()).isFalse();
-		assertThat(result.getFactorErrors()).containsExactly(RequiredFactorError.createMissing(REQUIRED_PASSWORD));
+		assertThat(result.isGranted()).isTrue();
 	}
 
 	@Test
 	void authorizeWhenExpired() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(EXPIRING_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(EXPIRING_PASSWORD)
 			.build();
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(EXPIRING_PASSWORD.getAuthority())
 			.issuedAt(Instant.now().minus(Duration.ofHours(2)))
@@ -173,8 +172,8 @@ class AllFactorsAuthorizationManagerTests {
 		RequiredFactor expiringPassword = RequiredFactor.withAuthority(FactorGrantedAuthority.PASSWORD_AUTHORITY)
 			.validDuration(expiresIn)
 			.build();
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(expiringPassword)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(expiringPassword)
 			.build();
 		allFactors.setClock(clock);
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(expiringPassword.getAuthority())
@@ -195,8 +194,8 @@ class AllFactorsAuthorizationManagerTests {
 		RequiredFactor expiringPassword = RequiredFactor.withAuthority(FactorGrantedAuthority.PASSWORD_AUTHORITY)
 			.validDuration(expiresIn)
 			.build();
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(expiringPassword)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(expiringPassword)
 			.build();
 		allFactors.setClock(clock);
 		FactorGrantedAuthority passwordFactor = FactorGrantedAuthority.withAuthority(expiringPassword.getAuthority())
@@ -209,8 +208,8 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void authorizeWhenDifferentFactorGrantedAuthorityThenMissing() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		Authentication authentication = new TestingAuthenticationToken("user", "password",
 				FactorGrantedAuthority.fromAuthority(REQUIRED_PASSWORD.getAuthority()) + "DIFFERENT");
@@ -221,28 +220,28 @@ class AllFactorsAuthorizationManagerTests {
 
 	@Test
 	void setClockWhenNullThenIllegalArgumentException() {
-		AllFactorsAuthorizationManager<Object> allFactors = AllFactorsAuthorizationManager.builder()
-			.requiredFactor(REQUIRED_PASSWORD)
+		AllRequiredFactorsAuthorizationManager<Object> allFactors = AllRequiredFactorsAuthorizationManager.builder()
+			.requireFactor(REQUIRED_PASSWORD)
 			.build();
 		assertThatIllegalArgumentException().isThrownBy(() -> allFactors.setClock(null));
 	}
 
 	@Test
 	void builderBuildWhenEmpty() {
-		assertThatIllegalArgumentException().isThrownBy(() -> AllFactorsAuthorizationManager.builder().build());
+		assertThatIllegalArgumentException().isThrownBy(() -> AllRequiredFactorsAuthorizationManager.builder().build());
 	}
 
 	@Test
 	void builderWhenNullRequiredFactor() {
-		AllFactorsAuthorizationManager.Builder builder = AllFactorsAuthorizationManager.builder();
-		assertThatIllegalArgumentException().isThrownBy(() -> builder.requiredFactor((RequiredFactor) null));
+		AllRequiredFactorsAuthorizationManager.Builder builder = AllRequiredFactorsAuthorizationManager.builder();
+		assertThatIllegalArgumentException().isThrownBy(() -> builder.requireFactor((RequiredFactor) null));
 	}
 
 	@Test
 	void builderWhenNullConsumerRequiredFactorBuilder() {
-		AllFactorsAuthorizationManager.Builder builder = AllFactorsAuthorizationManager.builder();
+		AllRequiredFactorsAuthorizationManager.Builder builder = AllRequiredFactorsAuthorizationManager.builder();
 		assertThatIllegalArgumentException()
-			.isThrownBy(() -> builder.requiredFactor((Consumer<RequiredFactor.Builder>) null));
+			.isThrownBy(() -> builder.requireFactor((Consumer<RequiredFactor.Builder>) null));
 	}
 
 }