diff --git a/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java index db472808e890..f44b39d1e97e 100755 --- a/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java @@ -34,6 +34,7 @@ public class IdentityProviderRepresentation { public static final String UPFLM_ON = "on"; public static final String UPFLM_MISSING = "missing"; + public static final String UPFLM_MISSING_ONLY = "missing-only"; public static final String UPFLM_OFF = "off"; /** @@ -41,11 +42,13 @@ public class IdentityProviderRepresentation { * * * @see #UPFLM_ON * @see #UPFLM_MISSING + * @see #UPFLM_MISSING_ONLY * @see #UPFLM_OFF */ @Deprecated diff --git a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java index baf64fae38c4..845c874c3fda 100755 --- a/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java @@ -35,7 +35,7 @@ public interface LoginFormsProvider extends Provider { String UPDATE_PROFILE_CONTEXT_ATTR = "updateProfileCtx"; - String IS_UPDATE_PROFILE = "isUpdateProfile"; + String UPDATE_PROFILE_FIRST_LOGIN = "updateProfileFirstLogin"; String TERMS_ACCEPTANCE_REQUIRED = "termsAcceptanceRequired"; String IDENTITY_PROVIDER_BROKER_CONTEXT = "identityProviderBrokerCtx"; diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java index abaff35f11c8..ec02fd2fbc1c 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java @@ -64,14 +64,15 @@ protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredI MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); String action = formData.getFirst("submitAction"); - if (action != null && action.equals("updateProfile")) { - context.resetFlow(() -> { - AuthenticationSessionModel authSession = context.getAuthenticationSession(); - - serializedCtx.saveToAuthenticationSession(authSession, BROKERED_CONTEXT_NOTE); - authSession.setAuthNote(ENFORCE_UPDATE_PROFILE, "true"); - }); - } else if (action != null && action.equals("linkAccount")) { +// if (action != null && action.equals("updateProfile")) { +// context.resetFlow(() -> { +// AuthenticationSessionModel authSession = context.getAuthenticationSession(); +// +// serializedCtx.saveToAuthenticationSession(authSession, BROKERED_CONTEXT_NOTE); +// authSession.setAuthNote(ENFORCE_UPDATE_PROFILE, "true"); +// }); +// } else + if (action != null && action.equals("linkAccount")) { context.success(); } else { throw new AuthenticationFlowException("Unknown action: " + action, diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java index e344671ff43b..83ab7e2f35fd 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java @@ -59,8 +59,6 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator { private static final Logger logger = Logger.getLogger(IdpReviewProfileAuthenticator.class); private static final String TERMS_FIELD ="termsAccepted"; private boolean enabledRequiredAction= false; - private boolean updateProfile= false; - @Override public boolean requiresUser() { return false; @@ -69,9 +67,9 @@ public boolean requiresUser() { @Override protected void authenticateImpl(AuthenticationFlowContext context, SerializedBrokeredIdentityContext userCtx, BrokeredIdentityContext brokerContext) { IdentityProviderModel idpConfig = brokerContext.getIdpConfig(); - AuthenticatorConfigModel authenticatorConfig = context.getAuthenticatorConfig(); - enabledRequiredAction = context.getRealm().getRequiredActionProviderByAlias(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name()).isEnabled() && Boolean.valueOf(authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.TERMS_AND_CONDITIONS)); - updateProfile = requiresUpdateProfilePage(authenticatorConfig, context, userCtx); + enabledRequiredAction = context.getRealm().getRequiredActionProviderByAlias(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name()).isEnabled() && Boolean.valueOf(context.getAuthenticatorConfig().getConfig().get(IdpReviewProfileAuthenticatorFactory.TERMS_AND_CONDITIONS)); + String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(context); + boolean updateProfile = requiresUpdateProfilePage(context, userCtx, updateProfileFirstLogin); if ( enabledRequiredAction || updateProfile) { @@ -84,7 +82,7 @@ protected void authenticateImpl(AuthenticationFlowContext context, SerializedBro // No formData for first render. The profile is rendered from userCtx Response challengeResponse = context.form() .setAttribute(LoginFormsProvider.UPDATE_PROFILE_CONTEXT_ATTR, userCtx) - .setAttribute(LoginFormsProvider.IS_UPDATE_PROFILE, updateProfile) + .setAttribute(LoginFormsProvider.UPDATE_PROFILE_FIRST_LOGIN, updateProfileFirstLogin) .setAttribute(LoginFormsProvider.TERMS_ACCEPTANCE_REQUIRED, enabledRequiredAction) .setFormData(null) .createUpdateProfilePage(); @@ -95,14 +93,8 @@ protected void authenticateImpl(AuthenticationFlowContext context, SerializedBro } } - protected boolean requiresUpdateProfilePage( AuthenticatorConfigModel authenticatorConfig, AuthenticationFlowContext context, SerializedBrokeredIdentityContext userCtx) { - if (Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(ENFORCE_UPDATE_PROFILE))) { - return true; - } - - String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(authenticatorConfig); - - if(IdentityProviderRepresentation.UPFLM_MISSING.equals(updateProfileFirstLogin)) { + protected boolean requiresUpdateProfilePage(AuthenticationFlowContext context, SerializedBrokeredIdentityContext userCtx, String updateProfileFirstLogin) { + if(IdentityProviderRepresentation.UPFLM_MISSING.equals(updateProfileFirstLogin) || IdentityProviderRepresentation.UPFLM_MISSING_ONLY.equals(updateProfileFirstLogin)) { try { UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class); profileProvider.create(UserProfileContext.IDP_REVIEW, userCtx.getAttributes()).validate(); @@ -115,7 +107,12 @@ protected boolean requiresUpdateProfilePage( AuthenticatorConfigModel authentica } } - private String calculateUpdateProfileFirstLogin(AuthenticatorConfigModel authenticatorConfig) { + private String calculateUpdateProfileFirstLogin(AuthenticationFlowContext context) { + AuthenticatorConfigModel authenticatorConfig = context.getAuthenticatorConfig(); + if (Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(ENFORCE_UPDATE_PROFILE))) { + return authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN); + } + if (authenticatorConfig == null || !authenticatorConfig.getConfig().containsKey(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN)) { return IdentityProviderRepresentation.UPFLM_MISSING; } else { @@ -128,12 +125,13 @@ protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredI EventBuilder event = context.getEvent(); event.event(EventType.UPDATE_PROFILE).detail(Details.CONTEXT, UserProfileContext.IDP_REVIEW.name()); MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); + String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(context); if (enabledRequiredAction && ! formData.containsKey(TERMS_FIELD)) { Response challengeForTerms = context.form() .setErrors(Collections.singletonList(new FormMessage(TERMS_FIELD, "termsAcceptanceRequired"))) .setAttribute(LoginFormsProvider.UPDATE_PROFILE_CONTEXT_ATTR, userCtx) - .setAttribute(LoginFormsProvider.IS_UPDATE_PROFILE, updateProfile) + .setAttribute(LoginFormsProvider.UPDATE_PROFILE_FIRST_LOGIN, updateProfileFirstLogin) .setAttribute(LoginFormsProvider.TERMS_ACCEPTANCE_REQUIRED, enabledRequiredAction) .setFormData(formData) .createUpdateProfilePage(); @@ -194,8 +192,7 @@ public String getServiceAccountClientLink() { attributes.put(TermsAndConditions.USER_ATTRIBUTE, Arrays.asList(Integer.toString(Time.currentTime()))); } //for security reason keep old values equal to broken data - String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(context.getAuthenticatorConfig()); - if (IdentityProviderRepresentation.UPFLM_MISSING.equals(updateProfileFirstLogin)) { + if (IdentityProviderRepresentation.UPFLM_MISSING_ONLY.equals(updateProfileFirstLogin)) { if (userCtx.getUsername() != null) { attributes.put("username", Stream.of(userCtx.getUsername()).toList()); } @@ -243,7 +240,7 @@ public String getServiceAccountClientLink() { Response challenge = context.form() .setErrors(errors) .setAttribute(LoginFormsProvider.UPDATE_PROFILE_CONTEXT_ATTR, userCtx) - .setAttribute(LoginFormsProvider.IS_UPDATE_PROFILE, updateProfile) + .setAttribute(LoginFormsProvider.UPDATE_PROFILE_FIRST_LOGIN, updateProfileFirstLogin) .setAttribute(LoginFormsProvider.TERMS_ACCEPTANCE_REQUIRED, enabledRequiredAction) .setFormData(formData) .createUpdateProfilePage(); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java index c7141fec0d50..76c154fe35e9 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java @@ -104,7 +104,7 @@ public boolean isUserSetupAllowed() { property.setName(UPDATE_PROFILE_ON_FIRST_LOGIN); property.setLabel("Update Profile on First Login"); property.setType(ProviderConfigProperty.LIST_TYPE); - List updateProfileValues = Arrays.asList(IdentityProviderRepresentation.UPFLM_ON, IdentityProviderRepresentation.UPFLM_MISSING, IdentityProviderRepresentation.UPFLM_OFF); + List updateProfileValues = Arrays.asList(IdentityProviderRepresentation.UPFLM_ON, IdentityProviderRepresentation.UPFLM_MISSING, IdentityProviderRepresentation.UPFLM_MISSING_ONLY, IdentityProviderRepresentation.UPFLM_OFF); property.setOptions(updateProfileValues); property.setDefaultValue(IdentityProviderRepresentation.UPFLM_MISSING); property.setHelpText("Define conditions under which a user has to review and update his profile after first-time login. Value 'On' means that" diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java index 2ec25d953991..c6c6fd541fdc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java @@ -104,7 +104,7 @@ public void testErrorExistingUserWithUpdateProfile() { waitForPage(driver, "account already exists", false); assertTrue(idpConfirmLinkPage.isCurrent()); - assertEquals("User with email user@localhost.com already exists. How do you want to continue?", idpConfirmLinkPage.getMessage()); + assertEquals("User with username consumer already exists. How do you want to continue?", idpConfirmLinkPage.getMessage()); } @@ -870,8 +870,6 @@ public void testVerifyEmailRequiredActionWhenChangingEmailDuringFirstLogin() { realm.update(realmRep); - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); - IdentityProviderRepresentation idpRep = identityProviderResource.toRepresentation(); idpRep.setTrustEmail(true); @@ -1090,8 +1088,6 @@ public void testEventsOnUpdateProfileNoEmailChange() { @Test public void testEventsOnUpdateProfileWithEmailChange() { - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); - createUser(bc.providerRealmName(), "no-first-name", "password", null, "LastName", "no-first-name@localhost.com"); oauth.clientId("broker-app"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java index 1d0cb06b5c5a..a50f683ef566 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java @@ -269,7 +269,7 @@ public void testLoginWithDifferentBrokerWhenUpdatingProfile() { @Test public void testEditUsername() { - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin); createUser(bc.providerRealmName(), "no-first-name", "password", null, "LastName", "no-first-name@localhost.com"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java index 3cfa25866915..9cb1d70148ac 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java @@ -1,6 +1,5 @@ package org.keycloak.testsuite.broker; -import org.keycloak.admin.client.resource.AuthenticationManagementResource; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.admin.client.resource.UsersResource; @@ -14,9 +13,7 @@ import org.keycloak.dom.saml.v2.assertion.NameIDType; import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; import org.keycloak.dom.saml.v2.protocol.ResponseType; -import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.protocol.saml.SamlPrincipalType; -import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; @@ -42,7 +39,6 @@ import java.io.InputStream; import java.net.URI; import java.util.*; -import java.util.function.BiConsumer; import java.util.stream.Collectors; import jakarta.ws.rs.core.Response; import org.jboss.arquillian.graphene.page.Page; @@ -150,17 +146,9 @@ public void addTestRealms(List testRealms) { testRealms.add(loadFromClasspath("kc3731-broker-realm.json", p)); } - private void updateExecutions(BiConsumer< AuthenticationExecutionInfoRepresentation, AuthenticationManagementResource > action) { - AuthenticationManagementResource flows = adminClient.realm(REALM_PROV_NAME).flows(); - - for (AuthenticationExecutionInfoRepresentation execution : flows.getExecutions(DefaultAuthenticationFlows.FIRST_BROKER_LOGIN_FLOW)) { - action.accept(execution, flows); - } - } - @Test public void testProviderIdpInitiatedLogin() throws Exception { - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + driver.navigate().to(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")); waitForPage("sign in to", true); @@ -227,7 +215,7 @@ private void assertAudience(ResponseType resp, String expectedAudience) throws E @Test public void testProviderIdpInitiatedLoginToApp() throws Exception { - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) // Login in provider realm @@ -267,7 +255,7 @@ public void testConsumerIdpInitiatedLoginToApp() throws Exception { principals.add(pr); rep.getConfig().put(SAMLIdentityProviderConfig.MULTIPLE_PRINCIPALS, JsonSerialization.writeValueAsString(principals)); idp.update(rep); - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_CONS_NAME, "sales")) @@ -307,7 +295,7 @@ public void testConsumerIdpInitiatedLoginToApp() throws Exception { @Test public void testTwoConsequentIdpInitiatedLogins() throws Exception { - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) @@ -387,7 +375,7 @@ public void testProviderIdpInitiatedLoginWithPrincipalAttribute() throws Excepti rep.getConfig().put(SAMLIdentityProviderConfig.MULTIPLE_PRINCIPALS, JsonSerialization.writeValueAsString(principals)); idp.update(rep); - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) @@ -449,7 +437,7 @@ public void testProviderTransientIdpInitiatedLogin() throws Exception { rep.getConfig().put(SAMLIdentityProviderConfig.MULTIPLE_PRINCIPALS, JsonSerialization.writeValueAsString(principals)); idp.update(rep); - updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) diff --git a/themes/src/main/resources/theme/base/login/login-idp-link-confirm.ftl b/themes/src/main/resources/theme/base/login/login-idp-link-confirm.ftl index c3537c5d3e91..666e9994d292 100644 --- a/themes/src/main/resources/theme/base/login/login-idp-link-confirm.ftl +++ b/themes/src/main/resources/theme/base/login/login-idp-link-confirm.ftl @@ -5,7 +5,6 @@ <#elseif section = "form">
-
diff --git a/themes/src/main/resources/theme/base/login/login-update-profile.ftl b/themes/src/main/resources/theme/base/login/login-update-profile.ftl index c97d34a84178..fa842db75c9e 100755 --- a/themes/src/main/resources/theme/base/login/login-update-profile.ftl +++ b/themes/src/main/resources/theme/base/login/login-update-profile.ftl @@ -12,7 +12,7 @@
disabled + class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.username?has_content)>disabled aria-invalid="<#if messagesPerField.existsError('username')>true" /> @@ -31,7 +31,7 @@
disabled + class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.email?has_content) >disabled aria-invalid="<#if messagesPerField.existsError('email')>true" /> @@ -50,7 +50,7 @@
disabled + class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.firstName?has_content)>disabled aria-invalid="<#if messagesPerField.existsError('firstName')>true" /> @@ -68,7 +68,7 @@
disabled + class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.lastName?has_content)>disabled aria-invalid="<#if messagesPerField.existsError('lastName')>true" /> diff --git a/themes/src/main/resources/theme/base/login/user-profile-commons.ftl b/themes/src/main/resources/theme/base/login/user-profile-commons.ftl index 56db2368cddc..5650f4a94ce7 100644 --- a/themes/src/main/resources/theme/base/login/user-profile-commons.ftl +++ b/themes/src/main/resources/theme/base/login/user-profile-commons.ftl @@ -76,7 +76,7 @@ <#macro inputTag attribute> disabled + <#if attribute.readOnly || updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && attribute.value?has_content)>disabled <#if attribute.autocomplete??>autocomplete="${attribute.autocomplete}" <#if attribute.annotations.inputTypePlaceholder??>placeholder="${attribute.annotations.inputTypePlaceholder}" <#if attribute.annotations.inputTypePattern??>pattern="${attribute.annotations.inputTypePattern}" @@ -106,7 +106,7 @@ <#macro textareaTag attribute>