This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.10.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/branch-1.10.x by this push:
new 0a45b4d [sentry] add require_db_privileges flag for ListTables
0a45b4d is described below
commit 0a45b4dcbbe76d25f0ce4ae96c31191625b23726
Author: Hao Hao <[email protected]>
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 <[email protected]>
(cherry picked from commit 45f03d69bdc73d1c7e08f249b4a40ecfbfd7f810)
Reviewed-on: http://gerrit.cloudera.org:8080/13678
Tested-by: Hao Hao <[email protected]>
---
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()) {
