[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