[keycloak-user] identity broker role mapping bug?

Simon Payne simonpayne58 at gmail.com
Wed Nov 8 05:30:00 EST 2017


i made the change on our system by rebuilding the keycloak-service at
3.2.1.final jar and deployed separately.  The code was changed as follows
and after some testing appears to provide the functionality we need.  I can
open a Jira ticket no problem as it would be useful to get it pushed into
the main build, as we have since upgraded to 3.3.0.final and lost the
changes.

@Override
public void importNewUser(KeycloakSession session, RealmModel realm,
UserModel user, IdentityProviderMapperModel mapperModel,
BrokeredIdentityContext context) {
    mapRole(realm, user, mapperModel, context);
}

@Override
public void updateBrokeredUser(KeycloakSession session, RealmModel
realm, UserModel user, IdentityProviderMapperModel mapperModel,
BrokeredIdentityContext context) {
    mapRole(realm, user, mapperModel, context);

}

private void mapRole(RealmModel realm, UserModel user,
IdentityProviderMapperModel mapperModel, BrokeredIdentityContext
context) {

    String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE);
    RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName);
    if (role == null) throw new IdentityBrokerException("Unable to
find role: " + roleName);

    if (hasClaimValue(mapperModel, context)) {
        user.grantRole(role);
    }else{
        user.deleteRoleMapping(role);
    }
}


On Mon, Nov 6, 2017 at 8:56 AM, Marek Posolda <mposolda at redhat.com> wrote:

> It makes sense to me. Feel free to create JIRA and ideally also submit PR
> with the test.
>
> Just one note: Make sure that "user.grantRole" is called during upgrade
> just in case that user doesn't yet have this role. I guess you would need
> to add some additional check like:
>
> if (!user.hasRole(role)) {
>     user.grantRole(role);
> }
>
> That's because of performance (Calling of user.grantRole always
> invalidates the user in the cache and require to reload him from DB, which
> is not needed if he has the role already).
>
> Marek
>
>
> On 26/10/17 11:54, Simon Payne wrote:
>
>> further to this, i think i've located the responsible code in the class
>> org.keycloak.broker.oidc.mappers.ClaimToRoleMapper, which in my opinion
>> doesn't perform the expected behavior as previously highlighted.  I'm
>> making a couple of assumptions that importNewUser method only gets called
>> on first broker login and that updateBrokeredUser gets called on every
>> login thereafter.
>>
>> we can see in the code below that importNewUser grants the role if the
>> claim is found - which is fine.  However, updateBrokeredUser only removes
>> if not found in the claim.
>>
>> I'm proposing that we alter the method updateBrokeredUser to include the
>> addition of the role as well.  I can't see how anyone is currently using
>> this code in any meaningful way as currently the code doesn't support the
>> addition of role after first login.
>>
>> any thoughts?  i'm happy to make the changes myself.
>>
>> Simon.
>>
>>
>>
>> @Override
>> public void importNewUser(KeycloakSession session, RealmModel realm,
>> UserModel user, IdentityProviderMapperModel mapperModel,
>> BrokeredIdentityContext context) {
>>      String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE);
>>      if (hasClaimValue(mapperModel, context)) {
>>          RoleModel role = KeycloakModelUtils.getRoleFromString(realm,
>> roleName);
>>          if (role == null) throw new IdentityBrokerException("Unable to
>> find role: " + roleName);
>>          user.grantRole(role);
>>      }
>> }
>>
>> @Override
>> public void updateBrokeredUser(KeycloakSession session, RealmModel
>> realm, UserModel user, IdentityProviderMapperModel mapperModel,
>> BrokeredIdentityContext context) {
>>      String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE);
>>      if (!hasClaimValue(mapperModel, context)) {
>>          RoleModel role = KeycloakModelUtils.getRoleFromString(realm,
>> roleName);
>>          if (role == null) throw new IdentityBrokerException("Unable to
>> find role: " + roleName);
>>          user.deleteRoleMapping(role);
>>      }
>>
>> }
>>
>>
>>
>>
>> On Wed, Oct 25, 2017 at 2:46 PM, Simon Payne <simonpayne58 at gmail.com>
>> wrote:
>>
>> Hi, i think i may have found a bug in the identity provider mapping of
>>> claims to roles.
>>>
>>> it appears that if i have an identity provider with claims in the token,
>>> which i want to map to a role in the identity broker, then it only does
>>> this once during the first time login.  if i remove the claim from the
>>> identity provider token, then this successfully removes it from the
>>> broker
>>> - but never remaps if i then add it again.
>>>
>>> the scenario i am trying to create here is that the identity provider is
>>> responsible for authentication where active directory groups appears as
>>> claim in the token.  the broker then map this claim to the role providing
>>> the authorization.
>>>
>>> this behaviour appears to be the same whether i map a broker role to a
>>> custom claim or a realm role in the provider token.
>>>
>>> hope this makes sense, thanks
>>>
>>> Simon.
>>>
>>>
>>>
>>> _______________________________________________
>> keycloak-user mailing list
>> keycloak-user at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-user
>>
>
>
>


More information about the keycloak-user mailing list