You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ab...@apache.org on 2020/04/10 18:19:07 UTC
[kudu] 01/02: [ranger] Fix and refactor RangerClient
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit e7044a5cbe1d1f9d36204279c73901d37451a570
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Thu Apr 9 14:58:31 2020 +0200
[ranger] Fix and refactor RangerClient
While working on integration tests we found a discrepancy handling
column-level privileges between Ranger and Sentry. This was caused by
RangerClient returning NotAuthorized when the requested action couldn't
be authorized on any of the resources. This commit moves all
authorization logic to the authorization provider instead of the client.
Change-Id: I3fb7272601c9cb7ebe2e250bea728e76894d242a
Reviewed-on: http://gerrit.cloudera.org:8080/15696
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Attila Bukor <ab...@apache.org>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/master/ranger_authz_provider.cc | 209 +++++++++++++++++++++++--------
src/kudu/ranger/ranger_client-test.cc | 42 ++++---
src/kudu/ranger/ranger_client.cc | 94 +++-----------
src/kudu/ranger/ranger_client.h | 40 +++---
4 files changed, 219 insertions(+), 166 deletions(-)
diff --git a/src/kudu/master/ranger_authz_provider.cc b/src/kudu/master/ranger_authz_provider.cc
index 8fe4e70..4129dd4 100644
--- a/src/kudu/master/ranger_authz_provider.cc
+++ b/src/kudu/master/ranger_authz_provider.cc
@@ -23,20 +23,16 @@
#include <glog/logging.h>
#include "kudu/common/common.pb.h"
+#include "kudu/common/table_util.h"
#include "kudu/gutil/map-util.h"
+#include "kudu/gutil/strings/substitute.h"
#include "kudu/ranger/ranger.pb.h"
#include "kudu/security/token.pb.h"
+#include "kudu/util/slice.h"
#include "kudu/util/status.h"
DECLARE_string(ranger_config_path);
-namespace kudu {
-
-class Env;
-class MetricEntity;
-
-namespace master {
-
using kudu::security::ColumnPrivilegePB;
using kudu::security::TablePrivilegePB;
using kudu::ranger::ActionPB;
@@ -44,6 +40,34 @@ using kudu::ranger::ActionHash;
using kudu::ranger::RangerClient;
using std::string;
using std::unordered_set;
+using strings::Substitute;
+
+namespace kudu {
+
+class Env;
+class MetricEntity;
+
+namespace master {
+
+namespace {
+
+const char* kUnauthorizedAction = "Unauthorized action";
+const char* kDenyNonRangerTableTemplate = "Denying action on table with invalid name $0. "
+ "Use 'kudu table rename_table' to rename it to "
+ "a Ranger-compatible name.";
+
+Status ParseTableIdentifier(const string& table_name, string* db, string* table) {
+ Slice tbl;
+ auto s = ParseRangerTableIdentifier(table_name, db, &tbl);
+ if (PREDICT_FALSE(!s.ok())) {
+ LOG(WARNING) << Substitute(kDenyNonRangerTableTemplate, table_name);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+ *table = tbl.ToString();
+ return Status::OK();
+}
+
+} // anonymous namespace
RangerAuthzProvider::RangerAuthzProvider(Env* env,
const scoped_refptr<MetricEntity>& metric_entity) :
@@ -61,9 +85,23 @@ Status RangerAuthzProvider::AuthorizeCreateTable(const string& table_name,
if (IsTrustedUser(user)) {
return Status::OK();
}
+
+ string db;
+ string tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(table_name, &db, &tbl));
+
+ bool authorized;
// Table creation requires 'CREATE ON DATABASE' privilege.
- return client_.AuthorizeAction(user, ActionPB::CREATE, table_name,
- RangerClient::Scope::DATABASE);
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::CREATE, db, tbl, &authorized,
+ RangerClient::Scope::DATABASE));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to CREATE $1", user, table_name);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
Status RangerAuthzProvider::AuthorizeDropTable(const string& table_name,
@@ -71,8 +109,20 @@ Status RangerAuthzProvider::AuthorizeDropTable(const string& table_name,
if (IsTrustedUser(user)) {
return Status::OK();
}
- // Table deletion requires 'DROP ON TABLE' privilege.
- return client_.AuthorizeAction(user, ActionPB::DROP, table_name);
+
+ string db;
+ string tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(table_name, &db, &tbl));
+ bool authorized;
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::DROP, db, tbl, &authorized));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to DROP $1", user, table_name);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
Status RangerAuthzProvider::AuthorizeAlterTable(const string& old_table,
@@ -81,16 +131,45 @@ Status RangerAuthzProvider::AuthorizeAlterTable(const string& old_table,
if (IsTrustedUser(user)) {
return Status::OK();
}
+
+ string old_db;
+ string old_tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(old_table, &old_db, &old_tbl));
// Table alteration (without table rename) requires ALTER ON TABLE.
+ bool authorized;
if (old_table == new_table) {
- return client_.AuthorizeAction(user, ActionPB::ALTER, old_table);
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALTER, old_db, old_tbl, &authorized));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to ALTER $1", user, old_table);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
// To prevent privilege escalation we require ALL on the old TABLE
// and CREATE on the new DATABASE for table rename.
- RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL, old_table));
- return client_.AuthorizeAction(user, ActionPB::CREATE, new_table,
- RangerClient::Scope::DATABASE);
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL, old_db, old_tbl, &authorized));
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to perform ALL on $1", user, old_table);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ string new_db;
+ string new_tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(new_table, &new_db, &new_tbl));
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::CREATE, new_db, new_tbl, &authorized,
+ RangerClient::Scope::DATABASE));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to CREATE $1", user, new_table);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
Status RangerAuthzProvider::AuthorizeGetTableMetadata(const string& table_name,
@@ -98,8 +177,22 @@ Status RangerAuthzProvider::AuthorizeGetTableMetadata(const string& table_name,
if (IsTrustedUser(user)) {
return Status::OK();
}
+
+ string db;
+ string tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(table_name, &db, &tbl));
+ bool authorized;
// Get table metadata requires 'METADATA ON TABLE' privilege.
- return client_.AuthorizeAction(user, ActionPB::METADATA, table_name);
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::METADATA, db, tbl, &authorized));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to access METADATA on $1", user,
+ table_name);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
Status RangerAuthzProvider::AuthorizeListTables(const string& user,
@@ -126,9 +219,21 @@ Status RangerAuthzProvider::AuthorizeGetTableStatistics(const string& table_name
if (IsTrustedUser(user)) {
return Status::OK();
}
+ string db;
+ string tbl;
+
+ RETURN_NOT_OK(ParseTableIdentifier(table_name, &db, &tbl));
+ bool authorized;
// Statistics contain data (e.g. number of rows) that requires the 'SELECT ON TABLE'
// privilege.
- return client_.AuthorizeAction(user, ActionPB::SELECT, table_name);
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::SELECT, db, tbl, &authorized));
+
+ if (PREDICT_FALSE(!authorized)) {
+ LOG(WARNING) << Substitute("User $0 is not authorized to SELECT on $1", user, table_name);
+ return Status::NotAuthorized(kUnauthorizedAction);
+ }
+
+ return Status::OK();
}
Status RangerAuthzProvider::FillTablePrivilegePB(const string& table_name,
@@ -137,7 +242,18 @@ Status RangerAuthzProvider::FillTablePrivilegePB(const string& table_name,
TablePrivilegePB* pb) {
DCHECK(pb);
DCHECK(pb->has_table_id());
- if (IsTrustedUser(user) || client_.AuthorizeAction(user, ActionPB::ALL, table_name).ok()) {
+
+ string db;
+ string tbl;
+ RETURN_NOT_OK(ParseTableIdentifier(table_name, &db, &tbl));
+
+ bool authorized;
+ if (IsTrustedUser(user)) {
+ authorized = true;
+ } else {
+ RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL, db, tbl, &authorized));
+ }
+ if (authorized) {
pb->set_delete_privilege(true);
pb->set_insert_privilege(true);
pb->set_scan_privilege(true);
@@ -154,29 +270,27 @@ Status RangerAuthzProvider::FillTablePrivilegePB(const string& table_name,
// Check if the user has any table-level privileges. If yes, we set them. If
// select is included, we can also return.
- auto s = client_.AuthorizeActions(user, table_name, &actions);
- if (s.ok()) {
- for (const ActionPB& action : actions) {
- switch (action) {
- case ActionPB::DELETE:
- pb->set_delete_privilege(true);
- break;
- case ActionPB::UPDATE:
- pb->set_update_privilege(true);
- break;
- case ActionPB::INSERT:
- pb->set_insert_privilege(true);
- break;
- case ActionPB::SELECT:
- pb->set_scan_privilege(true);
- break;
- default:
- LOG(WARNING) << "Unexpected action returned by Ranger: " << ActionPB_Name(action);
- break;
- }
- if (pb->scan_privilege()) {
- return Status::OK();
- }
+ RETURN_NOT_OK(client_.AuthorizeActions(user, db, tbl, &actions));
+ for (const ActionPB& action : actions) {
+ switch (action) {
+ case ActionPB::DELETE:
+ pb->set_delete_privilege(true);
+ break;
+ case ActionPB::UPDATE:
+ pb->set_update_privilege(true);
+ break;
+ case ActionPB::INSERT:
+ pb->set_insert_privilege(true);
+ break;
+ case ActionPB::SELECT:
+ pb->set_scan_privilege(true);
+ break;
+ default:
+ LOG(WARNING) << "Unexpected action returned by Ranger: " << ActionPB_Name(action);
+ break;
+ }
+ if (pb->scan_privilege()) {
+ return Status::OK();
}
}
@@ -190,15 +304,10 @@ Status RangerAuthzProvider::FillTablePrivilegePB(const string& table_name,
column_names.emplace(col.name());
}
- // If AuthorizeAction returns NotAuthorized, that means no column-level select
- // is allowed to the user. In this case we return the previous status.
- // Otherwise we populate schema_pb and return OK.
- //
- // TODO(abukor): revisit if it's worth merge this into the previous request
- if (!client_.AuthorizeActionMultipleColumns(user, ActionPB::SELECT, table_name,
- &column_names).ok()) {
- return s;
- }
+
+ // TODO(abukor): revisit if it's worth merging this into the previous request
+ RETURN_NOT_OK(client_.AuthorizeActionMultipleColumns(user, ActionPB::SELECT, db, tbl,
+ &column_names));
for (const auto& col : schema_pb.columns()) {
if (ContainsKey(column_names, col.name())) {
diff --git a/src/kudu/ranger/ranger_client-test.cc b/src/kudu/ranger/ranger_client-test.cc
index 02fe0cd..c08dd75 100644
--- a/src/kudu/ranger/ranger_client-test.cc
+++ b/src/kudu/ranger/ranger_client-test.cc
@@ -171,13 +171,16 @@ class RangerClientTest : public KuduTest {
};
TEST_F(RangerClientTest, TestAuthorizeCreateTableUnauthorized) {
- auto s = client_.AuthorizeAction("jdoe", ActionPB::CREATE, "bar.baz");
- ASSERT_TRUE(s.IsNotAuthorized());
+ bool authorized;
+ ASSERT_OK(client_.AuthorizeAction("jdoe", ActionPB::CREATE, "bar", "baz", &authorized));
+ ASSERT_FALSE(authorized);
}
TEST_F(RangerClientTest, TestAuthorizeCreateTableAuthorized) {
Allow("jdoe", ActionPB::CREATE, "foo", "bar");
- ASSERT_OK(client_.AuthorizeAction("jdoe", ActionPB::CREATE, "foo.bar"));
+ bool authorized;
+ ASSERT_OK(client_.AuthorizeAction("jdoe", ActionPB::CREATE, "foo", "bar", &authorized));
+ ASSERT_TRUE(authorized);
}
TEST_F(RangerClientTest, TestAuthorizeListNoTables) {
@@ -257,7 +260,7 @@ TEST_F(RangerClientTest, TestAuthorizeScanSubsetAuthorized) {
columns.emplace("col3");
columns.emplace("col4");
ASSERT_OK(client_.AuthorizeActionMultipleColumns("jdoe", ActionPB::SELECT,
- "default.foobar", &columns));
+ "default", "foobar", &columns));
ASSERT_EQ(2, columns.size());
ASSERT_TRUE(ContainsKey(columns, "col1"));
ASSERT_TRUE(ContainsKey(columns, "col3"));
@@ -276,7 +279,7 @@ TEST_F(RangerClientTest, TestAuthorizeScanAllColumnsAuthorized) {
columns.emplace("col3");
columns.emplace("col4");
ASSERT_OK(client_.AuthorizeActionMultipleColumns("jdoe", ActionPB::SELECT,
- "default.foobar", &columns));
+ "default", "foobar", &columns));
ASSERT_EQ(4, columns.size());
ASSERT_TRUE(ContainsKey(columns, "col1"));
ASSERT_TRUE(ContainsKey(columns, "col2"));
@@ -289,10 +292,9 @@ TEST_F(RangerClientTest, TestAuthorizeScanNoColumnsAuthorized) {
for (int i = 0; i < 4; ++i) {
columns.emplace(Substitute("col$0", i));
}
- auto s = client_.AuthorizeActionMultipleColumns("jdoe", ActionPB::SELECT,
- "default.foobar", &columns);
- ASSERT_TRUE(s.IsNotAuthorized());
- ASSERT_EQ(4, columns.size());
+ ASSERT_OK(client_.AuthorizeActionMultipleColumns("jdoe", ActionPB::SELECT,
+ "default", "foobar", &columns));
+ ASSERT_EQ(0, columns.size());
}
TEST_F(RangerClientTest, TestAuthorizeActionsNoneAuthorized) {
@@ -300,9 +302,8 @@ TEST_F(RangerClientTest, TestAuthorizeActionsNoneAuthorized) {
actions.emplace(ActionPB::DROP);
actions.emplace(ActionPB::SELECT);
actions.emplace(ActionPB::INSERT);
- auto s = client_.AuthorizeActions("jdoe", "default.foobar", &actions);
- ASSERT_TRUE(s.IsNotAuthorized());
- ASSERT_EQ(3, actions.size());
+ ASSERT_OK(client_.AuthorizeActions("jdoe", "default", "foobar", &actions));
+ ASSERT_EQ(0, actions.size());
}
TEST_F(RangerClientTest, TestAuthorizeActionsSomeAuthorized) {
@@ -311,7 +312,7 @@ TEST_F(RangerClientTest, TestAuthorizeActionsSomeAuthorized) {
actions.emplace(ActionPB::DROP);
actions.emplace(ActionPB::SELECT);
actions.emplace(ActionPB::INSERT);
- ASSERT_OK(client_.AuthorizeActions("jdoe", "default.foobar", &actions));
+ ASSERT_OK(client_.AuthorizeActions("jdoe", "default", "foobar", &actions));
ASSERT_EQ(1, actions.size());
ASSERT_TRUE(ContainsKey(actions, ActionPB::SELECT));
}
@@ -324,7 +325,7 @@ TEST_F(RangerClientTest, TestAuthorizeActionsAllAuthorized) {
actions.emplace(ActionPB::DROP);
actions.emplace(ActionPB::SELECT);
actions.emplace(ActionPB::INSERT);
- ASSERT_OK(client_.AuthorizeActions("jdoe", "default.foobar", &actions));
+ ASSERT_OK(client_.AuthorizeActions("jdoe", "default", "foobar", &actions));
ASSERT_EQ(3, actions.size());
}
@@ -393,8 +394,9 @@ TEST_F(RangerClientTestBase, TestLogging) {
}
// Make a request. It doesn't matter whether it succeeds or not -- debug logs
// should include info about each request.
- Status s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
- ASSERT_TRUE(s.IsNotAuthorized());
+ bool authorized;
+ ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", &authorized));
+ ASSERT_FALSE(authorized);
{
// Check that the Ranger client logs some DEBUG messages.
vector<string> lines;
@@ -412,8 +414,8 @@ TEST_F(RangerClientTestBase, TestLogging) {
FLAGS_ranger_overwrite_log_config = false;
client_.reset(new RangerClient(env_, metric_entity_));
ASSERT_OK(client_->Start());
- s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
- ASSERT_TRUE(s.IsNotAuthorized());
+ ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", &authorized));
+ ASSERT_FALSE(authorized);
{
// Our logs should still contain DEBUG messages since we didn't update the
// logging configuration.
@@ -428,8 +430,8 @@ TEST_F(RangerClientTestBase, TestLogging) {
FLAGS_ranger_overwrite_log_config = true;
client_.reset(new RangerClient(env_, metric_entity_));
ASSERT_OK(client_->Start());
- s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
- ASSERT_TRUE(s.IsNotAuthorized());
+ ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", &authorized));
+ ASSERT_FALSE(authorized);
{
// We shouldn't see any DEBUG messages since the client is configured to
// use INFO-level logging.
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index 00352e2..3e6dee1 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -18,7 +18,6 @@
#include "kudu/ranger/ranger_client.h"
#include <algorithm>
-#include <cstdint>
#include <cstdlib>
#include <memory>
#include <ostream>
@@ -181,7 +180,6 @@ using strings::Substitute;
namespace {
-const char* kUnauthorizedAction = "Unauthorized action";
const char* kDenyNonRangerTableTemplate = "Denying action on table with invalid name $0. "
"Use 'kudu table rename_table' to rename it to "
"a Ranger-compatible name.";
@@ -189,15 +187,6 @@ const char* kMainClass = "org.apache.kudu.subprocess.ranger.RangerSubprocessMain
const char* kRangerClientLogFilename = "kudu-ranger-subprocess";
const char* kRangerClientPropertiesFilename = "kudu-ranger-subprocess-log4j2.properties";
-const char* ScopeToString(RangerClient::Scope scope) {
- switch (scope) {
- case RangerClient::Scope::DATABASE: return "database";
- case RangerClient::Scope::TABLE: return "table";
- }
- LOG(FATAL) << static_cast<uint16_t>(scope) << ": unknown scope";
- __builtin_unreachable();
-}
-
// Returns the path to the JAR file containing the Ranger subprocess.
string RangerJarPath() {
if (FLAGS_ranger_jar_path.empty()) {
@@ -406,20 +395,10 @@ Status RangerClient::Start() {
}
// TODO(abukor): refactor to avoid code duplication
-Status RangerClient::AuthorizeAction(const string& user_name,
- const ActionPB& action,
- const string& table_name,
+Status RangerClient::AuthorizeAction(const string& user_name, const ActionPB& action,
+ const string& database, const string& table, bool* authorized,
Scope scope) {
DCHECK(subprocess_);
- string db;
- Slice tbl;
-
- auto s = ParseRangerTableIdentifier(table_name, &db, &tbl);
- if (PREDICT_FALSE(!s.ok())) {
- LOG(WARNING) << Substitute(kDenyNonRangerTableTemplate, table_name);
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
RangerRequestListPB req_list;
RangerResponseListPB resp_list;
req_list.set_user(user_name);
@@ -427,40 +406,25 @@ Status RangerClient::AuthorizeAction(const string& user_name,
RangerRequestPB* req = req_list.add_requests();
req->set_action(action);
- req->set_database(db);
+ req->set_database(database);
// Only pass the table name if this is table level request.
if (scope == Scope::TABLE) {
- req->set_table(tbl.ToString());
+ req->set_table(table);
}
RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list));
CHECK_EQ(1, resp_list.responses_size());
- if (resp_list.responses().begin()->allowed()) {
- return Status::OK();
- }
-
- LOG(WARNING) << Substitute("User $0 is not authorized to perform $1 on $2 at scope ($3)",
- user_name, ActionPB_Name(action), table_name, ScopeToString(scope));
- return Status::NotAuthorized(kUnauthorizedAction);
+ *authorized = resp_list.responses().begin()->allowed();
+ return Status::OK();
}
-Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name,
- const ActionPB& action,
- const string& table_name,
+Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name, const ActionPB& action,
+ const string& database, const string& table,
unordered_set<string>* column_names) {
DCHECK(subprocess_);
DCHECK(!column_names->empty());
- string db;
- Slice tbl;
-
- auto s = ParseRangerTableIdentifier(table_name, &db, &tbl);
- if (PREDICT_FALSE(!s.ok())) {
- LOG(WARNING) << Substitute(kDenyNonRangerTableTemplate, table_name);
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
RangerRequestListPB req_list;
RangerResponseListPB resp_list;
req_list.set_user(user_name);
@@ -468,8 +432,8 @@ Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name,
for (const auto& col : *column_names) {
auto req = req_list.add_requests();
req->set_action(action);
- req->set_database(db);
- req->set_table(tbl.ToString());
+ req->set_database(database);
+ req->set_table(table);
req->set_column(col);
}
@@ -484,20 +448,13 @@ Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name,
}
}
- if (allowed_columns.empty()) {
- LOG(WARNING) << Substitute("User $0 is not authorized to perform $1 on table $2",
- user_name, ActionPB_Name(action), table_name);
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
*column_names = move(allowed_columns);
return Status::OK();
}
-Status RangerClient::AuthorizeActionMultipleTables(const string& user_name,
- const ActionPB& action,
- unordered_set<string>* table_names) {
+Status RangerClient::AuthorizeActionMultipleTables(const string& user_name, const ActionPB& action,
+ unordered_set<string>* tables) {
DCHECK(subprocess_);
RangerRequestListPB req_list;
@@ -506,7 +463,7 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name,
vector<string> orig_table_names;
- for (const auto& table : *table_names) {
+ for (const auto& table : *tables) {
string db;
Slice tbl;
@@ -534,26 +491,17 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name,
}
}
- *table_names = move(allowed_tables);
+ *tables = move(allowed_tables);
return Status::OK();
}
-Status RangerClient::AuthorizeActions(const string& user_name,
- const string& table_name,
+Status RangerClient::AuthorizeActions(const string& user_name, const string& database,
+ const string& table,
unordered_set<ActionPB, ActionHash>* actions) {
DCHECK(subprocess_);
DCHECK(!actions->empty());
- string db;
- Slice tbl;
-
- auto s = ParseRangerTableIdentifier(table_name, &db, &tbl);
- if (PREDICT_FALSE(!s.ok())) {
- LOG(WARNING) << Substitute(kDenyNonRangerTableTemplate, table_name);
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
RangerRequestListPB req_list;
RangerResponseListPB resp_list;
req_list.set_user(user_name);
@@ -561,8 +509,8 @@ Status RangerClient::AuthorizeActions(const string& user_name,
for (const auto& action : *actions) {
auto req = req_list.add_requests();
req->set_action(action);
- req->set_database(db);
- req->set_table(tbl.ToString());
+ req->set_database(database);
+ req->set_table(table);
}
RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list));
@@ -576,12 +524,6 @@ Status RangerClient::AuthorizeActions(const string& user_name,
}
}
- if (allowed_actions.empty()) {
- LOG(WARNING) << Substitute("User $0 is not authorized to perform actions $1 on table $2",
- user_name, JoinMapped(*actions, ActionPB_Name, ", "), table_name);
- return Status::NotAuthorized(kUnauthorizedAction);
- }
-
*actions = move(allowed_actions);
return Status::OK();
diff --git a/src/kudu/ranger/ranger_client.h b/src/kudu/ranger/ranger_client.h
index 79ab45a..8e119f9 100644
--- a/src/kudu/ranger/ranger_client.h
+++ b/src/kudu/ranger/ranger_client.h
@@ -57,6 +57,11 @@ typedef subprocess::SubprocessProxy<RangerRequestListPB, RangerResponseListPB,
// coming from this class are environmental, like Java binary location, location
// of config files, and krb5.conf file (setting it doesn't enable Kerberos, that
// depends on core-site.xml).
+//
+// These methods return non-OK status only if something goes wrong between the
+// client and the subprocess (e.g. IOError, EndOfFile, Corruption), but they
+// never return NotAuthorized. The authz provider is responsible to return
+// NotAuthorized based on RangerClient's out parameters and return Status.
class RangerClient {
public:
// Similar to SentryAuthorizableScope scope which indicates the
@@ -77,35 +82,30 @@ class RangerClient {
// Starts the RangerClient, initializes the subprocess server.
Status Start() WARN_UNUSED_RESULT;
- // Authorizes an action on the table. Returns OK if 'user_name' is authorized
- // to perform 'action' on 'table_name', NotAuthorized otherwise.
+ // Authorizes an action on the table. Sets 'authorized' to true if it's
+ // authorized, false otherwise.
Status AuthorizeAction(const std::string& user_name, const ActionPB& action,
- const std::string& table_name, Scope scope = Scope::TABLE)
- WARN_UNUSED_RESULT;
+ const std::string& database, const std::string& table, bool* authorized,
+ Scope scope = Scope::TABLE) WARN_UNUSED_RESULT;
// Authorizes action on multiple tables. It sets 'table_names' to the
- // tables the user is authorized to access and returns OK.
+ // tables the user is authorized to access.
Status AuthorizeActionMultipleTables(const std::string& user_name, const ActionPB& action,
- std::unordered_set<std::string>* table_names)
- WARN_UNUSED_RESULT;
+ std::unordered_set<std::string>* tables)
+ WARN_UNUSED_RESULT;
- // Authorizes action on multiple columns. If there is at least one column that
- // user is authorized to perform the action on, it sets 'column_names' to the
- // columns the user is authorized to access and returns OK, NotAuthorized
- // otherwise.
+ // Authorizes action on multiple columns. It sets 'column_names' to the
+ // columns the user is authorized to access.
Status AuthorizeActionMultipleColumns(const std::string& user_name, const ActionPB& action,
- const std::string& table_name,
+ const std::string& database, const std::string& table,
std::unordered_set<std::string>* column_names)
WARN_UNUSED_RESULT;
- // Authorizes multiple table-level actions on a single table. If there is at
- // least one action that user is authorized to perform on the table, it sets
- // 'actions' to the actions the user is authorized to perform and returns OK,
- // NotAuthorized otherwise.
- Status AuthorizeActions(const std::string& user_name,
- const std::string& table_name,
- std::unordered_set<ActionPB, ActionHash>* actions)
- WARN_UNUSED_RESULT;
+ // Authorizes multiple table-level actions on a single table. It sets
+ // 'actions' to the actions the user is authorized to perform.
+ Status AuthorizeActions(const std::string& user_name, const std::string& database,
+ const std::string& table,
+ std::unordered_set<ActionPB, ActionHash>* actions) WARN_UNUSED_RESULT;
// Replaces the subprocess server in the subprocess proxy.
void ReplaceServerForTests(std::unique_ptr<subprocess::SubprocessServer> server) {