[keycloak-user] identity broker role mapping bug?

[Marek Posolda]

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