Option for client to specify default acr level (#10364)

Closes #10160
keycloak · Feb 22, 2022 · 8c3fc5a · 8c3fc5a
1 parent 1df842e
commit 8c3fc5a
Show file tree

Hide file tree

Showing 12 changed files with 225 additions and 10 deletions.
diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java
@@ -135,6 +135,7 @@ public final class Constants {
     public static final String REQUESTED_LEVEL_OF_AUTHENTICATION = "requested-level-of-authentication";
     public static final String FORCE_LEVEL_OF_AUTHENTICATION = "force-level-of-authentication";
     public static final String ACR_LOA_MAP = "acr.loa.map";
+    public static final String DEFAULT_ACR_VALUES = "default.acr.values";
     public static final int MINIMUM_LOA = 0;
     public static final int NO_LOA = -1;
 }
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java
@@ -375,13 +375,13 @@ private void setAttribute(String attrKey, String attrValue) {
         }
     }
 
-    private List<String> getAttributeMultivalued(String attrKey) {
+    public List<String> getAttributeMultivalued(String attrKey) {
         String attrValue = getAttribute(attrKey);
         if (attrValue == null) return Collections.emptyList();
         return Arrays.asList(Constants.CFG_DELIMITER_PATTERN.split(attrValue));
     }
 
-    private void setAttributeMultivalued(String attrKey, List<String> attrValues) {
+    public void setAttributeMultivalued(String attrKey, List<String> attrValues) {
         if (attrValues == null || attrValues.size() == 0) {
             // Remove attribute
             setAttribute(attrKey, null);

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
@@ -903,7 +903,7 @@ private String getAcr(AuthenticatedClientSessionModel clientSession) {
         if (acr == null) {
             acr = AcrUtils.mapLoaToAcr(loa, acrLoaMap, AcrUtils.getAcrValues(
                 clientSession.getNote(OIDCLoginProtocol.CLAIMS_PARAM),
-                clientSession.getNote(OIDCLoginProtocol.ACR_PARAM)));
+                clientSession.getNote(OIDCLoginProtocol.ACR_PARAM), clientSession.getClient()));
             if (acr == null) {
                 acr = AcrUtils.mapLoaToAcr(loa, acrLoaMap, acrLoaMap.keySet());
                 if (acr == null) {

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
@@ -296,7 +296,7 @@ private void updateAuthenticationSession() {
         List<String> acrValues = AcrUtils.getRequiredAcrValues(request.getClaims());
 
         if (acrValues.isEmpty()) {
-            acrValues = AcrUtils.getAcrValues(request.getClaims(), request.getAcr());
+            acrValues = AcrUtils.getAcrValues(request.getClaims(), request.getAcr(), authenticationSession.getClient());
         } else {
             authenticationSession.setClientNote(Constants.FORCE_LEVEL_OF_AUTHENTICATION, "true");
         }
@@ -309,11 +309,11 @@ private void updateAuthenticationSession() {
                 // this is an unknown acr. In case of an essential claim, we directly reject authentication as we cannot met the specification requirement. Otherwise fallback to minimum LoA
                 boolean essential = Boolean.parseBoolean(authenticationSession.getClientNote(Constants.FORCE_LEVEL_OF_AUTHENTICATION));
                 if (essential) {
-                    logger.errorf("Requested essential acr value '%s' is not a number and it is not mapped in the client mappings. Please doublecheck your client configuration or correct ACR passed in the 'claims' parameter.", acr);
+                    logger.errorf("Requested essential acr value '%s' is not a number and it is not mapped in the ACR-To-Loa mappings of realm or client. Please doublecheck ACR-to-LOA mapping or correct ACR passed in the 'claims' parameter.", acr);
                     event.error(Errors.INVALID_REQUEST);
                     throw new ErrorPageException(session, authenticationSession, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.CLAIMS_PARAM);
                 } else {
-                    logger.warnf("Requested acr value '%s' is not a number and it is not mapped in the client mappings. Please doublecheck your client configuration or correct ACR passed in the 'claims' parameter. Ignoring passed acr", acr);
+                    logger.warnf("Requested acr value '%s' is not a number and it is not mapped in the ACR-To-Loa mappings of realm or client. Please doublecheck ACR-to-LOA mapping or correct used ACR.", acr);
                     return Constants.MINIMUM_LOA;
                 }
             }

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/AcrUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/AcrUtils.java
@@ -29,6 +29,7 @@
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.RealmModel;
+import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.representations.ClaimsRepresentation;
 import org.keycloak.representations.IDToken;
 import org.keycloak.util.JsonSerialization;
@@ -41,8 +42,14 @@ public static List<String> getRequiredAcrValues(String claimsParam) {
         return getAcrValues(claimsParam, null, true);
     }
 
-    public static List<String> getAcrValues(String claimsParam, String acrValuesParam) {
-        return getAcrValues(claimsParam, acrValuesParam, false);
+    public static List<String> getAcrValues(String claimsParam, String acrValuesParam, ClientModel client) {
+        List<String> fromParams = getAcrValues(claimsParam, acrValuesParam, false);
+        if (!fromParams.isEmpty()) {
+            return fromParams;
+        }
+
+        // Fallback to default ACR values of client (if configured)
+        return getDefaultAcrValues(client);
     }
 
     private static List<String> getAcrValues(String claimsParam, String acrValuesParam, boolean essential) {
@@ -140,4 +147,9 @@ public static String mapLoaToAcr(int loa, Map<String, Integer> acrLoaMap, Collec
         }
         return acr;
     }
+
+
+    public static List<String> getDefaultAcrValues(ClientModel client) {
+        return OIDCAdvancedConfigWrapper.fromClientModel(client).getAttributeMultivalued(Constants.DEFAULT_ACR_VALUES);
+    }
 }
diff --git a/...ces/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java b/...ces/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java
@@ -28,12 +28,14 @@
 import org.keycloak.jose.jwk.JWKParser;
 import org.keycloak.jose.jws.Algorithm;
 import org.keycloak.models.CibaConfig;
+import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ParConfig;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.mappers.PairwiseSubMapperHelper;
+import org.keycloak.protocol.oidc.utils.AcrUtils;
 import org.keycloak.protocol.oidc.utils.AuthorizeClientUtil;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.protocol.oidc.utils.PairwiseSubMapperUtils;
@@ -234,6 +236,10 @@ public static ClientRepresentation toInternal(KeycloakSession session, OIDCClien
 
         configWrapper.setFrontChannelLogoutUrl(Optional.ofNullable(clientOIDC.getFrontChannelLogoutUri()).orElse(null));
 
+        if (clientOIDC.getDefaultAcrValues() != null) {
+            configWrapper.setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, clientOIDC.getDefaultAcrValues());
+        }
+
         return client;
     }
 
@@ -414,6 +420,11 @@ public static OIDCClientRepresentation toExternalResponse(KeycloakSession sessio
 
         response.setFrontChannelLogoutUri(config.getFrontChannelLogoutUrl());
 
+        List<String> defaultAcrValues = config.getAttributeMultivalued(Constants.DEFAULT_ACR_VALUES);
+        if (!defaultAcrValues.isEmpty()) {
+            response.setDefaultAcrValues(defaultAcrValues);
+        }
+
         return response;
     }
 

diff --git a/services/src/main/java/org/keycloak/validation/DefaultClientValidationProvider.java b/services/src/main/java/org/keycloak/validation/DefaultClientValidationProvider.java
@@ -16,13 +16,15 @@
  */
 package org.keycloak.validation;
 
+import org.keycloak.authentication.AuthenticatorUtil;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientScopeModel;
 import org.keycloak.protocol.ProtocolMapperConfigException;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCConfigAttributes;
 import org.keycloak.protocol.oidc.grants.ciba.CibaClientValidation;
 import org.keycloak.protocol.oidc.mappers.PairwiseSubMapperHelper;
+import org.keycloak.protocol.oidc.utils.AcrUtils;
 import org.keycloak.protocol.oidc.utils.PairwiseSubMapperUtils;
 import org.keycloak.protocol.oidc.utils.PairwiseSubMapperValidator;
 import org.keycloak.protocol.oidc.utils.SubjectType;
@@ -35,6 +37,7 @@
 import java.net.URISyntaxException;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation;
@@ -133,6 +136,7 @@ public ValidationResult validate(ValidationContext<ClientModel> context) {
         validatePairwiseInClientModel(context);
         new CibaClientValidation(context).validate();
         validateJwks(context);
+        validateDefaultAcrValues(context);
 
         return context.toResult();
     }
@@ -142,6 +146,7 @@ public ValidationResult validate(ClientValidationContext.OIDCContext context) {
         validateUrls(context);
         validatePairwiseInOIDCClient(context);
         new CibaClientValidation(context).validate();
+        validateDefaultAcrValues(context);
 
         return context.toResult();
     }
@@ -264,4 +269,20 @@ private void validateJwks(ValidationContext<ClientModel> context) {
             context.addError("jwksUrl", "Illegal to use both jwks_uri and jwks_string", "duplicatedJwksSettings");
         }
     }
+
+    private void validateDefaultAcrValues(ValidationContext<ClientModel> context) {
+        ClientModel client = context.getObjectToValidate();
+        List<String> defaultAcrValues = AcrUtils.getDefaultAcrValues(client);
+        Map<String, Integer> acrToLoaMap = AcrUtils.getAcrLoaMap(client);
+        if (acrToLoaMap.isEmpty()) {
+            acrToLoaMap = AcrUtils.getAcrLoaMap(client.getRealm());
+        }
+        for (String configuredAcr : defaultAcrValues) {
+            if (acrToLoaMap.containsKey(configuredAcr)) continue;
+            if (!AuthenticatorUtil.getLoAConfiguredInRealmBrowserFlow(client.getRealm())
+                    .anyMatch(level -> configuredAcr.equals(String.valueOf(level)))) {
+                context.addError("defaultAcrValues", "Default ACR values need to contain values specified in the ACR-To-Loa mapping or number levels from set realm browser flow");
+            }
+        }
+    }
 }
diff --git a/...an/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java b/...an/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java
@@ -32,6 +32,7 @@
 import org.keycloak.jose.jwe.JWEConstants;
 import org.keycloak.jose.jws.Algorithm;
 import org.keycloak.models.CibaConfig;
+import org.keycloak.models.Constants;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
@@ -44,6 +45,7 @@
 import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.util.KeycloakModelUtils;
+import org.keycloak.util.JsonSerialization;
 
 import java.util.*;
 import java.util.stream.Collectors;
@@ -729,4 +731,38 @@ public void testClientWithoutGrantTypes() throws Exception {
         OIDCAdvancedConfigWrapper config = OIDCAdvancedConfigWrapper.fromClientRepresentation(kcClient);
         Assert.assertTrue(config.isUseRefreshToken());
     }
+
+    @Test
+    public void testDefaultAcrValues() throws Exception {
+        // Set realm acr-to-loa mapping
+        RealmRepresentation realmRep = adminClient.realm("test").toRepresentation();
+        Map<String, Integer> acrLoaMap = new HashMap<>();
+        acrLoaMap.put("copper", 0);
+        acrLoaMap.put("silver", 1);
+        acrLoaMap.put("gold", 2);
+        realmRep.getAttributes().put(Constants.ACR_LOA_MAP, JsonSerialization.writeValueAsString(acrLoaMap));
+        adminClient.realm("test").update(realmRep);
+
+        OIDCClientRepresentation clientRep = createRep();
+        clientRep.setDefaultAcrValues(Arrays.asList("silver", "foo"));
+        try {
+            OIDCClientRepresentation response = reg.oidc().create(clientRep);
+            fail("Expected 400");
+        } catch (ClientRegistrationException e) {
+            assertEquals(400, ((HttpErrorException) e.getCause()).getStatusLine().getStatusCode());
+        }
+
+        clientRep.setDefaultAcrValues(Arrays.asList("silver", "gold"));
+        OIDCClientRepresentation response = reg.oidc().create(clientRep);
+        Assert.assertNames(response.getDefaultAcrValues(), "silver", "gold");
+
+        // Test Keycloak representation
+        ClientRepresentation kcClient = getClient(response.getClientId());
+        OIDCAdvancedConfigWrapper config = OIDCAdvancedConfigWrapper.fromClientRepresentation(kcClient);
+        Assert.assertNames(config.getAttributeMultivalued(Constants.DEFAULT_ACR_VALUES), "silver", "gold");
+
+        // Revert realm acr-to-loa mappings
+        realmRep.getAttributes().remove(Constants.ACR_LOA_MAP);
+        adminClient.realm("test").update(realmRep);
+    }
 }
diff --git a/...llian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java b/...llian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java
@@ -22,6 +22,8 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+
+import javax.ws.rs.BadRequestException;
 import javax.ws.rs.core.UriBuilder;
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.After;
@@ -37,6 +39,7 @@
 import org.keycloak.events.Details;
 import org.keycloak.models.AuthenticationExecutionModel.Requirement;
 import org.keycloak.models.Constants;
+import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.representations.ClaimsRepresentation;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.ClientRepresentation;
@@ -337,6 +340,79 @@ public void testRealmAcrLoaMapping() throws IOException {
         testRealm().update(realmRep);
     }
 
+    @Test
+    public void testClientDefaultAcrValues() {
+        ClientResource testClient = ApiUtil.findClientByClientId(testRealm(), "test-app");
+        ClientRepresentation testClientRep = testClient.toRepresentation();
+        OIDCAdvancedConfigWrapper.fromClientRepresentation(testClientRep).setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, Arrays.asList("silver", "gold"));
+        testClient.update(testClientRep);
+
+        // Should request client to authenticate with silver
+        oauth.openLoginForm();
+        authenticateWithUsername();
+        assertLoggedInWithAcr("silver");
+
+        // Re-configure to level gold
+        OIDCAdvancedConfigWrapper.fromClientRepresentation(testClientRep).setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, Arrays.asList("gold"));
+        testClient.update(testClientRep);
+        oauth.openLoginForm();
+        authenticateWithPassword();
+        assertLoggedInWithAcr("gold");
+
+        // Value from essential ACR should have preference
+        openLoginFormWithAcrClaim(true, "silver");
+        assertLoggedInWithAcr("0");
+
+        // Value from non-essential ACR should have preference
+        openLoginFormWithAcrClaim(false, "silver");
+        assertLoggedInWithAcr("0");
+
+        // Revert
+        testClientRep.getAttributes().put(Constants.DEFAULT_ACR_VALUES, null);
+        testClient.update(testClientRep);
+    }
+
+    @Test
+    public void testClientDefaultAcrValuesValidation() throws IOException {
+        // Setup realm acr-to-loa mapping
+        RealmRepresentation realmRep = testRealm().toRepresentation();
+        Map<String, Integer> acrLoaMap = new HashMap<>();
+        acrLoaMap.put("realm:copper", 0);
+        acrLoaMap.put("realm:silver", 1);
+        realmRep.getAttributes().put(Constants.ACR_LOA_MAP, JsonSerialization.writeValueAsString(acrLoaMap));
+        testRealm().update(realmRep);
+
+        // Value "foo" not used in any ACR-To-Loa mapping
+        ClientResource testClient = ApiUtil.findClientByClientId(testRealm(), "test-app");
+        ClientRepresentation testClientRep = testClient.toRepresentation();
+        OIDCAdvancedConfigWrapper.fromClientRepresentation(testClientRep).setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, Arrays.asList("silver", "2", "foo"));
+        try {
+            testClient.update(testClientRep);
+            Assert.fail("Should not successfully update client");
+        } catch (BadRequestException bre) {
+            // Expected
+        }
+
+        // Value "5" too big
+        OIDCAdvancedConfigWrapper.fromClientRepresentation(testClientRep).setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, Arrays.asList("silver", "2", "5"));
+        try {
+            testClient.update(testClientRep);
+            Assert.fail("Should not successfully update client");
+        } catch (BadRequestException bre) {
+            // Expected
+        }
+
+        // Should be fine
+        OIDCAdvancedConfigWrapper.fromClientRepresentation(testClientRep).setAttributeMultivalued(Constants.DEFAULT_ACR_VALUES, Arrays.asList("silver", "2"));
+        testClient.update(testClientRep);
+
+        // Revert
+        testClientRep.getAttributes().put(Constants.DEFAULT_ACR_VALUES, null);
+        testClient.update(testClientRep);
+        realmRep.getAttributes().remove(Constants.ACR_LOA_MAP);
+        testRealm().update(realmRep);
+    }
+
     public void openLoginFormWithAcrClaim(boolean essential, String... acrValues) {
         openLoginFormWithAcrClaim(oauth, essential, acrValues);
     }

diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -1943,6 +1943,8 @@ use-idtoken-as-detached-signature.tooltip=This makes ID token returned from Auth
 acr-loa-map=ACR to LoA Mapping
 acr-loa-map.tooltip=Define which ACR (Authentication Context Class Reference) value is mapped to which LoA (Level of Authentication). The ACR can be any value, whereas the LoA must be numeric. The LoA typically refers to the numbers configured as levels in the conditions in the authentication flow. The ACR refers to the value used in the OIDC/SAML authorization request and returned to client in the tokens.
 acr-loa-map-client.tooltip=Define which ACR (Authentication Context Class Reference) value is mapped to which LoA (Level of Authentication). The ACR can be any value, whereas the LoA must be numeric. This is recommended to be configured at the realm level where it is shared for all the clients. If you configure at the client level, the client mapping will take precedence over the mapping from the realm level.
+default-acr-values=Default ACR Values
+default-acr-values.tooltip=Default values to be used as voluntary ACR in case that there is no explicit ACR requested by 'claims' or 'acr_values' parameter in the OIDC request.
 
 key-not-allowed-here=Key '{{character}}' is not allowed here.