From 229b3b199d88d0c86c110ebb764211780e73f683 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Tue, 5 Dec 2023 17:36:12 +0200 Subject: [PATCH] Fix != expression in @PreAuthorize check Fixes: #37526 (cherry picked from commit 4616d52b43d44b86c373bcfaf3e6d75ce4190ea1) --- .../deployment/SpringSecurityProcessor.java | 8 ++++++- .../deployment/SpringPreAuthorizeTest.java | 10 ++++++++ .../deployment/springapp/SpringComponent.java | 5 ++++ .../interceptor/SpringSecurityRecorder.java | 5 ++-- ...lNameFromParameterObjectSecurityCheck.java | 23 +++++++++++++++---- 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/extensions/spring-security/deployment/src/main/java/io/quarkus/spring/security/deployment/SpringSecurityProcessor.java b/extensions/spring-security/deployment/src/main/java/io/quarkus/spring/security/deployment/SpringSecurityProcessor.java index 70d16090e0090..d5d70eb2fbace 100644 --- a/extensions/spring-security/deployment/src/main/java/io/quarkus/spring/security/deployment/SpringSecurityProcessor.java +++ b/extensions/spring-security/deployment/src/main/java/io/quarkus/spring/security/deployment/SpringSecurityProcessor.java @@ -48,6 +48,7 @@ import io.quarkus.spring.security.runtime.interceptor.SpringPreauthorizeInterceptor; import io.quarkus.spring.security.runtime.interceptor.SpringSecuredInterceptor; import io.quarkus.spring.security.runtime.interceptor.SpringSecurityRecorder; +import io.quarkus.spring.security.runtime.interceptor.check.PrincipalNameFromParameterObjectSecurityCheck; import io.quarkus.spring.security.runtime.interceptor.check.PrincipalNameFromParameterSecurityCheck; class SpringSecurityProcessor { @@ -466,13 +467,18 @@ void addSpringPreAuthorizeSecurityCheck(CombinedIndexBuildItem index, propertyName, index.getIndex(), part); + PrincipalNameFromParameterObjectSecurityCheck.CheckType checkType = part.contains("==") + ? PrincipalNameFromParameterObjectSecurityCheck.CheckType.EQ + : PrincipalNameFromParameterObjectSecurityCheck.CheckType.NEQ; + securityChecks.add(springSecurityRecorder.principalNameFromParameterObjectSecurityCheck( parameterNameAndIndex.getIndex(), stringPropertyAccessorData.getMatchingParameterClassInfo().name().toString(), StringPropertyAccessorGenerator .getAccessorClassName( stringPropertyAccessorData.getMatchingParameterClassInfo().name()), - stringPropertyAccessorData.getMatchingParameterFieldInfo().name())); + stringPropertyAccessorData.getMatchingParameterFieldInfo().name(), + checkType)); } } else if (part.matches(SpringSecurityProcessorUtil.BASIC_BEAN_METHOD_INVOCATION_REGEX)) { diff --git a/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/SpringPreAuthorizeTest.java b/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/SpringPreAuthorizeTest.java index eaab4280b3647..c7dc3deff1f77 100644 --- a/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/SpringPreAuthorizeTest.java +++ b/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/SpringPreAuthorizeTest.java @@ -116,6 +116,16 @@ public void testPrincipalNameFromObject() { assertSuccess(() -> springComponent.principalNameFromObject(new Person("user")), "user", USER); } + @Test + public void testPrincipalNameFromObjectIsNot() { + assertFailureFor(() -> springComponent.principalNameFromObjectIsNot(new Person("whatever")), + UnauthorizedException.class, + ANONYMOUS); + assertSuccess(() -> springComponent.principalNameFromObjectIsNot(new Person("whatever")), "whatever", USER); + assertFailureFor(() -> springComponent.principalNameFromObjectIsNot(new Person("user")), ForbiddenException.class, + USER); + } + @Test public void testNotSecured() { assertSuccess(() -> springComponent.notSecured(), "notSecured", ANONYMOUS); diff --git a/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/springapp/SpringComponent.java b/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/springapp/SpringComponent.java index 0d86e8406fcf3..68a8af6552c6d 100644 --- a/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/springapp/SpringComponent.java +++ b/extensions/spring-security/deployment/src/test/java/io/quarkus/spring/security/deployment/springapp/SpringComponent.java @@ -37,6 +37,11 @@ public String principalNameFromObject(Person person) { return person.getName(); } + @PreAuthorize("#person.name != authentication.principal.username") + public String principalNameFromObjectIsNot(Person person) { + return person.getName(); + } + public String notSecured() { return "notSecured"; } diff --git a/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/SpringSecurityRecorder.java b/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/SpringSecurityRecorder.java index 8101bdd0991b6..591c08d8bd892 100644 --- a/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/SpringSecurityRecorder.java +++ b/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/SpringSecurityRecorder.java @@ -70,8 +70,9 @@ public SecurityCheck fromGeneratedClass(String generatedClassName) { } public SecurityCheck principalNameFromParameterObjectSecurityCheck(int index, String expectedParameterClass, - String stringPropertyAccessorClass, String propertyName) { + String stringPropertyAccessorClass, String propertyName, + PrincipalNameFromParameterObjectSecurityCheck.CheckType checkType) { return PrincipalNameFromParameterObjectSecurityCheck.of(index, expectedParameterClass, stringPropertyAccessorClass, - propertyName); + propertyName, checkType); } } diff --git a/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java b/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java index c34e97b6d5f7b..63da575e48187 100644 --- a/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java +++ b/extensions/spring-security/runtime/src/main/java/io/quarkus/spring/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java @@ -23,22 +23,24 @@ public class PrincipalNameFromParameterObjectSecurityCheck implements SecurityCh private final Class expectedParameterClass; private final Class stringPropertyAccessorClass; private final String propertyName; + private final CheckType checkType; private PrincipalNameFromParameterObjectSecurityCheck(int index, String expectedParameterClass, - String stringPropertyAccessorClass, String propertyName) throws ClassNotFoundException { + String stringPropertyAccessorClass, String propertyName, CheckType checkType) throws ClassNotFoundException { this.index = index; this.expectedParameterClass = Class.forName(expectedParameterClass, false, Thread.currentThread().getContextClassLoader()); this.stringPropertyAccessorClass = (Class) Class.forName(stringPropertyAccessorClass, false, Thread.currentThread().getContextClassLoader()); this.propertyName = propertyName; + this.checkType = checkType; } public static PrincipalNameFromParameterObjectSecurityCheck of(int index, String expectedParameterClass, - String stringPropertyAccessorClass, String propertyName) { + String stringPropertyAccessorClass, String propertyName, CheckType checkType) { try { return new PrincipalNameFromParameterObjectSecurityCheck(index, expectedParameterClass, stringPropertyAccessorClass, - propertyName); + propertyName, checkType); } catch (ClassNotFoundException e) { throw new RuntimeException(e); } @@ -70,8 +72,14 @@ private void doApply(SecurityIdentity identity, Object[] parameters, String clas } String name = identity.getPrincipal().getName(); - if (!name.equals(parameterValueStr)) { - throw new ForbiddenException(); + if (checkType == CheckType.EQ) { + if (!name.equals(parameterValueStr)) { + throw new ForbiddenException(); + } + } else if (checkType == CheckType.NEQ) { + if (name.equals(parameterValueStr)) { + throw new ForbiddenException(); + } } } @@ -84,4 +92,9 @@ private IllegalStateException genericNotApplicableException(String className, St "PrincipalNameFromParameterObjectSecurityCheck with index " + index + " cannot be applied to '" + className + "#" + methodName + "'"); } + + public enum CheckType { + EQ, + NEQ + } }