hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thejas Nair" <the...@hortonworks.com>
Subject Re: Review Request 25616: HIVE-7790 Update privileges to check for update and delete
Date Tue, 16 Sep 2014 20:59:30 GMT


> On Sept. 16, 2014, 6:42 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 741
> > <https://reviews.apache.org/r/25616/diff/2/?file=690379#file690379line741>
> >
> >     should we skip it from ReadEntity if none of the columns are being used ? Though,
that case is not going to be common.
> >     
> >     eg a query like 'update table set j=null;' should not require select privileges
on the table, as there is no columns in where clause or value expression.
> >     
> >     Note that this is a change that we can also make in future without breaking
users. (making a change in future to require fewer privileges will not break users). Ie, it
does not have to be addressed in this patch.
> 
> Alan Gates wrote:
>     I don't think that makes any sense.  If I have delete permissions but not select
permissions I can delete all rows from a table but not some rows?  That definitely violates
the law of least astonishment.

You need select privileges only for use of columns in the value expression (in update) and
where clause (in update/delete).
In case of an authorization implementation that supports column level access control on table,
this is the behavior you get. You will be able to delete all rows even if you don't have select
privileges on the columns.

For example, you could have an application that sets some fields with PII to null. It would
not need select privileges. 

I am fine with current approach, as we have the ability to change it in future, based on user
feedback.


- Thejas


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


On Sept. 16, 2014, 7:37 p.m., Alan Gates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25616/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 7:37 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-7790
>     https://issues.apache.org/jira/browse/HIVE-7790
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Adds update and delete as action and adds checks for authorization during update and
delete. Also adds passing of updated columns in case authorizer wishes to check them.
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java
53d88b0 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 298f429 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java b2f66e0 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java a4df8b4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java 3aaa09c

>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
93df9f4 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
093b4fd 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
3236341 
>   ql/src/test/queries/clientnegative/authorization_delete_nodeletepriv.q PRE-CREATION

>   ql/src/test/queries/clientnegative/authorization_update_noupdatepriv.q PRE-CREATION

>   ql/src/test/queries/clientpositive/authorization_delete.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_delete_own_table.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_update.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_update_own_table.q PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_delete_nodeletepriv.q.out PRE-CREATION

>   ql/src/test/results/clientnegative/authorization_update_noupdatepriv.q.out PRE-CREATION

>   ql/src/test/results/clientpositive/authorization_delete.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_delete_own_table.q.out PRE-CREATION

>   ql/src/test/results/clientpositive/authorization_update.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_update_own_table.q.out PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/25616/diff/
> 
> 
> Testing
> -------
> 
> Added tests, both positive and negative, for update and delete, including ability to
update and delete tables created by user.  Also added tests for passing correct update columns.
> 
> 
> Thanks,
> 
> Alan Gates
> 
>


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