Sfoglia il codice sorgente

Some Security Expressions cause NPE when used within Query annotation

Added trustResolver, roleHierarchy, permissionEvaluator, defaultRolePrefix
fields to SecurityEvaluationContextExtension along with setter methods to override defaults.

Closes gh-11196
Evgeniy Cheban 3 anni fa
parent
commit
9235a841fe

+ 71 - 2
data/src/main/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,10 +17,17 @@
 package org.springframework.security.data.repository.query;
 
 import org.springframework.data.spel.spi.EvaluationContextExtension;
+import org.springframework.security.access.PermissionEvaluator;
+import org.springframework.security.access.expression.DenyAllPermissionEvaluator;
 import org.springframework.security.access.expression.SecurityExpressionRoot;
+import org.springframework.security.access.hierarchicalroles.NullRoleHierarchy;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
+import org.springframework.security.authentication.AuthenticationTrustResolver;
+import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.util.Assert;
 
 /**
  * <p>
@@ -77,12 +84,21 @@ import org.springframework.security.core.context.SecurityContextHolder;
  * it.
  *
  * @author Rob Winch
+ * @author Evgeniy Cheban
  * @since 4.0
  */
 public class SecurityEvaluationContextExtension implements EvaluationContextExtension {
 
 	private Authentication authentication;
 
+	private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+
+	private RoleHierarchy roleHierarchy = new NullRoleHierarchy();
+
+	private PermissionEvaluator permissionEvaluator = new DenyAllPermissionEvaluator();
+
+	private String defaultRolePrefix = "ROLE_";
+
 	/**
 	 * Creates a new instance that uses the current {@link Authentication} found on the
 	 * {@link org.springframework.security.core.context.SecurityContextHolder}.
@@ -106,8 +122,13 @@ public class SecurityEvaluationContextExtension implements EvaluationContextExte
 	@Override
 	public SecurityExpressionRoot getRootObject() {
 		Authentication authentication = getAuthentication();
-		return new SecurityExpressionRoot(authentication) {
+		SecurityExpressionRoot root = new SecurityExpressionRoot(authentication) {
 		};
+		root.setTrustResolver(this.trustResolver);
+		root.setRoleHierarchy(this.roleHierarchy);
+		root.setPermissionEvaluator(this.permissionEvaluator);
+		root.setDefaultRolePrefix(this.defaultRolePrefix);
+		return root;
 	}
 
 	private Authentication getAuthentication() {
@@ -118,4 +139,52 @@ public class SecurityEvaluationContextExtension implements EvaluationContextExte
 		return context.getAuthentication();
 	}
 
+	/**
+	 * Sets the {@link AuthenticationTrustResolver} to be used. Default is
+	 * {@link AuthenticationTrustResolverImpl}. Cannot be null.
+	 * @param trustResolver the {@link AuthenticationTrustResolver} to use
+	 * @since 5.8
+	 */
+	public void setTrustResolver(AuthenticationTrustResolver trustResolver) {
+		Assert.notNull(trustResolver, "trustResolver cannot be null");
+		this.trustResolver = trustResolver;
+	}
+
+	/**
+	 * Sets the {@link RoleHierarchy} to be used. Default is {@link NullRoleHierarchy}.
+	 * Cannot be null.
+	 * @param roleHierarchy the {@link RoleHierarchy} to use
+	 * @since 5.8
+	 */
+	public void setRoleHierarchy(RoleHierarchy roleHierarchy) {
+		Assert.notNull(roleHierarchy, "roleHierarchy cannot be null");
+		this.roleHierarchy = roleHierarchy;
+	}
+
+	/**
+	 * Sets the {@link PermissionEvaluator} to be used. Default is
+	 * {@link DenyAllPermissionEvaluator}. Cannot be null.
+	 * @param permissionEvaluator the {@link PermissionEvaluator} to use
+	 * @since 5.8
+	 */
+	public void setPermissionEvaluator(PermissionEvaluator permissionEvaluator) {
+		Assert.notNull(permissionEvaluator, "permissionEvaluator cannot be null");
+		this.permissionEvaluator = permissionEvaluator;
+	}
+
+	/**
+	 * Sets the default prefix to be added to
+	 * {@link org.springframework.security.access.expression.SecurityExpressionRoot#hasAnyRole(String...)}
+	 * or
+	 * {@link org.springframework.security.access.expression.SecurityExpressionRoot#hasRole(String)}.
+	 * For example, if hasRole("ADMIN") or hasRole("ROLE_ADMIN") is passed in, then the
+	 * role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default).
+	 * @param defaultRolePrefix the default prefix to add to roles. The default is
+	 * "ROLE_".
+	 * @since 5.8
+	 */
+	public void setDefaultRolePrefix(String defaultRolePrefix) {
+		this.defaultRolePrefix = defaultRolePrefix;
+	}
+
 }

+ 78 - 1
data/src/test/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtensionTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,7 +20,13 @@ import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.security.access.PermissionEvaluator;
+import org.springframework.security.access.expression.DenyAllPermissionEvaluator;
 import org.springframework.security.access.expression.SecurityExpressionRoot;
+import org.springframework.security.access.hierarchicalroles.NullRoleHierarchy;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
+import org.springframework.security.authentication.AuthenticationTrustResolver;
+import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.context.SecurityContextHolder;
 
@@ -69,6 +75,77 @@ public class SecurityEvaluationContextExtensionTests {
 		assertThat(getRoot().getAuthentication()).isSameAs(explicit);
 	}
 
+	@Test
+	public void setTrustResolverWhenNullThenIllegalArgumentException() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		assertThatIllegalArgumentException().isThrownBy(() -> this.securityExtension.setTrustResolver(null))
+				.withMessage("trustResolver cannot be null");
+	}
+
+	@Test
+	public void setTrustResolverWhenNotNullThenVerifyRootObject() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		this.securityExtension.setTrustResolver(trustResolver);
+		assertThat(getRoot()).extracting("trustResolver").isEqualTo(trustResolver);
+	}
+
+	@Test
+	public void setRoleHierarchyWhenNullThenIllegalArgumentException() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		assertThatIllegalArgumentException().isThrownBy(() -> this.securityExtension.setRoleHierarchy(null))
+				.withMessage("roleHierarchy cannot be null");
+	}
+
+	@Test
+	public void setRoleHierarchyWhenNotNullThenVerifyRootObject() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		RoleHierarchy roleHierarchy = new NullRoleHierarchy();
+		this.securityExtension.setRoleHierarchy(roleHierarchy);
+		assertThat(getRoot()).extracting("roleHierarchy").isEqualTo(roleHierarchy);
+	}
+
+	@Test
+	public void setPermissionEvaluatorWhenNullThenIllegalArgumentException() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		assertThatIllegalArgumentException().isThrownBy(() -> this.securityExtension.setPermissionEvaluator(null))
+				.withMessage("permissionEvaluator cannot be null");
+	}
+
+	@Test
+	public void setPermissionEvaluatorWhenNotNullThenVerifyRootObject() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		PermissionEvaluator permissionEvaluator = new DenyAllPermissionEvaluator();
+		this.securityExtension.setPermissionEvaluator(permissionEvaluator);
+		assertThat(getRoot()).extracting("permissionEvaluator").isEqualTo(permissionEvaluator);
+	}
+
+	@Test
+	public void setDefaultRolePrefixWhenCustomThenVerifyRootObject() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		String defaultRolePrefix = "CUSTOM_";
+		this.securityExtension.setDefaultRolePrefix(defaultRolePrefix);
+		assertThat(getRoot()).extracting("defaultRolePrefix").isEqualTo(defaultRolePrefix);
+	}
+
+	@Test
+	public void getRootObjectWhenAdditionalFieldsNotSetThenVerifyDefaults() {
+		TestingAuthenticationToken explicit = new TestingAuthenticationToken("explicit", "password", "ROLE_EXPLICIT");
+		this.securityExtension = new SecurityEvaluationContextExtension(explicit);
+		SecurityExpressionRoot root = getRoot();
+		assertThat(root).extracting("trustResolver").isInstanceOf(AuthenticationTrustResolverImpl.class);
+		assertThat(root).extracting("roleHierarchy").isInstanceOf(NullRoleHierarchy.class);
+		assertThat(root).extracting("permissionEvaluator").isInstanceOf(DenyAllPermissionEvaluator.class);
+		assertThat(root).extracting("defaultRolePrefix").isEqualTo("ROLE_");
+	}
+
 	private SecurityExpressionRoot getRoot() {
 		return this.securityExtension.getRootObject();
 	}