Uploaded image for project: 'KFS Archive'
  1. KFS Archive
  2. KFSOLD-19812

IM Person doc should only allow editing of direct group memberships

    Details

    • Type: Bug Fix
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.x
    • Labels:
      None
    • Sub-Committee:
      SYS
    • Impacted Modules:
      System

      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.

        Attachments

          Issue Links

            Activity

            Hide
            jksmith James Smith added a comment -

            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)
              Unknown macro: { List<GroupInfo> groups = new ArrayList<GroupInfo>(); for (String groupId }
            Show
            jksmith James Smith added a comment - 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) Unknown macro: { List<GroupInfo> groups = new ArrayList<GroupInfo>(); for (String groupId }
            Hide
            kymber Kymber Horn added a comment -

            Hi James, should this be resolved to IU's branch?

            Show
            kymber Kymber Horn added a comment - Hi James, should this be resolved to IU's branch?
            Hide
            jksmith James Smith added a comment -

            This was a UA issue and, since it's technically a contribution to Rice, I technically did not contribute anything to the Rice project. So I believe not...

            Show
            jksmith James Smith added a comment - This was a UA issue and, since it's technically a contribution to Rice, I technically did not contribute anything to the Rice project. So I believe not...
            Hide
            kymber Kymber Horn added a comment -

            Thanks James -

            Hi Dan, should this be assigned to RICE? Thanks, Kymber

            Show
            kymber Kymber Horn added a comment - Thanks James - Hi Dan, should this be assigned to RICE? Thanks, Kymber
            Hide
            dlemus Dan Lemus (Inactive) added a comment -

            Yes it should Kymber, I've added a new KULRICE issue and am assigning this to Rice Team.

            Show
            dlemus Dan Lemus (Inactive) added a comment - Yes it should Kymber, I've added a new KULRICE issue and am assigning this to Rice Team.
            Hide
            kymber Kymber Horn added a comment -

            The RICE issue is resolved.

            Show
            kymber Kymber Horn added a comment - The RICE issue is resolved.

              People

              • Assignee:
                riceteam Rice Team (Inactive)
                Reporter:
                abyrne Ailish Byrne
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: