impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From k...@apache.org
Subject [2/4] incubator-impala git commit: IMPALA-4965: Authorize access to runtime profile and exec summary
Date Sat, 10 Jun 2017 05:11:50 GMT
IMPALA-4965: Authorize access to runtime profile and exec summary

Bug:
When Sentry-based authorization is enabled, a user that isn't authorized
to EXPLAIN a statement that uses a view can still access unauthorized
information, such as view's definition, by running the statement and
asking for the query profile or the execution summary.

Fix:
During query compilation, determine if the user can access the the runtime
profile or the execution summary. Upon request for a runtime profile or
execution summary from a user, determine based on that information and
the user that is asking for the profile if the runtime profile
(or execution summary) will be returned or an authorization error.

The authorization rule enforced is the following:
- User A runs statement S, A asks for profile, A has profile access:
  Runtime profile is returned
- User A runs statement S, A asks for profile, A doesn't have profile access:
  Authorization error
- User A runs statement S, user B asks for profile:
  Authorization error.

This patch doesn't enforce access to the runtime profile or execution summary
through the Web UI.

Change-Id: I2255d587367c2d328590ae8534a5406c4b0c9b15
Reviewed-on: http://gerrit.cloudera.org:8080/7064
Reviewed-by: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/aba37d32
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/aba37d32
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/aba37d32

Branch: refs/heads/master
Commit: aba37d3218a8b2d97f41c463af51517376317bc3
Parents: fa174fc
Author: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Authored: Fri Jun 2 11:31:30 2017 -0700
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Sat Jun 10 02:08:37 2017 +0000

----------------------------------------------------------------------
 be/src/service/client-request-state.cc          |   2 +
 be/src/service/client-request-state.h           |   8 +-
 be/src/service/impala-beeswax-server.cc         |  25 +++-
 be/src/service/impala-hs2-server.cc             |  15 ++-
 be/src/service/impala-http-handler.cc           |   4 +-
 be/src/service/impala-server.cc                 |  15 ++-
 be/src/service/impala-server.h                  |  23 +++-
 be/src/util/auth-util.cc                        |  15 ++-
 be/src/util/auth-util.h                         |  12 ++
 common/thrift/Frontend.thrift                   |   5 +
 .../apache/impala/analysis/AnalysisContext.java |  16 ++-
 .../org/apache/impala/analysis/Analyzer.java    |  35 +++--
 .../apache/impala/analysis/InlineViewRef.java   |   7 +-
 .../impala/analysis/ShowCreateTableStmt.java    |   6 +-
 .../org/apache/impala/service/Frontend.java     |   1 +
 shell/impala_shell.py                           |  19 ++-
 tests/authorization/test_authorization.py       | 132 ++++++++++++++++++-
 17 files changed, 285 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/client-request-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index dcea8cb..6af0501 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -83,6 +83,7 @@ ClientRequestState::ClientRequestState(
     is_cancelled_(false),
     eos_(false),
     query_state_(beeswax::QueryState::CREATED),
+    user_has_profile_access_(true),
     current_batch_(NULL),
     current_batch_row_(0),
     num_rows_fetched_(0),
@@ -1083,4 +1084,5 @@ void ClientRequestState::UpdateQueryState(
   query_state_ = query_state;
   summary_profile_.AddInfoString("Query State", PrintQueryState(query_state_));
 }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/client-request-state.h
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.h b/be/src/service/client-request-state.h
index 5990b88..ea66022 100644
--- a/be/src/service/client-request-state.h
+++ b/be/src/service/client-request-state.h
@@ -147,6 +147,7 @@ class ClientRequestState {
       return GetEffectiveUser(query_ctx_.session);
   }
   const std::string& connected_user() const { return query_ctx_.session.connected_user;
}
+  bool user_has_profile_access() const { return user_has_profile_access_; }
   const std::string& do_as_user() const { return query_ctx_.session.delegated_user; }
   TSessionType::type session_type() const { return query_ctx_.session.session_type; }
   const TUniqueId& session_id() const { return query_ctx_.session.session_id; }
@@ -180,6 +181,9 @@ class ClientRequestState {
   beeswax::QueryState::type query_state() const { return query_state_; }
   const Status& query_status() const { return query_status_; }
   void set_result_metadata(const TResultSetMetadata& md) { result_metadata_ = md; }
+  void set_user_profile_access(bool user_has_profile_access) {
+    user_has_profile_access_ = user_has_profile_access;
+  }
   const RuntimeProfile& profile() const { return profile_; }
   const RuntimeProfile& summary_profile() const { return summary_profile_; }
   const TimestampValue& start_time() const { return start_time_; }
@@ -315,7 +319,9 @@ class ClientRequestState {
   beeswax::QueryState::type query_state_;
   Status query_status_;
   TExecRequest exec_request_;
-
+  /// If true, effective_user() has access to the runtime profile and execution
+  /// summary.
+  bool user_has_profile_access_;
   TResultSetMetadata result_metadata_; // metadata for select query
   RowBatch* current_batch_; // the current row batch; only applicable if coord is set
   int current_batch_row_; // number of rows fetched within the current batch

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/impala-beeswax-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-beeswax-server.cc b/be/src/service/impala-beeswax-server.cc
index 2f80fcd..a867d14 100644
--- a/be/src/service/impala-beeswax-server.cc
+++ b/be/src/service/impala-beeswax-server.cc
@@ -28,6 +28,7 @@
 #include "service/client-request-state.h"
 #include "service/query-options.h"
 #include "service/query-result-set.h"
+#include "util/auth-util.h"
 #include "util/impalad-metrics.h"
 #include "util/webserver.h"
 #include "util/runtime-profile.h"
@@ -348,14 +349,19 @@ void ImpalaServer::CloseInsert(TInsertResult& insert_result,
 // getting the profile, such as no matching queries found.
 void ImpalaServer::GetRuntimeProfile(string& profile_output, const QueryHandle& handle)
{
   ScopedSessionState session_handle(this);
-  RAISE_IF_ERROR(session_handle.WithSession(ThriftServer::GetThreadConnectionId()),
+  const TUniqueId& session_id = ThriftServer::GetThreadConnectionId();
+  stringstream ss;
+  shared_ptr<SessionState> session;
+  RAISE_IF_ERROR(session_handle.WithSession(session_id, &session),
       SQLSTATE_GENERAL_ERROR);
-
+  if (session == NULL) {
+    ss << Substitute("Invalid session id: $0", PrintId(session_id));
+    RaiseBeeswaxException(ss.str(), SQLSTATE_GENERAL_ERROR);
+  }
   TUniqueId query_id;
   QueryHandleToTUniqueId(handle, &query_id);
   VLOG_RPC << "GetRuntimeProfile(): query_id=" << PrintId(query_id);
-  stringstream ss;
-  Status status = GetRuntimeProfileStr(query_id, false, &ss);
+  Status status = GetRuntimeProfileStr(query_id, GetEffectiveUser(*session), false, &ss);
   if (!status.ok()) {
     ss << "GetRuntimeProfile error: " << status.GetDetail();
     RaiseBeeswaxException(ss.str(), SQLSTATE_GENERAL_ERROR);
@@ -366,12 +372,19 @@ void ImpalaServer::GetRuntimeProfile(string& profile_output, const
QueryHandle&
 void ImpalaServer::GetExecSummary(impala::TExecSummary& result,
       const beeswax::QueryHandle& handle) {
   ScopedSessionState session_handle(this);
-  RAISE_IF_ERROR(session_handle.WithSession(ThriftServer::GetThreadConnectionId()),
+  const TUniqueId& session_id = ThriftServer::GetThreadConnectionId();
+  shared_ptr<SessionState> session;
+  RAISE_IF_ERROR(session_handle.WithSession(session_id, &session),
       SQLSTATE_GENERAL_ERROR);
+  if (session == NULL) {
+    stringstream ss;
+    ss << Substitute("Invalid session id: $0", PrintId(session_id));
+    RaiseBeeswaxException(ss.str(), SQLSTATE_GENERAL_ERROR);
+  }
   TUniqueId query_id;
   QueryHandleToTUniqueId(handle, &query_id);
   VLOG_RPC << "GetExecSummary(): query_id=" << PrintId(query_id);
-  Status status = GetExecSummary(query_id, &result);
+  Status status = GetExecSummary(query_id, GetEffectiveUser(*session), &result);
   if (!status.ok()) RaiseBeeswaxException(status.GetDetail(), SQLSTATE_GENERAL_ERROR);
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/impala-hs2-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-hs2-server.cc b/be/src/service/impala-hs2-server.cc
index 6f50f64..1bb2e8a 100644
--- a/be/src/service/impala-hs2-server.cc
+++ b/be/src/service/impala-hs2-server.cc
@@ -41,6 +41,7 @@
 #include "service/client-request-state.h"
 #include "service/query-options.h"
 #include "service/query-result-set.h"
+#include "util/auth-util.h"
 #include "util/debug-util.h"
 #include "util/impalad-metrics.h"
 #include "util/runtime-profile-counters.h"
@@ -847,13 +848,17 @@ void ImpalaServer::GetExecSummary(TGetExecSummaryResp& return_val,
   shared_ptr<SessionState> session;
   HS2_RETURN_IF_ERROR(return_val, session_handle.WithSession(session_id, &session),
       SQLSTATE_GENERAL_ERROR);
+  if (session == NULL) {
+    HS2_RETURN_ERROR(return_val, Substitute("Invalid session id: $0",
+        PrintId(session_id)), SQLSTATE_GENERAL_ERROR);
+  }
 
   TUniqueId query_id;
   HS2_RETURN_IF_ERROR(return_val, THandleIdentifierToTUniqueId(
       request.operationHandle.operationId, &query_id, &secret), SQLSTATE_GENERAL_ERROR);
 
   TExecSummary summary;
-  Status status = GetExecSummary(query_id, &summary);
+  Status status = GetExecSummary(query_id, GetEffectiveUser(*session), &summary);
   HS2_RETURN_IF_ERROR(return_val, status, SQLSTATE_GENERAL_ERROR);
   return_val.__set_summary(summary);
   return_val.status.__set_statusCode(thrift::TStatusCode::SUCCESS_STATUS);
@@ -869,14 +874,18 @@ void ImpalaServer::GetRuntimeProfile(TGetRuntimeProfileResp& return_val,
   shared_ptr<SessionState> session;
   HS2_RETURN_IF_ERROR(return_val, session_handle.WithSession(session_id, &session),
       SQLSTATE_GENERAL_ERROR);
+  if (session == NULL) {
+    HS2_RETURN_ERROR(return_val, Substitute("Invalid session id: $0",
+        PrintId(session_id)), SQLSTATE_GENERAL_ERROR);
+  }
 
   TUniqueId query_id;
   HS2_RETURN_IF_ERROR(return_val, THandleIdentifierToTUniqueId(
       request.operationHandle.operationId, &query_id, &secret), SQLSTATE_GENERAL_ERROR);
 
   stringstream ss;
-  HS2_RETURN_IF_ERROR(return_val, GetRuntimeProfileStr(query_id, false, &ss),
-      SQLSTATE_GENERAL_ERROR);
+  HS2_RETURN_IF_ERROR(return_val, GetRuntimeProfileStr(query_id,
+      GetEffectiveUser(*session), false, &ss), SQLSTATE_GENERAL_ERROR);
   return_val.__set_profile(ss.str());
   return_val.status.__set_statusCode(thrift::TStatusCode::SUCCESS_STATUS);
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/impala-http-handler.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-http-handler.cc b/be/src/service/impala-http-handler.cc
index f441c4d..e6d5323 100644
--- a/be/src/service/impala-http-handler.cc
+++ b/be/src/service/impala-http-handler.cc
@@ -204,7 +204,7 @@ void ImpalaHttpHandler::QueryProfileHandler(const Webserver::ArgumentMap&
args,
   }
 
   stringstream ss;
-  Status status = server_->GetRuntimeProfileStr(unique_id, false, &ss);
+  Status status = server_->GetRuntimeProfileStr(unique_id, "", false, &ss);
   if (!status.ok()) {
     Value error(status.GetDetail().c_str(), document->GetAllocator());
     document->AddMember("error", error, document->GetAllocator());
@@ -225,7 +225,7 @@ void ImpalaHttpHandler::QueryProfileEncodedHandler(const Webserver::ArgumentMap&
   if (!status.ok()) {
     ss << status.GetDetail();
   } else {
-    Status status = server_->GetRuntimeProfileStr(unique_id, true, &ss);
+    Status status = server_->GetRuntimeProfileStr(unique_id, "", true, &ss);
     if (!status.ok()) {
       ss.str(Substitute("Could not obtain runtime profile: $0", status.GetDetail()));
     }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index da432dc..6e51e44 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -593,7 +593,7 @@ Status ImpalaServer::InitProfileLogging() {
 }
 
 Status ImpalaServer::GetRuntimeProfileStr(const TUniqueId& query_id,
-    bool base64_encoded, stringstream* output) {
+    const string& user, bool base64_encoded, stringstream* output) {
   DCHECK(output != nullptr);
   // Search for the query id in the active query map
   {
@@ -604,6 +604,8 @@ Status ImpalaServer::GetRuntimeProfileStr(const TUniqueId& query_id,
         return Status::Expected("Query plan is not ready.");
       }
       lock_guard<mutex> l(*request_state->lock());
+      RETURN_IF_ERROR(CheckProfileAccess(user, request_state->effective_user(),
+          request_state->user_has_profile_access()));
       if (base64_encoded) {
         request_state->profile().SerializeToArchiveString(output);
       } else {
@@ -622,6 +624,8 @@ Status ImpalaServer::GetRuntimeProfileStr(const TUniqueId& query_id,
       ss << "Query id " << PrintId(query_id) << " not found.";
       return Status(ss.str());
     }
+    RETURN_IF_ERROR(CheckProfileAccess(user, query_record->second->effective_user,
+        query_record->second->user_has_profile_access));
     if (base64_encoded) {
       (*output) << query_record->second->encoded_profile_str;
     } else {
@@ -631,12 +635,15 @@ Status ImpalaServer::GetRuntimeProfileStr(const TUniqueId& query_id,
   return Status::OK();
 }
 
-Status ImpalaServer::GetExecSummary(const TUniqueId& query_id, TExecSummary* result)
{
+Status ImpalaServer::GetExecSummary(const TUniqueId& query_id, const string& user,
+    TExecSummary* result) {
   // Search for the query id in the active query map.
   {
     shared_ptr<ClientRequestState> request_state = GetClientRequestState(query_id);
     if (request_state != nullptr) {
       lock_guard<mutex> l(*request_state->lock());
+      RETURN_IF_ERROR(CheckProfileAccess(user, request_state->effective_user(),
+          request_state->user_has_profile_access()));
       if (request_state->coord() != nullptr) {
         request_state->coord()->GetTExecSummary(result);
         TExecProgress progress;
@@ -659,6 +666,8 @@ Status ImpalaServer::GetExecSummary(const TUniqueId& query_id, TExecSummary*
res
       ss << "Query id " << PrintId(query_id) << " not found.";
       return Status(ss.str());
     }
+    RETURN_IF_ERROR(CheckProfileAccess(user, query_record->second->effective_user,
+        query_record->second->user_has_profile_access));
     *result = query_record->second->exec_summary;
   }
   return Status::OK();
@@ -833,6 +842,7 @@ Status ImpalaServer::ExecuteInternal(
         exec_env_->frontend()->GetExecRequest(query_ctx, &result)));
 
     (*request_state)->query_events()->MarkEvent("Planning finished");
+    (*request_state)->set_user_profile_access(result.user_has_profile_access);
     (*request_state)->summary_profile()->AddEventSequence(
         result.timeline.name, result.timeline);
     if (result.__isset.result_set_metadata) {
@@ -1677,6 +1687,7 @@ ImpalaServer::QueryStateRecord::QueryStateRecord(const ClientRequestState&
reque
   all_rows_returned = request_state.eos();
   last_active_time_ms = request_state.last_active_ms();
   request_pool = request_state.request_pool();
+  user_has_profile_access = request_state.user_has_profile_access();
 }
 
 bool ImpalaServer::QueryStateRecordLessThan::operator() (

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/service/impala-server.h
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index 7cdb319..344368b 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -473,12 +473,19 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf,
   /// output stream will not be modified on error.
   /// If base64_encoded, outputs the base64 encoded profile output, otherwise the human
   /// readable string.
-  Status GetRuntimeProfileStr(const TUniqueId& query_id, bool base64_encoded,
-      std::stringstream* output) WARN_UNUSED_RESULT;
-
-  /// Returns the exec summary for this query.
-  Status GetExecSummary(const TUniqueId& query_id, TExecSummary* result)
-      WARN_UNUSED_RESULT;
+  /// If the user asking for this profile is the same user that runs the query
+  /// and that user has access to the runtime profile, the profile is written to
+  /// the output. Otherwise, nothing is written to output and an error code is
+  /// returned to indicate an authorization error.
+  Status GetRuntimeProfileStr(const TUniqueId& query_id, const std::string& user,
+      bool base64_encoded, std::stringstream* output) WARN_UNUSED_RESULT;
+
+  /// Returns the exec summary for this query if the user asking for the exec
+  /// summary is the same user that run the query and that user has access to the full
+  /// query profile. Otherwise, an error status is returned to indicate an
+  /// authorization error.
+  Status GetExecSummary(const TUniqueId& query_id, const std::string& user,
+      TExecSummary* result) WARN_UNUSED_RESULT;
 
   /// Initialize "default_configs_" to show the default values for ImpalaQueryOptions and
   /// "support_start_over/false" to indicate that Impala does not support start over
@@ -560,6 +567,10 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf,
     /// will be set to the delegated user.
     std::string effective_user;
 
+    /// If true, effective_user has access to the runtime profile and execution
+    /// summary.
+    bool user_has_profile_access;
+
     /// default db for this query
     std::string default_db;
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/util/auth-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/auth-util.cc b/be/src/util/auth-util.cc
index 6e97537..e0a1719 100644
--- a/be/src/util/auth-util.cc
+++ b/be/src/util/auth-util.cc
@@ -15,8 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "service/client-request-state.h"
 #include "util/auth-util.h"
-
 #include "gen-cpp/ImpalaInternalService_types.h"
 
 using namespace std;
@@ -30,4 +30,17 @@ namespace impala {
     return session.connected_user;
   }
 
+  const std::string& GetEffectiveUser(const ImpalaServer::SessionState& session)
{
+    return session.do_as_user.empty() ? session.connected_user : session.do_as_user;
+  }
+
+  Status CheckProfileAccess(const string& user, const string& effective_user,
+      bool has_access) {
+    if (user.empty() || (user == effective_user && has_access)) return Status::OK();
+    stringstream ss;
+    ss << "User " << user << " is not authorized to access the runtime
profile or "
+       << "execution summary.";
+    return Status(ss.str());
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/be/src/util/auth-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/auth-util.h b/be/src/util/auth-util.h
index d42307c..8862b7e 100644
--- a/be/src/util/auth-util.h
+++ b/be/src/util/auth-util.h
@@ -20,6 +20,7 @@
 #define IMPALA_UTIL_AUTH_UTIL_H
 
 #include <string>
+#include "service/impala-server.h"
 
 namespace impala {
 
@@ -32,5 +33,16 @@ class TSessionState;
 /// delegated_user is empty, the effective user is the connected user.
 const std::string& GetEffectiveUser(const TSessionState& session);
 
+/// Same behavior as the function above with different input parameter type.
+const std::string& GetEffectiveUser(const ImpalaServer::SessionState& session);
+
+/// Checks if 'user' can access the runtime profile or execution summary of a
+/// statement by comparing 'user' with the user that run the statement, 'effective_user',
+/// and checking if 'effective_user' is authorized to access the profile, as indicated by
+/// 'has_access'. An error Status is returned if 'user' is not authorized to
+/// access the runtime profile or execution summary.
+Status CheckProfileAccess(const std::string& user, const std::string& effective_user,
+    bool has_access);
+
 } // namespace impala
 #endif

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/common/thrift/Frontend.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/Frontend.thrift b/common/thrift/Frontend.thrift
index 4337851..e07fe84 100644
--- a/common/thrift/Frontend.thrift
+++ b/common/thrift/Frontend.thrift
@@ -564,6 +564,11 @@ struct TExecRequest {
 
   // Timeline of planner's operation, for profiling
   11: optional RuntimeProfile.TEventSequence timeline
+
+  // If false, the user that runs this statement doesn't have access to the runtime
+  // profile. For example, a user can't access the runtime profile of a query
+  // that has a view for which the user doesn't have access to the underlying tables.
+  12: optional bool user_has_profile_access
 }
 
 // Parameters to FeSupport.cacheJar().

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index 34ac107..efde6b0 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -44,6 +44,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
@@ -93,6 +94,7 @@ public class AnalysisContext {
     private StatementBase stmt_;
     private Analyzer analyzer_;
     private EventSequence timeline_;
+    private boolean userHasProfileAccess_ = true;
 
     public boolean isAlterTableStmt() { return stmt_ instanceof AlterTableStmt; }
     public boolean isAlterViewStmt() { return stmt_ instanceof AlterViewStmt; }
@@ -345,6 +347,8 @@ public class AnalysisContext {
     public TLineageGraph getThriftLineageGraph() {
       return analyzer_.getThriftSerializedLineageGraph();
     }
+    public void setUserHasProfileAccess(boolean value) { userHasProfileAccess_ = value; }
+    public boolean userHasProfileAccess() { return userHasProfileAccess_; }
   }
 
   /**
@@ -490,10 +494,18 @@ public class AnalysisContext {
       }
     }
 
-    // Check any masked requests.
+    // Check all masked requests. If a masked request has an associated error message,
+    // an AuthorizationException is thrown if authorization fails. Masked requests with no
+    // error message are used to check if the user can access the runtime profile.
+    // These checks don't result in an AuthorizationException but set the
+    // 'user_has_profile_access' flag in queryCtx_.
     for (Pair<PrivilegeRequest, String> maskedReq: analyzer.getMaskedPrivilegeReqs())
{
       if (!authzChecker.hasAccess(analyzer.getUser(), maskedReq.first)) {
-        throw new AuthorizationException(maskedReq.second);
+        analysisResult_.setUserHasProfileAccess(false);
+        if (!Strings.isNullOrEmpty(maskedReq.second)) {
+          throw new AuthorizationException(maskedReq.second);
+        }
+        break;
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index ab3da02..7fd6cf7 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -150,12 +150,15 @@ public class Analyzer {
   // Flag indicating whether this analyzer belongs to a WITH clause view.
   private boolean isWithClause_ = false;
 
-  // If set, when privilege requests are registered they will use this error
+  // If set, when masked privilege requests are registered they will use this error
   // error message.
   private String authErrorMsg_;
 
-  // If false, privilege requests will not be registered in the analyzer.
-  // Note: it's not the purpose of this flag to control if security is enabled in general.
+  // If true privilege requests are added in maskedPrivileReqs_. Otherwise, privilege
+  // requests are added to privilegeReqs_.
+  private boolean maskPrivChecks_ = false;
+
+  // If false, privilege requests are not registered.
   private boolean enablePrivChecks_ = true;
 
   // By default, all registered semi-joined tuples are invisible, i.e., their slots
@@ -408,6 +411,7 @@ public class Analyzer {
     user_ = parentAnalyzer.getUser();
     useHiveColLabels_ = parentAnalyzer.useHiveColLabels_;
     authErrorMsg_ = parentAnalyzer.authErrorMsg_;
+    maskPrivChecks_ = parentAnalyzer.maskPrivChecks_;
     enablePrivChecks_ = parentAnalyzer.enablePrivChecks_;
     isWithClause_ = parentAnalyzer.isWithClause_;
   }
@@ -2448,10 +2452,12 @@ public class Analyzer {
     return new TableName(getDefaultDb(), tableName.getTbl());
   }
 
-  public void setEnablePrivChecks(boolean value) {
-    enablePrivChecks_ = value;
+  public void setMaskPrivChecks(String errMsg) {
+    maskPrivChecks_ = true;
+    authErrorMsg_ = errMsg;
   }
-  public void setAuthErrMsg(String errMsg) { authErrorMsg_ = errMsg; }
+
+  public void setEnablePrivChecks(boolean value) { enablePrivChecks_ = value; }
   public void setIsStraightJoin() { isStraightJoin_ = true; }
   public boolean isStraightJoin() { return isStraightJoin_; }
   public void setIsExplain() { globalState_.isExplain = true; }
@@ -2518,22 +2524,15 @@ public class Analyzer {
   }
 
   /**
-   * Registers a new PrivilegeRequest in the analyzer. If authErrorMsg_ is set,
-   * the privilege request will be added to the list of "masked" privilege requests,
-   * using authErrorMsg_ as the auth failure error message. Otherwise it will get
-   * added as a normal privilege request that will use the standard error message
-   * on authorization failure.
-   * If enablePrivChecks_ is false, the registration request will be ignored. This
-   * is used when analyzing catalog views since users should be able to query a view
-   * even if they do not have privileges on the underlying tables.
+   * Registers a new PrivilegeRequest in the analyzer.
    */
   public void registerPrivReq(PrivilegeRequest privReq) {
     if (!enablePrivChecks_) return;
-
-    if (Strings.isNullOrEmpty(authErrorMsg_)) {
-      globalState_.privilegeReqs.add(privReq);
+    if (maskPrivChecks_) {
+      globalState_.maskedPrivilegeReqs.add(
+          Pair.<PrivilegeRequest, String>create(privReq, authErrorMsg_));
     } else {
-      globalState_.maskedPrivilegeReqs.add(Pair.create(privReq, authErrorMsg_));
+      globalState_.privilegeReqs.add(privReq);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
index 3d309e0..3f6bc68 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
@@ -150,13 +150,14 @@ public class InlineViewRef extends TableRef {
         // If the user does not have privileges on the view's definition
         // then we report a masked authorization error so as not to reveal
         // privileged information (e.g., the existence of a table).
-        inlineViewAnalyzer_.setAuthErrMsg(
+        inlineViewAnalyzer_.setMaskPrivChecks(
             String.format("User '%s' does not have privileges to " +
             "EXPLAIN this statement.", analyzer.getUser().getName()));
       } else {
         // If this is not an EXPLAIN statement, auth checks for the view
-        // definition should be disabled.
-        inlineViewAnalyzer_.setEnablePrivChecks(false);
+        // definition are still performed in order to determine if the user has access
+        // to the runtime profile but don't trigger authorization errors.
+        inlineViewAnalyzer_.setMaskPrivChecks(null);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
index 13206c9..d2b57e8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
@@ -60,9 +60,9 @@ public class ShowCreateTableStmt extends StatementBase {
       // Only show the view's query if the user has permissions to execute the query, to
       // avoid revealing information, e.g. tables the user does not have access to.
       // Report a masked authorization message if authorization fails.
-      viewAnalyzer.setAuthErrMsg(String.format("User '%s' does not have privileges to " +
-          "see the definition of view '%s'.",
-          analyzer.getUser().getName(), view.getFullName()));
+      viewAnalyzer.setMaskPrivChecks(String.format("User '%s' does not have privileges "
+
+          "to see the definition of view '%s'.", analyzer.getUser().getName(),
+          view.getFullName()));
       QueryStmt viewQuery = view.getQueryStmt().clone();
       // Views from the Hive metastore may rely on Hive's column naming if the SQL
       // statement references a column by its implicitly defined column names.

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/fe/src/main/java/org/apache/impala/service/Frontend.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 3f4cc8a..d0a3b4d 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -1057,6 +1057,7 @@ public class Frontend {
     result.setQuery_options(queryCtx.client_request.getQuery_options());
     result.setAccess_events(analysisResult.getAccessEvents());
     result.analysis_warnings = analysisResult.getAnalyzer().getWarnings();
+    result.setUser_has_profile_access(analysisResult.userHasProfileAccess());
 
     TQueryOptions queryOptions = queryCtx.client_request.query_options;
     if (analysisResult.isCatalogOp()) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/shell/impala_shell.py
----------------------------------------------------------------------
diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index 8af2b57..d3495f7 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -511,10 +511,13 @@ class ImpalaShell(cmd.Cmd):
     summary = None
     try:
       summary = self.imp_client.get_summary(self.last_query_handle)
-    except RPCException:
-      pass
-    if summary is None:
-      print_to_stderr("Could not retrieve summary for query.")
+    except RPCException, e:
+      import re
+      error_pattern = re.compile("ERROR: Query id \d+:\d+ not found.")
+      if error_pattern.match(e.value):
+        print_to_stderr("Could not retrieve summary for query.")
+      else:
+        print_to_stderr(e)
       return CmdStatus.ERROR
     if summary.nodes is None:
       print_to_stderr("Summary not available")
@@ -943,9 +946,11 @@ class ImpalaShell(cmd.Cmd):
       if not is_dml:
         self.imp_client.close_query(self.last_query_handle, self.query_handle_closed)
       self.query_handle_closed = True
-
-      profile = self.imp_client.get_runtime_profile(self.last_query_handle)
-      self.print_runtime_profile(profile)
+      try:
+        profile = self.imp_client.get_runtime_profile(self.last_query_handle)
+        self.print_runtime_profile(profile)
+      except RPCException, e:
+        if self.show_profiles: raise e
       return CmdStatus.SUCCESS
     except RPCException, e:
       # could not complete the rpc successfully

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/aba37d32/tests/authorization/test_authorization.py
----------------------------------------------------------------------
diff --git a/tests/authorization/test_authorization.py b/tests/authorization/test_authorization.py
index 65833e0..38a1ee3 100644
--- a/tests/authorization/test_authorization.py
+++ b/tests/authorization/test_authorization.py
@@ -24,6 +24,7 @@ import tempfile
 import json
 from time import sleep, time
 from getpass import getuser
+from ImpalaService import ImpalaHiveServer2Service
 from TCLIService import TCLIService
 from thrift.transport.TSocket import TSocket
 from thrift.transport.TTransport import TBufferedTransport
@@ -44,7 +45,7 @@ class TestAuthorization(CustomClusterTestSuite):
     self.transport = TBufferedTransport(self.socket)
     self.transport.open()
     self.protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
-    self.hs2_client = TCLIService.Client(self.protocol)
+    self.hs2_client = ImpalaHiveServer2Service.Client(self.protocol)
 
   def teardown(self):
     if self.socket:
@@ -82,6 +83,94 @@ class TestAuthorization(CustomClusterTestSuite):
     execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
     TestHS2.check_response(execute_statement_resp)
 
+
+  @pytest.mark.execute_serially
+  @CustomClusterTestSuite.with_args("--server_name=server1\
+      --authorization_policy_file=%s\
+      --authorized_proxy_user_config=hue=%s" % (AUTH_POLICY_FILE, getuser()))
+  def test_access_runtime_profile(self):
+    from tests.hs2.test_hs2 import TestHS2
+    open_session_req = TCLIService.TOpenSessionReq()
+    open_session_req.username = getuser()
+    open_session_req.configuration = dict()
+    resp = self.hs2_client.OpenSession(open_session_req)
+    TestHS2.check_response(resp)
+
+    # Current user can't access view's underlying tables
+    self.session_handle = resp.sessionHandle
+    execute_statement_req = TCLIService.TExecuteStatementReq()
+    execute_statement_req.sessionHandle = self.session_handle
+    execute_statement_req.statement = "explain select * from functional.complex_view"
+    execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
+    assert 'User \'%s\' does not have privileges to EXPLAIN' % getuser() in\
+        str(execute_statement_resp)
+    # User should not have access to the runtime profile
+    self.__run_stmt_and_verify_profile_access("select * from functional.complex_view",
+        False, False)
+    self.__run_stmt_and_verify_profile_access("select * from functional.complex_view",
+        False, True)
+
+    # Repeat as a delegated user
+    open_session_req.username = 'hue'
+    open_session_req.configuration = dict()
+    # Delegated user is the current user
+    open_session_req.configuration['impala.doas.user'] = getuser()
+    resp = self.hs2_client.OpenSession(open_session_req)
+    TestHS2.check_response(resp)
+    self.session_handle = resp.sessionHandle
+    # User should not have access to the runtime profile
+    self.__run_stmt_and_verify_profile_access("select * from functional.complex_view",
+        False, False)
+    self.__run_stmt_and_verify_profile_access("select * from functional.complex_view",
+        False, True)
+
+    # Create a view for which the user has access to the underlying tables.
+    open_session_req.username = getuser()
+    open_session_req.configuration = dict()
+    resp = self.hs2_client.OpenSession(open_session_req)
+    TestHS2.check_response(resp)
+    self.session_handle = resp.sessionHandle
+    execute_statement_req = TCLIService.TExecuteStatementReq()
+    execute_statement_req.sessionHandle = self.session_handle
+    execute_statement_req.statement = """create view if not exists tpch.customer_view as
+        select * from tpch.customer limit 1"""
+    execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
+    TestHS2.check_response(execute_statement_resp)
+
+    # User should be able to run EXPLAIN
+    execute_statement_req = TCLIService.TExecuteStatementReq()
+    execute_statement_req.sessionHandle = self.session_handle
+    execute_statement_req.statement = """explain select * from tpch.customer_view"""
+    execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
+    TestHS2.check_response(execute_statement_resp)
+
+    # User should have access to the runtime profile and exec summary
+    self.__run_stmt_and_verify_profile_access("select * from tpch.customer_view", True,
+        False)
+    self.__run_stmt_and_verify_profile_access("select * from tpch.customer_view", True,
+        True)
+
+    # Repeat as a delegated user
+    open_session_req.username = 'hue'
+    open_session_req.configuration = dict()
+    # Delegated user is the current user
+    open_session_req.configuration['impala.doas.user'] = getuser()
+    resp = self.hs2_client.OpenSession(open_session_req)
+    TestHS2.check_response(resp)
+    self.session_handle = resp.sessionHandle
+    # User should have access to the runtime profile and exec summary
+    self.__run_stmt_and_verify_profile_access("select * from tpch.customer_view",
+        True, False)
+    self.__run_stmt_and_verify_profile_access("select * from tpch.customer_view",
+        True, True)
+
+    # Clean up
+    execute_statement_req = TCLIService.TExecuteStatementReq()
+    execute_statement_req.sessionHandle = self.session_handle
+    execute_statement_req.statement = "drop view if exists tpch.customer_view"
+    execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
+    TestHS2.check_response(execute_statement_resp)
+
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args("--server_name=server1\
       --authorization_policy_file=%s\
@@ -195,3 +284,44 @@ class TestAuthorization(CustomClusterTestSuite):
               json_dict[min(json_dict)]['impersonator'] == impersonator:
             return True
     return False
+
+  def __run_stmt_and_verify_profile_access(self, stmt, has_access, close_operation):
+    """Runs 'stmt' and retrieves the runtime profile and exec summary. If
+      'has_access' is true, it verifies that no runtime profile or exec summary are
+      returned. If 'close_operation' is true, make sure the operation is closed before
+      retrieving the profile and exec summary."""
+    from tests.hs2.test_hs2 import TestHS2
+    execute_statement_req = TCLIService.TExecuteStatementReq()
+    execute_statement_req.sessionHandle = self.session_handle
+    execute_statement_req.statement = stmt
+    execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req)
+    TestHS2.check_response(execute_statement_resp)
+
+    if close_operation:
+      close_operation_req = TCLIService.TCloseOperationReq()
+      close_operation_req.operationHandle = execute_statement_resp.operationHandle
+      TestHS2.check_response(self.hs2_client.CloseOperation(close_operation_req))
+
+    get_profile_req = ImpalaHiveServer2Service.TGetRuntimeProfileReq()
+    get_profile_req.operationHandle = execute_statement_resp.operationHandle
+    get_profile_req.sessionHandle = self.session_handle
+    get_profile_resp = self.hs2_client.GetRuntimeProfile(get_profile_req)
+
+    if has_access:
+      TestHS2.check_response(get_profile_resp)
+      assert "Plan: " in get_profile_resp.profile
+    else:
+      assert "User %s is not authorized to access the runtime profile or "\
+          "execution summary." % (getuser()) in str(get_profile_resp)
+
+    exec_summary_req = ImpalaHiveServer2Service.TGetExecSummaryReq()
+    exec_summary_req.operationHandle = execute_statement_resp.operationHandle
+    exec_summary_req.sessionHandle = self.session_handle
+    exec_summary_resp = self.hs2_client.GetExecSummary(exec_summary_req)
+
+    if has_access:
+      TestHS2.check_response(exec_summary_resp)
+      assert exec_summary_resp.summary.nodes is not None
+    else:
+      assert "User %s is not authorized to access the runtime profile or "\
+          "execution summary." % (getuser()) in str(exec_summary_resp)



Mime
View raw message