hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Navis Ryu" <navis....@nexr.com>
Subject Re: Review Request 16034: Add explain authorize for checking privileges
Date Sat, 24 May 2014 08:44:44 GMT


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java, line 325
> > <https://reviews.apache.org/r/16034/diff/4/?file=545573#file545573line325>
> >
> >     This if condition can never be false, right ?

Simple explain needs full authorization. This should be true only for "explain authorize".


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java, line 302
> > <https://reviews.apache.org/r/16034/diff/4/?file=545573#file545573line302>
> >
> >     Better name : collectReferedEntitiesNDoAuth()

Renamed to collectAuthRelatedEntities. This just collects(ignores) authorization exception.



> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 297
> > <https://reviews.apache.org/r/16034/diff/4/?file=545576#file545576line297>
> >
> >     We need to add this to nonReserved list in IdentifiersParser.g as well.

ah, sure.


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 298
> > <https://reviews.apache.org/r/16034/diff/4/?file=545576#file545576line298>
> >
> >     Also I think 
> >     explain authorization select * from T
> >     is more clear than 
> >     explain authorize select * from T

done.


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 496
> > <https://reviews.apache.org/r/16034/diff/4/?file=545572#file545572line496>
> >
> >     This could be
> >     HiveAuthorizationProvider authorizer = AuthorizationFactory.create(ss.getAuthorizer());

> >     See my corresponding comment in create().
> >     
> >     I want to avoid DelegtableAuthProvider in non-explain case, if possible.

done.


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java,
line 40
> > <https://reviews.apache.org/r/16034/diff/4/?file=545580#file545580line40>
> >
> >     This should instead be 
> >     return delegated;
> >     
> >     That will avoid creating DelegatableAuthProvider in non-explain case.

done.


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java,
lines 50-56
> > <https://reviews.apache.org/r/16034/diff/4/?file=545580#file545580line50>
> >
> >     Not able to understand this. Can you add comments whats happening here?

It's not related to this issue. There was an authorization issue on view which should be authorized
by the owner of view, not by current user, which was fixed long before than this(yes, in my
version). 

I should book that as a following issue. Will remove this part for that.


> On May 24, 2014, 12:51 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java,
line 104
> > <https://reviews.apache.org/r/16034/diff/4/?file=545580#file545580line104>
> >
> >     Can you add comments when this class is used and for what?

same as above


- Navis


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


On April 2, 2014, 9:07 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16034/
> -----------------------------------------------------------
> 
> (Updated April 2, 2014, 9:07 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5961
>     https://issues.apache.org/jira/browse/HIVE-5961
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> For easy checking of need privileges for a query, 
> {noformat}
> explain authorize select * from src join srcpart
> INPUTS: 
>   default@srcpart
>   default@srcpart@ds=2008-04-08/hr=11
>   default@srcpart@ds=2008-04-08/hr=12
>   default@srcpart@ds=2008-04-09/hr=11
>   default@srcpart@ds=2008-04-09/hr=12
>   default@src
> OUTPUTS: 
>   file:/home/navis/apache/oss-hive/itests/qtest/target/tmp/localscratchdir/hive_2013-12-04_21-57-53_748_5323811717799107868-1/-mr-10000
> CURRENT_USER: 
>   hive_test_user
> OPERATION: 
>   QUERY
> AUTHORIZATION_FAILURES: 
>   No privilege 'Select' found for inputs { database:default, table:srcpart, columnName:key}
>   No privilege 'Select' found for inputs { database:default, table:src, columnName:key}
>   No privilege 'Select' found for inputs { database:default, table:src, columnName:key}
> {noformat}
> 
> Hopefully good for debugging of authorization, which is in progress on HIVE-5837.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java d42895a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java 35f4fa9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java db9fa74 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSemanticAnalyzer.java 77fb8bd 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java e7d0359 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainWork.java d7140ca 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DelegatableAuthorizationProvider.java
PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_explain.q PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_explain.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/16034/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message