diff --git a/core/src/main/java/org/springframework/security/authorization/AuthoritiesAuthorizationManager.java b/core/src/main/java/org/springframework/security/authorization/AuthoritiesAuthorizationManager.java index 37b51df3f7..295d93bf89 100644 --- a/core/src/main/java/org/springframework/security/authorization/AuthoritiesAuthorizationManager.java +++ b/core/src/main/java/org/springframework/security/authorization/AuthoritiesAuthorizationManager.java @@ -69,7 +69,11 @@ public final class AuthoritiesAuthorizationManager implements AuthorizationManag private boolean isAuthorized(Authentication authentication, Collection authorities) { for (GrantedAuthority grantedAuthority : getGrantedAuthorities(authentication)) { - if (authorities.contains(grantedAuthority.getAuthority())) { + String authority = grantedAuthority.getAuthority(); + if (authority == null) { + continue; + } + if (authorities.contains(authority)) { return true; } } diff --git a/core/src/test/java/org/springframework/security/authorization/AuthoritiesAuthorizationManagerTests.java b/core/src/test/java/org/springframework/security/authorization/AuthoritiesAuthorizationManagerTests.java index 1105dfdd05..e9c7ae3fe1 100644 --- a/core/src/test/java/org/springframework/security/authorization/AuthoritiesAuthorizationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authorization/AuthoritiesAuthorizationManagerTests.java @@ -17,7 +17,9 @@ package org.springframework.security.authorization; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Set; import java.util.function.Supplier; import org.junit.jupiter.api.Test; @@ -30,11 +32,13 @@ import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatNullPointerException; /** * Tests for {@link AuthoritiesAuthorizationManager}. * * @author Evgeniy Cheban + * @author Khyojae */ class AuthoritiesAuthorizationManagerTests { @@ -83,4 +87,20 @@ class AuthoritiesAuthorizationManagerTests { assertThat(manager.authorize(authentication, Collections.singleton("ROLE_USER")).isGranted()).isTrue(); } + @Test + // gh-18543 + void authorizeWhenAuthorityIsNullThenDoesNotThrowNullPointerException() { + AuthoritiesAuthorizationManager manager = new AuthoritiesAuthorizationManager(); + + Authentication authentication = new TestingAuthenticationToken("user", "password", + Collections.singletonList(() -> null)); + + Collection authoritiesContainsThrowsNPE = Set.of("ROLE_USER"); + + // must be Collection that throws NPE when .contains(null) is invoked + // to replicate the issue in gh-18543 + assertThatNullPointerException().isThrownBy(() -> authoritiesContainsThrowsNPE.contains(null)); + assertThat(manager.authorize(() -> authentication, authoritiesContainsThrowsNPE).isGranted()).isFalse(); + } + }