Uploaded image for project: 'Kuali Rice Development'
  1. Kuali Rice Development
  2. KULRICE-8252

Permission check for super user tab seems to be using the wrong namespace

    Details

    • Type: Bug Fix
    • Status: Closed
    • Priority: Blocker
    • Resolution: Complete
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.1.3
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Rice Module:
      KEW
    • KRAD Feature Area:
      Business Rules
    • Application Requirement:
      KFS
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required

      Description

      I'm assuming this is an issue unless someone corrects me.

      There are two permission checks in the DocumentTypePermissionServiceImpl. One's for the global administration of routing for a document, and the other is for the use of the super user tab on documents.

      What is awkward is that they are using the same permission template name, but different namespaces.

      Further, the necessary template for the second case does not exist in the 2.1 dataset.

      So:

      1. Was this intentional?
      2. If so, KFS needs to duplicate the matching permission template. The KR-NS template checks pass in more than just the document type, so I assume a different "KIM Type" should be used.
      3. It will be very confusing to users to have templates with the same name but different functions. The KR-NS one should be renamed.
      	public boolean canAdministerRouting(String principalId, DocumentType documentType) {
      		validatePrincipalId(principalId);
      		validateDocumentType(documentType);
      
      		final Boolean result;
              if (documentType.isSuperUserGroupDefined()) {
                  result = documentType.isSuperUser(principalId);
              } else {
                  Map<String, String> permissionDetails = buildDocumentTypePermissionDetails(documentType);
                  result = getPermissionService().isAuthorizedByTemplate(principalId, KewApiConstants.KEW_NAMESPACE,
                          KewApiConstants.ADMINISTER_ROUTING_PERMISSION, permissionDetails, new HashMap<String, String>());
              }
      		return result;
      	}
      	
          public boolean canAdministerRoutingOnSuTab(String principalId, org.kuali.rice.kew.doctype.bo.DocumentType documentType, 
                  List<RouteNodeInstance> routeNodeInstances, String actionEvent) {
              validatePrincipalId(principalId);
              validateDocumentType(documentType);
      
              List<Map<String, String>> permissionDetailList = buildDocumentTypePermissionDetailsForSuTab(documentType, routeNodeInstances, actionEvent);
              // loop over permission details, only one of them needs to be authorized
              for (Map<String, String> permissionDetails : permissionDetailList) {
                  if (getPermissionService().isAuthorizedByTemplate(principalId, KRADConstants.KNS_NAMESPACE,
                       KewApiConstants.ADMINISTER_ROUTING_PERMISSION, permissionDetails, new HashMap<String, String>())) {
                       return true;
                  }
              }
              return false;
          }
      

        Attachments

          Issue Links

            Activity

            Hide
            jruch Jeff Ruch added a comment -

            Looks like this was resolved with KULRICE-7799. I will confirm with Shannon at standup to today before closing.

            Show
            jruch Jeff Ruch added a comment - Looks like this was resolved with KULRICE-7799 . I will confirm with Shannon at standup to today before closing.
            Hide
            jruch Jeff Ruch added a comment -

            Looks like this is resolved by KULRICE-7799. Assigned to Shannon, so that we are not stepping on each other.

            Show
            jruch Jeff Ruch added a comment - Looks like this is resolved by KULRICE-7799 . Assigned to Shannon, so that we are not stepping on each other.
            Hide
            shahess Shannon Hess added a comment -

            Jonathan,

            1. This was intentional. In KULRICE-7799, Damon had indicated "the permission template can be KR-NS Administer Routing for Document (same as the KR-WKFLW version)" so that was the way I set it up.

            2. There is SQL to create the new KIM type and permission template. There is currently a policy that allows no database changes to a patch release, so the database changes for KULRICE-7799 were added to the 2.2 updates in /rice/scripts/upgrades/2.1 to 2.2/db-updates/2012-08-21.sql. This policy is being revisited so these may be moved shortly and added to 2.1 dataset.

            3. Do you still feel that the templates need to have different names? If so, I could rename the KR-NS - Administer Routing for Document to KR-NS - Administer Routing on Super User Tab (Or something else if you have already come up with a better name)

            Thanks,
            Shannon

            SQL for Kim type and template (for oracle):

            INSERT INTO KRIM_TYP_T(KIM_TYP_ID, OBJ_ID, VER_NBR, NM, SRVC_NM, ACTV_IND, NMSPC_CD)
              VALUES((SELECT (max(to_number(KIM_TYP_ATTR_ID)) + 1) from KRIM_TYP_ATTR_T where KIM_TYP_ATTR_ID is not NULL and regexp_like(KIM_TYP_ATTR_ID, '^[1-9][0-9]{0,3}$')), 
                       sys_guid(), 1, 'Document Type, Routing Node and Action Event', 'documentTypeAndNodeAndActionEventService', 'Y', 'KR-SYS')
            /
            
            INSERT INTO KRIM_PERM_TMPL_T (ACTV_IND,KIM_TYP_ID,NM,NMSPC_CD,OBJ_ID,PERM_TMPL_ID,VER_NBR)
              VALUES ('Y',
              (SELECT KIM_TYP_ID FROM KRIM_TYP_T where NM = 'Document Type, Routing Node and Action Event' and SRVC_NM = 'documentTypeAndNodeAndActionEventService'), 'Administer Routing for Document', 'KR-NS', sys_guid(), 
              (SELECT (max(to_number(perm_tmpl_id)) + 1) from krim_perm_tmpl_t where perm_tmpl_id is not NULL and regexp_like(perm_tmpl_id, '^[1-9][0-9]{0,3}$')), 1)
            /
            
            Show
            shahess Shannon Hess added a comment - Jonathan, 1. This was intentional. In KULRICE-7799 , Damon had indicated "the permission template can be KR-NS Administer Routing for Document (same as the KR-WKFLW version)" so that was the way I set it up. 2. There is SQL to create the new KIM type and permission template. There is currently a policy that allows no database changes to a patch release, so the database changes for KULRICE-7799 were added to the 2.2 updates in /rice/scripts/upgrades/2.1 to 2.2/db-updates/2012-08-21.sql. This policy is being revisited so these may be moved shortly and added to 2.1 dataset. 3. Do you still feel that the templates need to have different names? If so, I could rename the KR-NS - Administer Routing for Document to KR-NS - Administer Routing on Super User Tab (Or something else if you have already come up with a better name) Thanks, Shannon SQL for Kim type and template (for oracle): INSERT INTO KRIM_TYP_T(KIM_TYP_ID, OBJ_ID, VER_NBR, NM, SRVC_NM, ACTV_IND, NMSPC_CD) VALUES((SELECT (max(to_number(KIM_TYP_ATTR_ID)) + 1) from KRIM_TYP_ATTR_T where KIM_TYP_ATTR_ID is not NULL and regexp_like(KIM_TYP_ATTR_ID, '^[1-9][0-9]{0,3}$')), sys_guid(), 1, 'Document Type, Routing Node and Action Event', 'documentTypeAndNodeAndActionEventService', 'Y', 'KR-SYS') / INSERT INTO KRIM_PERM_TMPL_T (ACTV_IND,KIM_TYP_ID,NM,NMSPC_CD,OBJ_ID,PERM_TMPL_ID,VER_NBR) VALUES ('Y', (SELECT KIM_TYP_ID FROM KRIM_TYP_T where NM = 'Document Type, Routing Node and Action Event' and SRVC_NM = 'documentTypeAndNodeAndActionEventService'), 'Administer Routing for Document', 'KR-NS', sys_guid(), (SELECT (max(to_number(perm_tmpl_id)) + 1) from krim_perm_tmpl_t where perm_tmpl_id is not NULL and regexp_like(perm_tmpl_id, '^[1-9][0-9]{0,3}$')), 1) /
            Hide
            jkeller Jonathan Keller added a comment -

            Thanks.

            We're talking about #3 right now within the KFS team. But, yes, we will want to change the permission names.

            This all came up because (1) I did not know of the original request and (2) that there are deeper permission checks within workflow (called from the SuperUserActionTakenEvent class) which check the original permission. So, the net effect was that the users could see the tab, but then it was blowing up whenever they took an action from it.

            So - hold on - we are probably going to have a small set of changes for this one. But, AFIAC, you can close this one out as we will be working out how we will be addressing these changes at a meeting tomorrow. Thanks.

            Show
            jkeller Jonathan Keller added a comment - Thanks. We're talking about #3 right now within the KFS team. But, yes, we will want to change the permission names. This all came up because (1) I did not know of the original request and (2) that there are deeper permission checks within workflow (called from the SuperUserActionTakenEvent class) which check the original permission. So, the net effect was that the users could see the tab, but then it was blowing up whenever they took an action from it. So - hold on - we are probably going to have a small set of changes for this one. But, AFIAC, you can close this one out as we will be working out how we will be addressing these changes at a meeting tomorrow. Thanks.
            Hide
            shahess Shannon Hess added a comment -

            Great, sounds good. I definitely understand how this would have been confusing, especially since you didn't know of the original request.

            Show
            shahess Shannon Hess added a comment - Great, sounds good. I definitely understand how this would have been confusing, especially since you didn't know of the original request.
            Hide
            shahess Shannon Hess added a comment -

            Closing this as the KFS team is going to discuss and open a new JIRA with additional information.

            Show
            shahess Shannon Hess added a comment - Closing this as the KFS team is going to discuss and open a new JIRA with additional information.

              People

              • Assignee:
                shahess Shannon Hess
                Reporter:
                jkeller Jonathan Keller
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: