You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2020/04/01 22:24:18 UTC
[kudu] 01/02: [ranger] authorize list tables should never throw
NotAuthorized
This is an automated email from the ASF dual-hosted git repository.
todd pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit abd29cc98af31d57251f19de1b68037afce2aff1
Author: Hao Hao <ha...@cloudera.com>
AuthorDate: Mon Mar 30 17:49:32 2020 -0700
[ranger] authorize list tables should never throw NotAuthorized
When authorizing list tables, the tables the user has no privileges
should be filtered out. However, that should not report an NotAuthorized
error even the user has no privileges over any tables. This also matches
the behavior with Sentry authorization.
Change-Id: I2d1df45a82acc70000783f0cbcce4d3c81840176
Reviewed-on: http://gerrit.cloudera.org:8080/15609
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/master/ranger_authz_provider.cc | 6 ++++++
src/kudu/ranger/ranger_client-test.cc | 13 ++++++-------
src/kudu/ranger/ranger_client.cc | 10 ----------
src/kudu/ranger/ranger_client.h | 6 ++----
4 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/src/kudu/master/ranger_authz_provider.cc b/src/kudu/master/ranger_authz_provider.cc
index 43ecfe5..8cbb49f 100644
--- a/src/kudu/master/ranger_authz_provider.cc
+++ b/src/kudu/master/ranger_authz_provider.cc
@@ -111,6 +111,12 @@ Status RangerAuthzProvider::AuthorizeListTables(const string& user,
}
*checked_table_names = true;
+
+ // Return immediately if there is no tables to authorize against.
+ if (table_names->empty()) {
+ return Status::OK();
+ }
+
// List tables requires 'METADATA ON TABLE' privilege on all tables being listed.
return client_.AuthorizeActionMultipleTables(user, ActionPB::METADATA, table_names);
}
diff --git a/src/kudu/ranger/ranger_client-test.cc b/src/kudu/ranger/ranger_client-test.cc
index b15be50..017423e 100644
--- a/src/kudu/ranger/ranger_client-test.cc
+++ b/src/kudu/ranger/ranger_client-test.cc
@@ -170,9 +170,8 @@ TEST_F(RangerClientTest, TestAuthorizeListNoTablesAuthorized) {
unordered_set<string> tables;
tables.emplace("foo.bar");
tables.emplace("foo.baz");
- auto s = client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables);
- ASSERT_TRUE(s.IsNotAuthorized());
- ASSERT_EQ(2, tables.size());
+ ASSERT_OK(client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables));
+ ASSERT_EQ(0, tables.size());
}
TEST_F(RangerClientTest, TestAuthorizeMetadataSubsetOfTablesAuthorized) {
@@ -201,8 +200,8 @@ TEST_F(RangerClientTest, TestAuthorizeMetadataAllNonRanger) {
unordered_set<string> tables;
tables.emplace("foo.");
tables.emplace(".bar");
- auto s = client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables);
- ASSERT_TRUE(s.IsNotAuthorized());
+ ASSERT_OK(client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables));
+ ASSERT_EQ(0, tables.size());
}
TEST_F(RangerClientTest, TestAuthorizeMetadataNoneAuthorizedContainsNonRanger) {
@@ -211,8 +210,8 @@ TEST_F(RangerClientTest, TestAuthorizeMetadataNoneAuthorizedContainsNonRanger) {
tables.emplace(".bar");
tables.emplace("foo.bar");
tables.emplace("foo.baz");
- auto s = client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables);
- ASSERT_TRUE(s.IsNotAuthorized());
+ ASSERT_OK(client_.AuthorizeActionMultipleTables("jdoe", ActionPB::METADATA, &tables));
+ ASSERT_EQ(0, tables.size());
}
TEST_F(RangerClientTest, TestAuthorizeMetadataAllAuthorizedContainsNonRanger) {
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index 53371ee..2f0ba21 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -352,10 +352,6 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name,
const ActionPB& action,
unordered_set<string>* table_names) {
DCHECK(subprocess_);
- // Return immediately if there is no tables to authorize against.
- if (table_names->empty()) {
- return Status::OK();
- }
RangerRequestListPB req_list;
RangerResponseListPB resp_list;
@@ -391,12 +387,6 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name,
}
}
- if (allowed_tables.empty()) {
- LOG(WARNING) << Substitute("User $0 is not authorized to perform $1 on $2 tables",
- user_name, ActionPB_Name(action), table_names->size());
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
*table_names = move(allowed_tables);
return Status::OK();
diff --git a/src/kudu/ranger/ranger_client.h b/src/kudu/ranger/ranger_client.h
index 1117785..4578f6c 100644
--- a/src/kudu/ranger/ranger_client.h
+++ b/src/kudu/ranger/ranger_client.h
@@ -77,10 +77,8 @@ class RangerClient {
const std::string& table_name, Scope scope = Scope::TABLE)
WARN_UNUSED_RESULT;
- // Authorizes action on multiple tables. If there is at least one table that
- // user is authorized to perform the action on, it sets 'table_names' to the
- // tables the user is authorized to access and returns OK, NotAuthorized
- // otherwise.
+ // Authorizes action on multiple tables. It sets 'table_names' to the
+ // tables the user is authorized to access and returns OK.
Status AuthorizeActionMultipleTables(const std::string& user_name, const ActionPB& action,
std::unordered_set<std::string>* table_names)
WARN_UNUSED_RESULT;