[jira] [Commented] (SENTRY-1660) Useless code in ResourceAuthorizationProvider

[Alexander Kolbasov (JIRA)]

    [ 
https://issues.apache.org/jira/browse/SENTRY-1660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15926797#comment-15926797
 ] 

Alexander Kolbasov commented on SENTRY-1660:
--------------------------------------------

Here is how this function looked way back when SENTRY-26 was committed:

{code}
  private boolean doHasAccess(Subject subject,
      List<? extends Authorizable> authorizables, Set<? extends Action> 
actions) {
    List<String> groups = groupService.getGroups(subject.getName());
    List<String> hierarchy = new ArrayList<String>();
    for (Authorizable authorizable : authorizables) {
      hierarchy.add(KV_JOINER.join(authorizable.getTypeName(), 
authorizable.getName()));
    }
    Iterable<Permission> permissions = getPermissions(authorizables, groups);
    for (Action action : actions) {
      String requestPermission = AUTHORIZABLE_JOINER.join(hierarchy);
      requestPermission = AUTHORIZABLE_JOINER.join(requestPermission,
          KV_JOINER.join(PRIVILEGE_NAME, action.getValue()));
      for (Permission permission : permissions) {
        /*
         * Does the permission granted in the policy file imply the requested 
action?
         */
        boolean result = 
permission.implies(permissionFactory.createPermission(requestPermission));
        if(LOGGER.isDebugEnabled()) {
          LOGGER.debug("FilePermission {}, RequestPermission {}, result {}",
              new Object[]{ permission, requestPermission, result});
        }
        if (result) {
          return true;
        }
      }
    }
    return false;
  }
{code}

SENTRY-143 changed this to

{code}
  private boolean doHasAccess(Subject subject,
      List<? extends Authorizable> authorizables, Set<? extends Action> actions,
      ActiveRoleSet roleSet) {
    Set<String> groups =  getGroups(subject);
    Set<String> hierarchy = new HashSet<String>();
    for (Authorizable authorizable : authorizables) {
      hierarchy.add(KV_JOINER.join(authorizable.getTypeName(), 
authorizable.getName()));
    }
    Iterable<Privilege> privileges = getPrivileges(groups, roleSet);
    List<String> requestPrivileges = buildPermissions(authorizables, actions);
    lastFailedPrivileges.get().clear();

    for (String requestPrivilege : requestPrivileges) {
      for (Privilege permission : privileges) {
        /*
         * Does the permission granted in the policy file imply the requested 
action?
         */
        boolean result = 
permission.implies(privilegeFactory.createPrivilege(requestPrivilege));
        if(LOGGER.isDebugEnabled()) {
          LOGGER.debug("ProviderPrivilege {}, RequestPrivilege {}, RoleSet, {}, 
Result {}",
              new Object[]{ permission, requestPrivilege, roleSet, result});
        }
        if (result) {
          return true;
        }
      }
    }
    lastFailedPrivileges.get().addAll(requestPrivileges);
    return false;
  }
{code}

so that's where the issue was introduced.

> Useless code in ResourceAuthorizationProvider
> ---------------------------------------------
>
>                 Key: SENTRY-1660
>                 URL: https://issues.apache.org/jira/browse/SENTRY-1660
>             Project: Sentry
>          Issue Type: Bug
>          Components: Sentry
>    Affects Versions: 1.8.0
>            Reporter: Alexander Kolbasov
>            Assignee: Jan Hentschel
>            Priority: Minor
>              Labels: bite-sized, newbie
>         Attachments: SENTRY-1660.001.patch
>
>
> The ResourceAuthorizationProvider class has the following code:
> {code}
>     Set<String> hierarchy = new HashSet<String>();
>     for (Authorizable authorizable : authorizables) {
>       hierarchy.add(KV_JOINER.join(authorizable.getTypeName(), 
> authorizable.getName()));
>     }
> {code}
> The {{hierarchy}} variable is never used afterwords, so this code is 
> completely useless.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)