Merge pull request #44667 from michalvavrik/feature/preauthorize-spri…

…ng-resteasy-fix Pass secured method arguments into security checks for `@PreAuthorize` security annotation on SpringWeb endpoints
quarkusio · Nov 25, 2024 · fc548b3 · fc548b3
2 parents fd27f9a + 245a281
commit fc548b3
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 0 deletions.
diff --git a/...nt/src/main/java/io/quarkus/spring/security/deployment/BeanMethodInvocationGenerator.java b/...nt/src/main/java/io/quarkus/spring/security/deployment/BeanMethodInvocationGenerator.java
@@ -130,6 +130,7 @@ final String generateSecurityCheck(String expression, MethodInfo securedMethodIn
                 instanceNullTrue.returnValue(newInstance);
             }
 
+            boolean checkRequiresMethodArguments = false;
             try (MethodCreator check = cc.getMethodCreator("check", boolean.class, SecurityIdentity.class, Object[].class)
                     .setModifiers(Modifier.PROTECTED)) {
                 ResultHandle arcContainer = check
@@ -155,6 +156,7 @@ final String generateSecurityCheck(String expression, MethodInfo securedMethodIn
 
                         argHandles[i] = check.load(argumentExpression.replace("'", ""));
                     } else if (trimmedArgumentExpression.matches(METHOD_PARAMETER_REGEX)) { // secured method's parameter case
+                        checkRequiresMethodArguments = true;
                         Matcher parameterMatcher = METHOD_PARAMETER_PATTERN.matcher(trimmedArgumentExpression);
                         if (!parameterMatcher.find()) { // should never happen
                             throw createGenericMalformedException(securedMethodInfo, expression);
@@ -206,6 +208,13 @@ final String generateSecurityCheck(String expression, MethodInfo securedMethodIn
 
                 check.returnValue(result);
             }
+
+            if (checkRequiresMethodArguments) {
+                try (MethodCreator check = cc.getMethodCreator("requiresMethodArguments", boolean.class)
+                        .setModifiers(Modifier.PUBLIC)) {
+                    check.returnBoolean(true);
+                }
+            }
         }
 
         beansReferencedInPreAuthorized.add(beanClassInfo.name().toString());

diff --git a/integration-tests/spring-web/src/main/java/io/quarkus/it/spring/security/PersonChecker.java b/integration-tests/spring-web/src/main/java/io/quarkus/it/spring/security/PersonChecker.java
@@ -0,0 +1,10 @@
+package io.quarkus.it.spring.security;
+
+import org.springframework.stereotype.Component;
+
+@Component
+public class PersonChecker {
+    public boolean check(String name) {
+        return name.equals("correct-name");
+    }
+}
diff --git a/integration-tests/spring-web/src/main/java/io/quarkus/it/spring/security/TheController.java b/integration-tests/spring-web/src/main/java/io/quarkus/it/spring/security/TheController.java
@@ -4,6 +4,7 @@
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -63,4 +64,9 @@ public String preAuthorizeOnController() {
         return "preAuthorizeOnController";
     }
 
+    @GetMapping("/preAuthorizeOnControllerWithArgs/{user}")
+    @PreAuthorize("@personChecker.check(#user)")
+    public String preAuthorizeOnControllerWithArgs(@PathVariable String user) {
+        return "Hello " + user + "!";
+    }
 }
diff --git a/integration-tests/spring-web/src/test/java/io/quarkus/it/spring/web/SecurityTest.java b/integration-tests/spring-web/src/test/java/io/quarkus/it/spring/web/SecurityTest.java
@@ -50,6 +50,16 @@ public void testPreAuthorizeOnController() {
                 Optional.of("preAuthorizeOnController"));
     }
 
+    @Test
+    public void preAuthorizeOnControllerWithArgs() {
+        String path = "/api/preAuthorizeOnControllerWithArgs/";
+        assertForAnonymous(path + "correct-name", 401, Optional.empty());
+        assertStatusAndContent(RestAssured.given().auth().preemptive().basic("stuart", "test"), path + "wrong-name", 403,
+                Optional.empty());
+        assertStatusAndContent(RestAssured.given().auth().preemptive().basic("aurea", "auri"), path + "correct-name", 200,
+                Optional.of("Hello correct-name!"));
+    }
+
     @Test
     public void shouldAccessAllowed() {
         assertForAnonymous("/api/accessibleForAllMethod", 200, Optional.of("accessibleForAll"));