Revert code

Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>

Author: ngocnhan-tran1996

core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java

@@ -169,21 +169,20 @@ public final boolean hasAllRoles(String... roles) {
 
 	@Override
 	public final boolean hasScope(String scope) {
-		return isGranted(this.authorizationManagerFactory.hasScope(scope));
+		assertScope(scope);
+		return isGranted(this.authorizationManagerFactory.hasAuthority(this.defaultScopePrefix + scope));
 	}
 
 	@Override
-	public boolean hasAnyScope(String... scopes) {
+	public final boolean hasAnyScope(String... scopes) {
+		Assert.notNull(scopes, "scopes cannot be null");
 		if (this.authorizationManagerFactory instanceof DefaultAuthorizationManagerFactory<T>) {
-			String scopePrefix = this.defaultScopePrefix;
-			for (int index = 0; index < scopes.length; index++) {
-				String scope = scopes[index];
-				if (scope.startsWith(scopePrefix)) {
-					scopes[index] = scope.substring(scopePrefix.length());
-				}
+			for (int i = 0; i < scopes.length; i++) {
+				assertScope(scopes[i]);
+				scopes[i] = this.defaultScopePrefix + scopes[i];
 			}
 		}
-		return isGranted(this.authorizationManagerFactory.hasAnyScope(scopes));
+		return isGranted(this.authorizationManagerFactory.hasAnyAuthority(scopes));
 	}
 
 	@Override
@@ -326,4 +325,11 @@ public void setPermissionEvaluator(PermissionEvaluator permissionEvaluator) {
 		this.permissionEvaluator = permissionEvaluator;
 	}
 
+	private void assertScope(String scope) {
+		Assert.notNull(scope, "scope cannot be null");
+		Assert.isTrue(!scope.startsWith(this.defaultScopePrefix), () -> scope + " should not start with '"
+				+ this.defaultScopePrefix + "' since '" + this.defaultScopePrefix
+				+ "' is automatically prepended when using hasScope and hasAnyScope. Consider using AuthorityAuthorizationManager#hasAuthority or #hasAnyAuthority instead.");
+	}
+
 }

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

@@ -124,59 +124,6 @@ public static <T> AuthorityAuthorizationManager<T> hasAnyAuthority(String... aut
 		return new AuthorityAuthorizationManager<>(authorities);
 	}
 
-	/**
-	 * Create an {@link AuthorityAuthorizationManager} that requires an
-	 * {@link Authentication} to have a {@code SCOPE_scope} authority.
-	 *
-	 * <p>
-	 * For example, if you call {@code hasScope("read")}, then this will require that each
-	 * authentication have a {@link org.springframework.security.core.GrantedAuthority}
-	 * whose value is {@code SCOPE_read}.
-	 *
-	 * <p>
-	 * This would equivalent to calling
-	 * {@code AuthorityAuthorizationManager#hasAuthority("SCOPE_read")}.
-	 * @param scope the scope value to require
-	 * @param <T> the secure object
-	 * @return an {@link AuthorityAuthorizationManager} that requires a
-	 * {@code "SCOPE_scope"} authority
-	 */
-	public static <T> AuthorityAuthorizationManager<T> hasScope(String scope) {
-		assertScope(scope);
-		return hasAuthority("SCOPE_" + scope);
-	}
-
-	/**
-	 * Create an {@link AuthorityAuthorizationManager} that requires an
-	 * {@link Authentication} to have at least one authority among {@code SCOPE_scope1},
-	 * {@code SCOPE_scope2}, ... {@code SCOPE_scopeN}.
-	 *
-	 * <p>
-	 * For example, if you call {@code hasAnyScope("read", "write")}, then this will
-	 * require that each authentication have at least a
-	 * {@link org.springframework.security.core.GrantedAuthority} whose value is either
-	 * {@code SCOPE_read} or {@code SCOPE_write}.
-	 *
-	 * <p>
-	 * This would equivalent to calling
-	 * {@code AuthorityAuthorizationManager#hasAnyAuthority("SCOPE_read", "SCOPE_write")}.
-	 * @param scopes the scope values to allow
-	 * @param <T> the secure object
-	 * @return an {@link AuthorityAuthorizationManager} that requires at least one
-	 * authority among {@code "SCOPE_scope1"}, {@code SCOPE_scope2}, ...
-	 * {@code SCOPE_scopeN}.
-	 *
-	 */
-	public static <T> AuthorityAuthorizationManager<T> hasAnyScope(String... scopes) {
-		Assert.notNull(scopes, "scopes cannot be null");
-		String[] mappedScopes = new String[scopes.length];
-		for (int i = 0; i < scopes.length; i++) {
-			assertScope(scopes[i]);
-			mappedScopes[i] = "SCOPE_" + scopes[i];
-		}
-		return hasAnyAuthority(mappedScopes);
-	}
-
 	private static String[] toNamedRolesArray(String rolePrefix, String[] roles) {
 		String[] result = new String[roles.length];
 		for (int i = 0; i < roles.length; i++) {
@@ -189,14 +136,6 @@ private static String[] toNamedRolesArray(String rolePrefix, String[] roles) {
 		return result;
 	}
 
-	private static void assertScope(String scope) {
-		Assert.notNull(scope, "scope cannot be null");
-		Assert.isTrue(!scope.startsWith("SCOPE_"),
-				() -> scope + " should not start with SCOPE_ since SCOPE_"
-						+ " is automatically prepended when using hasScope and hasAnyScope. Consider using "
-						+ " AuthorityReactiveAuthorizationManager#hasAuthority or #hasAnyAuthority instead.");
-	}
-
 	/**
 	 * {@inheritDoc}
 	 */

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

@@ -109,28 +109,6 @@ default AuthorizationManager<T> hasAllAuthorities(String... authorities) {
 		return AllAuthoritiesAuthorizationManager.hasAllAuthorities(authorities);
 	}
 
-	/**
-	 * Creates an {@link AuthorizationManager} that requires users to have the specified
-	 * scope.
-	 * @param scope the scope (automatically prepended with SCOPE_) that should be
-	 * required to allow access (i.e. write, read, etc.)
-	 * @return A new {@link AuthorizationManager} instance
-	 */
-	default AuthorizationManager<T> hasScope(String scope) {
-		return AuthorityAuthorizationManager.hasScope(scope);
-	}
-
-	/**
-	 * Creates an {@link AuthorizationManager} that requires users to have one of many
-	 * scopes.
-	 * @param scopes the scopes (automatically prepended with SCOPE_) that the user should
-	 * have at least one of to allow access (i.e. write, read, etc.)
-	 * @return A new {@link AuthorizationManager} instance
-	 */
-	default AuthorizationManager<T> hasAnyScope(String... scopes) {
-		return AuthorityAuthorizationManager.hasAnyScope(scopes);
-	}
-
 	/**
 	 * Creates an {@link AuthorizationManager} that allows any authenticated user.
 	 * @return A new {@link AuthorizationManager} instance

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

@@ -29,7 +29,6 @@
 import org.springframework.security.core.GrantedAuthority;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 
 /**
@@ -272,24 +271,4 @@ void hasAnyRoleWhenEmptyRolePrefixThenNoException() {
 		AuthorityAuthorizationManager.hasAnyRole("", new String[] { "USER" });
 	}
 
-	@Test
-	void hasAnyScopeWhenInvalidScopeThenThrowIllegalArgument() {
-		String[] scopes = { "read", "write", "SCOPE_invalid" };
-		assertThatExceptionOfType(IllegalArgumentException.class)
-			.isThrownBy(() -> AuthorityAuthorizationManager.hasAnyScope(scopes))
-			.withMessageContaining("SCOPE_invalid should not start with SCOPE_");
-	}
-
-	@Test
-	void hasScopeWhenValidScope() {
-		String scope = "read";
-		assertThat(AuthorityAuthorizationManager.hasScope(scope)).isNotNull();
-	}
-
-	@Test
-	void hasAnyScopeWhenValidScopes() {
-		String[] scopes = { "read", "write" };
-		assertThat(AuthorityAuthorizationManager.hasAnyScope(scopes)).isNotNull();
-	}
-
 }

core/src/test/java/org/springframework/security/authorization/method/PreAuthorizeReactiveAuthorizationManagerTests.java

@@ -90,6 +90,42 @@ public void checkDoSomethingStringWhenArgIsNotGrantThenDeniedDecision() throws E
 		assertThat(decision.isGranted()).isFalse();
 	}
 
+	@Test
+	public void checkSecuredScope() throws Exception {
+		MockMethodInvocation methodInvocation = new MockMethodInvocation(
+				new PreAuthorizeAuthorizationManagerTests.TestClass(),
+				PreAuthorizeAuthorizationManagerTests.TestClass.class, "securedScope");
+		PreAuthorizeReactiveAuthorizationManager manager = new PreAuthorizeReactiveAuthorizationManager();
+		Mono<Authentication> authentication = Mono
+			.just(new TestingAuthenticationToken("user", "password", "SCOPE_read"));
+		AuthorizationResult decision = manager.authorize(authentication, methodInvocation).block();
+		assertThat(decision).isNotNull();
+		assertThat(decision.isGranted()).isFalse();
+
+		authentication = Mono.just(new TestingAuthenticationToken("user", "password", "SCOPE_write"));
+		decision = manager.authorize(authentication, methodInvocation).block();
+		assertThat(decision).isNotNull();
+		assertThat(decision.isGranted()).isTrue();
+	}
+
+	@Test
+	public void checkSecuredAnyScope() throws Exception {
+		MockMethodInvocation methodInvocation = new MockMethodInvocation(
+				new PreAuthorizeAuthorizationManagerTests.TestClass(),
+				PreAuthorizeAuthorizationManagerTests.TestClass.class, "securedAnyScope");
+		PreAuthorizeReactiveAuthorizationManager manager = new PreAuthorizeReactiveAuthorizationManager();
+		Mono<Authentication> authentication = Mono
+			.just(new TestingAuthenticationToken("user", "password", "SCOPE_read"));
+		AuthorizationResult decision = manager.authorize(authentication, methodInvocation).block();
+		assertThat(decision).isNotNull();
+		assertThat(decision.isGranted()).isTrue();
+
+		authentication = Mono.just(new TestingAuthenticationToken("user", "password", "SCOPE_write"));
+		decision = manager.authorize(authentication, methodInvocation).block();
+		assertThat(decision).isNotNull();
+		assertThat(decision.isGranted()).isTrue();
+	}
+
 	@Test
 	public void checkRequiresAdminWhenClassAnnotationsThenMethodAnnotationsTakePrecedence() throws Exception {
 		Mono<Authentication> authentication = Mono
@@ -149,6 +185,16 @@ public void inheritedAnnotations() {
 
 		}
 
+		@PreAuthorize("hasScope('write')")
+		public void securedScope() {
+
+		}
+
+		@PreAuthorize("hasAnyScope('write', 'read')")
+		public void securedAnyScope() {
+
+		}
+
 	}
 
 	@PreAuthorize("hasRole('USER')")