Skip to content

Commit d29c984

Browse files
authored
Merge pull request #18544 from Khyojae/gh-18543
Fix GrantedAuthority.authority null in AuthoritiesAuthorizationManager
2 parents ac556a4 + 1116241 commit d29c984

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ private boolean isGranted(Authentication authentication, Collection<String> auth
6767

6868
private boolean isAuthorized(Authentication authentication, Collection<String> authorities) {
6969
for (GrantedAuthority grantedAuthority : getGrantedAuthorities(authentication)) {
70-
if (authorities.contains(grantedAuthority.getAuthority())) {
70+
String authority = grantedAuthority.getAuthority();
71+
if (authority == null) {
72+
continue;
73+
}
74+
if (authorities.contains(authority)) {
7175
return true;
7276
}
7377
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
package org.springframework.security.authorization;
1818

1919
import java.util.Arrays;
20+
import java.util.Collection;
2021
import java.util.Collections;
22+
import java.util.Set;
2123
import java.util.function.Supplier;
2224

2325
import org.junit.jupiter.api.Test;
@@ -30,11 +32,13 @@
3032

3133
import static org.assertj.core.api.Assertions.assertThat;
3234
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
35+
import static org.assertj.core.api.Assertions.assertThatNullPointerException;
3336

3437
/**
3538
* Tests for {@link AuthoritiesAuthorizationManager}.
3639
*
3740
* @author Evgeniy Cheban
41+
* @author Khyojae
3842
*/
3943
class AuthoritiesAuthorizationManagerTests {
4044

@@ -84,4 +88,20 @@ void checkWhenRoleHierarchySetThenGreaterRoleTakesPrecedence() {
8488
assertThat(manager.check(authentication, Collections.singleton("ROLE_USER")).isGranted()).isTrue();
8589
}
8690

91+
@Test
92+
// gh-18543
93+
void authorizeWhenAuthorityIsNullThenDoesNotThrowNullPointerException() {
94+
AuthoritiesAuthorizationManager manager = new AuthoritiesAuthorizationManager();
95+
96+
Authentication authentication = new TestingAuthenticationToken("user", "password",
97+
Collections.singletonList(() -> null));
98+
99+
Collection<String> authoritiesContainsThrowsNPE = Set.of("ROLE_USER");
100+
101+
// must be Collection that throws NPE when .contains(null) is invoked
102+
// to replicate the issue in gh-18543
103+
assertThatNullPointerException().isThrownBy(() -> authoritiesContainsThrowsNPE.contains(null));
104+
assertThat(manager.authorize(() -> authentication, authoritiesContainsThrowsNPE).isGranted()).isFalse();
105+
}
106+
87107
}

0 commit comments

Comments
 (0)