User profile attributes should only accept a single value unless configured otherwise

[jonkoops]

When a user profile attribute is configured it is still possible to give it a multi-valued string. For example, if an attribute named age is defined the following are all valid representations of that attribute on the user:

{
  "attributes": { "age": ["18"] }
}

{
  "attributes": { "age": ["18", "42"] }
}

This makes the value of the attribute extremely ambiguous for whomever is consuming this data from the REST API, making it had to reason about. One cannot make assumptions about which of these values is correct, allowing room for bugs to be introduced.

Therefore the proposal is as follows:

• Add a boolean property called isMultiValued to UserProfileAttribute, defaulting to false.
• If isMultiValued is true an array of multiple strings is accepted as a value (e.g. "age": ["18", "42"]).
• If isMultiValued is false an array of a single string accepted as a value (e.g. "age": ["18"]).
• A checkbox will be added to the form in the admin console to toggle the value of isMultiValued.

[pedroigor]

One of the assumptions we have around attributes is that they are user-case-specific. If you have set age you know how to properly handle it.

To communicate UI-related information [1] with clients we have some built-in annotations. The annotations are basically supported by Freemarker templates. We don't necessarily need to have a specific multivalued property if we can actually communicate to clients how the attribute should actually be rendered.

In the case of the administration console, if the built-in annotations are not enough to help render attributes we can still leverage annotations and add those we are missing. For instance, if it is easier for the admin console to know in advance which attributes are multivalued we can have a multivalued annotation and change the Attribute Page to show this attribute as an ON/OFF switch.

[1] https://www.keycloak.org/docs/latest/server_admin/#using-dynamic-forms

[pedroigor]

@jonkoops Can we close this one in favor of #19606?

That one covers both UI and API ends.

[jonkoops]

Can we close this one in favor of #19606?

@pedroigor Since this one is a bit more implementation specific I think it would be better if we close the other issue. This one also covers the UI regardless.

[jonkoops]

Ok, so let me try to break down your comments here.

To communicate UI-related information [1] with clients we have some built-in annotations. The annotations are basically supported by Freemarker templates. We don't necessarily need to have a specific multivalued property if we can actually communicate to clients how the attribute should actually be rendered.

So this is kinda hard, as you really can't know in advance where these attributes will be consumed. It might be correctly implemented in our case, but that doesn't mean that it is logical for 3rd party consumers of this data. It is also perfectly valid at the moment to create a user profile attribute without any annotations or validations. Making it practically impossible to determine this accurately.

I am going to operate on the assumption that you referring here to the 'input type' being the specifier for applications to understand whether this is a multivalue or not. This has several distinct problems:

It's ambiguous about whether a field is multi-valued

For example, imagine we had a user profile attribute called phoneNumber with an input type set to html5-tel:

This would create the following user profile configuration for the client to consume:

{
  "name": "phoneNumber",
  "displayName": "Phone Number",
  "required": false,
  "readOnly": false,
  "annotations": {
    "inputType": "html5-tel"
  }
}

With something like the following attribute data backing it up:

{
  "attributes": {
    "phoneNumber": [
      "06123456"
    ]
  }
}

There is no way for a consumer of this API to know whether this needs to be rendered as a single field. Or if it would be possible to add multiple phone numbers. In fact, we will just have to assume that it's either always a single field, or multi-valued. In the case of the Account Console we simply render out a single input for this field:

So if we wanted to accomplish having a single attribute with multiple values in the form (e.g. a new input for each value), there is currently no way of doing so.

So either we need to have a new input type of html5-tel-multivalue, or we need a new property on the attribute itself. For example, if we had the multiValued property:

{
  "name": "phoneNumbers",
  "displayName": "Phone Numbers",
  "required": false,
  "readOnly": false,
  "multiValue": true,
  "annotations": {
    "inputType": "html5-tel"
  }
}

Now it is explicitly clear to the API consumer that phoneNumbers can contain multiple values, which would allow us to build forms like this:

There is no validation if a single or multiple values are set

Even if I set an input type (such as html5-tel) in the earlier example. There is no validation that guarantees that only a single value is set. So in our earlier example, even if I would assume a an attribute is only backed by a single field in the form, a multi-value can still be set anywhere:

{
  "attributes": {
    "phoneNumbers": [
      "+13469763709",
      "+15736337698"
    ]
  }
}

In this case, which is the correct value? The first, the last? It's just not possible to really tell. People make mistakes, and things can and will be set in this way by mistake. By making it explicit if something is multi-value, we can apply validations when users attempt to set invalid values.

Annotations are not required

Even if we discard all the aforementioned problems, we're still left with another: Annotations are not required to create a user attribute. This means that in the worst case scenario the client only has the following metadata to go on:

{
  "name": "phoneNumbers",
  "displayName": "Phone Numbers",
  "required": false,
  "readOnly": false
}

With this information, essentially nothing valid can be determined and we will likely just have to assume this is a multi-valued text.

I hope this elaborates as to why I think we need to be able to configure attributes as multi-value (or not) and ensure that data is validated correctly.

[jonkoops]

@antikalk I think the discussion above might also be relevant for you, considering you logged the original issue asking for support for a similar feature.

[pedroigor]

@jonkoops I'm not talking about the built-in inputType annotation necessarily. As I mentioned, you can always have your own.

If we choose another annotation for the admin console, that would probably be documented so that 3rd-party admin clients can operate in the same way.

In your example, we could just add a multivalued annotation to use in conjunction with html5-tel. From the UI and an admin perspective is just a switch that behind the scenes configures this annotation. As you have annotations as metadata, you can create a component that operates on top of this annotation and render a field as single or multi-valued.

I see your point but I'm trying to leverage what we already have to support the requirements you are asking for. If we include a multivalued field it would mean we need to:

• Have validations to make sure a single-valued field is never added with multiple-values
• Deal with cases where the attribute was created as multivalued and afterward changed to single-valued. Which value do we need to return?

I mean, it is additional complexity on the server side that can be avoided while still addressing the same requirements. From an implementation perspective, we are basically checking the location from where you check if an attribute is multivalued.

For the server, all attributes are multivalued and annotations are the way to indicate intent to the UI.

[antikalk]

@jonkoops thanks for the examples, this is exactly what I am looking for! 👍

@pedroigor imo it would also make total sense to have it in an annotation, to indicate the intend that this field is multivalued.

But, please correct me if I am wrong, annotations do not have any impact on how the server side validates the input, right? E.g. if I am using html5-tel inputType it is not validated on the server side that the input is really a phone number. You would have to add a validator for that additionally. Which I don't think is too much of an issue - as this can just simply be configured once.

But for the multivalue this would become tricky: As the default would be "multivalued": false or the annotation not being set at all you would have to add a "singleValueValidator" to every field that is not multivalued if you want to make sure that not multiple values are stored.

[pedroigor]

But, please correct me if I am wrong, annotations do not have any impact on how the server side validates the input, right?

Yeah, the server is blind about the annotations set to an attribute. It adds no value to the processing of the user profile and its attributes.

As the default would be "multivalued": false or the annotation not being set at all you would have to add a "singleValueValidator" to every field that is not multivalued if you want to make sure that no multiple values are stored.

It is also tricky when you have multivalued: true and now you change the attribute to be single-valued. I'm not sure what the server should do in this case. If we want to add validation for multivalued, IMO that would be a built-in validation. Therefore, the proposal from @jonkoops to make it a first-class citizen property in the attribute metadata makes sense.

However, as I mentioned before, we are also going to face other tricky situations. For instance, if you create a field as multivalued and then switch to single-valued, what the server should do? Truncate the values? Which values the server should ignore/remove? Does the admin need to first remove values and then switch to single-valued?

That is why I think we can start simple and rely on annotations as much as possible to drive UI rendering. Completely decoupling the server (to which attributes are always multivalued) from the client. In your example, we could have something in the UI that automatically adds/removes a validator depending on how the multivalued attribute is set.

[jonkoops]

@pedroigor I agree with your assessment that this is definitely an enhancement we can do later. Let's wait until after we have stabilized User Profile as a feature before we start to consider what the implementation for this should be.

[mposolda]

+1 for something like having multivalued property on user-profile configuration for individual attributes.

Especially since some of our validators are validating just the 1st value - like for example RegistrationUsernameExistsValidator calling String value = values.get(0); and working just with the very 1st value and ignoring the other values. And ignoring all the values besides the 1st can have some unexpected side-effects...

I can see that migration can be the concern as mentioned already. For example when someone switches attribute foo from multivalued: true to multivalued: false, but attribute foo already has multiple values for user john. I think that when this is switched, we should enforce that update of user john won't be allowed (either by Account REST API or by admin REST API) if the JSON sent in the UPDATE request contains multiple values of attribute foo. This will give user/admin to adapt to the change and make sure that he is able to update user just with valid content (so single value for attribute foo).
I suppose that for our UI layers (both Account UI and admin UI), we can have annotations, which will allow only to make sure that single value is rendered for such attribute and hence also update request will contain just the single value, which was rendered in UI? So update of such user will be easily possible unless I am missing something.

[jonkoops]

I suppose that for our UI layers (both Account UI and admin UI), we can have annotations, which will allow only to make sure that single value is rendered for such attribute and hence also update request will contain just the single value, which was rendered in UI? So update of such user will be easily possible unless I am missing something.

Yeah, I think we would simply truncate the existing value if an attribute is not multivalued, but has multiple values assigned to it. We already have logic in the front-end to determine an input type based on the length of the values, but this of course quite a bit more error prone to determine than an explicit field.

[pedroigor]

@jonkoops After talking with @mposolda, we can continue the discussion about this one once we deliver the feature. We also need to review server-side code because we lack consistency when handling attribute values. For instance, #23539 (comment).

We are thinking about moving this one to a follow-up epic.

[jonkoops]

Sounds good to me @pedroigor

[mposolda]

Removing the milestone 24 for now. This was added to the epic with "User profile additional capabilities" - #25542 . We may revisit this (together with other things) as a follow-up task for user profile

[ahus1]

I just tried out user profiles for a demo, and I found the following behavior in the admin UI when changing a newly added user attribute:

• If the database already stores a multi-value field with two entries, in the admin UI I can remove that second entry and save it.
• Once I removed everything but the last entry, the UI changes and I can no longer add additional values.

I didn't add any validations or attributes, as I didn't find any one that looked to me like it would influence this behavior.

I then searched the open issues and found that seems to relate to multivalued fields, and that it is no longer planned for KC24, which surprises me as users won't be able to undo changes as described above.

So I wonder if the the behavior I describe in this comment is related to this issue, or if it should be a new bug.

[pedroigor]

@ahus1 Can you please open a new bug? I think it is somewhat related but it does not necessarily mean we need to introduce changes to the configuration schema or the SPI.

[jonkoops]

This bug was already reported several times, I don't think it can be fixed unless a concept of explicit multi-value is introduced. The algorithm used to determine whether a field is multi-valued or not on the client side is flawed, and needs to infer this from a lack of detail. We can keep changing the default behaviour all we want (we already had 4 PRS to do so), but it will always be a workaround and result in undefined and unwanted behaviour.

[ahus1]

@pedroigor - given @jonkoops's comment, I won't open a new issue. Still, I think this should be considered high priority (a blocker?) for user attribute handling in KC24 as handling multi-values attributes is IMHO not working before this is implemented

I think is a regression compared to previous versions of Keycloak unless someone can describe me how users will be able to work around this.

cc: @mposolda