From 431f137c37b8877054551a59d29ec2a43a1e49da Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 12 Feb 2021 21:54:15 -0300 Subject: [PATCH] [KEYCLOAK-17123] - Avoid validation and updates for read-only attributes during updates --- .../userprofile/UserProfileAttributes.java | 14 ++- .../userprofile/UserProfileContext.java | 2 + .../userprofile/UserProfileProvider.java | 2 + .../UserProfileValidationResult.java | 16 ++- .../broker/IdpReviewProfileAuthenticator.java | 21 ++-- .../forms/RegistrationProfile.java | 14 +-- .../forms/RegistrationUserCreation.java | 19 ++- .../requiredactions/UpdateProfile.java | 19 ++- .../resources/account/AccountFormService.java | 17 ++- .../resources/account/AccountRestService.java | 13 +-- .../resources/admin/UserResource.java | 13 +-- .../LegacyUserProfileProvider.java | 46 ++++---- .../profile/AbstractUserProfile.java | 25 +++- .../profile/DefaultUserProfileContext.java | 51 +++----- .../profile/UserProfileContextFactory.java | 109 ++++++++++++++++++ .../AccountUserRepresentationUserProfile.java | 5 +- .../representations/AttributeUserProfile.java | 7 +- .../representations/IdpUserProfile.java | 5 +- .../representations/UserModelUserProfile.java | 5 +- .../UserRepresentationUserProfile.java | 7 +- .../userprofile/utils/UserUpdateHelper.java | 21 +++- .../validation/StaticValidators.java | 6 +- .../validation/ValidationChainTest.java | 27 ++++- ...ountRestServiceReadOnlyAttributesTest.java | 7 +- .../ldap/LDAPAccountRestApiTest.java | 42 ++++++- .../federation/ldap/LDAPAdminRestApiTest.java | 4 +- 26 files changed, 358 insertions(+), 159 deletions(-) create mode 100644 services/src/main/java/org/keycloak/userprofile/profile/UserProfileContextFactory.java diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileAttributes.java index f5f5b765a7..26dbfd893a 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileAttributes.java @@ -24,10 +24,18 @@ import java.util.List; public class UserProfileAttributes extends HashMap> { - public UserProfileAttributes(Map> attribtues){ + private final UserProfileProvider profileProvider; + + public UserProfileAttributes(Map> attribtues, + UserProfileProvider profileProvider){ + this.profileProvider = profileProvider; this.putAll(attribtues); } + public UserProfileAttributes(Map> attribtues){ + this(attribtues, null); + } + public void setAttribute(String key, List value){ this.put(key,value); } @@ -47,4 +55,8 @@ public class UserProfileAttributes extends HashMap> { public void removeAttribute(String attr) { this.remove(attr); } + + public boolean isReadOnlyAttribute(String key) { + return profileProvider != null && profileProvider.isReadOnlyAttribute(key); + } } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileContext.java b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileContext.java index e6c7e88bd0..c3d67fd3a5 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileContext.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileContext.java @@ -17,6 +17,7 @@ package org.keycloak.userprofile; +import org.keycloak.userprofile.validation.UserProfileValidationResult; import org.keycloak.userprofile.validation.UserUpdateEvent; /** @@ -26,4 +27,5 @@ public interface UserProfileContext { UserUpdateEvent getUpdateEvent(); UserProfile getCurrentProfile(); + UserProfileValidationResult validate(); } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileProvider.java b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileProvider.java index d5e95eb2e6..74c12d469b 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/UserProfileProvider.java @@ -26,4 +26,6 @@ import org.keycloak.userprofile.validation.UserProfileValidationResult; public interface UserProfileProvider extends Provider { UserProfileValidationResult validate(UserProfileContext updateContext, UserProfile updatedProfile); + + boolean isReadOnlyAttribute(String key); } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/validation/UserProfileValidationResult.java b/server-spi-private/src/main/java/org/keycloak/userprofile/validation/UserProfileValidationResult.java index d3fcb69655..9cec63869b 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/validation/UserProfileValidationResult.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/validation/UserProfileValidationResult.java @@ -21,6 +21,8 @@ import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; +import org.keycloak.userprofile.UserProfile; + /** * @author Markus Till */ @@ -28,9 +30,12 @@ public class UserProfileValidationResult { List attributeValidationResults; + private final UserProfile updatedProfile; - public UserProfileValidationResult(List attributeValidationResults) { + public UserProfileValidationResult(List attributeValidationResults, + UserProfile updatedProfile) { this.attributeValidationResults = attributeValidationResults; + this.updatedProfile = updatedProfile; } public List getValidationResults() { @@ -50,4 +55,13 @@ public class UserProfileValidationResult { public boolean hasAttributeChanged(String attribute) { return this.attributeValidationResults.stream().filter(o -> o.getField().equals(attribute)).collect(Collectors.toList()).get(0).hasChanged(); } + + /** + * Returns the {@link UserProfile} used during validations. + * + * @return the profile user during validations + */ + public UserProfile getProfile() { + return updatedProfile; + } } 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 571b6c7355..492fcf035c 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 @@ -17,6 +17,8 @@ package org.keycloak.authentication.authenticators.broker; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forIdpReview; + import org.jboss.logging.Logger; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; @@ -33,12 +35,8 @@ import org.keycloak.models.UserModel; import org.keycloak.models.utils.FormMessage; import org.keycloak.models.utils.UserModelDelegate; import org.keycloak.representations.idm.IdentityProviderRepresentation; -import org.keycloak.services.resources.AttributeFormDataProcessor; import org.keycloak.services.validation.Validation; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; -import org.keycloak.userprofile.UserProfileProvider; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; -import org.keycloak.userprofile.profile.representations.AttributeUserProfile; +import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; import org.keycloak.userprofile.validation.UserProfileValidationResult; @@ -104,14 +102,8 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator { EventBuilder event = context.getEvent(); event.event(EventType.UPDATE_PROFILE); MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); + UserProfileValidationResult result = forIdpReview(userCtx, formData, context.getSession()).validate(); - UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - AttributeUserProfile updatedProfile = AttributeFormDataProcessor.toUserProfile(formData); - - String oldEmail = userCtx.getEmail(); - String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); - - UserProfileValidationResult result = profileProvider.validate(DefaultUserProfileContext.forIdpReview(userCtx), updatedProfile); List errors = Validation.getFormErrorsFromValidation(result); if (errors != null && !errors.isEmpty()) { @@ -124,6 +116,8 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator { return; } + UserProfile updatedProfile = result.getProfile(); + UserUpdateHelper.updateIdpReview(context.getRealm(), new UserModelDelegate(null) { @Override public Map> getAttributes() { @@ -150,6 +144,9 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator { logger.debugf("Profile updated successfully after first authentication with identity provider '%s' for broker user '%s'.", brokerContext.getIdpConfig().getAlias(), userCtx.getUsername()); + String oldEmail = userCtx.getEmail(); + String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + if (result.hasAttributeChanged(UserModel.EMAIL)) { context.getAuthenticationSession().setAuthNote(UPDATE_PROFILE_EMAIL_CHANGED, "true"); event.clone().event(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail).success(); diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java index c474b17463..aa4ea6416f 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java @@ -17,6 +17,8 @@ package org.keycloak.authentication.forms; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forRegistrationProfile; + import org.keycloak.Config; import org.keycloak.authentication.FormAction; import org.keycloak.authentication.FormActionFactory; @@ -34,12 +36,9 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.services.messages.Messages; import org.keycloak.services.resources.AttributeFormDataProcessor; import org.keycloak.services.validation.Validation; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; import org.keycloak.userprofile.UserProfile; -import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.profile.representations.AttributeUserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; import org.keycloak.userprofile.validation.UserProfileValidationResult; import javax.ws.rs.core.MultivaluedMap; @@ -65,18 +64,17 @@ public class RegistrationProfile implements FormAction, FormActionFactory { @Override public void validate(org.keycloak.authentication.ValidationContext context) { MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); - UserProfile updatedProfile = AttributeFormDataProcessor.toUserProfile(formData); - - UserProfileProvider userProfile = context.getSession().getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); context.getEvent().detail(Details.REGISTER_METHOD, "form"); - UserProfileValidationResult result = userProfile.validate(DefaultUserProfileContext.forRegistrationProfile(), updatedProfile); + UserProfileValidationResult result = forRegistrationProfile(context.getSession(), formData).validate(); List errors = Validation.getFormErrorsFromValidation(result); if (errors.size() > 0) { - if (result.hasFailureOfErrorType(Messages.EMAIL_EXISTS, Messages.INVALID_EMAIL)) + if (result.hasFailureOfErrorType(Messages.EMAIL_EXISTS, Messages.INVALID_EMAIL)) { + UserProfile updatedProfile = result.getProfile(); context.getEvent().detail(Details.EMAIL, updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL)); + } if (result.hasFailureOfErrorType(Messages.EMAIL_EXISTS)) { context.error(Errors.EMAIL_IN_USE); diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java index c60d71b3ea..9e0576a48a 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java @@ -17,6 +17,8 @@ package org.keycloak.authentication.forms; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forRegistrationUserCreation; + import org.keycloak.Config; import org.keycloak.authentication.FormAction; import org.keycloak.authentication.FormActionFactory; @@ -37,12 +39,9 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.services.messages.Messages; import org.keycloak.services.resources.AttributeFormDataProcessor; import org.keycloak.services.validation.Validation; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; import org.keycloak.userprofile.UserProfile; -import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.profile.representations.AttributeUserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; import org.keycloak.userprofile.validation.UserProfileValidationResult; import javax.ws.rs.core.MultivaluedMap; @@ -71,17 +70,17 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); context.getEvent().detail(Details.REGISTER_METHOD, "form"); - UserProfile newProfile = AttributeFormDataProcessor.toUserProfile(context.getHttpRequest().getDecodedFormParameters()); + + UserProfileValidationResult result = forRegistrationUserCreation(context.getSession(), formData).validate(); + UserProfile newProfile = result.getProfile(); String email = newProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); - String username = newProfile.getAttributes().getFirstAttribute(UserModel.USERNAME); + context.getEvent().detail(Details.EMAIL, email); + + String username = newProfile.getAttributes().getFirstAttribute(UserModel.USERNAME); + context.getEvent().detail(Details.USERNAME, username); - UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - - context.getEvent().detail(Details.REGISTER_METHOD, "form"); - UserProfileValidationResult result = profileProvider.validate(DefaultUserProfileContext.forRegistrationUserCreation(), newProfile); - List errors = Validation.getFormErrorsFromValidation(result); if (context.getRealm().isRegistrationEmailAsUsername()) { context.getEvent().detail(Details.USERNAME, email); diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java index 01c1887106..19322a59da 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java @@ -17,6 +17,8 @@ package org.keycloak.authentication.requiredactions; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forUpdateProfile; + import org.keycloak.Config; import org.keycloak.OAuth2Constants; import org.keycloak.authentication.DisplayTypeRequiredActionFactory; @@ -31,13 +33,9 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.UserModel; import org.keycloak.models.utils.FormMessage; -import org.keycloak.services.resources.AttributeFormDataProcessor; import org.keycloak.services.validation.Validation; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; -import org.keycloak.userprofile.UserProfileProvider; -import org.keycloak.userprofile.profile.representations.AttributeUserProfile; +import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; import org.keycloak.userprofile.validation.UserProfileValidationResult; import javax.ws.rs.core.MultivaluedMap; @@ -71,14 +69,9 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact event.event(EventType.UPDATE_PROFILE); MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); UserModel user = context.getUser(); - - AttributeUserProfile updatedProfile = AttributeFormDataProcessor.toUserProfile(formData); - String oldEmail = user.getEmail(); - String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); - - UserProfileProvider userProfile = context.getSession().getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - UserProfileValidationResult result = userProfile.validate(DefaultUserProfileContext.forUpdateProfile(user),updatedProfile); + UserProfileValidationResult result = forUpdateProfile(user, formData, context.getSession()).validate(); + final UserProfile updatedProfile = result.getProfile(); List errors = Validation.getFormErrorsFromValidation(result); if (errors != null && !errors.isEmpty()) { @@ -90,6 +83,8 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact return; } + String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + UserUpdateHelper.updateUserProfile(context.getRealm(), user, updatedProfile); if (result.hasAttributeChanged(UserModel.EMAIL)) { user.setEmailVerified(false); diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java index 06a6c58de9..f511fc4e32 100755 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java @@ -16,6 +16,8 @@ */ package org.keycloak.services.resources.account; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forOldAccount; + import org.jboss.logging.Logger; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.PermissionTicket; @@ -66,17 +68,13 @@ import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.messages.Messages; import org.keycloak.services.resources.AbstractSecuredLocalService; -import org.keycloak.services.resources.AttributeFormDataProcessor; import org.keycloak.services.resources.RealmsResource; import org.keycloak.services.util.ResolveRelative; import org.keycloak.services.validation.Validation; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.storage.ReadOnlyException; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; -import org.keycloak.userprofile.UserProfileProvider; -import org.keycloak.userprofile.profile.representations.AttributeUserProfile; +import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; import org.keycloak.userprofile.validation.UserProfileValidationResult; import org.keycloak.util.JsonSerialization; import org.keycloak.utils.CredentialHelper; @@ -365,15 +363,11 @@ public class AccountFormService extends AbstractSecuredLocalService { csrfCheck(formData); UserModel user = auth.getUser(); - AttributeUserProfile updatedProfile = AttributeFormDataProcessor.toUserProfile(formData); String oldEmail = user.getEmail(); - String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); event.event(EventType.UPDATE_PROFILE).client(auth.getClient()).user(auth.getUser()); - UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - - UserProfileValidationResult result = profileProvider.validate(DefaultUserProfileContext.forAccountService(user), updatedProfile); + UserProfileValidationResult result = forOldAccount(user, formData, session).validate(); List errors = Validation.getFormErrorsFromValidation(result); if (!errors.isEmpty()) { @@ -389,6 +383,9 @@ public class AccountFormService extends AbstractSecuredLocalService { return account.setErrors(status, errors).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT); } + UserProfile updatedProfile = result.getProfile(); + String newEmail = updatedProfile.getAttributes().getFirstAttribute(UserModel.EMAIL); + try { // backward compatibility with old account console where attributes are not removed if missing UserUpdateHelper.updateAccountOldConsole(realm, user, updatedProfile); diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java index 6eef412bdf..e2573ea418 100755 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java @@ -16,6 +16,8 @@ */ package org.keycloak.services.resources.account; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forAccountService; + import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.spi.HttpRequest; import org.keycloak.common.ClientConnection; @@ -45,11 +47,6 @@ import org.keycloak.services.resources.account.resources.ResourcesService; import org.keycloak.services.util.ResolveRelative; import org.keycloak.storage.ReadOnlyException; import org.keycloak.theme.Theme; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; -import org.keycloak.userprofile.UserProfile; -import org.keycloak.userprofile.UserProfileProvider; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; -import org.keycloak.userprofile.profile.representations.AccountUserRepresentationUserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; import org.keycloak.userprofile.validation.UserProfileValidationResult; @@ -160,9 +157,7 @@ public class AccountRestService { event.event(EventType.UPDATE_PROFILE).client(auth.getClient()).user(auth.getUser()); - UserProfile updatedUser = new AccountUserRepresentationUserProfile(rep); - UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - UserProfileValidationResult result = profileProvider.validate(DefaultUserProfileContext.forAccountService(user), updatedUser); + UserProfileValidationResult result = forAccountService(user, rep, session).validate(); if (result.hasFailureOfErrorType(Messages.READ_ONLY_USERNAME)) return ErrorResponse.error(Messages.READ_ONLY_USERNAME, Response.Status.BAD_REQUEST); @@ -177,7 +172,7 @@ public class AccountRestService { } try { - UserUpdateHelper.updateAccount(realm, user, updatedUser); + UserUpdateHelper.updateAccount(realm, user, result.getProfile()); event.success(); return Response.noContent().build(); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 05356a32b7..1838f674a0 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -71,13 +71,7 @@ import org.keycloak.services.resources.account.AccountFormService; import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator; import org.keycloak.services.validation.Validation; import org.keycloak.storage.ReadOnlyException; -import org.keycloak.userprofile.LegacyUserProfileProviderFactory; -import org.keycloak.userprofile.UserProfile; -import org.keycloak.userprofile.UserProfileProvider; -import org.keycloak.userprofile.profile.DefaultUserProfileContext; -import org.keycloak.userprofile.profile.representations.AccountUserRepresentationUserProfile; import org.keycloak.userprofile.utils.UserUpdateHelper; -import org.keycloak.userprofile.profile.representations.UserRepresentationUserProfile; import org.keycloak.userprofile.validation.AttributeValidationResult; import org.keycloak.userprofile.validation.UserProfileValidationResult; import org.keycloak.userprofile.validation.ValidationResult; @@ -119,6 +113,7 @@ import java.util.stream.Stream; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_ID; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_USERNAME; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forUserResource; /** * Base resource for managing users @@ -201,9 +196,7 @@ public class UserResource { } public static Response validateUserProfile(UserModel user, UserRepresentation rep, KeycloakSession session) { - UserProfile updatedUser = new UserRepresentationUserProfile(rep); - UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); - UserProfileValidationResult result = profileProvider.validate(DefaultUserProfileContext.forUserResource(user), updatedUser); + UserProfileValidationResult result = forUserResource(user, rep, session).validate(); if (!result.getErrors().isEmpty()) { for (AttributeValidationResult attrValidation : result.getErrors()) { StringBuilder s = new StringBuilder("Failed to update attribute " + attrValidation.getField() + ": "); @@ -220,7 +213,7 @@ public class UserResource { public static void updateUserFromRep(UserModel user, UserRepresentation rep, KeycloakSession session, boolean isUpdateExistingUser) { boolean removeMissingRequiredActions = isUpdateExistingUser; - UserUpdateHelper.updateUserResource(session.getContext().getRealm(), user, new UserRepresentationUserProfile(rep), isUpdateExistingUser); + UserUpdateHelper.updateUserResource(session, user, rep, isUpdateExistingUser); if (rep.isEnabled() != null) user.setEnabled(rep.isEnabled()); if (rep.isEmailVerified() != null) user.setEmailVerified(rep.isEmailVerified()); diff --git a/services/src/main/java/org/keycloak/userprofile/LegacyUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/LegacyUserProfileProvider.java index 562cbe05cf..d6f7ac45c5 100644 --- a/services/src/main/java/org/keycloak/userprofile/LegacyUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/LegacyUserProfileProvider.java @@ -1,28 +1,26 @@ /* - * Copyright 2020 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at + * * Copyright 2021 Red Hat, Inc. and/or its affiliates + * * and other contributors as indicated by the @author tags. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. */ package org.keycloak.userprofile; -import java.util.Collection; -import java.util.List; import java.util.regex.Pattern; -import java.util.stream.Collectors; -import org.jboss.logging.Logger; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -36,7 +34,6 @@ import org.keycloak.userprofile.validation.ValidationChainBuilder; */ public class LegacyUserProfileProvider implements UserProfileProvider { - private static final Logger logger = Logger.getLogger(LegacyUserProfileProvider.class); private final KeycloakSession session; private final Pattern readOnlyAttributes; private final Pattern adminReadOnlyAttributes; @@ -77,7 +74,12 @@ public class LegacyUserProfileProvider implements UserProfileProvider { addReadOnlyAttributeValidators(builder, readOnlyAttributes, updateContext, updatedProfile); break; } - return new UserProfileValidationResult(builder.build().validate(updateContext,updatedProfile)); + return new UserProfileValidationResult(builder.build().validate(updateContext,updatedProfile), updatedProfile); + } + + @Override + public boolean isReadOnlyAttribute(String key) { + return readOnlyAttributes.matcher(key).find() || adminReadOnlyAttributes.matcher(key).find(); } private void addUserCreationValidators(ValidationChainBuilder builder) { @@ -130,12 +132,12 @@ public class LegacyUserProfileProvider implements UserProfileProvider { } private void addReadOnlyAttributeValidators(ValidationChainBuilder builder, Pattern configuredReadOnlyAttrs, UserProfileContext updateContext, UserProfile updatedProfile) { - addValidatorsForAllAttributeOfUser(builder, configuredReadOnlyAttrs, updatedProfile); - addValidatorsForAllAttributeOfUser(builder, configuredReadOnlyAttrs, updateContext.getCurrentProfile()); + addValidatorsForReadOnlyAttributes(builder, configuredReadOnlyAttrs, updatedProfile); + addValidatorsForReadOnlyAttributes(builder, configuredReadOnlyAttrs, updateContext.getCurrentProfile()); } - private void addValidatorsForAllAttributeOfUser(ValidationChainBuilder builder, Pattern configuredReadOnlyAttrsPattern, UserProfile profile) { + private void addValidatorsForReadOnlyAttributes(ValidationChainBuilder builder, Pattern configuredReadOnlyAttrsPattern, UserProfile profile) { if (profile == null) { return; } @@ -144,7 +146,7 @@ public class LegacyUserProfileProvider implements UserProfileProvider { .filter(currentAttrName -> configuredReadOnlyAttrsPattern.matcher(currentAttrName).find()) .forEach((currentAttrName) -> builder.addAttributeValidator().forAttribute(currentAttrName) - .addValidationFunction(Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED, StaticValidators.isAttributeUnchanged(currentAttrName)).build() + .addValidationFunction(Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED, StaticValidators.isReadOnlyAttributeUnchanged(currentAttrName)).build() ); } } \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/userprofile/profile/AbstractUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/AbstractUserProfile.java index 7b7532a2cc..91f751afda 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/AbstractUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/AbstractUserProfile.java @@ -1,7 +1,25 @@ +/* + * Copyright 2020 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.keycloak.userprofile.profile; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileAttributes; +import org.keycloak.userprofile.UserProfileProvider; import java.util.List; import java.util.Map; @@ -11,15 +29,12 @@ public abstract class AbstractUserProfile implements UserProfile { private final UserProfileAttributes attributes; - public AbstractUserProfile(Map> attributes) { - this.attributes = new UserProfileAttributes(attributes); + public AbstractUserProfile(Map> attributes, UserProfileProvider profileProvider) { + this.attributes = new UserProfileAttributes(attributes, profileProvider); } @Override public UserProfileAttributes getAttributes() { return this.attributes; } - - - } diff --git a/services/src/main/java/org/keycloak/userprofile/profile/DefaultUserProfileContext.java b/services/src/main/java/org/keycloak/userprofile/profile/DefaultUserProfileContext.java index e0ae0f7bce..4e29c048bd 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/DefaultUserProfileContext.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/DefaultUserProfileContext.java @@ -17,15 +17,10 @@ package org.keycloak.userprofile.profile; -import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; -import org.keycloak.models.UserModel; -import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.userprofile.UserProfile; -import org.keycloak.userprofile.UserProfileAttributes; import org.keycloak.userprofile.UserProfileContext; -import org.keycloak.userprofile.profile.representations.IdpUserProfile; -import org.keycloak.userprofile.profile.representations.UserModelUserProfile; -import org.keycloak.userprofile.profile.representations.UserRepresentationUserProfile; +import org.keycloak.userprofile.UserProfileProvider; +import org.keycloak.userprofile.validation.UserProfileValidationResult; import org.keycloak.userprofile.validation.UserUpdateEvent; /** @@ -33,40 +28,17 @@ import org.keycloak.userprofile.validation.UserUpdateEvent; */ public class DefaultUserProfileContext implements UserProfileContext { private UserProfile currentUserProfile; + private final UserProfile updatedProfile; + private final UserProfileProvider profileProvider; private UserUpdateEvent userUpdateEvent; - private DefaultUserProfileContext(UserUpdateEvent userUpdateEvent, UserProfile currentUserProfile) { + DefaultUserProfileContext(UserUpdateEvent userUpdateEvent, UserProfile currentUserProfile, + UserProfile updatedProfile, + UserProfileProvider profileProvider) { this.userUpdateEvent = userUpdateEvent; this.currentUserProfile = currentUserProfile; - } - - public static DefaultUserProfileContext forIdpReview(SerializedBrokeredIdentityContext currentUser) { - return new DefaultUserProfileContext(UserUpdateEvent.IdpReview, new IdpUserProfile(currentUser)); - } - - public static DefaultUserProfileContext forUpdateProfile(UserModel currentUser) { - return new DefaultUserProfileContext(UserUpdateEvent.UpdateProfile, new UserModelUserProfile(currentUser)); - } - - public static DefaultUserProfileContext forAccountService(UserModel currentUser) { - return new DefaultUserProfileContext(UserUpdateEvent.Account, new UserModelUserProfile(currentUser)); - } - - public static DefaultUserProfileContext forRegistrationUserCreation() { - return new DefaultUserProfileContext(UserUpdateEvent.RegistrationUserCreation, null); - } - - public static DefaultUserProfileContext forRegistrationProfile() { - return new DefaultUserProfileContext(UserUpdateEvent.RegistrationProfile, null); - } - - /** - * @param currentUser if this is null, then we're creating new user. If it is not null, we're updating existing user - * @return user profile context for the validation of user when called from admin REST API - */ - public static DefaultUserProfileContext forUserResource(UserModel currentUser) { - UserProfile currentUserProfile = currentUser == null ? null : new UserModelUserProfile(currentUser); - return new DefaultUserProfileContext(UserUpdateEvent.UserResource, currentUserProfile); + this.updatedProfile = updatedProfile; + this.profileProvider = profileProvider; } @Override @@ -78,4 +50,9 @@ public class DefaultUserProfileContext implements UserProfileContext { public UserUpdateEvent getUpdateEvent(){ return userUpdateEvent; } + + @Override + public UserProfileValidationResult validate() { + return profileProvider.validate(this, updatedProfile); + } } diff --git a/services/src/main/java/org/keycloak/userprofile/profile/UserProfileContextFactory.java b/services/src/main/java/org/keycloak/userprofile/profile/UserProfileContextFactory.java new file mode 100644 index 0000000000..699473e8f9 --- /dev/null +++ b/services/src/main/java/org/keycloak/userprofile/profile/UserProfileContextFactory.java @@ -0,0 +1,109 @@ +/* + * Copyright 2021 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.userprofile.profile; + +import javax.ws.rs.core.MultivaluedMap; + +import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.UserModel; +import org.keycloak.representations.account.UserRepresentation; +import org.keycloak.services.resources.AttributeFormDataProcessor; +import org.keycloak.userprofile.LegacyUserProfileProviderFactory; +import org.keycloak.userprofile.UserProfile; +import org.keycloak.userprofile.UserProfileProvider; +import org.keycloak.userprofile.profile.representations.AccountUserRepresentationUserProfile; +import org.keycloak.userprofile.profile.representations.IdpUserProfile; +import org.keycloak.userprofile.profile.representations.UserModelUserProfile; +import org.keycloak.userprofile.profile.representations.UserRepresentationUserProfile; +import org.keycloak.userprofile.validation.UserUpdateEvent; + +/** + * @author Pedro Igor + */ +public final class UserProfileContextFactory { + + public static DefaultUserProfileContext forIdpReview(SerializedBrokeredIdentityContext currentUser, + MultivaluedMap formData, KeycloakSession session) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.IdpReview, new IdpUserProfile(currentUser, profileProvider), + AttributeFormDataProcessor.toUserProfile(formData), profileProvider); + } + + public static DefaultUserProfileContext forUpdateProfile(UserModel currentUser, + MultivaluedMap formData, + KeycloakSession session) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.UpdateProfile, new UserModelUserProfile(currentUser, profileProvider), + AttributeFormDataProcessor.toUserProfile(formData), profileProvider); + } + + public static DefaultUserProfileContext forAccountService(UserModel currentUser, + UserRepresentation rep, KeycloakSession session) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.Account, new UserModelUserProfile(currentUser, profileProvider), + new AccountUserRepresentationUserProfile(rep, profileProvider), + profileProvider); + } + + public static DefaultUserProfileContext forOldAccount(UserModel currentUser, + MultivaluedMap formData, KeycloakSession session) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.Account, new UserModelUserProfile(currentUser, profileProvider), + AttributeFormDataProcessor.toUserProfile(formData), + profileProvider); + } + + public static DefaultUserProfileContext forRegistrationUserCreation( + KeycloakSession session, MultivaluedMap formData) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.RegistrationUserCreation, null, + AttributeFormDataProcessor.toUserProfile(formData), profileProvider); + } + + public static DefaultUserProfileContext forRegistrationProfile(KeycloakSession session, + MultivaluedMap formData) { + UserProfileProvider profileProvider = getProfileProvider(session); + return new DefaultUserProfileContext(UserUpdateEvent.RegistrationProfile, null, + AttributeFormDataProcessor.toUserProfile(formData), profileProvider); + } + + /** + * @param currentUser if this is null, then we're creating new user. If it is not null, we're updating existing user + * @param rep + * @return user profile context for the validation of user when called from admin REST API + */ + public static DefaultUserProfileContext forUserResource(UserModel currentUser, + org.keycloak.representations.idm.UserRepresentation rep, KeycloakSession session) { + UserProfileProvider profileProvider = getProfileProvider(session); + UserProfile currentUserProfile = currentUser == null ? null : new UserModelUserProfile(currentUser, profileProvider); + return new DefaultUserProfileContext(UserUpdateEvent.UserResource, currentUserProfile, + new UserRepresentationUserProfile(rep, profileProvider), profileProvider); + } + + public static DefaultUserProfileContext forProfile(UserUpdateEvent event) { + return new DefaultUserProfileContext(event, null, null, null); + } + + private static UserProfileProvider getProfileProvider(KeycloakSession session) { + if (session == null) { + return null; + } + return session.getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); + } +} diff --git a/services/src/main/java/org/keycloak/userprofile/profile/representations/AccountUserRepresentationUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/representations/AccountUserRepresentationUserProfile.java index 0eee9502d7..5df34fd93b 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/representations/AccountUserRepresentationUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/representations/AccountUserRepresentationUserProfile.java @@ -21,6 +21,7 @@ package org.keycloak.userprofile.profile.representations; import org.keycloak.models.UserModel; import org.keycloak.representations.account.UserRepresentation; import org.keycloak.userprofile.UserProfileAttributes; +import org.keycloak.userprofile.UserProfileProvider; import java.util.Collections; import java.util.HashMap; @@ -32,8 +33,8 @@ import java.util.Map; */ public class AccountUserRepresentationUserProfile extends AttributeUserProfile { - public AccountUserRepresentationUserProfile(UserRepresentation user) { - super(flattenUserRepresentation(user)); + public AccountUserRepresentationUserProfile(UserRepresentation user, UserProfileProvider profileProvider) { + super(flattenUserRepresentation(user), profileProvider); } private static UserProfileAttributes flattenUserRepresentation(UserRepresentation user) { diff --git a/services/src/main/java/org/keycloak/userprofile/profile/representations/AttributeUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/representations/AttributeUserProfile.java index cdda97a26a..aebfbd754f 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/representations/AttributeUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/representations/AttributeUserProfile.java @@ -17,6 +17,7 @@ package org.keycloak.userprofile.profile.representations; +import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.profile.AbstractUserProfile; import javax.ws.rs.NotSupportedException; @@ -28,8 +29,12 @@ import java.util.Map; */ public class AttributeUserProfile extends AbstractUserProfile { + public AttributeUserProfile(Map> attributes, UserProfileProvider profileProvider) { + super(attributes, profileProvider); + } + public AttributeUserProfile(Map> attributes) { - super(attributes); + super(attributes, null); } @Override diff --git a/services/src/main/java/org/keycloak/userprofile/profile/representations/IdpUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/representations/IdpUserProfile.java index 0ea3ab1297..574b8a494c 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/representations/IdpUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/representations/IdpUserProfile.java @@ -18,6 +18,7 @@ package org.keycloak.userprofile.profile.representations; import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; +import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.profile.AbstractUserProfile; @@ -28,8 +29,8 @@ public class IdpUserProfile extends AbstractUserProfile { private final SerializedBrokeredIdentityContext user; - public IdpUserProfile(SerializedBrokeredIdentityContext user) { - super(user.getAttributes()); + public IdpUserProfile(SerializedBrokeredIdentityContext user, UserProfileProvider profileProvider) { + super(user.getAttributes(), profileProvider); this.user = user; } diff --git a/services/src/main/java/org/keycloak/userprofile/profile/representations/UserModelUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/representations/UserModelUserProfile.java index 9c7539818e..67a1863003 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/representations/UserModelUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/representations/UserModelUserProfile.java @@ -18,6 +18,7 @@ package org.keycloak.userprofile.profile.representations; import org.keycloak.models.UserModel; +import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.profile.AbstractUserProfile; /** @@ -26,8 +27,8 @@ import org.keycloak.userprofile.profile.AbstractUserProfile; public class UserModelUserProfile extends AbstractUserProfile { - public UserModelUserProfile(UserModel user) { - super(user.getAttributes()); + public UserModelUserProfile(UserModel user, UserProfileProvider profileProvider) { + super(user.getAttributes(), profileProvider); this.user = user; } diff --git a/services/src/main/java/org/keycloak/userprofile/profile/representations/UserRepresentationUserProfile.java b/services/src/main/java/org/keycloak/userprofile/profile/representations/UserRepresentationUserProfile.java index 681e10a9c9..9b720dfb72 100644 --- a/services/src/main/java/org/keycloak/userprofile/profile/representations/UserRepresentationUserProfile.java +++ b/services/src/main/java/org/keycloak/userprofile/profile/representations/UserRepresentationUserProfile.java @@ -20,6 +20,7 @@ package org.keycloak.userprofile.profile.representations; import org.keycloak.models.UserModel; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.userprofile.UserProfileAttributes; +import org.keycloak.userprofile.UserProfileProvider; import java.util.Collections; import java.util.HashMap; @@ -32,8 +33,12 @@ import java.util.Map; public class UserRepresentationUserProfile extends AttributeUserProfile { + public UserRepresentationUserProfile(UserRepresentation user, UserProfileProvider profileProvider) { + super(flattenUserRepresentation(user), profileProvider); + } + public UserRepresentationUserProfile(UserRepresentation user) { - super(flattenUserRepresentation(user)); + super(flattenUserRepresentation(user), null); } private static UserProfileAttributes flattenUserRepresentation(UserRepresentation user) { diff --git a/services/src/main/java/org/keycloak/userprofile/utils/UserUpdateHelper.java b/services/src/main/java/org/keycloak/userprofile/utils/UserUpdateHelper.java index 156904c051..a58bd542f9 100644 --- a/services/src/main/java/org/keycloak/userprofile/utils/UserUpdateHelper.java +++ b/services/src/main/java/org/keycloak/userprofile/utils/UserUpdateHelper.java @@ -17,11 +17,16 @@ package org.keycloak.userprofile.utils; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.userprofile.LegacyUserProfileProviderFactory; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileAttributes; +import org.keycloak.userprofile.UserProfileProvider; +import org.keycloak.userprofile.profile.representations.UserRepresentationUserProfile; import org.keycloak.userprofile.validation.UserUpdateEvent; import java.util.Collections; @@ -71,8 +76,11 @@ public class UserUpdateHelper { update(UserUpdateEvent.Account, realm, user, updatedProfile.getAttributes(), false); } - public static void updateUserResource(RealmModel realm, UserModel user, UserProfile userRepresentationUserProfile, boolean removeExistingAttributes) { - update(UserUpdateEvent.UserResource, realm, user, userRepresentationUserProfile.getAttributes(), removeExistingAttributes); + public static void updateUserResource(KeycloakSession session, UserModel user, UserRepresentation rep, boolean removeExistingAttributes) { + UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class, LegacyUserProfileProviderFactory.PROVIDER_ID); + RealmModel realm = session.getContext().getRealm(); + UserRepresentationUserProfile userProfile = new UserRepresentationUserProfile(rep, profileProvider); + update(UserUpdateEvent.UserResource, realm, user, userProfile.getAttributes(), removeExistingAttributes); } /** @@ -128,8 +136,8 @@ public class UserUpdateHelper { } } - private static void updateAttributes(UserModel currentUser, Map> updatedUser, boolean removeMissingAttributes) { - for (Map.Entry> attr : updatedUser.entrySet()) { + private static void updateAttributes(UserModel currentUser, UserProfileAttributes attributes, boolean removeMissingAttributes) { + for (Map.Entry> attr : attributes.entrySet()) { List currentValue = currentUser.getAttributeStream(attr.getKey()).collect(Collectors.toList()); //In case of username we need to provide lower case values List updatedValue = attr.getKey().equals(UserModel.USERNAME) ? AttributeToLower(attr.getValue()) : attr.getValue(); @@ -139,9 +147,12 @@ public class UserUpdateHelper { } if (removeMissingAttributes) { Set attrsToRemove = new HashSet<>(currentUser.getAttributes().keySet()); - attrsToRemove.removeAll(updatedUser.keySet()); + attrsToRemove.removeAll(attributes.keySet()); for (String attr : attrsToRemove) { + if (attributes.isReadOnlyAttribute(attr)) { + continue; + } currentUser.removeAttribute(attr); } diff --git a/services/src/main/java/org/keycloak/userprofile/validation/StaticValidators.java b/services/src/main/java/org/keycloak/userprofile/validation/StaticValidators.java index 4b98d9c6ed..cda5386088 100644 --- a/services/src/main/java/org/keycloak/userprofile/validation/StaticValidators.java +++ b/services/src/main/java/org/keycloak/userprofile/validation/StaticValidators.java @@ -107,10 +107,14 @@ public class StaticValidators { && session.users().getUserByEmail(session.getContext().getRealm(), value) != null); } - public static BiFunction, UserProfileContext, Boolean> isAttributeUnchanged(String attributeName) { + public static BiFunction, UserProfileContext, Boolean> isReadOnlyAttributeUnchanged(String attributeName) { return (newAttrValues, context) -> { + if (newAttrValues == null) { + return true; + } List existingAttrValues = context.getCurrentProfile() == null ? null : context.getCurrentProfile().getAttributes().getAttribute(attributeName); boolean result = ObjectUtil.isEqualOrBothNull(newAttrValues, existingAttrValues); + if (!result) { logger.warnf("Attempt to edit denied attribute '%s' of user '%s'", attributeName, context.getCurrentProfile() == null ? "new user" : context.getCurrentProfile().getAttributes().getFirstAttribute(UserModel.USERNAME)); } diff --git a/services/src/test/java/org/keycloak/userprofile/validation/ValidationChainTest.java b/services/src/test/java/org/keycloak/userprofile/validation/ValidationChainTest.java index 9c5c3039c1..e70848ef06 100644 --- a/services/src/test/java/org/keycloak/userprofile/validation/ValidationChainTest.java +++ b/services/src/test/java/org/keycloak/userprofile/validation/ValidationChainTest.java @@ -1,5 +1,24 @@ +/* + * Copyright 2020 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.keycloak.userprofile.validation; +import static org.keycloak.userprofile.profile.UserProfileContextFactory.forProfile; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -35,14 +54,14 @@ public class ValidationChainTest { rep.singleAttribute("FAKE_FIELD", "content"); rep.singleAttribute("NULLABLE_FIELD", null); - updateContext = DefaultUserProfileContext.forRegistrationProfile(); + updateContext = forProfile(UserUpdateEvent.RegistrationProfile); } @Test public void validate() { testchain = builder.build(); - UserProfileValidationResult results = new UserProfileValidationResult(testchain.validate(updateContext, new UserRepresentationUserProfile(rep))); + UserProfileValidationResult results = new UserProfileValidationResult(testchain.validate(updateContext, new UserRepresentationUserProfile(rep)), null); Assert.assertEquals(true, results.hasFailureOfErrorType("FAKE_FIELD_ERRORKEY")); Assert.assertEquals(false, results.hasFailureOfErrorType("FIRST_NAME_FIELD_ERRORKEY")); Assert.assertEquals(true, results.getValidationResults().stream().filter(o -> o.getField().equals("firstName")).collect(Collectors.toList()).get(0).isValid()); @@ -57,7 +76,7 @@ public class ValidationChainTest { .addAttributeValidator().forAttribute("FAKE_FIELD") .addSingleAttributeValueValidationFunction("FAKE_FIELD_ERRORKEY_2", (value, updateUserProfileContext) -> false).build().build(); - UserProfileValidationResult results = new UserProfileValidationResult(testchain.validate(updateContext, new UserRepresentationUserProfile(rep))); + UserProfileValidationResult results = new UserProfileValidationResult(testchain.validate(updateContext, new UserRepresentationUserProfile(rep)), null); Assert.assertEquals(true, results.hasFailureOfErrorType("FAKE_FIELD_ERRORKEY_1")); Assert.assertEquals(true, results.hasFailureOfErrorType("FAKE_FIELD_ERRORKEY_2")); Assert.assertEquals(true, results.getValidationResults().stream().filter(o -> o.getField().equals("firstName")).collect(Collectors.toList()).get(0).isValid()); @@ -67,7 +86,7 @@ public class ValidationChainTest { @Test public void emptyChain() { - UserProfileValidationResult results = new UserProfileValidationResult(ValidationChainBuilder.builder().build().validate(updateContext,new UserRepresentationUserProfile(rep) )); + UserProfileValidationResult results = new UserProfileValidationResult(ValidationChainBuilder.builder().build().validate(updateContext,new UserRepresentationUserProfile(rep) ), null); Assert.assertEquals(Collections.emptyList(), results.getValidationResults()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java index 99252eeeef..4c790ccd92 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceReadOnlyAttributesTest.java @@ -36,6 +36,8 @@ import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.QUARKUS; import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE; @@ -135,9 +137,10 @@ public class AccountRestServiceReadOnlyAttributesTest extends AbstractRestServic user.singleAttribute(attrName, "foo-updated"); updateError(user, 400, Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED); - // Remove attribute from the user with account REST (Case when we are removing existing attribute) + // Ignore removal of read-only attributes user.getAttributes().remove(attrName); - updateError(user, 400, Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED); + user = updateAndGet(user); + assertTrue(user.getAttributes().containsKey(attrName)); // Revert with admin REST adminUserRep.getAttributes().remove(attrName); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java index c0d2c35091..c62a3aec67 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAccountRestApiTest.java @@ -52,6 +52,7 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author Marek Posolda @@ -142,8 +143,11 @@ public class LDAPAccountRestApiTest extends AbstractLDAPTest { user.getAttributes().get(LDAPConstants.LDAP_ID).remove(0); updateProfileExpectError(user, 400, Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED); + // ignore removal for read-only attributes user.getAttributes().remove(LDAPConstants.LDAP_ID); - updateProfileExpectError(user, 400, Messages.UPDATE_READ_ONLY_ATTRIBUTES_REJECTED); + updateProfileExpectSuccess(user); + user = getProfile(); + assertFalse(user.getAttributes().get(LDAPConstants.LDAP_ID).isEmpty()); // Trying to update LDAP_ENTRY_DN should fail user.getAttributes().put(LDAPConstants.LDAP_ID, origLdapId); @@ -208,6 +212,42 @@ public class LDAPAccountRestApiTest extends AbstractLDAPTest { } + @Test + public void testIgnoreReadOnlyAttributes() throws IOException { + testingClient.server().run(session -> { + LDAPTestContext ctx = LDAPTestContext.init(session); + RealmModel appRealm = ctx.getRealm(); + appRealm.setEditUsernameAllowed(false); + }); + UserRepresentation user = SimpleHttp.doGet(getAccountUrl(null), httpClient).auth(tokenUtil.getToken()).asJson(UserRepresentation.class); + user.setEmail("john-alias@email.org"); + SimpleHttp.doPost(getAccountUrl(null), httpClient).json(user).auth(tokenUtil.getToken()).asStatus(); + + UserRepresentation usernew = SimpleHttp.doGet(getAccountUrl(null), httpClient).auth(tokenUtil.getToken()).asJson(UserRepresentation.class); + assertEquals("johnkeycloak", usernew.getUsername()); + assertEquals("John", usernew.getFirstName()); + assertEquals("Doe", usernew.getLastName()); + assertEquals("john-alias@email.org", usernew.getEmail()); + assertFalse(usernew.isEmailVerified()); + + usernew.getAttributes().clear(); + + //clean up + usernew.setEmail("john@email.org"); + final int i = SimpleHttp.doPost(getAccountUrl(null), httpClient).json(usernew).auth(tokenUtil.getToken()).asStatus(); + + org.keycloak.representations.idm.UserRepresentation userRep = testRealm().users() + .search(usernew.getUsername()).get(0); + + userRep.setAttributes(null); + + testRealm().users().get(userRep.getId()).update(userRep); + usernew = SimpleHttp.doGet(getAccountUrl(null), httpClient).auth(tokenUtil.getToken()).asJson(UserRepresentation.class); + + assertTrue(usernew.getAttributes().containsKey(LDAPConstants.LDAP_ID)); + assertTrue(usernew.getAttributes().containsKey(LDAPConstants.LDAP_ENTRY_DN)); + } + private String getAccountUrl(String resource) { return suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/realms/test/account" + (resource != null ? "/" + resource : ""); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAdminRestApiTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAdminRestApiTest.java index 15fac62730..7913c5e3b3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAdminRestApiTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPAdminRestApiTest.java @@ -152,7 +152,9 @@ public class LDAPAdminRestApiTest extends AbstractLDAPTest { updateUserExpectError(userRes, user); user.getAttributes().remove(LDAPConstants.LDAP_ID); - updateUserExpectError(userRes, user); + userRes.update(user); + user = userRes.toRepresentation(); + assertEquals(origLdapId, user.getAttributes().get(LDAPConstants.LDAP_ID)); // Trying to update LDAP_ENTRY_DN should fail user.getAttributes().put(LDAPConstants.LDAP_ID, origLdapId);