kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [kudu] 01/02: [sentry] add require_db_privileges flag for ListTables
Date Mon, 17 Jun 2019 12:41:43 GMT
This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 45f03d69bdc73d1c7e08f249b4a40ecfbfd7f810
Author: Hao Hao <hao.hao@cloudera.com>
AuthorDate: Fri Jun 14 11:02:35 2019 -0700

    [sentry] add require_db_privileges flag for ListTables
    
    This patch adds a sentry_require_db_privileges_for_list_tables flag to
    allow enforcing database level privileges for ListTables. Without this
    flag, there will be a number of requests to Sentry related to the number
    of tables in Kudu. With it, that number is limited to the number of
    databases in Kudu. However, note that when the flag is set to true,
    users with no database-level privileges on a database will not be able
    to see any tables within it.
    
    Using ListTablesBenchmark without the flag:
     $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10
--num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false
    
    sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s
sys 0.016s
    
    With the flag:
     $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10
--num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true
    
    sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s
sys 0.000s
    
    Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
    Reviewed-on: http://gerrit.cloudera.org:8080/13657
    Tested-by: Kudu Jenkins
    Reviewed-by: Hao Hao <hao.hao@cloudera.com>
---
 src/kudu/master/sentry_authz_provider-test.cc |  8 ++++++++
 src/kudu/master/sentry_authz_provider.cc      | 11 ++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/kudu/master/sentry_authz_provider-test.cc b/src/kudu/master/sentry_authz_provider-test.cc
index 3a281df..fd8230a 100644
--- a/src/kudu/master/sentry_authz_provider-test.cc
+++ b/src/kudu/master/sentry_authz_provider-test.cc
@@ -83,6 +83,7 @@ DEFINE_bool(has_db_privileges, true,
     "Whether the user should have db-level privileges in testing");
 TAG_FLAG(has_db_privileges, hidden);
 
+DECLARE_bool(sentry_require_db_privileges_for_list_tables);
 DECLARE_int32(sentry_service_recv_timeout_seconds);
 DECLARE_int32(sentry_service_send_timeout_seconds);
 DECLARE_uint32(sentry_privileges_cache_capacity_mb);
@@ -453,6 +454,13 @@ TEST_F(SentryAuthzProviderTest, TestListTables) {
   ASSERT_OK(sentry_authz_provider_->AuthorizeListTables(kTestUser, &tables, &checked_table_names));
   ASSERT_TRUE(checked_table_names);
   ASSERT_EQ(kNumDbs, tables.size());
+
+  // When requires database level privileges for list tables, user shouldn't see
+  // any tables with only table level privileges.
+  FLAGS_sentry_require_db_privileges_for_list_tables = true;
+  ASSERT_OK(sentry_authz_provider_->AuthorizeListTables(kTestUser, &tables, &checked_table_names));
+  ASSERT_TRUE(checked_table_names);
+  ASSERT_EQ(0, tables.size());
 }
 
 class SentryAuthzProviderFilterPrivilegesTest : public SentryAuthzProviderTest {
diff --git a/src/kudu/master/sentry_authz_provider.cc b/src/kudu/master/sentry_authz_provider.cc
index 80b8074..355bdae 100644
--- a/src/kudu/master/sentry_authz_provider.cc
+++ b/src/kudu/master/sentry_authz_provider.cc
@@ -22,20 +22,29 @@
 #include <utility>
 #include <vector>
 
+#include <gflags/gflags.h>
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
 #include "kudu/common/common.pb.h"
 #include "kudu/common/table_util.h"
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/sentry_privileges_fetcher.h"
 #include "kudu/security/token.pb.h"
 #include "kudu/sentry/sentry_action.h"
 #include "kudu/sentry/sentry_authorizable_scope.h"
+#include "kudu/util/flag_tags.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/trace.h"
 
+DEFINE_bool(sentry_require_db_privileges_for_list_tables, false,
+            "Whether Kudu will require database-level privileges to authorize "
+            "ListTables requests. When set to false, table-level privileges are "
+            "required for each table.");
+TAG_FLAG(sentry_require_db_privileges_for_list_tables, advanced);
+
 DECLARE_string(sentry_service_rpc_addresses);
 
 using kudu::security::ColumnPrivilegePB;
@@ -230,7 +239,7 @@ Status SentryAuthzProvider::AuthorizeListTables(const string& user,
       for (auto table_name : tables_in_db) {
         EmplaceOrDie(&authorized_tables, std::move(table_name));
       }
-    } else {
+    } else if (!FLAGS_sentry_require_db_privileges_for_list_tables) {
       for (auto table_name : tables_in_db) {
         s = AuthorizeGetTableMetadata(table_name, user);
         if (s.ok()) {


Mime
View raw message