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

Wed Oct 9 06:19:17 EDT 2019

On Fri, 4 Oct 2019 at 03:35, Martin Maher <gentoo at penguindreams.us> wrote:

> Stian:
>
> >> On Oct 2, 2019, at 04:40, Stian Thorgersen <sthorger at redhat.com> wrote:
> >>
> >>
> >>
> >> On Fri, 27 Sep 2019 at 20:05, Martin Maher <gentoo at penguindreams.us>
> wrote:
> >> Stian:
> >>
> >> LDAP would be my mail client’s idea of a spellcheck joke.  It was
> familiar with LDAP but not with IdP, so it did me the “favor” of
> “correcting” the spelling “error" and I did not notice.
> >>
> >> > On Sep 27, 2019, at 03:50, Stian Thorgersen <sthorger at redhat.com>
> wrote:
> >> >
> >> >
> >> >
> >> > On Wed, 25 Sep 2019 at 20:35, Martin Maher <gentoo at penguindreams.us>
> wrote:
> >> >> 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.
> >> >>
> >> > For the record I was only talking about IdPs (identity brokering),
> not LDAP. For LDAP we have very fine-grained control already and it also
> supports read/write sync, with that regards it's quite a different problem.
> >> >
> >> > I think the configuration should be at the IdP level, with an option
> to override on individual mappers. Not sure why it would be a realm/domain
> level, or what you actually mean by that?
> >>
> >> What I mean by realm/domain is what I will call the “location
> qualifier” of the submitted userID, e.g., elwood at dev.somedomain.tld
> versus elwood at somedomain.tld versus elwood at someotherdomain.tld.  Even if
> all three were handled by the same IdP, wouldn’t it be conceivable that
> differing requirements might applicable?
> >
> > I can imagine there would be such a case, but would suggest we start
> simple with a single option on the IdP, then consider something more
> advanced later.
>
> This is a reasonable approach.  Walk before crawling and all.
>
> >> >>
> >> >> 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.
> >> >>
> >> > I'm afraid you've lost me a little. Perhaps it's because I'm talking
> about external OIDC/SAML providers, while you are talking about LDAP?
> >>
> >> What I was trying articulate was the addition of a fourth option,
> “local”, to the proposed three “import”, “owner”, and “force”, which would
> permit the user to be defined only in the Keycloak server db.  While this
> would not be particularly useful in IdP configurations, it would allow for
> a non-IdP situation, such as locally-defined administrators of the Keycloak
> system or other small scope use cases.
> >
> > Not sure I understand how that would apply to a config at the IdP level.
> Users managed fully within Keycloak would not be managed by an IdP at all,
> hence there's no need for such an option at the IdP level. Unless I'm
> missunderstanding you that is.
>
> The idea here is to prevent a user acquired from an IdP from potentially
> overriding a locally defined user.  Now that I think about the situation,
> it might be more of a preference on Keycloak that prevents overwrite or
> modification of locally-managed users by user data acquired from an IdP.
> There is every possibility that this preference already exists, or that the
> outcome I am imagining is not possible.
>

Keycloak doesn't overwrite an existing user, but rather adds a link to the
IdP. Further, a user is able to login with multiple IdPs. Before adding the
link the user is also requested to re-authenticate.

>
> >> As for my final paragraph, I put it into the response in the wrong
> place.  The thought was associated with the concept of mappers being able
> to be unique configured at the realm/domain level, and how that would lend
> itself to working out collisions of nominated privilege.
> >>
> >> Hope this helps,
> >>
> >> Martin
> >>
> >> > 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
> >> >
> >>
> >
>