From 48ef3f47195c19d960d6f61c97e409b1e0ae8b43 Mon Sep 17 00:00:00 2001 From: Evgeniy Cheban Date: Tue, 17 May 2022 19:55:45 +0300 Subject: [PATCH] Some Security Expressions cause NPE when used within Query annotation Added trustResolver, roleHierarchy, permissionEvaluator, defaultRolePrefix fields to SecurityEvaluationContextExtension. Closes gh-11196 Closes gh-11289 --- .../SecurityEvaluationContextExtension.java | 24 +++++++++++++++++-- ...curityEvaluationContextExtensionTests.java | 16 ++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/data/src/main/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtension.java b/data/src/main/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtension.java index 3696904a9a..02c6027ecc 100644 --- a/data/src/main/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtension.java +++ b/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,7 +17,13 @@ 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; @@ -77,12 +83,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 +121,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() { diff --git a/data/src/test/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtensionTests.java b/data/src/test/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtensionTests.java index a890a463af..40ef5f8920 100644 --- a/data/src/test/java/org/springframework/security/data/repository/query/SecurityEvaluationContextExtensionTests.java +++ b/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,10 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +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.authentication.AuthenticationTrustResolverImpl; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.context.SecurityContextHolder; @@ -69,6 +72,17 @@ public class SecurityEvaluationContextExtensionTests { assertThat(getRoot().getAuthentication()).isSameAs(explicit); } + @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(); }