Merge pull request #37536 from geoand/#37526

Fix != expression in @PreAuthorize check
quarkusio · Dec 5, 2023 · 08cf46e · 08cf46e
2 parents 6e48533 + 4616d52
commit 08cf46e
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 8 deletions.
diff --git a/...ployment/src/main/java/io/quarkus/spring/security/deployment/SpringSecurityProcessor.java b/...ployment/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/...eployment/src/test/java/io/quarkus/spring/security/deployment/SpringPreAuthorizeTest.java b/...eployment/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/...oyment/src/test/java/io/quarkus/spring/security/deployment/springapp/SpringComponent.java b/...oyment/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/.../src/main/java/io/quarkus/spring/security/runtime/interceptor/SpringSecurityRecorder.java b/.../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/...ing/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java b/...ing/security/runtime/interceptor/check/PrincipalNameFromParameterObjectSecurityCheck.java
@@ -23,22 +23,24 @@ public class PrincipalNameFromParameterObjectSecurityCheck implements SecurityCh
     private final Class<?> expectedParameterClass;
     private final Class<? extends StringPropertyAccessor> 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<? extends StringPropertyAccessor>) 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
+    }
 }