You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2014/05/24 02:51:12 UTC
Re: Review Request 16034: Add explain authorize for checking privileges
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16034/#review43887
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/16034/#comment78163>
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.
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java
<https://reviews.apache.org/r/16034/#comment78169>
Better name : collectReferedEntitiesNDoAuth()
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java
<https://reviews.apache.org/r/16034/#comment78168>
This if condition can never be false, right ?
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g
<https://reviews.apache.org/r/16034/#comment78164>
We need to add this to nonReserved list in IdentifiersParser.g as well.
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g
<https://reviews.apache.org/r/16034/#comment78165>
Also I think
explain authorization select * from T
is more clear than
explain authorize select * from T
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java
<https://reviews.apache.org/r/16034/#comment78162>
This should instead be
return delegated;
That will avoid creating DelegatableAuthProvider in non-explain case.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java
<https://reviews.apache.org/r/16034/#comment78166>
Not able to understand this. Can you add comments whats happening here?
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationFactory.java
<https://reviews.apache.org/r/16034/#comment78167>
Can you add comments when this class is used and for what?
- Ashutosh Chauhan
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
>
>
Re: Review Request 16034: Add explain authorize for checking privileges
Posted by Navis Ryu <na...@nexr.com>.
> 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
>
>