> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1503 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1566>
> >
> >     Can we maintain the same consistency with other audit methods?Maybe 
> > just pass in the request object?
> >     
> >     Also, Can you explain why onGrantOwnerPrivileges needs the Status 
> > parameter to be passed in?

I cannot keep the consistency unless I create a thrift object for user 
privileges. Because the owner privilege is granted implicitly by Sentry, then 
we don't need the Thrift object, so I had to have this method with those 
parameters.

The Status is used by the createJsonLogEntity() method to append the status.


> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Line 1619 (original), 1571 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1635>
> >
> >     Why don't we have an audit log for revoking owner privileges?

Because there is not information about what is being revoked. The revoke 
happens by the revokeOwnerPrivileges() call.

I initially though on breaking the code to allow auditing the revoke, but 
something special of the OWNER privilege is that only one role or user can have 
it and it cannot be revoked. So, if you grant the OWNER to a user or role, it 
immediatly means nobody else has that privilege, so in this case, it does not 
make sense to log that as the OWNER privilege is assigned to one user or role.


> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
> > Lines 215 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070545#file2070545line216>
> >
> >     I like this. Was curious since this should return the command that the 
> > user ran. 
> >     Is there some way to display that?

I thought that too, but how to know what happend was executed? We know it could 
be a create database or table, or alter database or table; but that's a command 
from Hive. This audit log is for Sentry (no Hive). The only way I thought so is 
to write that message.


- Sergio


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


On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2018, 2:29 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2157
>     https://issues.apache.org/jira/browse/sentry-2157
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch logs owner privileges grants and revokes.
> 
> 
> Diffs
> -----
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  61f9168b1970144dbf0b7a7378f2d25e70f1761d 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
>  61becceac881443b02182e6ab1012add4c046499 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
>  6479a6055e8c7087f0e484080ec9d46a9c146212 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java
>  307f38eadb65bf12dc6225cfe43a5d590657d055 
> 
> 
> Diff: https://reviews.apache.org/r/68268/diff/1/
> 
> 
> Testing
> -------
> 
> I run the patch in a cluster and the audit logs is displaying the correct 
> messages.
> 
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER
>  privilege granted to USER: 
> sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Reply via email to