Skip to content

Commit

Permalink
Handling of changed profile information during sign-up when review pr…
Browse files Browse the repository at this point in the history 
…ofile is set to "missing" or T&C is requested
  • Loading branch information
cgeorgilakis-grnet committed Dec 19, 2024
1 parent c1ddf8e commit 5b286dc
Show file tree
Hide file tree
Showing 14 changed files with 239 additions and 201 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ Full Keycloak upstream jira issue can be shown if filtered by Fix version.

Our Keycloak version is working well with PostgreSQL database. For using other SQL databases, text field in database need to be evaluated.

## [Unreleased]
### Fixed
- Handling of changed profile information during sign-up when review profile is set to "missing" or T&C is requested

## [22.0.11-1.11] - 2024-11-19

### Added
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/keycloak/representations/idm/IdentityProviderRepresentation.java
View file
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

<quarkus.version>3.2.12.Final</quarkus.version>
<quarkus.build.version>3.2.12.Final</quarkus.build.version>
<eosc-kc.version>${project.version}-1.11</eosc-kc.version>
<eosc-kc.version>${project.version}-1.12rc1</eosc-kc.version>

<project.build-time>${timestamp}</project.build-time>

Expand Down
2 changes: 1 addition & 1 deletion server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java
View file
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
17 changes: 9 additions & 8 deletions ...n/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java
View file
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
75 changes: 54 additions & 21 deletions ...java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
View file
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,19 +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;
if (authenticatorConfig == null || !authenticatorConfig.getConfig().containsKey(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN)) {
updateProfileFirstLogin = IdentityProviderRepresentation.UPFLM_MISSING;
} else {
updateProfileFirstLogin = authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN);
}

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 @@ -120,17 +107,31 @@ protected boolean requiresUpdateProfilePage( AuthenticatorConfigModel authentica
}
}

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 {
return authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN);
}
}

@Override
protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredIdentityContext userCtx, BrokeredIdentityContext brokerContext) {
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 @@ -190,6 +191,38 @@ public String getServiceAccountClientLink() {
attributes.remove(TERMS_FIELD);
attributes.put(TermsAndConditions.USER_ATTRIBUTE, Arrays.asList(Integer.toString(Time.currentTime())));
}
//for security reason keep old values equal to broken data
if (IdentityProviderRepresentation.UPFLM_MISSING_ONLY.equals(updateProfileFirstLogin)) {
if (userCtx.getUsername() != null) {
attributes.put("username", Stream.of(userCtx.getUsername()).toList());
}
if (userCtx.getEmail() != null) {
attributes.put("email", Stream.of(userCtx.getEmail()).toList());
}
if (userCtx.getFirstName() != null) {
attributes.put("firstName", Stream.of(userCtx.getFirstName()).toList());
}
if (userCtx.getLastName() != null) {
attributes.put("lastName", Stream.of(userCtx.getLastName()).toList());
}
} else if (IdentityProviderRepresentation.UPFLM_OFF.equals(updateProfileFirstLogin)) {
attributes.put("username", Stream.of(userCtx.getUsername()).toList());
if (userCtx.getEmail() != null) {
attributes.put("email", Stream.of(userCtx.getEmail()).toList());
} else {
attributes.remove("email");
}
if (userCtx.getFirstName() != null) {
attributes.put("firstName", Stream.of(userCtx.getFirstName()).toList());
} else {
attributes.remove("firstName");
}
if (userCtx.getLastName() != null) {
attributes.put("lastName", Stream.of(userCtx.getLastName()).toList());
} else {
attributes.remove("lastName");
}
}
UserProfile profile = profileProvider.create(UserProfileContext.IDP_REVIEW, attributes, updatedProfile, formData.containsKey(TERMS_FIELD));

try {
Expand All @@ -207,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
2 changes: 1 addition & 1 deletion ...g/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java
View file
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
2 changes: 1 addition & 1 deletion ...uillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public String getDepartment() {
}

public boolean isDepartmentEnabled() {
return departmentInput.getAttribute("readOnly") == null;
return departmentInput.isEnabled();
}

public boolean isCurrent() {
Expand Down
4 changes: 2 additions & 2 deletions ...n-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/VerifyProfilePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public String getDepartment() {
}

public boolean isDepartmentEnabled() {
return departmentInput.getAttribute("readOnly") == null;
return departmentInput.isEnabled();
}

public boolean isUsernamePresent() {
Expand All @@ -142,7 +142,7 @@ public boolean isEmailPresent() {

public boolean isUsernameEnabled() {
try {
return driver.findElement(By.id("username")).getAttribute("readOnly") == null;
return driver.findElement(By.id("username")).isEnabled();
} catch (NoSuchElementException nse) {
return false;
}
Expand Down
Loading

0 comments on commit 5b286dc

Please sign in to comment.