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

Wed Sep 25 06:36:55 EDT 2019

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
>>
>>