> On 十月 2, 2017, 6:46 p.m., Ramesh Mani wrote:
> > plugin-sqoop/src/main/java/org/apache/ranger/authorization/sqoop/authorizer/RangerSqoopAuthorizer.java
> > Lines 99 (patched)
> > <https://reviews.apache.org/r/62710/diff/1/?file=1841282#file1841282line99>
> >
> >     can privilege be null? Is it test for this case?
> >     Please add some unit test also for various scenarios.

OK,I will submit another patch for some unit test.
And according to Sqoop AuthorizationEngine.checkPrivilege method,
the privilege would not be null, but privileges would be an empty list.
Sqoop code reference, please notice SQOOP-2256 commont:
org.apache.sqoop.security.authorization.AuthorizationEngine.checkPrivilege(MPrivilege...)
  private static void checkPrivilege(MPrivilege... privileges) {
    AuthorizationHandler handler = 
AuthorizationManager.getAuthorizationHandler();
    UserGroupInformation user = HttpUserGroupInformation.get();
    String user_name = user == null ? StringUtils.EMPTY : 
user.getShortUserName();
    MPrincipal principal = new MPrincipal(user_name, MPrincipal.TYPE.USER);

    // SQOOP-2256: Hack code, do not check privilege when the user is the 
creator
    // If the user is the owner/creator of this resource, then privilege will
    // not be checked. It is a hack code for the time being. The concept of
    // "Owner" will be added in the future and this code will be removed.
    ArrayList<MPrivilege> privilegesNeedCheck = new ArrayList<MPrivilege>();
    for (MPrivilege privilege : privileges) {
      Repository repository = RepositoryManager.getInstance().getRepository();
      if 
(MResource.TYPE.LINK.name().equalsIgnoreCase(privilege.getResource().getType()))
 {
        MLink link = 
repository.findLink(Long.valueOf(privilege.getResource().getName()));
        if (!user_name.equals(link.getCreationUser())) {
          privilegesNeedCheck.add(privilege);
        }
      } else if 
(MResource.TYPE.JOB.name().equalsIgnoreCase(privilege.getResource().getType())) 
{
        MJob job = 
repository.findJob(Long.valueOf(privilege.getResource().getName()));
        if (!user_name.equals(job.getCreationUser())) {
          privilegesNeedCheck.add(privilege);
        }
      } else {
        privilegesNeedCheck.add(privilege);
      }
    }

    handler.checkPrivileges(principal, privilegesNeedCheck);
  }
}


- Qiang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62710/#review186865
-----------------------------------------------------------


On 十月 12, 2017, 9:02 a.m., Qiang Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62710/
> -----------------------------------------------------------
> 
> (Updated 十月 12, 2017, 9:02 a.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Don Bosco Durai, Colm O 
> hEigeartaigh, Gautam Borad, Madhan Neethiraj, pengjianhua, Ramesh Mani, 
> Selvamohan Neethiraj, sam  rome, Venkat Ranganathan, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-1810
>     https://issues.apache.org/jira/browse/RANGER-1810
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Apache Sqoop is a tool designed for efficiently transferring bulk data 
> between Apache Hadoop and structured datastores such as relational databases. 
> You can use Sqoop to import data from external structured datastores into 
> Hadoop Distributed File System or related systems like Hive and HBase. 
> Conversely, Sqoop can be used to extract data from Hadoop and export it to 
> external structured datastores such as relational databases and enterprise 
> data warehouses.It successfully graduated from the Incubator in March of 2012 
> and is now a Top-Level Apache project.
> The Ranger will further expand the influence in the hadoop ecosystem if it 
> supports sqoop authorization. So we should develop sqoop plugin to enable, 
> monitor and manage apache Sqoop2.
> 
> Our test specialists have rigorously tested this feature.
> 
> 
> Diffs
> -----
> 
>   agents-common/scripts/enable-agent.sh d31a264 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/EmbeddedServiceDefsUtil.java
>  9463ab8 
>   agents-common/src/main/resources/service-defs/ranger-servicedef-sqoop.json 
> PRE-CREATION 
>   plugin-sqoop/.gitignore PRE-CREATION 
>   plugin-sqoop/conf/ranger-policymgr-ssl-changes.cfg PRE-CREATION 
>   plugin-sqoop/conf/ranger-policymgr-ssl.xml PRE-CREATION 
>   plugin-sqoop/conf/ranger-sqoop-audit-changes.cfg PRE-CREATION 
>   plugin-sqoop/conf/ranger-sqoop-audit.xml PRE-CREATION 
>   plugin-sqoop/conf/ranger-sqoop-security-changes.cfg PRE-CREATION 
>   plugin-sqoop/conf/ranger-sqoop-security.xml PRE-CREATION 
>   plugin-sqoop/pom.xml PRE-CREATION 
>   plugin-sqoop/scripts/install.properties PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/authorization/sqoop/authorizer/RangerSqoopAuthorizer.java
>  PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/services/sqoop/RangerServiceSqoop.java
>  PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/services/sqoop/client/SqoopClient.java
>  PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/services/sqoop/client/SqoopResourceMgr.java
>  PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/services/sqoop/client/json/model/SqoopConnectorResponse.java
>  PRE-CREATION 
>   
> plugin-sqoop/src/main/java/org/apache/ranger/services/sqoop/client/json/model/SqoopConnectorsResponse.java
>  PRE-CREATION 
>   pom.xml 3958014 
>   ranger-sqoop-plugin-shim/.gitignore PRE-CREATION 
>   ranger-sqoop-plugin-shim/pom.xml PRE-CREATION 
>   
> ranger-sqoop-plugin-shim/src/main/java/org/apache/ranger/authorization/sqoop/authorizer/RangerSqoopAuthorizer.java
>  PRE-CREATION 
>   src/main/assembly/admin-web.xml 0e97818 
>   src/main/assembly/plugin-sqoop.xml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62710/diff/2/
> 
> 
> Testing
> -------
> 
> Our test specialists have rigorously tested this feature.
> 
> 
> Thanks,
> 
> Qiang Zhang
> 
>

Reply via email to