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

Martin Maher gentoo at penguindreams.us
Wed Sep 25 14:35:31 EDT 2019 

Stian, et al:

It took me a few minutes to read through the entire thread and your last remarks strike me as a good path forward.  So +1 for that.

The follow-on thought I have about this is that mapping, and therefore per mapper configuration, would best be done on a per realm/domain level.  This would address the federation concerns, but it would also mean that there might be an LDAP mapper for the @firstDomain with a different configuration from an LDAP mapper for @secondDomain.

Proceeding along this path, if you have local admins of the Keycloak system, role, attribute and identity mapping, then they could be controlled separately from some realm/domain for which the Keycloak system has user responsibilities.  This might necessitate a “local” option that points to the Keycloak internal data, which would give you four options -

Import
Owner
Force
Local

This same method could also be utilized to resolve issues of collision of nominated privilege.  I have seen cases in the past, with other systems, where privilege collisions occur because a userID exists in two systems, which each nominate a differing level of privilege and may utilize the same token names for a privilege, e.g., Attribute= fwAdmin / value= superAdmin.

Respectfully submitted,

Martin

> On Sep 25, 2019, at 05:23 , Stian Thorgersen <sthorger at redhat.com> 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.
> 
> 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.
> 
> 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
>>>> 
>>>> 
> _______________________________________________
> 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