2:56 a.m.
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(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-user