Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============4580662092600592216=="
MIME-Version: 1.0
Subject: Re: Review Request 25616: HIVE-7790 Update privileges to check for
 update and delete
From: "Thejas Nair" <thejas@hortonworks.com>
To: "Thejas Nair" <thejas@hortonworks.com>
Cc: "Alan Gates" <gates@hortonworks.com>, "hive" <dev@hive.apache.org>
Date: Tue, 16 Sep 2014 20:59:30 -0000
Message-ID: <20140916205930.7803.18902@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Thejas Nair" <noreply@reviews.apache.org>
References: <20140916064231.7803.4493@reviews.apache.org>
In-Reply-To: <20140916064231.7803.4493@reviews.apache.org>
Reply-To: "Thejas Nair" <thejas@hortonworks.com>

--===============4580662092600592216==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> 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
> 
>


--===============4580662092600592216==--