Captain-P-Goldfish / scim-for-keycloak

a third party module that extends keycloak by SCIM functionality
BSD 3-Clause "New" or "Revised" License
183 stars 47 forks source link

Azure AD group member patch add issue #31

Closed jerboa-io closed 2 years ago

jerboa-io commented 2 years ago

Azure AD group membership from PATCH requests look like they are not being handled as the memberships are not updating in keycloak and the admin events aren't being added.

https://docs.microsoft.com/en-us/azure/active-directory/app-provisioning/use-scim-to-provision-users-and-groups#update-group-add-members

heres a request from azure aad

[de.captaingoldfish.scim.sdk.keycloak.scim.ScimEndpoint] (default task-2) scim requestURL: https://realm.domain.com/auth/realms/realm/scim/v2/Groups/b03edd26-a054-4e46-abf3-98b8fc177faa

[de.captaingoldfish.scim.sdk.keycloak.scim.ScimEndpoint] (default task-2) scim body: {"schemas":["urn:ietf:params:scim:api:messages:2.0:PatchOp"],"Operations":[{"op":"add","path":"members","value":[{"value":"132304dd-5616-4d48-845c-b0dec4c6658d"}]}]}

Captain-P-Goldfish commented 2 years ago

Yes I can exactly tell what the problem is. I should add an appropriate log-message.

The problem here is that it is possible to add Groups as members and Users as members to the group-resource. You request does not define which type the id is referencing. There are currently two possible alternatives to let the API know if the resource should be a user-resource or a group-resource.

  1. add the type attribute with either of these values Group or User.
  2. add the $ref attribute with the fully qualified reference-URL to the resource

Other possibilities do not exist currently

arheom commented 1 year ago

If it helps, we solved this issue by adding one method inside the GroupHandler.java file, in case the type attribute is not set, to try to set it using the existing Keycloak data, assuming the users and groups are added before the associations, which worked well so far. Whenever the AAD tries to create / update groups, then we decorate the Group object with the type, if not present. And this implementation won't change anything from the actual logic when the type is provided, just an extra call to Keycloak.

private void updateMembersType(ScimKeycloakContext scimKeycloakContext, Group group)
  {
    KeycloakSession keycloakSession = scimKeycloakContext.getKeycloakSession();
    RealmModel realm = keycloakSession.getContext().getRealm();
    List<String> groupIds = keycloakSession.groups()
                                           .getGroupsStream(realm)
                                           .map(g -> g.getId())
                                           .collect(Collectors.toList());

    group.getMembers().forEach(groupMember -> {
      if (!groupMember.getType().isPresent())
      {
        // the type is not set -> find the right type
        groupMember.setType((groupIds.contains(groupMember.getValue().get()) ? "Group" : "User"));
      }
    });
  }