Re: Review Request 67131: SENTRY-2174: Sentry authorization provider should now generate ACL for users

2018-05-15 Thread kalyan kumar kalvagadda via Review Board 

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

(Updated May 15, 2018, 11:45 p.m.)


Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.


Changes
---

addressed review comments from sergio.


Bugs: SENTRY-2174
https://issues.apache.org/jira/browse/SENTRY-2174


Repository: sentry


Description
---

SentryAuthorizationProvider should now additionally provide the ACL entries 
with the permissions that users have along with the permissions for the groups.

With the changes proposed in SENTRY-2173, PrivilegeInfo will not only have role 
to permission mapping. it will also have user to privilege mapping information.

SentryAuthorizationProvider should be using the new information added in 
PrivilegeInfo to add ACL for users.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
 a88d8e25ad745effe354aa6267252998b189a252 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryPermissions.java
 dbce40538cf4721b601cf63b1da713f3d57fc981 


Diff: https://reviews.apache.org/r/67131/diff/2/

Changes: https://reviews.apache.org/r/67131/diff/1-2/


Testing
---

Added new unit tests and also made sure that all the existing tests pass.


Thanks,

kalyan kumar kalvagadda

Re: Review Request 67134: SENTRY-2224: Support SHOW GRANT on HIVE_OBJECT

2018-05-15 Thread Sergio Pena via Review Board 

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



I just need to review the DefaultSentryAccessController. Btw, I see that Hive 
has something here in case you want to compare it:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java


sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
Lines 31-142 (original), 32-145 (patched)


This class should not be here. I removed it in another jira, mmm, I'll take 
a look.



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
Lines 191-192 (original), 192-206 (patched)


I think SentryHiveAuthorizationTaskFactory is not used anymore in Sentry. 
This is an old implementation of the Hive authz V1. We should ignore these 
changes and delete the class in another jira.



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
Lines 209 (patched)


If it is for object, why is the type == USER? If it is a bug in Hive, then, 
should we check this or can we ignore it?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
Lines 272 (patched)


Nit: space between for and (, and > and authList.



sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
Lines 476-493 (patched)


I think TestSentryHiveAuthorizationTaskFactory is not used anymore in 
Sentry. This is an old implementation of the Hive authz V1. We should ignore 
these changes and delete the class in another jira.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 1595 (original), 1595 (patched)


what's the difference between startsWith and endsWith?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java
Lines 37-38 (patched)


Are we sting 'show grant role' or just 'show grant'? I think to keep this 
variable name generic, we should use SHOW_GRANT_DB_POSITION, etc/ right?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java
Lines 116 (patched)


role2 and role3  has privileges here, but what about role1 who has all 
privileges in the server? How should this work? should we display it as well?


- Sergio Pena


On May 15, 2018, 4:17 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67134/
> ---
> 
> (Updated May 15, 2018, 4:17 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Currently Sentry doesn't support Hive command to show privileges on 
> authorizables without mentioning any role or user name
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java
>  4fa4221b4 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
>  203632d05 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
>  23246c903 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
>  fc2427cbf 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
>  2e3fd7f36 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  cafe2b597 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67134/diff/1/
> 
> 
> Testing
> ---
> 
> $ mvn -f sentry-binding/pom.xml test
> $ mvn -f sentry-provider/pom.xml test
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 67131: SENTRY-2174: Sentry authorization provider should now generate ACL for users

2018-05-15 Thread Sergio Pena via Review Board 

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




sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 92-93 (patched)


should these be private final?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 95-102 (patched)


I usually try to avoid throwing exceptions in the constructor because makes 
things easier to read and code.

So I was wondering if we could make this constructor private and use a 
static method to initialize the arguments intead, like:

public static HdfsAclEntity asGroup(String groupname) {
  return new HdfsAclEntity(AclEntryType.GROUP, groupname);
}

public static HdfsAclEntity asUser(String username) {
  return new HdfsAclEntity(AclEntryType.USER, username);
}

As you see, it above methods will avoid throwing exceptions. 
Do you think we should do the same here?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 141-143 (original), 214-216 (patched)


Should we have an if() that prevents constructing permissions if a 
TPrivilegeEntityType.GROUP is sent (which is not supported)?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 143-144 (original), 216-217 (patched)


Why are you constructing permissions twice?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 157-158 (original), 236-242 (patched)


Doesn't this if() be on the getPerms() function like it was on the 
getGrouopPerms?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Line 176 (original), 268-273 (patched)


we could use HdfsAclEntity.asGroup(group) here to avoid catching exceptions 
that should not happen.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Line 181 (original), 283-286 (patched)


we could use HdfsAclEntity.asUser(user) here to avoid catching exceptions 
that should not happen.


- Sergio Pena


On May 15, 2018, 4:03 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67131/
> ---
> 
> (Updated May 15, 2018, 4:03 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2174
> https://issues.apache.org/jira/browse/SENTRY-2174
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SentryAuthorizationProvider should now additionally provide the ACL entries 
> with the permissions that users have along with the permissions for the 
> groups.
> 
> With the changes proposed in SENTRY-2173, PrivilegeInfo will not only have 
> role to permission mapping. it will also have user to privilege mapping 
> information.
> 
> SentryAuthorizationProvider should be using the new information added in 
> PrivilegeInfo to add ACL for users.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
>  a88d8e25ad745effe354aa6267252998b189a252 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryPermissions.java
>  dbce40538cf4721b601cf63b1da713f3d57fc981 
> 
> 
> Diff: https://reviews.apache.org/r/67131/diff/1/
> 
> 
> Testing
> ---
> 
> Added new unit tests and also made sure that all the existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

2018-05-15 Thread Anthony Young-Garner via Review Board 

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

(Updated May 15, 2018, 6:47 p.m.)


Review request for sentry.


Changes
---

Removed sentry-dist/src/license/THIRD-PARTY.properties from patch.


Bugs: SENTRY-2208
https://issues.apache.org/jira/browse/SENTRY-2208


Repository: sentry


Description
---

Refactored the SentryService class and its dependencies from 
sentry-provider/sentry-provider-db into a new 
sentry-service/sentry-service-server module. In addition, refactored 
SentryServiceClientFactory into a new sentry-service/sentry-service-client 
module.

Work is based on previously completed work on SENTRY-1205.

NOTE: The diff looks large, but the overwhelming majority (9 pages) of the diff 
consists of file moves (from sentry-provider-db to sentry-service-server)

https://issues.apache.org/jira/browse/SENTRY-2208


Diffs (updated)
-

  sentry-hdfs/sentry-hdfs-service/pom.xml 
50a451d917eb7428cd71ab7cedd9c4363111c166 
  sentry-provider/sentry-provider-db/pom.xml 
48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
 876ee146ab31aaa82b83e88585c45bb2deabc0d2 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/AuditLoggerTestAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/DBAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/GMAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

2018-05-15 Thread Anthony Young-Garner via Review Board 

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

(Updated May 15, 2018, 6:38 p.m.)


Review request for sentry.


Changes
---

Provided more detailed description.


Bugs: SENTRY-2208
https://issues.apache.org/jira/browse/SENTRY-2208


Repository: sentry


Description (updated)
---

Refactored the SentryService class and its dependencies from 
sentry-provider/sentry-provider-db into a new 
sentry-service/sentry-service-server module. In addition, refactored 
SentryServiceClientFactory into a new sentry-service/sentry-service-client 
module.

Work is based on previously completed work on SENTRY-1205.

NOTE: The diff looks large, but the overwhelming majority (9 pages) of the diff 
consists of file moves (from sentry-provider-db to sentry-service-server)

https://issues.apache.org/jira/browse/SENTRY-2208


Diffs
-

  sentry-dist/src/license/THIRD-PARTY.properties 
b39e1b6ca7eba8c6a7695a4238104af7cd50da32 
  sentry-hdfs/sentry-hdfs-service/pom.xml 
50a451d917eb7428cd71ab7cedd9c4363111c166 
  sentry-provider/sentry-provider-db/pom.xml 
48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
 876ee146ab31aaa82b83e88585c45bb2deabc0d2 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/AuditLoggerTestAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/DBAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/GMAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java

Re: Review Request 67072: SENTRY-2228: Improve on how to handle unsupported Hive commands

2018-05-15 Thread Na Li via Review Board 

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Line 304 (original), 304-305 (patched)


I need to do some research on the best way to handle this.

1) have white list for commands that are processed in other places, like 
grant/revoke privileges, show databases.
2) For DDL command, if the user has all privilege on the level of the 
input/ouput, then allow.


- Na Li


On May 10, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67072/
> ---
> 
> (Updated May 10, 2018, 9:36 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Bugs: sentry-2228
> https://issues.apache.org/jira/browse/sentry-2228
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> throw exception for unsupported hive commands
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  09bd9b5 
> 
> 
> Diff: https://reviews.apache.org/r/67072/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 67072: SENTRY-2228: Improve on how to handle unsupported Hive commands

2018-05-15 Thread Sergio Pena via Review Board 

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Line 304 (original), 304-305 (patched)


Should we deny roles with ALL privileges to execute these commands that do 
not have an authorization map?


- Sergio Pena


On May 10, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67072/
> ---
> 
> (Updated May 10, 2018, 9:36 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Bugs: sentry-2228
> https://issues.apache.org/jira/browse/sentry-2228
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> throw exception for unsupported hive commands
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  09bd9b5 
> 
> 
> Diff: https://reviews.apache.org/r/67072/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 67134: SENTRY-2224: Support SHOW GRANT on HIVE_OBJECT

2018-05-15 Thread Arjun Mishra via Review Board 

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

(Updated May 15, 2018, 4:17 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
Sergio Pena.


Repository: sentry


Description
---

Currently Sentry doesn't support Hive command to show privileges on 
authorizables without mentioning any role or user name


Diffs
-

  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java
 4fa4221b4 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
 203632d05 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 23246c903 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 fc2427cbf 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 2e3fd7f36 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 cafe2b597 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67134/diff/1/


Testing (updated)
---

$ mvn -f sentry-binding/pom.xml test
$ mvn -f sentry-provider/pom.xml test


Thanks,

Arjun Mishra

Review Request 67131: SENTRY-2174: Sentry authorization provider should now generate ACL for users

2018-05-15 Thread kalyan kumar kalvagadda via Review Board 

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

Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.


Bugs: SENTRY-2174
https://issues.apache.org/jira/browse/SENTRY-2174


Repository: sentry


Description
---

SentryAuthorizationProvider should now additionally provide the ACL entries 
with the permissions that users have along with the permissions for the groups.

With the changes proposed in SENTRY-2173, PrivilegeInfo will not only have role 
to permission mapping. it will also have user to privilege mapping information.

SentryAuthorizationProvider should be using the new information added in 
PrivilegeInfo to add ACL for users.


Diffs
-

  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
 a88d8e25ad745effe354aa6267252998b189a252 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryPermissions.java
 dbce40538cf4721b601cf63b1da713f3d57fc981 


Diff: https://reviews.apache.org/r/67131/diff/1/


Testing
---

Added new unit tests and also made sure that all the existing tests pass.


Thanks,

kalyan kumar kalvagadda

Review Request 67134: SENTRY-2224: Support SHOW GRANT on HIVE_OBJECT

2018-05-15 Thread Arjun Mishra via Review Board 

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

Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
Sergio Pena.


Repository: sentry


Description
---

Currently Sentry doesn't support Hive command to show privileges on 
authorizables without mentioning any role or user name


Diffs
-

  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java
 4fa4221b4 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
 203632d05 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 23246c903 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 fc2427cbf 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 2e3fd7f36 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 cafe2b597 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestShowGrants.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67134/diff/1/


Testing
---


Thanks,

Arjun Mishra

Re: Review Request 66898: SENTRY-2214: Sentry should not allow URI grants to EMPTY or NULL locations

2018-05-15 Thread Arjun Mishra via Review Board 


> On May 10, 2018, 10:19 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
> > Lines 420 (patched)
> > 
> >
> > Arjun,
> > 
> > You could have used Strings.isNullOrEmpty to be inline with the current 
> > sentry code. Currently we use "Strings.isNullOrEmpty" for the same purpose 
> > in tons of places.

Will that detact blanks like "" ? I don't think so


- Arjun


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


On May 8, 2018, 8:20 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66898/
> ---
> 
> (Updated May 8, 2018, 8:20 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We can grant privileges to URI's with EMPTY or NULL locations. This should 
> not be allowed
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
>  3ac49fa6a 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  8485ca37f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  8ac3c0d51 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b41002783 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  ae1316423 
> 
> 
> Diff: https://reviews.apache.org/r/66898/diff/6/
> 
> 
> Testing
> ---
> 
> $ mvn -f sentry-binding/sentry-binding-hive/pom.xml test
> $ mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test
> $ mvn -f sentry-provider/sentry-provider-db/pom.xml test
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 67072: SENTRY-2228: Improve on how to handle unsupported Hive commands

2018-05-15 Thread kalyan kumar kalvagadda via Review Board 

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
Line 304 (original), 304 (patched)


Authorization may not be correct exception that should be thrown. Currently 
"SemanticException" is thrown when authorization fails. We need to maintain 
same behavior even for this case.


- kalyan kumar kalvagadda


On May 10, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67072/
> ---
> 
> (Updated May 10, 2018, 9:36 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Bugs: sentry-2228
> https://issues.apache.org/jira/browse/sentry-2228
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> throw exception for unsupported hive commands
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  09bd9b5 
> 
> 
> Diff: https://reviews.apache.org/r/67072/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

2018-05-15 Thread Anthony Young-Garner via Review Board 

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

(Updated May 15, 2018, 1:21 p.m.)


Review request for sentry.


Changes
---

Rebased on master.


Bugs: SENTRY-2208
https://issues.apache.org/jira/browse/SENTRY-2208


Repository: sentry


Description
---

Refactored the SentryService class and its dependencies from 
sentry-provider/sentry-provider-db into a new 
sentry-service/sentry-service-server module. In addition, refactored 
SentryServiceClientFactory into a new sentry-service/sentry-service-client 
module.

Work is based on previously completed work on SENTRY-1205.

https://issues.apache.org/jira/browse/SENTRY-2208


Diffs (updated)
-

  sentry-dist/src/license/THIRD-PARTY.properties 
b39e1b6ca7eba8c6a7695a4238104af7cd50da32 
  sentry-hdfs/sentry-hdfs-service/pom.xml 
50a451d917eb7428cd71ab7cedd9c4363111c166 
  sentry-provider/sentry-provider-db/pom.xml 
48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
 876ee146ab31aaa82b83e88585c45bb2deabc0d2 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/AuditLoggerTestAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/DBAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/GMAuditMetadataLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntity.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
  
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsSnapshotId.java