Re: [keycloak-user] org.keycloak.broker.oidc.mappers.ClaimToRoleMapper does not update user roles

Hi, I think the JIRA for this already exists and we want to improve in this area. One thing is, that the actual call of updating UserModel should be done just if user is not already in that role. Otherwise we will have unecessary DB calls and cache invalidations during each broker login. I think this was already discussed before. So feel free to add to that JIRA or even send PR for this. Thanks, Marek On 15/10/18 15:18, Philippe Gauthier wrote: > I Simon. > > > I posted the question on the mailing list. > > > I looked in the Keycloak devel branch on github and the code is still > the same as you posted last year. > > > We have a support contract with RedHat. Maybe I can use this path to > open the case? > > > Thankyou. > > > Philippe. > > ________________________________ > De : Simon Payne <simonpayne58(a)gmail.com&gt; > Envoyé : 15 octobre 2018 09:09:33 > À : Philippe Gauthier > Cc : keycloak-user; Étienne Sadio > Objet : Re: [keycloak-user] > org.keycloak.broker.oidc.mappers.ClaimToRoleMapper does not update > user roles > > Hi Philippe, > > yes i found that it wouldn't add or remove roles if the use was > already known.  I never got around to raising a Jira ticket to fix > the issue as i had some issues trying to get a dev environment up and > running - some units tests just wouldn't run for me. > > any way.. this was my solution which is running in our production, > which seems to still be working as expected.  I just rebuilt the > relevant service and deployed accordingly. > > I'm happy to work on the permanent fix.  I found it in 3.2.1 (i think > it was and it is still present in 4.3 which is the most up-to date > version we are running).  There were some additional requirements > which Marek mentioned to include in the fix, they will be in the > original thread. > >> @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); >>       } >> } > > Simon. > > > > > > > On Mon, Oct 15, 2018 at 1:46 PM Philippe Gauthier > <philippe.gauthier@inspq.qc.ca<mailto:philippe.gauthier@inspq.qc.ca>> > wrote: > Hi > > > I saw a 2017 post from Simon Payne about ClaimToRoleMapper and I > cannot find any answers for his question. > > http://lists.jboss.org/pipermail/keycloak-user/2017-October/012129.html&l... > > > > This post was about ClaimToRoleMapper class of the OIDC broker > component. This class search for a claim, check for its value and > grant a role if the value is equals to the value specified in the > configuration. > > > If the user from the IdP is not known by Keycloak, it will be created > by the First Broker Login Flow and the role will be granted. > > > If the user is already known by Keycloak, he have the role specified > by the mapper and he don't have the claim anymore, the role will be > revocated. > > > But. If the user is known by Keycloak, he don't have the role > specified by the mapper and he have the claim, Keycloak does not > grant him the role. > > > It is clear why it does this in the code but it is not clear why this > have been done that way: > > > Here is the code. > > @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); >      } >      /* Maybe we should add an else here that does what the > importNewUser does. > } > Thankyou > > Philippe Gauthier. > > _______________________________________________ > keycloak-user mailing list > keycloak-user@lists.jboss.org<mailto:keycloak-user@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/keycloak-user<https://na01.sa... > > _______________________________________________ > keycloak-user mailing list > keycloak-user(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-user