Skip to content

Commit 3935f4b

Browse files
terminuxmarcusdacoregio
authored andcommitted
Fix the bug that the custom GrantedAuthority comparison fails
Closes gh-10566
1 parent c68a75b commit 3935f4b

File tree

4 files changed

+52
-4
lines changed

4 files changed

+52
-4
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,10 @@ private boolean isGranted(Authentication authentication) {
133133

134134
private boolean isAuthorized(Authentication authentication) {
135135
for (GrantedAuthority grantedAuthority : authentication.getAuthorities()) {
136-
if (this.authorities.contains(grantedAuthority)) {
137-
return true;
136+
for (GrantedAuthority authority : this.authorities) {
137+
if (authority.getAuthority().equals(grantedAuthority.getAuthority())) {
138+
return true;
139+
}
138140
}
139141
}
140142
return false;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ public class AuthorityReactiveAuthorizationManager<T> implements ReactiveAuthori
4545
@Override
4646
public Mono<AuthorizationDecision> check(Mono<Authentication> authentication, T object) {
4747
// @formatter:off
48-
return authentication.filter((a) -> a.isAuthenticated())
48+
return authentication.filter(Authentication::isAuthenticated)
4949
.flatMapIterable(Authentication::getAuthorities)
50-
.any(this.authorities::contains)
50+
.map(GrantedAuthority::getAuthority)
51+
.any((grantedAuthority) -> this.authorities.stream().anyMatch((authority) -> authority.getAuthority().equals(grantedAuthority)))
5152
.map((granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities)))
5253
.defaultIfEmpty(new AuthorityAuthorizationDecision(false, this.authorities));
5354
// @formatter:on

core/src/test/java/org/springframework/security/authorization/AuthorityAuthorizationManagerTests.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616

1717
package org.springframework.security.authorization;
1818

19+
import java.util.Collections;
1920
import java.util.function.Supplier;
2021

2122
import org.junit.jupiter.api.Test;
2223

2324
import org.springframework.security.authentication.TestingAuthenticationToken;
2425
import org.springframework.security.core.Authentication;
26+
import org.springframework.security.core.GrantedAuthority;
2527

2628
import static org.assertj.core.api.Assertions.assertThat;
2729
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -133,6 +135,30 @@ public void hasAuthorityWhenUserHasNotAuthorityThenDeniedDecision() {
133135
assertThat(manager.check(authentication, object).isGranted()).isFalse();
134136
}
135137

138+
@Test
139+
public void hasAuthorityWhenUserHasCustomAuthorityThenGrantedDecision() {
140+
AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAuthority("ADMIN");
141+
GrantedAuthority customGrantedAuthority = () -> "ADMIN";
142+
143+
Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password",
144+
Collections.singletonList(customGrantedAuthority));
145+
Object object = new Object();
146+
147+
assertThat(manager.check(authentication, object).isGranted()).isTrue();
148+
}
149+
150+
@Test
151+
public void hasAuthorityWhenUserHasNotCustomAuthorityThenDeniedDecision() {
152+
AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAuthority("ADMIN");
153+
GrantedAuthority customGrantedAuthority = () -> "USER";
154+
155+
Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password",
156+
Collections.singletonList(customGrantedAuthority));
157+
Object object = new Object();
158+
159+
assertThat(manager.check(authentication, object).isGranted()).isFalse();
160+
}
161+
136162
@Test
137163
public void hasAnyRoleWhenUserHasAnyRoleThenGrantedDecision() {
138164
AuthorityAuthorizationManager<Object> manager = AuthorityAuthorizationManager.hasAnyRole("ADMIN", "USER");

core/src/test/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManagerTests.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import org.springframework.security.authentication.TestingAuthenticationToken;
2929
import org.springframework.security.core.Authentication;
30+
import org.springframework.security.core.GrantedAuthority;
3031

3132
import static org.assertj.core.api.Assertions.assertThat;
3233
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -88,6 +89,24 @@ public void checkWhenHasAuthorityAndAuthorizedThenReturnTrue() {
8889
assertThat(granted).isTrue();
8990
}
9091

92+
@Test
93+
public void checkWhenHasCustomAuthorityAndAuthorizedThenReturnTrue() {
94+
GrantedAuthority customGrantedAuthority = () -> "ADMIN";
95+
this.authentication = new TestingAuthenticationToken("rob", "secret",
96+
Collections.singletonList(customGrantedAuthority));
97+
boolean granted = this.manager.check(Mono.just(this.authentication), null).block().isGranted();
98+
assertThat(granted).isTrue();
99+
}
100+
101+
@Test
102+
public void checkWhenHasCustomAuthorityAndAuthenticatedAndWrongAuthoritiesThenReturnFalse() {
103+
GrantedAuthority customGrantedAuthority = () -> "USER";
104+
this.authentication = new TestingAuthenticationToken("rob", "secret",
105+
Collections.singletonList(customGrantedAuthority));
106+
boolean granted = this.manager.check(Mono.just(this.authentication), null).block().isGranted();
107+
assertThat(granted).isFalse();
108+
}
109+
91110
@Test
92111
public void checkWhenHasRoleAndAuthorizedThenReturnTrue() {
93112
this.manager = AuthorityReactiveAuthorizationManager.hasRole("ADMIN");

0 commit comments

Comments
 (0)