[keycloak-dev] A newly added Hardcoded Role mapper ignores users that have already logged in before

Stian Thorgersen sthorger at redhat.com
Fri Sep 27 06:57:46 EDT 2019 

I'll get a design proposal written up around this, then we have something
to refer to on how we want to address this issue.

On Fri, 27 Sep 2019 at 12:57, Stian Thorgersen <sthorger at redhat.com> wrote:

>
>
> On Thu, 26 Sep 2019 at 11:52, Marek Posolda <mposolda at redhat.com> wrote:
>
>> On 25. 09. 19 14:23, Stian Thorgersen wrote:
>>
>> Thinking about this some more. I think all mappers for identity brokering
>> should have an option to select sync-mode:
>>
>> * import - only update on first import
>> * owner - if the idp is the owner of the user (the user has not been
>> modified within KC, and only has a single IdP link)
>> * force - always update
>>
>> The mappers that today has a behaviour that doesn't match one of the
>> above could have an option "legacy".
>>
>>
>> On the IdP config itself there should be a default sync-mode user for
>> mappers that haven't explicitly set the sync-mode. The default value should
>> be import.
>>
>> +1 for this part. With the small note that not all mapper implementations
>> may need this option. For example UsernameTemplateMapper.
>>
>
> What does UsernameTemplateMapper actually do? Perhaps for those where it
> doesn't make sense to have an option it should display the sync-mode, but
> in non-editable mode?
>
>
>> The next piece of the puzzle would be to prevent editing of values that
>> shouldn't be possible to edit locally. For user attributes that should be
>> driven by User Profile SPI, where it would be somehow possible to for
>> example say don't allow editing this attribute if one of the following
>> IdPs. I'm working on a design proposal for the User Profile SPI currently,
>> so we can add that as a requirement there. The same feature could be used
>> for User Federation providers. For roles it is a bit harder, but would be
>> nice to somehow be able to flag what roles are managed by IdP/user-storage
>> and which are not. Perhaps we could add some metadata to the role mapping
>> for that.
>>
>> Yes, will be great to have this capability on User Profile SPI.
>>
>> I am not sure about metadata for role/group mappings. There will be some
>> added complexity in that and not sure it deserves the benefits - also
>> considering that role/group memberships is something, which is editable
>> just by administrator and not by user himself in account console.
>>
> Remember that very often there will be one person configuring Keycloak and
> setting up the IdPs and a completely different person managing users and
> roles/groups. I think it would be nice to be able to show that a role/group
> mapping is managed externally, but I agree it's not the top priority.
>
>> Regarding User Profile SPI, I can imagine that it will be nice to have
>> the ability to automatically update the metadata/requirements based on some
>> events? For example when we add support for "temporary" users registered
>> through IDP, those users will be typically read-only. So I can imagine that
>> when administrator creates IDP with temporary users, there will be an
>> event, which will automatically add rules to User Profile SPI that all
>> attributes of such user should be read-only. Similarly when you have
>> read-only LDAP provider or SSSD provider, you may also want auto-created
>> User Profile metadata specifying that users should be non-editable. I am
>> not sure whether to count with this in the User Profile SPI design or not?
>>
> Not sure. Right now I've not really covered this part in the user profile
> design as I haven't really thought it through. I'm kinda thinking about 2
> options. First 1 where attributes can have different config based on the
> source, and another where you define different user profiles. The latter is
> probably simpler and cleaner. Let's move further discussion on this part
> elsewhere though.
>
>> Marek
>>
>>
>> Would be great to start on a design proposal around this, so we can have
>> it documented the way it should work. Once we have that and have agreed on
>> the approach I don't mind having PRs for individual mappers merged as I
>> appreciate the fact that for this case you want a solution for hardcoded
>> role mapper quickly without having to do all of the work for all the other
>> mappers.
>>
>> On Wed, 25 Sep 2019 at 12:36, Stian Thorgersen <sthorger at redhat.com>
>> wrote:
>>
>>> https://issues.jboss.org/browse/KEYCLOAK-8690
>>>
>>> Adding to my point that we need a consistent solution/strategy for all
>>> mappers.
>>>
>>> On Wed, 25 Sep 2019 at 12:32, Stian Thorgersen <sthorger at redhat.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, 25 Sep 2019 at 09:42, EXTERNAL Thiele Frank (TNG,
>>>> INST-CSS/BSV-OS2) <external.Frank.Thiele at bosch-si.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>>
>>>>> That is an interesting point. I checked some mappers:
>>>>>
>>>>> -        AttributeToRoleMapper handles the update like the import
>>>>> with the exception that in case of an update, the role is deleted if the
>>>>> attribute is no longer present (I call this for now inverted logic).
>>>>>
>>>>> -        ClaimToRoleMapper and ExternalKeycloakRoleToRoleMapper
>>>>> handle an update with the inverted logic only – so they don’t set but only
>>>>> delete the role.
>>>>>
>>>>> -        HardcodeRoleMapper fully ignores updates whereas it could at
>>>>> least do it the same way as AttributeToRoleMapper.
>>>>>
>>>>> -        UserAttributeMapper is even more complex…
>>>>>
>>>>> So the currently used IdentityProviderMapper implementations are very
>>>>> inconsistent and hopefully documented and understood well for and by the
>>>>> end users.
>>>>>
>>>>
>>>> It is not documented and I doubt anyone can understand how it will
>>>> function. This is my concern when we have "random" things happening in each
>>>> mapper without an overall consistent plan.
>>>>
>>>>
>>>>>
>>>>>
>>>>> All I am saying is that it will become a breaking change to globally
>>>>> define this behavior as there are nowadays several, conflicting modes
>>>>> implemented. Due to that I would like to emphasize that the flag
>>>>> introduction (“handleUpdateToo”) still seems as the solution with the
>>>>> lowest friction.
>>>>>
>>>>
>>>> Adding a flag to an individual mapper is just a solution to your
>>>> problem and it doesn't address the wider issue. It is basically a
>>>> work-around for your use-case, and is introducing yet another behaviour on
>>>> top of the already inconsistent behaviour we have today.
>>>>
>>>> Of course we need anything new that is added to be able to match the
>>>> current behaviour, which will be more and more difficult the more random
>>>> switches and config options we have in mappers. So we really do need to
>>>> have a proper solution in thought out, then figure out how to address it.
>>>>
>>>>
>>>>>
>>>>>
>>>>> Mit freundlichen Grüßen / Best regards
>>>>>
>>>>>
>>>>> *Frank Thiele *
>>>>> Open Source Services 2 - Product Group Customer Success Services
>>>>> (INST-CSS/BSV-OS2)
>>>>> Bosch Software Innovations GmbH | Ullsteinstr. 128 | 12109 Berlin |
>>>>> GERMANY | www.bosch-si.com
>>>>> Tel. +49 30 726112-0 | Fax +49 30 726112-100 |
>>>>> external.Frank.Thiele at bosch-si.com
>>>>>
>>>>> Sitz: Berlin, Registergericht: Amtsgericht Charlottenburg; HRB 148411 B
>>>>> Aufsichtsratsvorsitzender: Dr.-Ing. Thorsten Lücke; Geschäftsführung:
>>>>> Dr. Stefan Ferber, Michael Hahn, Dr. Aleksandar Mitrovic
>>>>>
>>>>>
>>>>>
>>>>> *Von:* Stian Thorgersen <sthorger at redhat.com>
>>>>> *Gesendet:* Mittwoch, 25. September 2019 08:38
>>>>> *An:* Marek Posolda <mposolda at redhat.com>
>>>>> *Cc:* Schuster Sebastian (INST-CSS/BSV-OS2) <
>>>>> Sebastian.Schuster at bosch-si.com>; EXTERNAL Thiele Frank (TNG,
>>>>> INST-CSS/BSV-OS2) <external.Frank.Thiele at bosch-si.com>;
>>>>> keycloak-dev at lists.jboss.org
>>>>> *Betreff:* Re: [keycloak-dev] A newly added Hardcoded Role mapper
>>>>> ignores users that have already logged in before
>>>>>
>>>>>
>>>>>
>>>>> Adding config options on a single mappers is not really a great
>>>>> solution. We need to make sure there is a consistent approach throughout.
>>>>> We probably don't have consistent and predictable behaviour today, but I
>>>>> would rather not make it worse by introducing random config options on
>>>>> mappers.
>>>>>
>>>>>
>>>>>
>>>>> Main question is if this should be controlled on individual mappers or
>>>>> if it should mainly be a config on the identity provider.
>>>>>
>>>>>
>>>>>
>>>>> Having the config on the identity provider would make more sense as it
>>>>> would be simpler to configure and it would avoid corner cases.
>>>>>
>>>>>
>>>>>
>>>>> There's probably at least 3 different modes for identity brokering
>>>>> that should be supported:
>>>>>
>>>>>
>>>>>
>>>>> 1) Import only - User is only imported if it doesn't exist. If user
>>>>> already exists nothing is updated.
>>>>>
>>>>> 2) Sync - Allow changes to the user within Keycloak, but also sync
>>>>> changes from external IdP
>>>>>
>>>>> 3) External - Do not allow any changes to the user within Keycloak as
>>>>> the user should be fully managed from the external IdP
>>>>>
>>>>>
>>>>>
>>>>> Option 1 is trivial.
>>>>>
>>>>>
>>>>>
>>>>> Option 2 can be very complicated. Take the example of the hardcoded
>>>>> role for instance. User first logs in, the role is added. An admin then
>>>>> removes the role from the user. User logs in again and the role is
>>>>> re-added. Same example can be applied to last name for instance. User logs
>>>>> in. 6 months later the user changes the last name in Keycloak account
>>>>> console, but then next day when they re-login the last name is changed back.
>>>>>
>>>>>
>>>>>
>>>>> Option 3 is relatively trivial, but would need some tweaks within
>>>>> Keycloak. A user that is fully externally managed should not be able to use
>>>>> Keycloak account console and should be view-only in the admin console.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mon, 23 Sep 2019 at 22:21, Marek Posolda <mposolda at redhat.com>
>>>>> wrote:
>>>>>
>>>>> Makes sense to me. From me +1 for this.
>>>>>
>>>>> Marek
>>>>>
>>>>> On 20. 09. 19 15:57, Schuster Sebastian (INST-CSS/BSV-OS2) wrote:
>>>>> > I guess the point was just to add a configuration flag to the mapper
>>>>> enabling the update on existing users.
>>>>> > If that flag is not there or set to false, the old behavior stays.
>>>>> >
>>>>> > Best regards,
>>>>> > Sebastian
>>>>> >
>>>>> >
>>>>> > Mit freundlichen Grüßen / Best regards
>>>>> >
>>>>> > Dr.-Ing. Sebastian Schuster
>>>>> >
>>>>> > Open Source Services (INST-CSS/BSV-OS2)
>>>>> > Bosch Software Innovations GmbH | Ullsteinstr. 128 | 12109 Berlin |
>>>>> GERMANY | www.bosch-si.com
>>>>> > Tel. +49 30 726112-485 | Mobil +49 152 02177668 | Telefax +49 30
>>>>> 726112-100 | Sebastian.Schuster at bosch-si.com
>>>>> >
>>>>> > Sitz: Berlin, Registergericht: Amtsgericht Charlottenburg; HRB
>>>>> 148411 B
>>>>> > Aufsichtsratsvorsitzender: Dr.-Ing. Thorsten Lücke;
>>>>> Geschäftsführung: Dr. Stefan Ferber, Michael Hahn, Dr. Aleksandar Mitrovic
>>>>> >
>>>>> >
>>>>> >
>>>>> > -----Ursprüngliche Nachricht-----
>>>>> > Von: keycloak-dev-bounces at lists.jboss.org <
>>>>> keycloak-dev-bounces at lists.jboss.org> Im Auftrag von Stian Thorgersen
>>>>> > Gesendet: Freitag, 20. September 2019 15:25
>>>>> > An: EXTERNAL Thiele Frank (TNG, INST-CSS/BSV-OS2) <
>>>>> external.Frank.Thiele at bosch-si.com>
>>>>> > Cc: keycloak-dev at lists.jboss.org
>>>>> > Betreff: Re: [keycloak-dev] A newly added Hardcoded Role mapper
>>>>> ignores users that have already logged in before
>>>>> >
>>>>> > I'm afraid you've lost me on the last one as I'm not following ;)
>>>>> >
>>>>> > On Thu, 19 Sep 2019 at 16:17, EXTERNAL Thiele Frank (TNG,
>>>>> INST-CSS/BSV-OS2) <external.Frank.Thiele at bosch-si.com> wrote:
>>>>> >
>>>>> >> Hi,
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> What if I implement a newer version of the Hardcoded Role mapper
>>>>> that
>>>>> >> has a (optional, as configuration migration case) flag to activate
>>>>> >> update handling. So when the flag is set to false or not set at all
>>>>> >> (migration case), then behavior is as of today. If the flag is set,
>>>>> >> the import and update functions behave the same way.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Mit freundlichen Grüßen / Best regards
>>>>> >>
>>>>> >>
>>>>> >> *Frank Thiele *
>>>>> >> Open Source Services 2 - Product Group Customer Success Services
>>>>> >> (INST-CSS/BSV-OS2)
>>>>> >> Bosch Software Innovations GmbH | Ullsteinstr. 128 | 12109 Berlin |
>>>>> >> GERMANY | www.bosch-si.com Tel. +49 30 726112-0 | Fax +49 30
>>>>> >> 726112-100 | external.Frank.Thiele at bosch-si.com
>>>>> >>
>>>>> >> Sitz: Berlin, Registergericht: Amtsgericht Charlottenburg; HRB
>>>>> 148411
>>>>> >> B
>>>>> >> Aufsichtsratsvorsitzender: Dr.-Ing. Thorsten Lücke;
>>>>> Geschäftsführung: Dr.
>>>>> >> Stefan Ferber, Michael Hahn, Dr. Aleksandar Mitrovic
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> *Von:* Stian Thorgersen <sthorger at redhat.com>
>>>>> >> *Gesendet:* Donnerstag, 19. September 2019 13:51
>>>>> >> *An:* EXTERNAL Thiele Frank (TNG, INST-CSS/BSV-OS2) <
>>>>> >> external.Frank.Thiele at bosch-si.com>
>>>>> >> *Cc:* keycloak-dev at lists.jboss.org
>>>>> >> *Betreff:* Re: [keycloak-dev] A newly added Hardcoded Role mapper
>>>>> >> ignores users that have already logged in before
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> If memory serves me correctly this was on purpose where the
>>>>> thinking 5
>>>>> >> years ago was that users would be imported on first login, then
>>>>> >> managed from Keycloak after that. That is not always the case though
>>>>> >> and we should have some way of controlling if users updated on
>>>>> >> subsequent logins and perhaps also be able to fine-tune what is
>>>>> updated.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Thu, 19 Sep 2019 at 13:21, EXTERNAL Thiele Frank (TNG,
>>>>> >> INST-CSS/BSV-OS2) <external.Frank.Thiele at bosch-si.com> wrote:
>>>>> >>
>>>>> >> Hello,
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> In our project, we use the "Hardcoded role" mapper within a
>>>>> configured
>>>>> >> Identity Provider (also a Keycloak instance, in our case the same
>>>>> but
>>>>> >> a different realm) to describe that each user logging in via
>>>>> Keycloak
>>>>> >> shall be given a certain role.
>>>>> >>
>>>>> >> This works perfectly if the mapper is configured before the first
>>>>> >> login of the user. The configured role is granted to the (cloned)
>>>>> user
>>>>> >> when he logs in the first time via Keycloak.
>>>>> >>
>>>>> >> But when another "Hardcoded role" mapper is added to configure
>>>>> another
>>>>> >> role, then the user is not given the other role when he logs in.
>>>>> Only
>>>>> >> new users logging in the first time get both roles assigned.
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Is this on purpose or a bug?
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Mit freundlichen Grüßen / Best regards
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Frank Thiele
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Open Source Services 2 - Product Group Customer Success Services
>>>>> >> (INST-CSS/BSV-OS2) Bosch Software Innovations GmbH | Ullsteinstr.
>>>>> 128
>>>>> >> |
>>>>> >> 12109 Berlin | GERMANY | www.bosch-si.com<http://www.bosch-si.com<
>>>>> >> http://www.bosch-si.com%3chttp:/www.bosch-si.com>>
>>>>> >>
>>>>> >> external.Frank.Thiele at bosch-si.com<mailto:
>>>>> >> external.Frank.Thiele at bosch-si.com<mailto:
>>>>> >> external.Frank.Thiele at bosch-si.com%
>>>>> >> 3cmailto:external.Frank.Thiele at bosch-si.com>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Sitz: Berlin, Registergericht: Amtsgericht Charlottenburg; HRB
>>>>> 148411
>>>>> >> B
>>>>> >>
>>>>> >> Aufsichtsratsvorsitzender: Dr.-Ing. Thorsten Lücke;
>>>>> Geschäftsführung: Dr.
>>>>> >> Stefan Ferber, Michael Hahn, Dr. Aleksandar Mitrovic
>>>>> >>
>>>>> >> _______________________________________________
>>>>> >> keycloak-dev mailing list
>>>>> >> keycloak-dev at lists.jboss.org
>>>>> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>> >>
>>>>> >>
>>>>> > _______________________________________________
>>>>> > keycloak-dev mailing list
>>>>> > keycloak-dev at lists.jboss.org
>>>>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>> >
>>>>> > _______________________________________________
>>>>> > keycloak-dev mailing list
>>>>> > keycloak-dev at lists.jboss.org
>>>>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>>
>>>>>
>>

More information about the keycloak-dev mailing list