diff --git a/docs/documentation/server_admin/topics/authentication/conditions.adoc b/docs/documentation/server_admin/topics/authentication/conditions.adoc index e5077cba28..f559c87449 100644 --- a/docs/documentation/server_admin/topics/authentication/conditions.adoc +++ b/docs/documentation/server_admin/topics/authentication/conditions.adoc @@ -24,7 +24,7 @@ This checks if the other executions in the flow are configured for the user. The Execution requirements section includes an example of the OTP form. `Condition - User Attribute`:: -This checks if the user has set up the required attribute. +This checks if the user has set up the required attribute: optionally, the check can also evaluate the group attributes. There is a possibility to negate output, which means the user should not have the attribute. The xref:proc-configuring-user-attributes_{context}[User Attributes] section shows how to add a custom attribute. You can provide these fields: @@ -38,6 +38,9 @@ Name of the attribute to check. Expected attribute value::: Expected value in the attribute. +Include group attributes::: +If On, the condition checks if any of the joined group has one attribute matching the configured name and value: this option can affect performance + Negate output::: You can negate the output. In other words, the attribute should not be present. diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValue.java b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValue.java index 1f85d676d0..0cdeb19d34 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValue.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValue.java @@ -23,6 +23,7 @@ import org.keycloak.authentication.AuthenticationFlowException; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; import java.util.Map; import java.util.Objects; @@ -37,6 +38,7 @@ public class ConditionalUserAttributeValue implements ConditionalAuthenticator { Map config = context.getAuthenticatorConfig().getConfig(); String attributeName = config.get(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_NAME); String attributeValue = config.get(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_EXPECTED_VALUE); + boolean includeGroupAttributes = Boolean.parseBoolean(config.get(ConditionalUserAttributeValueFactory.CONF_INCLUDE_GROUP_ATTRIBUTES)); boolean negateOutput = Boolean.parseBoolean(config.get(ConditionalUserAttributeValueFactory.CONF_NOT)); UserModel user = context.getUser(); @@ -45,6 +47,9 @@ public class ConditionalUserAttributeValue implements ConditionalAuthenticator { } boolean result = user.getAttributeStream(attributeName).anyMatch(attr -> Objects.equals(attr, attributeValue)); + if (!result && includeGroupAttributes) { + result = KeycloakModelUtils.resolveAttribute(user, attributeName, true).stream().anyMatch(attr -> Objects.equals(attr, attributeValue)); + } return negateOutput != result; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValueFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValueFactory.java index 4712cc624d..95024c159f 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValueFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalUserAttributeValueFactory.java @@ -31,6 +31,7 @@ public class ConditionalUserAttributeValueFactory implements ConditionalAuthenti public static final String CONF_ATTRIBUTE_NAME = "attribute_name"; public static final String CONF_ATTRIBUTE_EXPECTED_VALUE = "attribute_expected_value"; + public static final String CONF_INCLUDE_GROUP_ATTRIBUTES = "include_group_attributes"; public static final String CONF_NOT = "not"; private static final AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = { @@ -96,13 +97,19 @@ public class ConditionalUserAttributeValueFactory implements ConditionalAuthenti authNoteExpectedValue.setLabel("Expected attribute value"); authNoteExpectedValue.setHelpText("Expected value in the attribute"); + ProviderConfigProperty includeGroupAttributes = new ProviderConfigProperty(); + includeGroupAttributes.setType(ProviderConfigProperty.BOOLEAN_TYPE); + includeGroupAttributes.setName(CONF_INCLUDE_GROUP_ATTRIBUTES); + includeGroupAttributes.setLabel("Include group attributes"); + includeGroupAttributes.setHelpText("If On, the condition checks if any of the joined groups has one attribute matching the configured name and value (this option can affect performance)"); + ProviderConfigProperty negateOutput = new ProviderConfigProperty(); negateOutput.setType(ProviderConfigProperty.BOOLEAN_TYPE); negateOutput.setName(CONF_NOT); negateOutput.setLabel("Negate output"); negateOutput.setHelpText("Apply a not to the check result"); - return Arrays.asList(authNoteName, authNoteExpectedValue, negateOutput); + return Arrays.asList(authNoteName, authNoteExpectedValue, includeGroupAttributes, negateOutput); } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ConditionalUserAttributeAuthenticatorTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ConditionalUserAttributeAuthenticatorTest.java new file mode 100644 index 0000000000..bf9e2d8a18 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ConditionalUserAttributeAuthenticatorTest.java @@ -0,0 +1,206 @@ +package org.keycloak.testsuite.forms; + +import org.jboss.arquillian.graphene.page.Page; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.authentication.authenticators.access.AllowAccessAuthenticatorFactory; +import org.keycloak.authentication.authenticators.access.DenyAccessAuthenticatorFactory; +import org.keycloak.authentication.authenticators.browser.PasswordFormFactory; +import org.keycloak.authentication.authenticators.browser.UsernameFormFactory; +import org.keycloak.events.Details; +import org.keycloak.events.Errors; +import org.keycloak.models.AuthenticationExecutionModel; +import org.keycloak.representations.idm.GroupRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; +import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.authentication.authenticators.conditional.ConditionalUserAttributeValueFactory; +import org.keycloak.testsuite.pages.ErrorPage; +import org.keycloak.testsuite.pages.LoginUsernameOnlyPage; +import org.keycloak.testsuite.pages.PasswordPage; +import org.keycloak.testsuite.util.AccountHelper; +import org.keycloak.testsuite.util.FlowUtil; +import org.keycloak.testsuite.util.GroupBuilder; +import org.keycloak.testsuite.util.UserBuilder; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.keycloak.testsuite.forms.BrowserFlowTest.revertFlows; + +/** + * @author Daniele Martinoli + */ +public class ConditionalUserAttributeAuthenticatorTest extends AbstractTestRealmKeycloakTest { + + private final static String X_APPROVE_ATTR = "x-approved"; + private final static String X_APPROVE_ATTR_VALUE = Boolean.toString(true); + + private final static String APPROVED_GROUP = "approved"; + private final static String SUBGROUP = "subgroup"; + + private final static String APPROVED_USER = "approved"; + private final static String APPROVED_BY_GROUP_USER = "approved-by-group"; + private final static String APPROVED_BY_SUBGROUP_USER = "approved-by-subgroup"; + private final static String PASSWORD = "password"; + + @Page + protected LoginUsernameOnlyPage loginUsernameOnlyPage; + + @Page + protected PasswordPage passwordPage; + + @Page + protected ErrorPage errorPage; + + @Rule + public AssertEvents events = new AssertEvents(this); + + @Override + public void configureTestRealm(RealmRepresentation testRealm) {} + + private void createUsers() { + GroupRepresentation subGroup = GroupBuilder.create().name(SUBGROUP).build(); + testRealm().groups().add(subGroup); + GroupRepresentation approvedGroup = GroupBuilder.create().name(APPROVED_GROUP).subGroups(List.of(subGroup)) + .attributes(Map.of(X_APPROVE_ATTR, List.of(X_APPROVE_ATTR_VALUE))) + .build(); + testRealm().groups().add(approvedGroup); + + UserRepresentation approved = UserBuilder.create().username(APPROVED_USER).password(PASSWORD) + .addAttribute(X_APPROVE_ATTR, X_APPROVE_ATTR_VALUE) + .build(); + testRealm().users().create(approved); + + UserRepresentation approvedByGroup = UserBuilder.create().username(APPROVED_BY_GROUP_USER).password(PASSWORD) + .addAttribute(X_APPROVE_ATTR, X_APPROVE_ATTR_VALUE) + .addGroups(APPROVED_GROUP) + .build(); + testRealm().users().create(approvedByGroup); + + UserRepresentation approvedBySubgroup = UserBuilder.create().username(APPROVED_BY_SUBGROUP_USER).password(PASSWORD) + .addAttribute(X_APPROVE_ATTR, X_APPROVE_ATTR_VALUE) + .addGroups(SUBGROUP) + .build(); + testRealm().users().create(approvedBySubgroup); + } + + @Test + public void testAllowedUsersWithApprovedAttribute(){ + final String flowAlias = "browser - user attribute condition"; + final String errorMessage = "You don't have necessary attribute."; + + createUsers(); + configureBrowserFlowWithConditionalUserAttribute(flowAlias, errorMessage); + + for (String user : List.of(APPROVED_USER, APPROVED_BY_GROUP_USER, APPROVED_BY_SUBGROUP_USER)) { + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.assertCurrent(); + loginUsernameOnlyPage.login(user); + + final String testUserId = testRealm().users().search(user).get(0).getId(); + + passwordPage.assertCurrent(); + passwordPage.login(PASSWORD); + + events.expectLogin() + .user(testUserId) + .detail(Details.USERNAME, user) + .removeDetail(Details.CONSENT) + .assertEvent(); + + AccountHelper.logout(testRealm(), user); + } + } + + /** + * This test checks that if user does not have specific attribute, then the access is denied. + */ + @Test + public void testDenyUserWithoutApprovedAttribute() { + final String flowAlias = "browser - user attribute condition"; + final String errorMessage = "You don't have necessary attribute."; + final String user = "test-user@localhost"; + + configureBrowserFlowWithConditionalUserAttribute(flowAlias, errorMessage); + + try { + loginUsernameOnlyPage.open(); + loginUsernameOnlyPage.assertCurrent(); + loginUsernameOnlyPage.login(user); + + errorPage.assertCurrent(); + assertThat(errorPage.getError(), is(errorMessage)); + + events.expectLogin() + .user((String) null) + .session((String) null) + .error(Errors.ACCESS_DENIED) + .detail(Details.USERNAME, user) + .removeDetail(Details.CONSENT) + .assertEvent(); + } finally { + revertFlows(testRealm(), flowAlias); + } + } + + /** + * This flow contains: + * UsernameForm REQUIRED + * Subflow CONDITIONAL + * ** conditional user attribute + * ** Allow Access REQUIRED + * Subflow CONDITIONAL + * ** conditional user attribute-negated + * ** Deny Access REQUIRED + * Password REQUIRED + * + * @param newFlowAlias + * @param conditionProviderId + * @param conditionConfig + * @param denyConfig + */ + private void configureBrowserFlowWithConditionalUserAttribute(String newFlowAlias, String errorMessage) { + Map hasApproveAttributeConfigMap = new HashMap<>(); + hasApproveAttributeConfigMap.put(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_NAME, X_APPROVE_ATTR); + hasApproveAttributeConfigMap.put(ConditionalUserAttributeValueFactory.CONF_ATTRIBUTE_EXPECTED_VALUE, X_APPROVE_ATTR_VALUE); + hasApproveAttributeConfigMap.put(ConditionalUserAttributeValueFactory.CONF_INCLUDE_GROUP_ATTRIBUTES, Boolean.toString(true)); + hasApproveAttributeConfigMap.put(ConditionalUserAttributeValueFactory.CONF_NOT, Boolean.toString(false)); + + Map missApproveAttributeConfigMap = new HashMap<>(hasApproveAttributeConfigMap); + missApproveAttributeConfigMap.put(ConditionalUserAttributeValueFactory.CONF_NOT, Boolean.toString(true)); + + Map denyAccessConfigMap = new HashMap<>(); + denyAccessConfigMap.put(DenyAccessAuthenticatorFactory.ERROR_MESSAGE, errorMessage); + + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias)); + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) + .selectFlow(newFlowAlias) + .inForms(forms -> forms + .clear() + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, UsernameFormFactory.PROVIDER_ID) + .addSubFlowExecution(AuthenticationExecutionModel.Requirement.CONDITIONAL, subflow -> subflow + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, + ConditionalUserAttributeValueFactory.PROVIDER_ID, + config -> config.setConfig(hasApproveAttributeConfigMap)) + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, + AllowAccessAuthenticatorFactory.PROVIDER_ID, config -> {}) + ) + .addSubFlowExecution(AuthenticationExecutionModel.Requirement.CONDITIONAL, subflow -> subflow + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, + ConditionalUserAttributeValueFactory.PROVIDER_ID, + config -> config.setConfig(missApproveAttributeConfigMap)) + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, + DenyAccessAuthenticatorFactory.PROVIDER_ID, config -> config.setConfig(denyAccessConfigMap)) + ) + .addAuthenticatorExecution(AuthenticationExecutionModel.Requirement.REQUIRED, PasswordFormFactory.PROVIDER_ID) + ) + .defineAsBrowserFlow() // Activate this new flow + ); + } +}