You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2019/08/27 14:26:36 UTC

[Impala-ASF-CR] IMPALA-8851: Do not throw authorization exception in drop if exists queries

Hello Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14121

to look at the new patch set (#5).

Change subject: IMPALA-8851: Do not throw authorization exception in drop if exists queries
......................................................................

IMPALA-8851: Do not throw authorization exception in drop if exists queries

Note that this is the continuation of work in
https://github.com/vihangk1/impala/commits/IMPALA-8851

This patch's goal is to change Impala's behavior in the following case:
- the query is a DROP TABLE/VIEW/DATABASE/FUNCTIONS IF EXISTS statement
- the given object does not exist
- the user has some kind of privilige on the object, which imples the
  privilege to know whether object exists, but does not have DROP
  privilege on the object

Until now this lead to an authorization exception, while it will be
allowed with this change.

An example where this is useful is a user who has CREATE privilege on
a database, and creates table t_owned, and gets ownership of the
table. In this case DROP TABLE IF EXISTS was non idempotent:
DROP TABLE IF EXISTS t_owned;
-> success
DROP TABLE IF EXISTS t_owned;
-> authorization error, as the privileges for the table were
   deleted when the table was successfully dropped

After this change the second statement will be also successful.

The authorization logic has to avoid leaking information that the
user has no right to know. For this reason DROP IF EXISTS has to
return the same error message regardless whether the object exists
or not if the user has no right to know it's existence. This is
achieved with the following pattern:
- in the IF EXISTS case first an ANY privilege is registered, then
  the existence of the object is checked and if it doesn't exist,
  the analysis returns successfully
- if the object exists, the DROP privilege is registered (if there is
  no IF EXISTS in the query, this always happens)
- as the authorization logic checks privileges in the order of
  registration, first the ANY will be checked, and DROP will be only
  checked if the user has ANY privileges

Testing:
- Added a new test case in the sentry tests which confirms that the
authorization exception is not thrown when a drop if exists query is
issued on a object which does not exist.
- Changed several tests affected by the new behavior.
- Ran core tests.

Change-Id: Iba068935e5da92d71e16e2321afdb8e7b781086a
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M testdata/workloads/functional-query/queries/QueryTest/create-database.test
M tests/authorization/test_owner_privileges.py
M tests/common/sentry_cache_test_suite.py
10 files changed, 223 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/14121/5
-- 
To view, visit http://gerrit.cloudera.org:8080/14121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iba068935e5da92d71e16e2321afdb8e7b781086a
Gerrit-Change-Number: 14121
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>