Skip to content

Commit

Permalink
missing-only in update user profile
Browse files Browse the repository at this point in the history
  • Loading branch information
cgeorgilakis-grnet committed Dec 18, 2024
1 parent c24dfc0 commit 22943c5
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,21 @@ 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";

/**
* Mode of profile update after first login when user is created over this identity provider. Possible values:
* <ul>
* <li><code>on</code> - update profile page is presented for all users
* <li><code>missing</code> - update profile page is presented for users with missing some of mandatory user profile fields
* <li><code>missing-only</code> - update profile page is presented for users with missing some of mandatory user profile fields. Only missing fields can be changed.
* <li><code>off</code> - update profile page is newer shown after first login
* </ul>
*
* @see #UPFLM_ON
* @see #UPFLM_MISSING
* @see #UPFLM_MISSING_ONLY
* @see #UPFLM_OFF
*/
@Deprecated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,15 @@ protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredI
MultivaluedMap<String, String> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 {
Expand All @@ -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<String, String> 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();
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> updateProfileValues = Arrays.asList(IdentityProviderRepresentation.UPFLM_ON, IdentityProviderRepresentation.UPFLM_MISSING, IdentityProviderRepresentation.UPFLM_OFF);
List<String> 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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void testErrorExistingUserWithUpdateProfile() {

waitForPage(driver, "account already exists", false);
assertTrue(idpConfirmLinkPage.isCurrent());
assertEquals("User with email [email protected] 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());
}


Expand Down Expand Up @@ -870,8 +870,6 @@ public void testVerifyEmailRequiredActionWhenChangingEmailDuringFirstLogin() {

realm.update(realmRep);

updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin);

IdentityProviderRepresentation idpRep = identityProviderResource.toRepresentation();

idpRep.setTrustEmail(true);
Expand Down Expand Up @@ -1090,8 +1088,6 @@ public void testEventsOnUpdateProfileNoEmailChange() {

@Test
public void testEventsOnUpdateProfileWithEmailChange() {
updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin);

createUser(bc.providerRealmName(), "no-first-name", "password", null, "LastName", "[email protected]");

oauth.clientId("broker-app");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]");
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -150,17 +146,9 @@ public void addTestRealms(List<RealmRepresentation> 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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<#elseif section = "form">
<form id="kc-register-form" action="${url.loginAction}" method="post">
<div class="${properties.kcFormGroupClass!}">
<button type="submit" class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" name="submitAction" id="updateProfile" value="updateProfile">${msg("confirmLinkIdpReviewProfile")}</button>
<button type="submit" class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" name="submitAction" id="linkAccount" value="linkAccount">${msg("confirmLinkIdpContinue", idpDisplayName)}</button>
</div>
</form>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</div>
<div class="${properties.kcInputWrapperClass!}">
<input type="text" id="username" name="username" value="${(user.username!'')}"
class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled</#if>
class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.username?has_content)>disabled</#if>
aria-invalid="<#if messagesPerField.existsError('username')>true</#if>"
/>

Expand All @@ -31,7 +31,7 @@
</div>
<div class="${properties.kcInputWrapperClass!}">
<input type="text" id="email" name="email" value="${(user.email!'')}"
class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled</#if>
class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.email?has_content) >disabled</#if>
aria-invalid="<#if messagesPerField.existsError('email')>true</#if>"
/>

Expand All @@ -50,7 +50,7 @@
</div>
<div class="${properties.kcInputWrapperClass!}">
<input type="text" id="firstName" name="firstName" value="${(user.firstName!'')}"
class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled</#if>
class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.firstName?has_content)>disabled</#if>
aria-invalid="<#if messagesPerField.existsError('firstName')>true</#if>"
/>

Expand All @@ -68,7 +68,7 @@
</div>
<div class="${properties.kcInputWrapperClass!}">
<input type="text" id="lastName" name="lastName" value="${(user.lastName!'')}"
class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled</#if>
class="${properties.kcInputClass!}" <#if updateProfileFirstLogin == 'off'|| ( updateProfileFirstLogin == 'missing-only' && user.lastName?has_content)>disabled</#if>
aria-invalid="<#if messagesPerField.existsError('lastName')>true</#if>"
/>

Expand Down
Loading

0 comments on commit 22943c5

Please sign in to comment.