Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-4575

IM Person doc should only allow editing of direct group memberships

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.0.3
    • Fix Version/s: 1.0.3
    • Component/s: Development
    • Labels:
      None
    • Similar issues:
      KULRICE-8542Person, role & group docs should not allow multiple docs editing the same record to be saved at the same time
      KULRICE-11043Person document goes into exception when attempting to deactivate a Person with a single group membership
      KULRICE-8337Adding line to group membership on person record causes NPE
      KULRICE-4368On the Group Document, if you forget to enter description, when you submit and get the validation errors it messes up the membership list
      KULRICE-6858If editing groups or roles, it validates the existence of all members before saving the doc, even those which are "inactive"
      KULRICE-4516KimDocumentRoleMemberLookupableHelperServiceImpl confused by multiple docs
      KULRICE-4538KIM Group maintenance screens allow creation of circular group memberships
      KULRICE-4812cannot add a principal to a group when principal is already a non direct memeber of that group
      KULRICE-8342Person document incorrectly validating "unique" attributes on role memberships
      KULRICE-7661AdHoc Group request - isAddHocRouteWorkgroupValid() method should validate the groups permission and it is not.
    • Rice Module:
      KIM
    • Application Requirement:
      KFS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required

      Description

      Currently, the UIDocumentServiceImpl#loadEntityToPersonDoc method pulls group memberships from GroupService#getGroupsForPrincipal. While wildly useful in other contexts (such as finding out which responsiblities a principal has through a group), in this context it's deadly: getGroupsForPrincipal returns all memberships in groups and all memberships implied by groups being members in other groups.

      Basically, if principal kmoutlaw is a member of group A, and group A is a member of group B, then when you do an IM Person doc on kmoutlaw, the memberships for both group A and group B are shown as editable. Editors might miss this and simply save the doc, thereby making kmoutlaw a direct member of group A and group B. The second time we open an IM person doc on kmoutlaw, both the direct membership to group B and the indirect membership (through group A) to group B are editable...which leads to optimistic lock exceptions.

      On the IM Person doc, only direct group memberships should be editable.

      Note from James on how to fix this issue (listed on KFSMI-5987):
      James Smith added a comment - 13/Sep/10 05:20 PM
      To fix said issue, I changed this line in UIDocumentServiceImpl#loadEntityToPersonDoc:

      List<? extends Group> groups = getGroupService().getGroupsForPrincipal(identityManagementPersonDocument.getPrincipalId());

      to this:

      List<? extends Group> groups = getGroupsByIds(getGroupService().getDirectGroupIdsForPrincipal(identityManagementPersonDocument.getPrincipalId()));

      and added this method (all of this is in UA's override of that service):

      /**

      • Looks up GroupInfo objects for each group id passed in
      • @param groupIds the List of group ids to look up GroupInfo records on
      • @return a List of GroupInfo records
        */
        protected List<? extends Group> getGroupsByIds(List<String> groupIds) {
        List<GroupInfo> groups = new ArrayList<GroupInfo>();
        for (String groupId : groupIds) { final GroupInfo groupInfo = getGroupService().getGroupInfo(groupId); groups.add(groupInfo); }

        return groups;
        }

        Issue Links

          Activity

          Hide
          Jeremy Hanson added a comment -

          change committed

          Show
          Jeremy Hanson added a comment - change committed
          Hide
          Jeremy Hanson added a comment -

          Currently, non direct group memberships are not shown on the IM Person document. Not sure when this was implemented, but it does check to make sure the principalId is a direct member before adding it to the doc.

          That said, the current way of doing this looks incredibly complicated for what it does.
          It currently gets all groups for a principal id, which returns non-direct groups.
          Then it grabs the direct members for the each of these groups and loops through them comparing them to the principalID
          If the direct member of the group matches the principal id, the group is added to the document.

          So I think I'm going to updated this code to James' solution as it should be more efficient.

          Show
          Jeremy Hanson added a comment - Currently, non direct group memberships are not shown on the IM Person document. Not sure when this was implemented, but it does check to make sure the principalId is a direct member before adding it to the doc. That said, the current way of doing this looks incredibly complicated for what it does. It currently gets all groups for a principal id, which returns non-direct groups. Then it grabs the direct members for the each of these groups and loops through them comparing them to the principalID If the direct member of the group matches the principal id, the group is added to the document. So I think I'm going to updated this code to James' solution as it should be more efficient.
          Hide
          Dan Lemus (Inactive) added a comment -

          Not sure if this should be listed more as a contribution, or fix. But the appropriate fix has been added to the description field of this JIRA.

          Show
          Dan Lemus (Inactive) added a comment - Not sure if this should be listed more as a contribution, or fix. But the appropriate fix has been added to the description field of this JIRA.

            People

            • Assignee:
              Jeremy Hanson
              Reporter:
              Dan Lemus (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Structure Helper Panel