Merge pull request #37436 from coiouhkc/bugfix/37390-roles-allowed-co…

…mma-in-property-value Respect comma escapes in property value for @RolesAllowed.
quarkusio · Dec 1, 2023 · 60d084f · 60d084f
2 parents 37c57c6 + b045e66
commit 60d084f
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/...yment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java b/...yment/src/test/java/io/quarkus/security/test/rolesallowed/RolesAllowedExpressionTest.java
@@ -40,7 +40,8 @@ public class RolesAllowedExpressionTest {
             "%test.test-profile-admin=admin\n" +
             "missing-profile-profile-admin=superman\n" +
             "%missing-profile.missing-profile-profile-admin=admin\n" +
-            "all-roles=Administrator,Software,Tester,User\n";
+            "all-roles=Administrator,Software,Tester,User\n" +
+            "ldap-roles=cn=Administrator\\\\,ou=Software\\\\,dc=Tester\\\\,dc=User\n";
 
     @RegisterExtension
     static final QuarkusUnitTest config = new QuarkusUnitTest()
@@ -90,6 +91,10 @@ public void shouldRestrictAccessToSpecificRole() {
         assertSuccess(() -> bean.list(), "list",
                 new AuthData(Set.of("Administrator", "Software", "Tester", "User"), false, "list"));
         assertFailureFor(() -> bean.list(), ForbiddenException.class, ADMIN);
+
+        // property expression with escaped collection separator should not be treated as list
+        assertSuccess(() -> bean.ldap(), "ldap",
+                new AuthData(Set.of("cn=Administrator,ou=Software,dc=Tester,dc=User"), false, "ldap"));
     }
 
     @Singleton
@@ -141,6 +146,11 @@ public final String list() {
             return "list";
         }
 
+        @RolesAllowed("${ldap-roles}")
+        public final String ldap() {
+            return "ldap";
+        }
+
     }
 
 }
diff --git a/...ons/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java b/...ons/security/runtime/src/main/java/io/quarkus/security/runtime/SecurityCheckRecorder.java
@@ -98,15 +98,17 @@ public String[] get() {
                         // @RolesAllowed({"${my.roles}"}) => my.roles=one,two <=> @RolesAllowed({"one", "two"})
                         if (strVal != null && strVal.contains(",")) {
                             var strArr = StringUtil.split(strVal);
-                            if (strArr.length > 1) {
+                            if (strArr.length >= 1) {
                                 // role order is irrelevant as logical operator between them is OR
 
-                                // first role will go to the original place
+                                // first role will go to the original place, double escaped comma will be parsed correctly
                                 strVal = strArr[0];
 
-                                // the rest of the roles will be appended at the end
-                                for (int i1 = 1; i1 < strArr.length; i1++) {
-                                    roles.add(strArr[i1]);
+                                if (strArr.length > 1) {
+                                    // the rest of the roles will be appended at the end
+                                    for (int i1 = 1; i1 < strArr.length; i1++) {
+                                        roles.add(strArr[i1]);
+                                    }
                                 }
                             }
                         }