diff --git a/core/src/main/java/org/keycloak/representations/account/UserRepresentation.java b/core/src/main/java/org/keycloak/representations/account/UserRepresentation.java index 1f561ff025..6276e36396 100755 --- a/core/src/main/java/org/keycloak/representations/account/UserRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/account/UserRepresentation.java @@ -19,6 +19,7 @@ package org.keycloak.representations.account; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import org.keycloak.json.StringListMapDeserializer; +import org.keycloak.representations.idm.UserProfileMetadata; import java.util.ArrayList; import java.util.Arrays; diff --git a/core/src/main/java/org/keycloak/representations/account/UserProfileAttributeMetadata.java b/core/src/main/java/org/keycloak/representations/idm/UserProfileAttributeMetadata.java similarity index 98% rename from core/src/main/java/org/keycloak/representations/account/UserProfileAttributeMetadata.java rename to core/src/main/java/org/keycloak/representations/idm/UserProfileAttributeMetadata.java index 1937e1f1ce..ef7723523c 100644 --- a/core/src/main/java/org/keycloak/representations/account/UserProfileAttributeMetadata.java +++ b/core/src/main/java/org/keycloak/representations/idm/UserProfileAttributeMetadata.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.keycloak.representations.account; +package org.keycloak.representations.idm; import java.util.Map; diff --git a/core/src/main/java/org/keycloak/representations/account/UserProfileMetadata.java b/core/src/main/java/org/keycloak/representations/idm/UserProfileMetadata.java similarity index 96% rename from core/src/main/java/org/keycloak/representations/account/UserProfileMetadata.java rename to core/src/main/java/org/keycloak/representations/idm/UserProfileMetadata.java index 5925e57036..fb8bd1ec40 100644 --- a/core/src/main/java/org/keycloak/representations/account/UserProfileMetadata.java +++ b/core/src/main/java/org/keycloak/representations/idm/UserProfileMetadata.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.keycloak.representations.account; +package org.keycloak.representations.idm; import java.util.List; diff --git a/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java index b80a708dea..6871796e35 100755 --- a/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java @@ -66,6 +66,7 @@ public class UserRepresentation { protected List groups; private Map access; + private UserProfileMetadata userProfileMetadata; public String getSelf() { return self; @@ -312,4 +313,12 @@ public class UserRepresentation { return attrs; } + + public void setUserProfileMetadata(UserProfileMetadata userProfileMetadata) { + this.userProfileMetadata = userProfileMetadata; + } + + public UserProfileMetadata getUserProfileMetadata() { + return userProfileMetadata; + } } diff --git a/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/UserResource.java b/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/UserResource.java index 15cb0bed6b..e8de29db23 100755 --- a/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/UserResource.java +++ b/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/UserResource.java @@ -48,6 +48,9 @@ public interface UserResource { @GET UserRepresentation toRepresentation(); + @GET + UserRepresentation toRepresentation(@QueryParam("userProfileMetadata") boolean userProfileMetadata); + @PUT void update(UserRepresentation userRepresentation); diff --git a/js/apps/admin-ui/src/authentication/policies/OtpPolicy.tsx b/js/apps/admin-ui/src/authentication/policies/OtpPolicy.tsx index c0f8360c61..d64295411e 100644 --- a/js/apps/admin-ui/src/authentication/policies/OtpPolicy.tsx +++ b/js/apps/admin-ui/src/authentication/policies/OtpPolicy.tsx @@ -42,7 +42,7 @@ type OtpPolicyProps = { type FormFields = Omit< RealmRepresentation, - "clients" | "components" | "groups" + "clients" | "components" | "groups" | "users" | "federatedUsers" >; export const OtpPolicy = ({ realm, realmUpdated }: OtpPolicyProps) => { diff --git a/js/apps/admin-ui/src/realm-settings/EmailTab.tsx b/js/apps/admin-ui/src/realm-settings/EmailTab.tsx index 9021eff79a..d1cb574b17 100644 --- a/js/apps/admin-ui/src/realm-settings/EmailTab.tsx +++ b/js/apps/admin-ui/src/realm-settings/EmailTab.tsx @@ -34,6 +34,8 @@ type RealmSettingsEmailTabProps = { save: (realm: RealmRepresentation) => void; }; +type FormType = Omit; + export const RealmSettingsEmailTab = ({ realm, save, @@ -51,7 +53,7 @@ export const RealmSettingsEmailTab = ({ reset: resetForm, getValues, formState: { errors }, - } = useForm({ defaultValues: realm }); + } = useForm({ defaultValues: realm }); const reset = () => resetForm(realm); const watchFromValue = watch("smtpServer.from", ""); diff --git a/js/apps/admin-ui/src/user/EditUser.tsx b/js/apps/admin-ui/src/user/EditUser.tsx index e0edfd30e9..6083904e91 100644 --- a/js/apps/admin-ui/src/user/EditUser.tsx +++ b/js/apps/admin-ui/src/user/EditUser.tsx @@ -61,7 +61,7 @@ export default function EditUser() { useFetch( async () => { const [user, currentRealm, attackDetection] = await Promise.all([ - adminClient.users.findOne({ id: id! }), + adminClient.users.findOne({ id: id!, userProfileMetadata: true }), adminClient.realms.findOne({ realm }), adminClient.attackDetection.findOne({ id: id! }), ]); @@ -103,7 +103,7 @@ const EditUserForm = ({ user, bruteForced, refresh }: EditUserFormProps) => { const { addAlert, addError } = useAlerts(); const navigate = useNavigate(); const { hasAccess } = useAccess(); - const userForm = useForm({ + const userForm = useForm>({ mode: "onChange", defaultValues: user, }); diff --git a/js/apps/admin-ui/src/user/UserForm.tsx b/js/apps/admin-ui/src/user/UserForm.tsx index f49d071991..4582de1620 100644 --- a/js/apps/admin-ui/src/user/UserForm.tsx +++ b/js/apps/admin-ui/src/user/UserForm.tsx @@ -224,7 +224,7 @@ export const UserForm = ({ )} {isUserProfileEnabled ? ( - + ) : ( <> {!realm?.registrationEmailAsUsername && ( diff --git a/js/apps/admin-ui/src/user/UserProfileFields.tsx b/js/apps/admin-ui/src/user/UserProfileFields.tsx index 33ada42f92..8b63a3e338 100644 --- a/js/apps/admin-ui/src/user/UserProfileFields.tsx +++ b/js/apps/admin-ui/src/user/UserProfileFields.tsx @@ -1,11 +1,11 @@ import type { UserProfileAttribute } from "@keycloak/keycloak-admin-client/lib/defs/userProfileConfig"; +import UserProfileConfig from "@keycloak/keycloak-admin-client/lib/defs/userProfileConfig"; import { Text } from "@patternfly/react-core"; import { Fragment } from "react"; +import { useFormContext } from "react-hook-form"; import { useTranslation } from "react-i18next"; -import { useFormContext } from "react-hook-form"; import { ScrollForm } from "../components/scroll-form/ScrollForm"; -import { useUserProfile } from "../realm-settings/user-profile/UserProfileContext"; import { OptionComponent } from "./components/OptionsComponent"; import { SelectComponent } from "./components/SelectComponent"; import { TextAreaComponent } from "./components/TextAreaComponent"; @@ -13,6 +13,7 @@ import { TextComponent } from "./components/TextComponent"; import { DEFAULT_ROLES, fieldName } from "./utils"; type UserProfileFieldsProps = { + config: UserProfileConfig; roles?: string[]; }; @@ -78,21 +79,21 @@ export const isValidComponentType = (value: string): value is Field => value in FIELDS; export const UserProfileFields = ({ + config, roles = ["admin"], }: UserProfileFieldsProps) => { const { t } = useTranslation("realm-settings"); - const { config } = useUserProfile(); return ( ({ + sections={[{ name: "" }, ...(config.groups || [])].map((g) => ({ title: g.displayHeader || g.name || t("general"), panel: (
{g.displayDescription && ( {g.displayDescription} )} - {config?.attributes?.map((attribute) => ( + {config.attributes?.map((attribute) => ( {(attribute.group || "") === g.name && (attribute.permissions?.view || DEFAULT_ROLES).some((r) => diff --git a/js/apps/admin-ui/src/user/components/OptionsComponent.tsx b/js/apps/admin-ui/src/user/components/OptionsComponent.tsx index 79f6456c70..8fce15ee23 100644 --- a/js/apps/admin-ui/src/user/components/OptionsComponent.tsx +++ b/js/apps/admin-ui/src/user/components/OptionsComponent.tsx @@ -42,6 +42,7 @@ export const OptionComponent = (attr: UserProfileAttribute) => { field.onChange([option]); } }} + readOnly={attr.readOnly} /> ))} diff --git a/js/apps/admin-ui/src/user/components/SelectComponent.tsx b/js/apps/admin-ui/src/user/components/SelectComponent.tsx index cadcc43482..2f80c8e9e4 100644 --- a/js/apps/admin-ui/src/user/components/SelectComponent.tsx +++ b/js/apps/admin-ui/src/user/components/SelectComponent.tsx @@ -9,13 +9,10 @@ import { import { useTranslation } from "react-i18next"; import { Options } from "../UserProfileFields"; -import { DEFAULT_ROLES, fieldName } from "../utils"; +import { fieldName } from "../utils"; import { UserProfileFieldsProps, UserProfileGroup } from "./UserProfileGroup"; -export const SelectComponent = ({ - roles = [], - ...attribute -}: UserProfileFieldsProps) => { +export const SelectComponent = (attribute: UserProfileFieldsProps) => { const { t } = useTranslation("users"); const { control } = useFormContext(); const [open, setOpen] = useState(false); @@ -74,11 +71,7 @@ export const SelectComponent = ({ variant={isMultiValue(field) ? "typeaheadmulti" : "single"} aria-label={t("common:selectOne")} isOpen={open} - isDisabled={ - !(attribute.permissions?.edit || DEFAULT_ROLES).some((r) => - roles.includes(r), - ) - } + readOnly={attribute.readOnly} > {options.map((option) => ( { {...register(fieldName(attr))} cols={attr.annotations?.["inputTypeCols"] as number} rows={attr.annotations?.["inputTypeRows"] as number} + readOnly={attr.readOnly} /> ); diff --git a/js/apps/admin-ui/src/user/components/TextComponent.tsx b/js/apps/admin-ui/src/user/components/TextComponent.tsx index da74389f5a..c1e21db5c9 100644 --- a/js/apps/admin-ui/src/user/components/TextComponent.tsx +++ b/js/apps/admin-ui/src/user/components/TextComponent.tsx @@ -18,6 +18,7 @@ export const TextComponent = (attr: UserProfileAttribute) => { data-testid={attr.name} type={type} placeholder={attr.annotations?.["inputTypePlaceholder"] as string} + readOnly={attr.readOnly} {...register(fieldName(attr))} /> diff --git a/js/libs/keycloak-admin-client/src/defs/userProfileConfig.ts b/js/libs/keycloak-admin-client/src/defs/userProfileConfig.ts index 0af67d28c1..1ba7c3b828 100644 --- a/js/libs/keycloak-admin-client/src/defs/userProfileConfig.ts +++ b/js/libs/keycloak-admin-client/src/defs/userProfileConfig.ts @@ -10,6 +10,7 @@ export interface UserProfileAttribute { validations?: Record>; annotations?: Record; required?: UserProfileAttributeRequired; + readOnly?: boolean; permissions?: UserProfileAttributePermissions; selector?: UserProfileAttributeSelector; displayName?: string; diff --git a/js/libs/keycloak-admin-client/src/defs/userRepresentation.ts b/js/libs/keycloak-admin-client/src/defs/userRepresentation.ts index 4215dc9c4d..aa6dd967f4 100644 --- a/js/libs/keycloak-admin-client/src/defs/userRepresentation.ts +++ b/js/libs/keycloak-admin-client/src/defs/userRepresentation.ts @@ -2,6 +2,7 @@ import type UserConsentRepresentation from "./userConsentRepresentation.js"; import type CredentialRepresentation from "./credentialRepresentation.js"; import type FederatedIdentityRepresentation from "./federatedIdentityRepresentation.js"; import type { RequiredActionAlias } from "./requiredActionProviderRepresentation.js"; +import type UserProfileConfig from "./userProfileConfig.js"; export default interface UserRepresentation { id?: string; @@ -30,4 +31,5 @@ export default interface UserRepresentation { realmRoles?: string[]; self?: string; serviceAccountClientId?: string; + userProfileMetadata?: UserProfileConfig; } diff --git a/js/libs/keycloak-admin-client/src/resources/users.ts b/js/libs/keycloak-admin-client/src/resources/users.ts index 9994094444..6c98f1d3bf 100644 --- a/js/libs/keycloak-admin-client/src/resources/users.ts +++ b/js/libs/keycloak-admin-client/src/resources/users.ts @@ -48,7 +48,7 @@ export class Users extends Resource<{ realm?: string }> { */ public findOne = this.makeRequest< - { id: string }, + { id: string; userProfileMetadata?: boolean }, UserRepresentation | undefined >({ method: "GET", diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/Attributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/Attributes.java index 4f8fd9f03e..080208f520 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/Attributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/Attributes.java @@ -105,11 +105,11 @@ public interface Attributes { Set nameSet(); /** - * Returns all attributes defined. + * Returns all attributes that can be written. * * @return the attributes */ - Set>> attributeSet(); + Map> getWritable(); /** *

Returns the metadata associated with the attribute with the given {@code name}. diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java index 774abd31b6..87a5140ded 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java @@ -58,7 +58,7 @@ public class DefaultAttributes extends HashMap> implements public static final String READ_ONLY_ATTRIBUTE_KEY = "kc.read.only"; protected final UserProfileContext context; - private final KeycloakSession session; + protected final KeycloakSession session; private final Map metadataByAttribute; protected final UserModel user; @@ -74,6 +74,18 @@ public class DefaultAttributes extends HashMap> implements @Override public boolean isReadOnly(String attributeName) { + if (UserModel.USERNAME.equals(attributeName)) { + if (isServiceAccountUser()) { + return true; + } + } + + if (UserModel.EMAIL.equals(attributeName)) { + if (isServiceAccountUser()) { + return false; + } + } + if (isReadOnlyFromMetadata(attributeName) || isReadOnlyInternalAttribute(attributeName)) { return true; } @@ -163,8 +175,18 @@ public class DefaultAttributes extends HashMap> implements } @Override - public Set>> attributeSet() { - return entrySet(); + public Map> getWritable() { + Map> attributes = new HashMap<>(this); + + for (String name : nameSet()) { + AttributeMetadata metadata = getMetadata(name); + + if (metadata == null || !metadata.canEdit(createAttributeContext(metadata))) { + attributes.remove(name); + } + } + + return attributes; } @Override @@ -198,6 +220,10 @@ public class DefaultAttributes extends HashMap> implements return this; } + protected boolean isServiceAccountUser() { + return user != null && user.getServiceAccountClientLink() != null; + } + private AttributeContext createAttributeContext(Entry> attribute, AttributeMetadata metadata) { return new AttributeContext(context, session, attribute, user, metadata); } @@ -262,8 +288,8 @@ public class DefaultAttributes extends HashMap> implements key = key.substring(Constants.USER_ATTRIBUTES_PREFIX.length()); } - List values; Object value = entry.getValue(); + List values; if (value instanceof String) { values = Collections.singletonList((String) value); @@ -292,29 +318,34 @@ public class DefaultAttributes extends HashMap> implements } if (user != null) { - List username = newAttributes.get(UserModel.USERNAME); + List username = newAttributes.getOrDefault(UserModel.USERNAME, Collections.emptyList()); - if (username == null || username.isEmpty() || (!realm.isEditUsernameAllowed() && UserProfileContext.USER_API.equals(context))) { + if (username.isEmpty() && isReadOnly(UserModel.USERNAME)) { setUserName(newAttributes, Collections.singletonList(user.getUsername())); } } - List email = newAttributes.get(UserModel.EMAIL); + List email = newAttributes.getOrDefault(UserModel.EMAIL, Collections.emptyList()); - if (email != null && realm.isRegistrationEmailAsUsername()) { - final List lowerCaseEmailList = email.stream() + if (!email.isEmpty() && realm.isRegistrationEmailAsUsername()) { + List lowerCaseEmailList = email.stream() .filter(Objects::nonNull) .map(String::toLowerCase) .collect(Collectors.toList()); setUserName(newAttributes, lowerCaseEmailList); + + if (user != null && isReadOnly(UserModel.EMAIL)) { + newAttributes.put(UserModel.EMAIL, Collections.singletonList(user.getEmail())); + setUserName(newAttributes, Collections.singletonList(user.getEmail())); + } } return newAttributes; } private void setUserName(Map> newAttributes, List lowerCaseEmailList) { - if (user != null && user.getServiceAccountClientLink() != null) { + if (isServiceAccountUser()) { return; } newAttributes.put(UserModel.USERNAME, lowerCaseEmailList); @@ -342,16 +373,11 @@ public class DefaultAttributes extends HashMap> implements return true; } - // expect any attribute if managing the user profile using REST - if (UserProfileContext.USER_API.equals(context) || UserProfileContext.ACCOUNT.equals(context)) { + if (isServiceAccountUser()) { return true; } - if (isReadOnly(name)) { - return true; - } - - if (user != null && user.getServiceAccountClientLink() != null) { + if (isReadOnlyInternalAttribute(name)) { return true; } diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java index cf861f07fa..67a9129cdf 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java @@ -104,13 +104,8 @@ public final class DefaultUserProfile implements UserProfile { } try { - for (Map.Entry> attribute : attributes.attributeSet()) { + for (Map.Entry> attribute : attributes.getWritable().entrySet()) { String name = attribute.getKey(); - - if (attributes.isReadOnly(name)) { - continue; - } - List currentValue = user.getAttributeStream(name).filter(Objects::nonNull).collect(Collectors.toList()); List updatedValue = attribute.getValue().stream().filter(Objects::nonNull).collect(Collectors.toList()); @@ -118,6 +113,7 @@ public final class DefaultUserProfile implements UserProfile { if (!removeAttributes && updatedValue.isEmpty()) { continue; } + user.setAttribute(name, updatedValue); if (UserModel.EMAIL.equals(name) && metadata.getContext().isResetEmailVerified()) { @@ -139,7 +135,7 @@ public final class DefaultUserProfile implements UserProfile { attrsToRemove.removeAll(attributes.nameSet()); for (String attr : attrsToRemove) { - if (this.attributes.isReadOnly(attr)) { + if (attributes.isReadOnly(attr)) { continue; } 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 a02120ba8a..5dd627daa9 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 @@ -41,6 +41,9 @@ import org.keycloak.userprofile.UserProfileProvider; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; + +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -153,7 +156,9 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator { }; UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class); - UserProfile profile = profileProvider.create(UserProfileContext.IDP_REVIEW, formData, updatedProfile); + Map> attributes = new HashMap<>(formData); + attributes.putIfAbsent(UserModel.USERNAME, Collections.singletonList(updatedProfile.getUsername())); + UserProfile profile = profileProvider.create(UserProfileContext.IDP_REVIEW, attributes, updatedProfile); try { String oldEmail = userCtx.getEmail(); diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java index 06363a4e7d..d136af06ed 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java @@ -157,6 +157,7 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor public static UserProfile validateEmailUpdate(KeycloakSession session, UserModel user, String newEmail) { MultivaluedMap formData = new MultivaluedHashMap<>(); + formData.putSingle(UserModel.USERNAME, user.getUsername()); formData.putSingle(UserModel.EMAIL, newEmail); UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class); UserProfile profile = profileProvider.create(UserProfileContext.UPDATE_EMAIL, formData, user); 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 d57f759064..bc1d40df10 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 @@ -70,8 +70,8 @@ import org.keycloak.provider.ConfiguredProvider; import org.keycloak.representations.account.ClientRepresentation; import org.keycloak.representations.account.ConsentRepresentation; import org.keycloak.representations.account.ConsentScopeRepresentation; -import org.keycloak.representations.account.UserProfileAttributeMetadata; -import org.keycloak.representations.account.UserProfileMetadata; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; +import org.keycloak.representations.idm.UserProfileMetadata; import org.keycloak.representations.account.UserRepresentation; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.GroupRepresentation; 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 45db164153..4f0ee0a8f9 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 @@ -58,7 +58,10 @@ import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.models.utils.RoleUtils; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.utils.RedirectUtils; +import org.keycloak.provider.ConfiguredProvider; import org.keycloak.provider.ProviderFactory; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; +import org.keycloak.representations.idm.UserProfileMetadata; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; @@ -80,6 +83,8 @@ import org.keycloak.services.resources.LoginActionsService; import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator; import org.keycloak.services.validation.Validation; import org.keycloak.storage.ReadOnlyException; +import org.keycloak.userprofile.AttributeMetadata; +import org.keycloak.userprofile.AttributeValidatorMetadata; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.ValidationException; @@ -102,6 +107,8 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; import jakarta.ws.rs.core.UriBuilder; +import org.keycloak.validate.Validators; + import java.net.URI; import java.text.MessageFormat; import java.util.ArrayList; @@ -302,7 +309,9 @@ public class UserResource { @Produces(MediaType.APPLICATION_JSON) @Tag(name = KeycloakOpenAPI.Admin.Tags.USERS) @Operation( summary = "Get representation of the user") - public UserRepresentation getUser() { + public UserRepresentation getUser( + @Parameter(description = "Indicates if the user profile metadata should be added to the response") @QueryParam("userProfileMetadata") boolean userProfileMetadata + ) { auth.users().requireView(user); UserRepresentation rep = ModelToRepresentation.toRepresentation(session, realm, user); @@ -325,6 +334,10 @@ public class UserResource { rep.setAttributes(readableAttributes); } + if (userProfileMetadata) { + rep.setUserProfileMetadata(createUserProfileMetadata(profile)); + } + return rep; } @@ -1054,4 +1067,35 @@ public class UserResource { rep.setLastAccess(Time.toMillis(clientSession.getTimestamp())); return rep; } + + private UserProfileMetadata createUserProfileMetadata(final UserProfile profile) { + Map> am = profile.getAttributes().getReadable(); + + if(am == null) + return null; + + List attributes = am.keySet().stream() + .map(name -> profile.getAttributes().getMetadata(name)) + .filter(Objects::nonNull) + .sorted((a,b) -> Integer.compare(a.getGuiOrder(), b.getGuiOrder())) + .map(sam -> toRestMetadata(sam, profile)) + .collect(Collectors.toList()); + return new UserProfileMetadata(attributes); + } + + private UserProfileAttributeMetadata toRestMetadata(AttributeMetadata am, UserProfile profile) { + return new UserProfileAttributeMetadata(am.getName(), + am.getAttributeDisplayName(), + profile.getAttributes().isRequired(am.getName()), + profile.getAttributes().isReadOnly(am.getName()), + am.getAnnotations(), + toValidatorMetadata(am)); + } + + private Map> toValidatorMetadata(AttributeMetadata am){ + // we return only validators which are instance of ConfiguredProvider. Others are expected as internal. + return am.getValidators() == null ? null : am.getValidators().stream() + .filter(avm -> (Validators.validator(session, avm.getValidatorId()) instanceof ConfiguredProvider)) + .collect(Collectors.toMap(AttributeValidatorMetadata::getValidatorId, AttributeValidatorMetadata::getValidatorConfig)); + } } diff --git a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java index 423f721762..7d5dad5c34 100644 --- a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java @@ -77,23 +77,13 @@ public abstract class AbstractUserProfileProvider KeycloakContext context = session.getContext(); RealmModel realm = context.getRealm(); - switch (c.getContext()) { - case REGISTRATION_PROFILE: - case IDP_REVIEW: - return !realm.isRegistrationEmailAsUsername(); - case ACCOUNT_OLD: - case ACCOUNT: - case UPDATE_PROFILE: - return realm.isEditUsernameAllowed(); - case UPDATE_EMAIL: - return realm.isRegistrationEmailAsUsername(); - case USER_API: - return true; - default: - return false; + if (IDP_REVIEW.equals(c.getContext())) { + return !realm.isRegistrationEmailAsUsername(); } + + return realm.isEditUsernameAllowed(); } - + private static boolean readUsernameCondition(AttributeContext c) { KeycloakSession session = c.getSession(); KeycloakContext context = session.getContext(); @@ -110,27 +100,47 @@ public abstract class AbstractUserProfileProvider return realm.isEditUsernameAllowed(); case UPDATE_EMAIL: return false; - default: - return true; } + + return true; } private static boolean editEmailCondition(AttributeContext c) { - return !Profile.isFeatureEnabled(Profile.Feature.UPDATE_EMAIL) || (c.getContext() != UPDATE_PROFILE && c.getContext() != ACCOUNT); + RealmModel realm = c.getSession().getContext().getRealm(); + + if (REGISTRATION_PROFILE.equals(c.getContext())) { + return true; + } + + if (Profile.isFeatureEnabled(Feature.UPDATE_EMAIL)) { + return !(UPDATE_PROFILE.equals(c.getContext()) || ACCOUNT.equals(c.getContext())); + } + + UserModel user = c.getUser(); + + if (user != null && realm.isRegistrationEmailAsUsername() && !realm.isEditUsernameAllowed()) { + return false; + } + + return true; } private static boolean readEmailCondition(AttributeContext c) { UserProfileContext context = c.getContext(); - if (UPDATE_PROFILE.equals(context)) { - if (Profile.isFeatureEnabled(Feature.UPDATE_EMAIL)) { - return false; - } + if (REGISTRATION_PROFILE.equals(context)) { + return true; + } + if (Profile.isFeatureEnabled(Feature.UPDATE_EMAIL)) { + return !UPDATE_PROFILE.equals(context); + } + + if (UPDATE_PROFILE.equals(context)) { RealmModel realm = c.getSession().getContext().getRealm(); - if (realm.isRegistrationEmailAsUsername() && !realm.isEditUsernameAllowed()) { - return false; + if (realm.isRegistrationEmailAsUsername()) { + return realm.isEditUsernameAllowed(); } } @@ -396,8 +406,10 @@ public abstract class AbstractUserProfileProvider UserProfileMetadata metadata = new UserProfileMetadata(USER_API); - metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(UsernameHasValueValidator.ID)); - metadata.addAttribute(UserModel.EMAIL, -1, new AttributeValidatorMetadata(EmailValidator.ID, ValidatorConfig.builder().config(EmailValidator.IGNORE_EMPTY_VALUE, true).build())); + metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(UsernameHasValueValidator.ID)) + .addWriteCondition(AbstractUserProfileProvider::editUsernameCondition); + metadata.addAttribute(UserModel.EMAIL, -1, new AttributeValidatorMetadata(EmailValidator.ID, ValidatorConfig.builder().config(EmailValidator.IGNORE_EMPTY_VALUE, true).build())) + .addWriteCondition(AbstractUserProfileProvider::editEmailCondition); List readonlyValidators = new ArrayList<>(); diff --git a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java index cc48000a81..c83f630fa7 100644 --- a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java +++ b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java @@ -1,11 +1,12 @@ package org.keycloak.userprofile; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; /** @@ -22,28 +23,66 @@ public class LegacyAttributes extends DefaultAttributes { @Override protected boolean isSupportedAttribute(String name) { - if (super.isSupportedAttribute(name)) { + return true; + } + + @Override + public boolean isReadOnly(String name) { + RealmModel realm = session.getContext().getRealm(); + + if (isReadOnlyInternalAttribute(name)) { return true; } - if (name.startsWith(Constants.USER_ATTRIBUTES_PREFIX)) { - return true; + if (user == null) { + return false; + } + + if (UserModel.USERNAME.equals(name)) { + if (isServiceAccountUser()) { + return true; + } + if (UserProfileContext.IDP_REVIEW.equals(context)) { + return false; + } + return !realm.isEditUsernameAllowed(); + } + + if (UserModel.EMAIL.equals(name)) { + if (isServiceAccountUser()) { + return false; + } + if (UserProfileContext.IDP_REVIEW.equals(context)) { + return false; + } + if (realm.isRegistrationEmailAsUsername() && !realm.isEditUsernameAllowed()) { + return true; + } } return false; } @Override - public boolean isReadOnly(String attributeName) { - return isReadOnlyFromMetadata(attributeName) || isReadOnlyInternalAttribute(attributeName); + public Map> getReadable() { + if(user == null || user.getAttributes() == null) { + return Collections.emptyMap(); + } + + return new HashMap<>(user.getAttributes()); } @Override - public Map> getReadable() { - if(user == null || user.getAttributes() == null) - return new HashMap<>(); + public Map> getWritable() { + Map> attributes = new HashMap<>(this); - return new HashMap<>(user.getAttributes()); + for (String name : nameSet()) { + if (isReadOnly(name)) { + attributes.remove(name); + } + } + + return attributes; } @Override diff --git a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java index 6531ffb61e..98d7d7f5a6 100644 --- a/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java +++ b/services/src/main/java/org/keycloak/userprofile/validator/ImmutableAttributeValidator.java @@ -51,11 +51,6 @@ public class ImmutableAttributeValidator implements SimpleValidator { public ValidationContext validate(Object input, String inputHint, ValidationContext context, ValidatorConfig config) { UserProfileAttributeValidationContext ac = (UserProfileAttributeValidationContext) context; AttributeContext attributeContext = ac.getAttributeContext(); - - if (!isReadOnly(attributeContext)) { - return context; - } - UserModel user = attributeContext.getUser(); if (user == null) { @@ -65,7 +60,7 @@ public class ImmutableAttributeValidator implements SimpleValidator { List currentValue = user.getAttributeStream(inputHint).collect(Collectors.toList()); List values = (List) input; - if (!CollectionUtil.collectionEquals(currentValue, values)) { + if (!CollectionUtil.collectionEquals(currentValue, values) && isReadOnly(attributeContext)) { if (currentValue.isEmpty() && !notBlankValidator().validate(values).isValid()) { return context; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java index 1d565f1367..c04f7c3fc1 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java @@ -22,12 +22,14 @@ import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.authentication.authenticators.browser.WebAuthnAuthenticatorFactory; import org.keycloak.authentication.authenticators.browser.WebAuthnPasswordlessAuthenticatorFactory; import org.keycloak.authentication.requiredactions.WebAuthnPasswordlessRegisterFactory; import org.keycloak.authentication.requiredactions.WebAuthnRegisterFactory; import org.keycloak.broker.provider.util.SimpleHttp; +import org.keycloak.common.Profile; import org.keycloak.common.enums.AccountRestApiVersion; import org.keycloak.common.util.ObjectUtil; import org.keycloak.credential.CredentialTypeMetadata; @@ -44,7 +46,7 @@ import org.keycloak.representations.account.ClientRepresentation; import org.keycloak.representations.account.ConsentRepresentation; import org.keycloak.representations.account.ConsentScopeRepresentation; import org.keycloak.representations.account.SessionRepresentation; -import org.keycloak.representations.account.UserProfileAttributeMetadata; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; import org.keycloak.representations.account.UserRepresentation; import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.AuthenticationExecutionRepresentation; @@ -62,6 +64,7 @@ import org.keycloak.services.util.ResolveRelative; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.admin.authentication.AbstractAuthenticationTest; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.TokenUtil; import org.keycloak.testsuite.util.UserBuilder; @@ -96,14 +99,79 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { public AssertEvents events = new AssertEvents(this); @Test - public void testGetUserProfileMetadata_EditUsernameAllowed() throws IOException { - + public void testEditUsernameAllowed() throws IOException { UserRepresentation user = getUser(); - assertNotNull(user.getUserProfileMetadata()); - assertUserProfileAttributeMetadata(user, "username", "${username}", true, false); - assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); - assertUserProfileAttributeMetadata(user, "firstName", "${firstName}", true, false); - assertUserProfileAttributeMetadata(user, "lastName", "${lastName}", true, false); + String originalUsername = user.getUsername(); + String originalEmail = user.getEmail(); + RealmResource realm = adminClient.realm("test"); + RealmRepresentation realmRep = realm.toRepresentation(); + Boolean registrationEmailAsUsername = realmRep.isRegistrationEmailAsUsername(); + Boolean editUsernameAllowed = realmRep.isEditUsernameAllowed(); + + try { + realmRep.setRegistrationEmailAsUsername(false); + realmRep.setEditUsernameAllowed(true); + realm.update(realmRep); + user = getUser(); + assertNotNull(user.getUserProfileMetadata()); + // can write both username and email + assertUserProfileAttributeMetadata(user, "username", "${username}", true, false); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); + assertUserProfileAttributeMetadata(user, "firstName", "${firstName}", true, false); + assertUserProfileAttributeMetadata(user, "lastName", "${lastName}", true, false); + user.setUsername("changed-username"); + user.setEmail("changed-email@keycloak.org"); + user = updateAndGet(user); + assertEquals("changed-username", user.getUsername()); + assertEquals("changed-email@keycloak.org", user.getEmail()); + + realmRep.setRegistrationEmailAsUsername(false); + realmRep.setEditUsernameAllowed(false); + realm.update(realmRep); + user = getUser(); + assertNotNull(user.getUserProfileMetadata()); + // username is readonly but email is writable + assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); + user.setUsername("should-not-change"); + user.setEmail("changed-email@keycloak.org"); + updateError(user, 400, Messages.READ_ONLY_USERNAME); + + realmRep.setRegistrationEmailAsUsername(true); + realmRep.setEditUsernameAllowed(true); + realm.update(realmRep); + user = getUser(); + assertNotNull(user.getUserProfileMetadata()); + // username is read-only and is the same as email, but email is writable + assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); + user.setUsername("should-be-the-email"); + user.setEmail("user@keycloak.org"); + user = updateAndGet(user); + assertEquals("user@keycloak.org", user.getUsername()); + assertEquals("user@keycloak.org", user.getEmail()); + + realmRep.setRegistrationEmailAsUsername(true); + realmRep.setEditUsernameAllowed(false); + realm.update(realmRep); + user = getUser(); + assertNotNull(user.getUserProfileMetadata()); + // username is read-only and is the same as email, but email is read-only + assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, true); + user.setUsername("should-be-the-email"); + user.setEmail("should-not-change@keycloak.org"); + user = updateAndGet(user); + assertEquals("user@keycloak.org", user.getUsername()); + assertEquals("user@keycloak.org", user.getEmail()); + } finally { + realmRep.setRegistrationEmailAsUsername(registrationEmailAsUsername); + realmRep.setEditUsernameAllowed(editUsernameAllowed); + realm.update(realmRep); + user.setUsername(originalUsername); + user.setEmail(originalEmail); + updateAndGet(user); + } } @Test @@ -113,23 +181,30 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { } @Test - public void testGetUserProfileMetadata_EditUsernameDisallowed() throws IOException { - + public void testEditUsernameDisallowed() throws IOException { try { - RealmRepresentation realmRep = adminClient.realm("test").toRepresentation(); + RealmResource realm = adminClient.realm("test"); + RealmRepresentation realmRep = realm.toRepresentation(); realmRep.setEditUsernameAllowed(false); - adminClient.realm("test").update(realmRep); + realm.update(realmRep); UserRepresentation user = getUser(); assertNotNull(user.getUserProfileMetadata()); UserProfileAttributeMetadata upm = assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); //makes sure internal validators are not exposed Assert.assertEquals(0, upm.getValidators().size()); - + upm = assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); Assert.assertEquals(1, upm.getValidators().size()); Assert.assertTrue(upm.getValidators().containsKey(EmailValidator.ID)); - + + realmRep.setRegistrationEmailAsUsername(true); + realm.update(realmRep); + user = getUser(); + upm = assertUserProfileAttributeMetadata(user, "email", "${email}", true, true); + Assert.assertEquals(1, upm.getValidators().size()); + Assert.assertTrue(upm.getValidators().containsKey(EmailValidator.ID)); + assertUserProfileAttributeMetadata(user, "firstName", "${firstName}", true, false); assertUserProfileAttributeMetadata(user, "lastName", "${lastName}", true, false); } finally { @@ -152,10 +227,12 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { protected UserProfileAttributeMetadata assertUserProfileAttributeMetadata(UserRepresentation user, String attName, String displayName, boolean required, boolean readOnly) { UserProfileAttributeMetadata uam = getUserProfileAttributeMetadata(user, attName); - assertNotNull(uam); - assertEquals("Unexpected display name for attribute " + uam.getName(), displayName, uam.getDisplayName()); - assertEquals("Unexpected required flag for attribute " + uam.getName(), required, uam.isRequired()); - assertEquals("Unexpected readonly flag for attribute " + uam.getName(), readOnly, uam.isReadOnly()); + if (isDeclarativeUserProfile()) { + assertNotNull(uam); + assertEquals("Unexpected display name for attribute " + uam.getName(), displayName, uam.getDisplayName()); + assertEquals("Unexpected required flag for attribute " + uam.getName(), required, uam.isRequired()); + assertEquals("Unexpected readonly flag for attribute " + uam.getName(), readOnly, uam.isReadOnly()); + } return uam; } @@ -378,7 +455,6 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { user = updateAndGet(user); assertEquals("test-user@localhost", user.getUsername()); - realmRep.setRegistrationEmailAsUsername(true); adminClient.realm("test").update(realmRep); @@ -1537,6 +1613,35 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { // custom-audience client is used only in this test so no need to revert the changes } + @Test + @EnableFeature(Profile.Feature.UPDATE_EMAIL) + public void testEmailWhenUpdateEmailEnabled() throws Exception { + reconnectAdminClient(); + RealmRepresentation realm = testRealm().toRepresentation(); + Boolean registrationEmailAsUsername = realm.isRegistrationEmailAsUsername(); + Boolean editUsernameAllowed = realm.isEditUsernameAllowed(); + + try { + realm.setRegistrationEmailAsUsername(true); + realm.setEditUsernameAllowed(true); + testRealm().update(realm); + UserRepresentation user = getUser(true); + assertNotNull(user.getEmail()); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, true); + + realm.setRegistrationEmailAsUsername(false); + realm.setEditUsernameAllowed(false); + testRealm().update(realm); + user = getUser(true); + assertNotNull(user.getEmail()); + assertUserProfileAttributeMetadata(user, "email", "${email}", true, true); + } finally { + realm.setRegistrationEmailAsUsername(registrationEmailAsUsername); + realm.setEditUsernameAllowed(editUsernameAllowed); + testRealm().update(realm); + } + } + protected boolean isDeclarativeUserProfile() { return false; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java index c48ca77e6c..7d6813f5d1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java @@ -37,8 +37,8 @@ import org.keycloak.common.Profile; import org.keycloak.events.Details; import org.keycloak.events.EventType; import org.keycloak.models.UserModel; -import org.keycloak.representations.account.UserProfileAttributeMetadata; -import org.keycloak.representations.account.UserProfileMetadata; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; +import org.keycloak.representations.idm.UserProfileMetadata; import org.keycloak.representations.account.UserRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; @@ -101,7 +101,7 @@ public class AccountRestServiceWithUserProfileTest extends AccountRestServiceTes @Test @Override - public void testGetUserProfileMetadata_EditUsernameAllowed() throws IOException { + public void testEditUsernameAllowed() throws IOException { setUserProfileConfiguration(UP_CONFIG_FOR_METADATA); @@ -221,7 +221,7 @@ public class AccountRestServiceWithUserProfileTest extends AccountRestServiceTes @Test @Override - public void testGetUserProfileMetadata_EditUsernameDisallowed() throws IOException { + public void testEditUsernameDisallowed() throws IOException { try { RealmRepresentation realmRep = adminClient.realm("test").toRepresentation(); @@ -261,6 +261,7 @@ public class AccountRestServiceWithUserProfileTest extends AccountRestServiceTes } finally { RealmRepresentation realmRep = testRealm().toRepresentation(); realmRep.setEditUsernameAllowed(true); + realmRep.setRegistrationEmailAsUsername(false); testRealm().update(realmRep); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java index 91246a24ff..917b042c60 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.actions; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import java.util.Arrays; import org.jboss.arquillian.drone.api.annotation.Drone; @@ -184,10 +185,18 @@ public abstract class AbstractRequiredActionUpdateEmailTest extends AbstractTest setRegistrationEmailAsUsername(testRealm(), true); try { + UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); + String firstName = user.getFirstName(); + String lastName = user.getLastName(); + assertNotNull(firstName); + assertNotNull(lastName); changeEmailUsingRequiredAction("new@localhost", true); - - UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "new@localhost"); + user = ActionUtil.findUserWithAdminClient(adminClient, "new@localhost"); Assert.assertNotNull(user); + firstName = user.getFirstName(); + lastName = user.getLastName(); + assertNotNull(firstName); + assertNotNull(lastName); } finally { setRegistrationEmailAsUsername(testRealm(), genuineRegistrationEmailAsUsername); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/DeclarativeUserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/DeclarativeUserTest.java index 91efded5f3..16a9d74ccf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/DeclarativeUserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/DeclarativeUserTest.java @@ -109,7 +109,6 @@ public class DeclarativeUserTest extends AbstractAdminTest { ApiUtil.getCreatedId(response); } catch (WebApplicationException e) { // it's ok when the client has already been created for a previous test - assertThat(e.getResponse().getStatus(), equalTo(409)); } testRealmUserManagerClient = AdminClientUtil.createAdminClient(true, realmRep.getRealm(), diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 00524d07cf..2105c7182d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -21,6 +21,7 @@ import org.hamcrest.Matchers; import org.jboss.arquillian.drone.api.annotation.Drone; import org.jboss.arquillian.graphene.page.Page; import org.junit.After; +import org.junit.Assume; import org.junit.Before; import org.junit.Assert; import org.junit.Rule; @@ -102,10 +103,12 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -156,6 +159,28 @@ public class UserTest extends AbstractAdminTest { @Page protected LoginPage loginPage; + protected Set managedAttributes = new HashSet<>(); + + { + managedAttributes.add("test"); + managedAttributes.add("attr"); + managedAttributes.add("attr1"); + managedAttributes.add("attr2"); + managedAttributes.add("attr3"); + managedAttributes.add("foo"); + managedAttributes.add("bar"); + managedAttributes.add("phoneNumber"); + managedAttributes.add("usercertificate"); + managedAttributes.add("saml.persistent.name.id.for.foo"); + managedAttributes.add(LDAPConstants.LDAP_ID); + managedAttributes.add("LDap_Id"); + managedAttributes.add("deniedSomeAdmin"); + + for (int i = 1; i < 10; i++) { + managedAttributes.add("test" + i); + } + } + @Before public void beforeUserTest() { createAppClientInRealm(REALM_NAME); @@ -630,11 +655,9 @@ public class UserTest extends AbstractAdminTest { user.setFirstName("First" + i); user.setLastName("Last" + i); - HashMap> attributes = new HashMap<>(); - attributes.put("test", Collections.singletonList("test" + i)); - attributes.put("test" + i, Collections.singletonList("test" + i)); - attributes.put("attr", Collections.singletonList("common")); - user.setAttributes(attributes); + addAttribute(user, "test", Collections.singletonList("test" + i)); + addAttribute(user, "test" + i, Collections.singletonList("test" + i)); + addAttribute(user, "attr", Collections.singletonList("common")); ids.add(createUser(user)); } @@ -642,6 +665,15 @@ public class UserTest extends AbstractAdminTest { return ids; } + private void addAttribute(UserRepresentation user, String name, List values) { + Map> attributes = Optional.ofNullable(user.getAttributes()).orElse(new HashMap<>()); + + attributes.put(name, values); + managedAttributes.add(name); + + user.setAttributes(attributes); + } + @Test public void countByAttribute() { createUsers(); @@ -1145,6 +1177,7 @@ public class UserTest extends AbstractAdminTest { @Test public void wildcardSearch() { + Assume.assumeFalse("Default validators do not allow special chars", isDeclarativeUserProfile()); createUser("0user\\\\0", "email0@emal"); createUser("1user\\\\", "email1@emal"); createUser("2user\\\\%", "email2@emal"); @@ -1435,12 +1468,20 @@ public class UserTest extends AbstractAdminTest { String user2Id = createUser(user2); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(2, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(2, user1.getAttributes().size()); + } assertAttributeValue("value1user1", user1.getAttributes().get("attr1")); assertAttributeValue("value2user1", user1.getAttributes().get("attr2")); user2 = realm.users().get(user2Id).toRepresentation(); - assertEquals(2, user2.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user2.getAttributes().size()); + } else { + assertEquals(2, user2.getAttributes().size()); + } assertAttributeValue("value1user2", user2.getAttributes().get("attr1")); vals = user2.getAttributes().get("attr2"); assertEquals(2, vals.size()); @@ -1452,7 +1493,11 @@ public class UserTest extends AbstractAdminTest { updateUser(realm.users().get(user1Id), user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(3, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(3, user1.getAttributes().size()); + } assertAttributeValue("value3user1", user1.getAttributes().get("attr1")); assertAttributeValue("value2user1", user1.getAttributes().get("attr2")); assertAttributeValue("value4user1", user1.getAttributes().get("attr3")); @@ -1461,7 +1506,11 @@ public class UserTest extends AbstractAdminTest { updateUser(realm.users().get(user1Id), user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(2, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(2, user1.getAttributes().size()); + } assertAttributeValue("value2user1", user1.getAttributes().get("attr2")); assertAttributeValue("value4user1", user1.getAttributes().get("attr3")); @@ -1470,7 +1519,11 @@ public class UserTest extends AbstractAdminTest { updateUser(realm.users().get(user1Id), user1); user1 = realm.users().get(user1Id).toRepresentation(); assertNotNull(user1.getAttributes()); - assertEquals(2, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(2, user1.getAttributes().size()); + } // empty attributes should remove attributes user1.setAttributes(Collections.emptyMap()); @@ -1488,13 +1541,21 @@ public class UserTest extends AbstractAdminTest { realm.users().get(user1Id).update(user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(2, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(2, user1.getAttributes().size()); + } user1.getAttributes().remove("foo"); realm.users().get(user1Id).update(user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(1, user1.getAttributes().size()); + if (isDeclarativeUserProfile()) { + assertEquals(managedAttributes.size(), user1.getAttributes().size()); + } else { + assertEquals(1, user1.getAttributes().size()); + } } @Test @@ -1547,7 +1608,11 @@ public class UserTest extends AbstractAdminTest { user1 = realm.users().get(user1Id).toRepresentation(); assertEquals("foo", user1.getAttributes().get("usercertificate").get(0)); assertEquals("bar", user1.getAttributes().get("saml.persistent.name.id.for.foo").get(0)); - assertFalse(user1.getAttributes().containsKey(LDAPConstants.LDAP_ID)); + if (isDeclarativeUserProfile()) { + assertTrue(user1.getAttributes().get(LDAPConstants.LDAP_ID).isEmpty()); + } else { + assertFalse(user1.getAttributes().containsKey(LDAPConstants.LDAP_ID)); + } } @Test @@ -2291,10 +2356,11 @@ public class UserTest extends AbstractAdminTest { } @Test - public void updateUserWithEmailAsUsername() { + public void updateUserWithEmailAsUsernameEditUsernameDisabled() { switchRegistrationEmailAsUsername(true); getCleanup().addCleanup(() -> switchRegistrationEmailAsUsername(false)); - + RealmRepresentation rep = realm.toRepresentation(); + assertFalse(rep.isEditUsernameAllowed()); String id = createUser(); UserResource user = realm.users().get(id); @@ -2304,6 +2370,25 @@ public class UserTest extends AbstractAdminTest { userRep.setEmail("user11@localhost"); updateUser(user, userRep); + userRep = realm.users().get(id).toRepresentation(); + assertEquals("user1@localhost", userRep.getUsername()); + } + + @Test + public void updateUserWithEmailAsUsernameEditUsernameAllowed() { + switchRegistrationEmailAsUsername(true); + getCleanup().addCleanup(() -> switchRegistrationEmailAsUsername(false)); + switchEditUsernameAllowedOn(true); + getCleanup().addCleanup(() -> switchEditUsernameAllowedOn(false)); + + String id = createUser(); + UserResource user = realm.users().get(id); + UserRepresentation userRep = user.toRepresentation(); + assertEquals("user1@localhost", userRep.getUsername()); + + userRep.setEmail("user11@localhost"); + updateUser(user, userRep); + userRep = realm.users().get(id).toRepresentation(); assertEquals("user11@localhost", userRep.getUsername()); } @@ -2349,12 +2434,20 @@ public class UserTest extends AbstractAdminTest { UserRepresentation update = new UserRepresentation(); update.setId(userId); + if (isDeclarativeUserProfile()) { + // user profile requires sending all attributes otherwise they are removed + update.setEmail(email); + } update.setAttributes(Map.of("phoneNumber", List.of("123"))); updateUser(realm.users().get(userId), update); UserRepresentation updated = realm.users().get(userId).toRepresentation(); assertThat(updated.getUsername(), equalTo(userName)); - assertThat(updated.getAttributes(), equalTo(Map.of("phoneNumber", List.of("123")))); + if (isDeclarativeUserProfile()) { + assertThat(updated.getAttributes().get("phoneNumber"), equalTo(List.of("123"))); + } else { + assertThat(updated.getAttributes(), equalTo(Map.of("phoneNumber", List.of("123")))); + } assertThat(updated.getEmail(), equalTo(email)); } @@ -2774,6 +2867,12 @@ public class UserTest extends AbstractAdminTest { firstRealm.setRealm("first-realm"); adminClient.realms().create(firstRealm); + getCleanup().addCleanup(new AutoCloseable() { + @Override + public void close() throws Exception { + adminClient.realms().realm(firstRealm.getRealm()).remove(); + } + }); realm = adminClient.realm(firstRealm.getRealm()); realmId = realm.toRepresentation().getId(); @@ -2798,6 +2897,8 @@ public class UserTest extends AbstractAdminTest { fail("Should not have access to firstUser from another realm"); } catch (NotFoundException nfe) { // ignore + } finally { + adminClient.realm(secondRealm.getRealm()).remove(); } } @@ -3319,4 +3420,8 @@ public class UserTest extends AbstractAdminTest { actualRepresentation ); } + + protected boolean isDeclarativeUserProfile() { + return false; + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTestWithUserProfile.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTestWithUserProfile.java new file mode 100755 index 0000000000..e8ca24a9ac --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTestWithUserProfile.java @@ -0,0 +1,103 @@ +/* + * Copyright 2016 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.testsuite.admin; + +import static org.junit.Assert.assertNotNull; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Set; + +import org.jetbrains.annotations.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.keycloak.common.Profile.Feature; +import org.keycloak.models.LDAPConstants; +import org.keycloak.representations.idm.UserProfileAttributeMetadata; +import org.keycloak.representations.idm.UserProfileMetadata; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; +import org.keycloak.testsuite.forms.VerifyProfileTest; +import org.keycloak.userprofile.config.UPAttribute; +import org.keycloak.userprofile.config.UPAttributePermissions; +import org.keycloak.userprofile.config.UPConfig; +import org.keycloak.util.JsonSerialization; + +@EnableFeature(Feature.DECLARATIVE_USER_PROFILE) +public class UserTestWithUserProfile extends UserTest { + + @Before + public void onBefore() throws IOException { + RealmRepresentation realmRep = realm.toRepresentation(); + VerifyProfileTest.disableDynamicUserProfile(realm); + assertAdminEvents.poll(); + realm.update(realmRep); + assertAdminEvents.poll(); + VerifyProfileTest.enableDynamicUserProfile(realmRep); + realm.update(realmRep); + assertAdminEvents.poll(); + VerifyProfileTest.setUserProfileConfiguration(realm, null); + UPConfig upConfig = JsonSerialization.readValue(realm.users().userProfile().getConfiguration(), UPConfig.class); + + for (String name : managedAttributes) { + upConfig.addAttribute(createAttributeMetadata(name)); + } + + VerifyProfileTest.setUserProfileConfiguration(realm, JsonSerialization.writeValueAsString(upConfig)); + } + + @Test + public void testUserProfileMetadata() { + String userId = createUser("user-metadata", "user-metadata@keycloak.org"); + UserRepresentation user = realm.users().get(userId).toRepresentation(true); + UserProfileMetadata metadata = user.getUserProfileMetadata(); + assertNotNull(metadata); + + for (String name : managedAttributes) { + assertNotNull(getAttributeMetadata(metadata, name)); + } + } + + @Nullable + private static UserProfileAttributeMetadata getAttributeMetadata(UserProfileMetadata metadata, String name) { + UserProfileAttributeMetadata attrMetadata = null; + + for (UserProfileAttributeMetadata m : metadata.getAttributes()) { + if (name.equals(m.getName())) { + attrMetadata = m; + } + } + return attrMetadata; + } + + private UPAttribute createAttributeMetadata(String name) { + UPAttribute attribute = new UPAttribute(); + attribute.setName(name); + UPAttributePermissions permissions = new UPAttributePermissions(); + permissions.setEdit(Set.of("user", "admin")); + attribute.setPermissions(permissions); + this.managedAttributes.add(name); + return attribute; + } + + @Override + protected boolean isDeclarativeUserProfile() { + return true; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java index 315291a8ca..692e741f1c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java @@ -48,6 +48,7 @@ import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentValidationException; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.representations.idm.ClientRepresentation; @@ -517,7 +518,7 @@ public class UserProfileTest extends AbstractUserProfileTest { UserModel user = profile.create(); assertThat(profile.getAttributes().nameSet(), - containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.LOCALE, "address", "department")); + containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.LOCALE, "department")); assertNull(user.getFirstAttribute("department")); @@ -644,17 +645,16 @@ public class UserProfileTest extends AbstractUserProfileTest { provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}}," + "{\"name\": \"phone\", \"permissions\": {\"edit\": [\"admin\"]}}," + "{\"name\": \"address\", \"permissions\": {\"edit\": [\"admin\"]}}]}"); - UserProfile profile = provider.create(UserProfileContext.ACCOUNT, attributes); UserModel user = profile.create(); - assertThat(profile.getAttributes().nameSet(), containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.LOCALE, "address", "department", "phone")); + attributes.put("address", Arrays.asList("change-address")); + attributes.put("department", Arrays.asList("changed-sales")); + attributes.put("phone", Arrays.asList("changed-phone")); profile = provider.create(UserProfileContext.USER_API, attributes, user); - Set attributesUpdated = new HashSet<>(); - profile.update((attributeName, userModel, oldValue) -> assertTrue(attributesUpdated.add(attributeName))); assertThat(attributesUpdated, containsInAnyOrder("department", "address", "phone")); @@ -1314,6 +1314,32 @@ public class UserProfileTest extends AbstractUserProfileTest { profile.validate(); } + @Test + public void testReadOnlyInternalAttributeValidation() { + getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testReadOnlyInternalAttributeValidation); + } + + private static void testReadOnlyInternalAttributeValidation(KeycloakSession session) throws IOException { + RealmModel realm = session.getContext().getRealm(); + UserModel maria = session.users().addUser(realm, "maria"); + + maria.setAttribute(LDAPConstants.LDAP_ID, List.of("1")); + + DeclarativeUserProfileProvider provider = getDynamicUserProfileProvider(session); + Map> attributes = new HashMap<>(); + + attributes.put(LDAPConstants.LDAP_ID, List.of("2")); + + UserProfile profile = provider.create(UserProfileContext.USER_API, attributes, maria); + + try { + profile.validate(); + fail("Should fail validation"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(LDAPConstants.LDAP_ID)); + } + } + @Test public void testRequiredByClientScope() { getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testRequiredByClientScope); @@ -1464,6 +1490,7 @@ public class UserProfileTest extends AbstractUserProfileTest { UserModel user = session.users().addUser(realm, username); Map attributes = new HashMap<>(); + attributes.put(UserModel.USERNAME, user.getUsername()); attributes.put(UserModel.EMAIL, "test@keycloak.com"); UserProfile profile = provider.create(UserProfileContext.USER_API, attributes, user); @@ -1502,10 +1529,20 @@ public class UserProfileTest extends AbstractUserProfileTest { attributes.put(UserModel.EMAIL, Arrays.asList("new-email@test.com")); attributes.put("foo", "changed"); profile = provider.create(UserProfileContext.USER_API, attributes, user); + try { + profile.update(false); + fail("Should fail validation"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(UserModel.USERNAME)); + assertTrue(ve.hasError(Messages.MISSING_USERNAME)); + } + + attributes.put(UserModel.USERNAME, Collections.singletonList(user.getUsername())); + profile = provider.create(UserProfileContext.USER_API, attributes, user); profile.update(false); + profile = provider.create(UserProfileContext.USER_API, user); Attributes userAttributes = profile.getAttributes(); - assertEquals("new-email@test.com", userAttributes.getFirstValue(UserModel.EMAIL)); assertEquals("Test Value", userAttributes.getFirstValue("test-attribute")); assertEquals("changed", userAttributes.getFirstValue("foo"));