Merge pull request quarkusio#50367 from michalvavrik/feature/fix-step-up-auth-with-kc-single-val-attr

Fix step authentication with Keycloak when protocol mapper has 'acr' user attribute configured as single valued string

Author: geoand

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StepUpAuthenticationPolicy.java

@@ -26,7 +26,7 @@ record StepUpAuthenticationPolicy(String[] expectedAcrValues, Long maxAge) imple
     private static volatile boolean enabled = false;
 
     StepUpAuthenticationPolicy(String acrValues, Duration maxAge) {
-        this(acrValues.split(","), maxAge == null ? null : maxAge.toSeconds());
+        this(splitAcrValues(acrValues), maxAge == null ? null : maxAge.toSeconds());
     }
 
     private static final Logger LOG = Logger.getLogger(StepUpAuthenticationPolicy.class);
@@ -63,17 +63,21 @@ private void verifyMaxAge(JsonObject json) {
     }
 
     private void verifyAcr(JsonObject json) {
-        JsonArray acr = json.getJsonArray(OidcConstants.ACR);
-        if (acr != null && !acr.isEmpty()) {
-            boolean acrFound = true;
-            for (String expectedAcrValue : expectedAcrValues) {
-                if (!acr.contains(expectedAcrValue)) {
-                    LOG.debug("Acr value " + expectedAcrValue + " is required but not found in token 'acr' claim: " + acr);
-                    acrFound = false;
-                    break;
+        Object acrObject = json.getValue(OidcConstants.ACR);
+        if (acrObject != null) {
+            if (acrObject instanceof JsonArray acr && !acr.isEmpty()) {
+                boolean acrFound = true;
+                for (String expectedAcrValue : expectedAcrValues) {
+                    if (!acr.contains(expectedAcrValue)) {
+                        LOG.debug("Acr value " + expectedAcrValue + " is required but not found in token 'acr' claim: " + acr);
+                        acrFound = false;
+                        break;
+                    }
                 }
-            }
-            if (acrFound) {
+                if (acrFound) {
+                    return;
+                }
+            } else if (expectedAcrValues.length == 1 && expectedAcrValues[0].equals(acrObject)) {
                 return;
             }
         }
@@ -147,4 +151,12 @@ static void markAsEnabled() {
     static boolean isEnabled() {
         return enabled;
     }
+
+    private static String[] splitAcrValues(String acrValues) {
+        String[] acrValuesArr = acrValues.split(",");
+        if (acrValuesArr.length > 1) {
+            return Set.of(acrValuesArr).toArray(String[]::new);
+        }
+        return acrValuesArr;
+    }
 }

integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java

@@ -425,7 +425,11 @@ private String jwt(String audience, String subject, String kid, boolean withEmpt
         }
 
         if (acr != null && !acr.isEmpty()) {
-            builder.claim("acr", Arrays.asList(acr.split(",")));
+            if (acr.endsWith("_string")) {
+                builder.claim("acr", acr);
+            } else {
+                builder.claim("acr", Arrays.asList(acr.split(",")));
+            }
         }
 
         if (authTime != null && !authTime.isEmpty()) {

integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenStepUpAuthenticationTest.java

@@ -232,6 +232,15 @@ public void testMaxAgeAndAcrRequired() {
                 .body(is("max-age-and-acr-required"));
     }
 
+    @Test
+    public void testSingleAcrStringValueRequired() {
+        // wrong single acr -> fail
+        stepUpMethodLevelRequest(Set.of("3_string"), "no-rbac-annotation-string").statusCode(401);
+        // correct acr -> pass
+        stepUpMethodLevelRequest(Set.of("1_string"), "no-rbac-annotation-string").statusCode(200)
+                .body(is("no-rbac-annotation"));
+    }
+
     private static ValidatableResponse stepUpMethodLevelRequest(Set<String> acrValues, String path) {
         return stepUpMethodLevelRequest(acrValues, path, null);
     }
@@ -350,6 +359,13 @@ public String noRbacAnnotationMethodLevel() {
             return "no-rbac-annotation";
         }
 
+        @GET
+        @AuthenticationContext("1_string")
+        @Path("no-rbac-annotation-string")
+        public String noRbacAnnotationMethodLevelString() {
+            return "no-rbac-annotation";
+        }
+
         @GET
         @AuthenticationContext("3")
         @RolesAllowed("user")