Fix the bug that the custom GrantedAuthority comparison fails

Closes gh-10566
spring-projects · Dec 8, 2021 · 86ed937 · 86ed937
1 parent 32ec8c3
commit 86ed937
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 4 deletions.
diff --git a/...c/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java b/...c/main/java/org/springframework/security/authorization/AuthorityAuthorizationManager.java
@@ -133,8 +133,10 @@ private boolean isGranted(Authentication authentication) {
 
 	private boolean isAuthorized(Authentication authentication) {
 		for (GrantedAuthority grantedAuthority : authentication.getAuthorities()) {
-			if (this.authorities.contains(grantedAuthority)) {
-				return true;
+			for (GrantedAuthority authority : this.authorities) {
+				if (authority.getAuthority().equals(grantedAuthority.getAuthority())) {
+					return true;
+				}
 			}
 		}
 		return false;

diff --git a/...ava/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java b/...ava/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java
@@ -45,9 +45,10 @@ public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthori
 	@Override
 	public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
 		// @formatter:off
-		return authentication.filter((a) -> a.isAuthenticated())
+		return authentication.filter(Authentication::isAuthenticated)
 				.flatMapIterable(Authentication::getAuthorities)
-				.any(this.authorities::contains)
+				.map(GrantedAuthority::getAuthority)
+				.any((grantedAuthority) -> this.authorities.stream().anyMatch((authority) -> authority.getAuthority().equals(grantedAuthority)))
 				.map((granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities)))
 				.defaultIfEmpty(new AuthorityAuthorizationDecision(false, this.authorities));
 		// @formatter:on

diff --git a/...t/java/org/springframework/security/authorization/AuthorityAuthorizationManagerTests.java b/...t/java/org/springframework/security/authorization/AuthorityAuthorizationManagerTests.java
@@ -16,12 +16,14 @@
 
 package org.springframework.security.authorization;
 
+import java.util.Collections;
 import java.util.function.Supplier;
 
 import org.junit.jupiter.api.Test;
 
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -133,6 +135,30 @@ public void hasAuthorityWhenUserHasNotAuthorityThenDeniedDecision() {
 		assertThat(manager.check(authentication, object).isGranted()).isFalse();
 	}
 
+	@Test
+	public void hasAuthorityWhenUserHasCustomAuthorityThenGrantedDecision() {
+		AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAuthority("ADMIN");
+		GrantedAuthority customGrantedAuthority = () -> "ADMIN";
+
+		Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password",
+				Collections.singletonList(customGrantedAuthority));
+		Object object = new Object();
+
+		assertThat(manager.check(authentication, object).isGranted()).isTrue();
+	}
+
+	@Test
+	public void hasAuthorityWhenUserHasNotCustomAuthorityThenDeniedDecision() {
+		AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAuthority("ADMIN");
+		GrantedAuthority customGrantedAuthority = () -> "USER";
+
+		Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password",
+				Collections.singletonList(customGrantedAuthority));
+		Object object = new Object();
+
+		assertThat(manager.check(authentication, object).isGranted()).isFalse();
+	}
+
 	@Test
 	public void hasAnyRoleWhenUserHasAnyRoleThenGrantedDecision() {
 		AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAnyRole("ADMIN", "USER");

diff --git a/...rg/springframework/security/authorization/AuthorityReactiveAuthorizationManagerTests.java b/...rg/springframework/security/authorization/AuthorityReactiveAuthorizationManagerTests.java
@@ -27,6 +27,7 @@
 
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -88,6 +89,24 @@ public void checkWhenHasAuthorityAndAuthorizedThenReturnTrue() {
 		assertThat(granted).isTrue();
 	}
 
+	@Test
+	public void checkWhenHasCustomAuthorityAndAuthorizedThenReturnTrue() {
+		GrantedAuthority customGrantedAuthority = () -> "ADMIN";
+		this.authentication = new TestingAuthenticationToken("rob", "secret",
+				Collections.singletonList(customGrantedAuthority));
+		boolean granted = this.manager.check(Mono.just(this.authentication), null).block().isGranted();
+		assertThat(granted).isTrue();
+	}
+
+	@Test
+	public void checkWhenHasCustomAuthorityAndAuthenticatedAndWrongAuthoritiesThenReturnFalse() {
+		GrantedAuthority customGrantedAuthority = () -> "USER";
+		this.authentication = new TestingAuthenticationToken("rob", "secret",
+				Collections.singletonList(customGrantedAuthority));
+		boolean granted = this.manager.check(Mono.just(this.authentication), null).block().isGranted();
+		assertThat(granted).isFalse();
+	}
+
 	@Test
 	public void checkWhenHasRoleAndAuthorizedThenReturnTrue() {
 		this.manager = AuthorityReactiveAuthorizationManager.hasRole("ADMIN");