From cd154cf3189a8ccda78da1d8f36d64b1ff2fff1b Mon Sep 17 00:00:00 2001 From: mposolda Date: Wed, 13 Dec 2023 10:30:59 +0100 Subject: [PATCH] User Profile: If required roles ('user') and reqired scopes are set, the required scopes have no effect closes #25475 Signed-off-by: mposolda --- .../DeclarativeUserProfileProvider.java | 11 +++ .../user/profile/AbstractUserProfileTest.java | 7 +- .../user/profile/UserProfileTest.java | 71 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java index d131bb0927..e08a65af8e 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java @@ -296,6 +296,17 @@ public class DeclarativeUserProfileProvider implements UserProfileProvider { if (rc != null) { if (rc.isAlways() || UPConfigUtils.isRoleForContext(context, rc.getRoles())) { required = AttributeMetadata.ALWAYS_TRUE; + + // If scopes are configured, we will use scope-based selector and require the attribute just if scope is + // in current authenticationSession (either default scope or by 'scope' parameter) + if (rc.getScopes() != null && !rc.getScopes().isEmpty()) { + if (UPConfigUtils.canBeAuthFlowContext(context)) { + required = (c) -> requestedScopePredicate(c, rc.getScopes()); + } else { + // Scopes not available for admin and account contexts + required = AttributeMetadata.ALWAYS_FALSE; + } + } } else if (UPConfigUtils.canBeAuthFlowContext(context) && rc.getScopes() != null && !rc.getScopes().isEmpty()) { // for contexts executed from auth flow and with configured scopes requirement // we have to create required validation with scopes based selector diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java index ab2827d9cc..a1dc3ea971 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/AbstractUserProfileTest.java @@ -29,6 +29,7 @@ import java.util.Optional; import java.util.Set; import org.junit.Before; +import org.keycloak.OAuth2Constants; import org.keycloak.common.Profile; import org.keycloak.component.ComponentModel; import org.keycloak.models.ClientModel; @@ -214,7 +215,11 @@ public abstract class AbstractUserProfileTest extends AbstractTestRealmKeycloakT @Override public String getClientNote(String name) { - return null; + if (OAuth2Constants.SCOPE.equals(name) && scopes != null && !scopes.isEmpty()) { + return String.join(" ", scopes); + } else { + return null; + } } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java index cf78703856..28c86d3757 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java @@ -61,6 +61,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.userprofile.config.UPConfig.UnmanagedAttributePolicy; import org.keycloak.representations.userprofile.config.UPGroup; import org.keycloak.services.messages.Messages; +import org.keycloak.testsuite.arquillian.annotation.ModelTest; import org.keycloak.testsuite.runonserver.RunOnServer; import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.userprofile.AttributeGroupMetadata; @@ -98,8 +99,10 @@ public class UserProfileTest extends AbstractUserProfileTest { testRealm.setClientScopes(new ArrayList<>()); testRealm.getClientScopes().add(ClientScopeBuilder.create().name("customer").protocol("openid-connect").build()); testRealm.getClientScopes().add(ClientScopeBuilder.create().name("client-a").protocol("openid-connect").build()); + testRealm.getClientScopes().add(ClientScopeBuilder.create().name("some-optional-scope").protocol("openid-connect").build()); ClientRepresentation client = KeycloakModelUtils.createClient(testRealm, "client-a"); client.setDefaultClientScopes(Collections.singletonList("customer")); + client.setOptionalClientScopes(Collections.singletonList("some-optional-scope")); KeycloakModelUtils.createClient(testRealm, "client-b"); } @@ -1429,6 +1432,74 @@ public class UserProfileTest extends AbstractUserProfileTest { } + @Test + @ModelTest + public void testRequiredByOptionalClientScope(KeycloakSession session) { + RealmModel realm = session.realms().getRealmByName("test"); + session.getContext().setRealm(realm); + + UserProfileProvider provider = getUserProfileProvider(session); + UPConfig config = parseDefaultConfig(); + config.addOrReplaceAttribute(new UPAttribute(ATT_ADDRESS, new UPAttributePermissions(Set.of(), Set.of(ROLE_ADMIN, ROLE_USER)), new UPAttributeRequired(Set.of(ROLE_ADMIN, ROLE_USER), Set.of("some-optional-scope")))); + provider.setConfiguration(config); + + Map attributes = new HashMap<>(); + + attributes.put(UserModel.USERNAME, "user"); + attributes.put(UserModel.FIRST_NAME, "John"); + attributes.put(UserModel.LAST_NAME, "Doe"); + attributes.put(UserModel.EMAIL, "user@email.test"); + + // client with default scopes. No address scope included + configureAuthenticationSession(session, "client-a", null); + + // No fail on admin and account console as they do not have scopes + UserProfile profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + profile = provider.create(UserProfileContext.ACCOUNT, attributes); + profile.validate(); + + // no fail on auth flow scopes when scope is not required + profile = provider.create(UserProfileContext.REGISTRATION, attributes); + profile.validate(); + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + profile = provider.create(UserProfileContext.IDP_REVIEW, attributes); + profile.validate(); + + // client with default scopes for which is attribute NOT configured as required + configureAuthenticationSession(session, "client-a", Set.of("some-optional-scope")); + + // No fail on admin and account console as they do not have scopes + profile = provider.create(UserProfileContext.USER_API, attributes); + profile.validate(); + profile = provider.create(UserProfileContext.ACCOUNT, attributes); + profile.validate(); + + // fail on auth flow scopes when scope is required + try { + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes); + profile.validate(); + fail("Should fail validation"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(ATT_ADDRESS)); + } + try { + profile = provider.create(UserProfileContext.REGISTRATION, attributes); + profile.validate(); + fail("Should fail validation"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(ATT_ADDRESS)); + } + try { + profile = provider.create(UserProfileContext.IDP_REVIEW, attributes); + profile.validate(); + fail("Should fail validation"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(ATT_ADDRESS)); + } + } + @Test public void testConfigurationInvalidScope() { getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testConfigurationInvalidScope);