diff --git a/auth/allow_all_authorizer.hh b/auth/allow_all_authorizer.hh index 2d1dd1f412..e8ad8b4c79 100644 --- a/auth/allow_all_authorizer.hh +++ b/auth/allow_all_authorizer.hh @@ -56,23 +56,23 @@ public: return allow_all_authorizer_name(); } - virtual future authorize(service&, stdx::string_view, resource) const override { + virtual future authorize(service&, stdx::string_view, const resource&) const override { return make_ready_future(permissions::ALL); } - virtual future<> grant(permission_set, resource, stdx::string_view) override { + virtual future<> grant(permission_set, const resource&, stdx::string_view) override { throw exceptions::invalid_request_exception("GRANT operation is not supported by AllowAllAuthorizer"); } - virtual future<> revoke(permission_set, resource, stdx::string_view) override { + virtual future<> revoke(permission_set, const resource&, stdx::string_view) override { throw exceptions::invalid_request_exception("REVOKE operation is not supported by AllowAllAuthorizer"); } virtual future> list( service&, permission_set, - std::optional, - std::optional) const override { + const std::optional&, + const std::optional&) const override { throw exceptions::invalid_request_exception("LIST PERMISSIONS operation is not supported by AllowAllAuthorizer"); } @@ -80,7 +80,7 @@ public: return make_ready_future(); } - virtual future<> revoke_all(resource) override { + virtual future<> revoke_all(const resource&) override { return make_ready_future(); } diff --git a/auth/authorizer.hh b/auth/authorizer.hh index 17b9d529f8..e74aab857c 100644 --- a/auth/authorizer.hh +++ b/auth/authorizer.hh @@ -90,17 +90,17 @@ public: /// /// The resulting permission set includes permissions obtained transitively through roles granted to this role. /// - virtual future authorize(service&, stdx::string_view role_name, resource) const = 0; + virtual future authorize(service&, stdx::string_view role_name, const resource&) const = 0; /// /// Grant a set of permissions to a user for a particular \ref resource. /// - virtual future<> grant(permission_set, resource, stdx::string_view role_name) = 0; + virtual future<> grant(permission_set, const resource&, stdx::string_view role_name) = 0; /// /// Revoke a set of permissions from a user for a particular \ref resource. /// - virtual future<> revoke(permission_set, resource, stdx::string_view role_name) = 0; + virtual future<> revoke(permission_set, const resource&, stdx::string_view role_name) = 0; /// /// Query for granted permissions. @@ -116,8 +116,8 @@ public: list( service&, permission_set matching, - std::optional resource, - std::optional role_name) const = 0; + const std::optional& resource, + const std::optional& role_name) const = 0; /// /// Revoke all permissions granted to a particular user. @@ -127,7 +127,7 @@ public: /// /// Revoke all permissions granted to any user for a particular resource. /// - virtual future<> revoke_all(resource) = 0; + virtual future<> revoke_all(const resource&) = 0; /// /// System resources used internally as part of the implementation. These are made inaccessible to users. diff --git a/auth/default_authorizer.cc b/auth/default_authorizer.cc index 57425cde3f..2a708b8092 100644 --- a/auth/default_authorizer.cc +++ b/auth/default_authorizer.cc @@ -153,8 +153,8 @@ future default_authorizer::authorize_role_directly( future default_authorizer::authorize( service& ser, stdx::string_view role_name, - resource resource) const { - return do_with(permission_set(), std::move(resource), [this, &ser, role_name](auto& ps, const auto& r) { + const resource& r) const { + return do_with(permission_set(), [this, &ser, role_name, &r](auto& ps) { return ser.get_roles(role_name).then([this, &ser, &ps, &r](std::unordered_set all_roles) { return do_with(std::move(all_roles), [this, &ser, &ps, &r](const auto& all_roles) { return parallel_for_each(all_roles, [this, &ser, &ps, &r](stdx::string_view role_name) { @@ -170,7 +170,11 @@ future default_authorizer::authorize( } future<> -default_authorizer::modify(permission_set set, resource resource, stdx::string_view role_name, stdx::string_view op) { +default_authorizer::modify( + permission_set set, + const resource& resource, + stdx::string_view role_name, + stdx::string_view op) { // TODO: why does this not check super user? auto query = sprint( "UPDATE %s.%s SET %s = %s %s ? WHERE %s = ? AND %s = ?", @@ -189,19 +193,19 @@ default_authorizer::modify(permission_set set, resource resource, stdx::string_v } -future<> default_authorizer::grant(permission_set set, resource resource, stdx::string_view role_name) { - return modify(std::move(set), std::move(resource), role_name, "+"); +future<> default_authorizer::grant(permission_set set, const resource& resource, stdx::string_view role_name) { + return modify(std::move(set), resource, role_name, "+"); } -future<> default_authorizer::revoke(permission_set set, resource resource, stdx::string_view role_name) { - return modify(std::move(set), std::move(resource), role_name, "-"); +future<> default_authorizer::revoke(permission_set set, const resource& resource, stdx::string_view role_name) { + return modify(std::move(set), resource, role_name, "-"); } future> default_authorizer::list( service& ser, permission_set set, - std::optional resource, - std::optional role_name) const { + const std::optional& resource, + const std::optional& role_name) const { sstring query = sprint( "SELECT %s, %s, %s FROM %s.%s", ROLE_NAME, @@ -215,7 +219,7 @@ future> default_authorizer::list( future<::shared_ptr> f = make_ready_future<::shared_ptr>(); if (role_name) { - f = ser.get_roles(*role_name).then([this, resource = std::move(resource), query, &f]( + f = ser.get_roles(*role_name).then([this, &resource, query, &f]( std::unordered_set all_roles) mutable { if (resource) { query += sprint(" WHERE %s IN ? AND %s = ?", ROLE_NAME, RESOURCE_NAME); @@ -268,7 +272,7 @@ future<> default_authorizer::revoke_all(stdx::string_view role_name) { }); } -future<> default_authorizer::revoke_all(resource resource) { +future<> default_authorizer::revoke_all(const resource& resource) { auto query = sprint( "SELECT %s FROM %s.%s WHERE %s = ? ALLOW FILTERING", ROLE_NAME, diff --git a/auth/default_authorizer.hh b/auth/default_authorizer.hh index 1ba64783d2..2f1edc3942 100644 --- a/auth/default_authorizer.hh +++ b/auth/default_authorizer.hh @@ -69,23 +69,27 @@ public: return default_authorizer_name(); } - virtual future authorize(service&, stdx::string_view, resource) const override; + virtual future authorize(service&, stdx::string_view, const resource&) const override; - virtual future<> grant(permission_set, resource, stdx::string_view) override; + virtual future<> grant(permission_set, const resource&, stdx::string_view) override; - virtual future<> revoke(permission_set, resource, stdx::string_view) override; + virtual future<> revoke(permission_set, const resource&, stdx::string_view) override; virtual future> - list(service&, permission_set, std::optional, std::optional) const override; + list( + service&, + permission_set, + const std::optional&, + const std::optional&) const override; virtual future<> revoke_all(stdx::string_view) override; - virtual future<> revoke_all(resource) override; + virtual future<> revoke_all(const resource&) override; virtual const resource_set& protected_resources() const override; private: - future<> modify(permission_set, resource, stdx::string_view, stdx::string_view); + future<> modify(permission_set, const resource&, stdx::string_view, stdx::string_view); /// /// Permissions granted directly to a role, rather than those inherited. diff --git a/auth/permissions_cache.cc b/auth/permissions_cache.cc index 3b86d1499c..c432108a67 100644 --- a/auth/permissions_cache.cc +++ b/auth/permissions_cache.cc @@ -44,8 +44,8 @@ permissions_cache::permissions_cache(const permissions_cache_config& c, service& }) { } -future permissions_cache::get(stdx::string_view role_name, resource r) { - return do_with(key_type(sstring(role_name), std::move(r)), [this](const auto& k) { +future permissions_cache::get(stdx::string_view role_name, const resource& r) { + return do_with(key_type(sstring(role_name), r), [this](const auto& k) { return _cache.get(k); }); } diff --git a/auth/permissions_cache.hh b/auth/permissions_cache.hh index 3fd8fd0753..26a27a347e 100644 --- a/auth/permissions_cache.hh +++ b/auth/permissions_cache.hh @@ -83,7 +83,7 @@ public: return _cache.stop(); } - future get(stdx::string_view role_name, resource); + future get(stdx::string_view role_name, const resource&); }; } diff --git a/auth/resource.cc b/auth/resource.cc index 54afda2da3..1e8d7ec2fc 100644 --- a/auth/resource.cc +++ b/auth/resource.cc @@ -280,4 +280,16 @@ resource parse_resource(stdx::string_view name) { return resource(kind, std::move(parts)); } +static const resource the_root_data_resource{resource_kind::data}; + +const resource& root_data_resource() { + return the_root_data_resource; +} + +static const resource the_root_role_resource{resource_kind::role}; + +const resource& root_role_resource() { + return the_root_role_resource; +} + } diff --git a/auth/resource.hh b/auth/resource.hh index c94d583d22..5097b1db4c 100644 --- a/auth/resource.hh +++ b/auth/resource.hh @@ -201,6 +201,8 @@ std::ostream& operator<<(std::ostream&, const role_resource_view&); /// resource parse_resource(stdx::string_view name); +const resource& root_data_resource(); + inline resource make_data_resource(stdx::string_view keyspace) { return resource(data_resource_t{}, keyspace); } @@ -208,6 +210,8 @@ inline resource make_data_resource(stdx::string_view keyspace, stdx::string_view return resource(data_resource_t{}, keyspace, table); } +const resource& root_role_resource(); + inline resource make_role_resource(stdx::string_view role) { return resource(role_resource_t{}, role); } diff --git a/auth/service.cc b/auth/service.cc index d5ae632917..c9a03273a4 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -240,9 +240,9 @@ future service::has_existing_legacy_users() const { }); } -future service::get_permissions(stdx::string_view role_name, resource r) const { - return validate_role_exists(*this, role_name).then([this, role_name, r = std::move(r)] { - return _permissions_cache->get(role_name, std::move(r)); +future service::get_permissions(stdx::string_view role_name, const resource& r) const { + return validate_role_exists(*this, role_name).then([this, role_name, &r] { + return _permissions_cache->get(role_name, r); }); } diff --git a/auth/service.hh b/auth/service.hh index 26400d1ae2..1c7c9d54e8 100644 --- a/auth/service.hh +++ b/auth/service.hh @@ -121,7 +121,7 @@ public: /// /// \returns an exceptional future with \ref nonexistant_role if the named role does not exist. /// - future get_permissions(stdx::string_view role_name, resource) const; + future get_permissions(stdx::string_view role_name, const resource&) const; /// /// Query whether the named role has been granted a role that is a superuser. diff --git a/auth/transitional.cc b/auth/transitional.cc index 437d0321b9..55499c4a24 100644 --- a/auth/transitional.cc +++ b/auth/transitional.cc @@ -205,8 +205,8 @@ public: } virtual future - authorize(service& ser, stdx::string_view role_name, resource resource) const override { - return ser.has_superuser(role_name).then([resource](bool s) { + authorize(service& ser, stdx::string_view role_name, const resource& resource) const override { + return ser.has_superuser(role_name).then([&resource](bool s) { static const permission_set transitional_permissions = permission_set::of< permission::CREATE, @@ -219,29 +219,29 @@ public: }); } - virtual future<> grant(permission_set ps, resource r, stdx::string_view s) override { - return _authorizer->grant(std::move(ps), std::move(r), s); + virtual future<> grant(permission_set ps, const resource& r, stdx::string_view s) override { + return _authorizer->grant(std::move(ps), r, s); } - virtual future<> revoke(permission_set ps, resource r, stdx::string_view s) override { - return _authorizer->revoke(std::move(ps), std::move(r), s); + virtual future<> revoke(permission_set ps, const resource& r, stdx::string_view s) override { + return _authorizer->revoke(std::move(ps), r, s); } virtual future> list( service& ser, permission_set ps, - std::optional r, - std::optional s) const override { - return _authorizer->list(ser, std::move(ps), std::move(r), std::move(s)); + const std::optional& r, + const std::optional& s) const override { + return _authorizer->list(ser, std::move(ps), r, s); } virtual future<> revoke_all(stdx::string_view s) override { return _authorizer->revoke_all(s); } - virtual future<> revoke_all(resource r) override { - return _authorizer->revoke_all(std::move(r)); + virtual future<> revoke_all(const resource& r) override { + return _authorizer->revoke_all(r); } virtual const resource_set& protected_resources() const override { diff --git a/cql3/statements/list_permissions_statement.cc b/cql3/statements/list_permissions_statement.cc index ba1ca65e36..5b1ed3c8fe 100644 --- a/cql3/statements/list_permissions_statement.cc +++ b/cql3/statements/list_permissions_statement.cc @@ -154,8 +154,10 @@ cql3::statements::list_permissions_statement::execute( return map_reduce( resources, [&state, this](opt_resource r) { - auto& auth_service = *state.get_client_state().get_auth_service(); - return auth_service.underlying_authorizer().list(auth_service, _permissions, std::move(r), _username); + return do_with(std::move(r), [this, &state](const opt_resource& r) { + auto& auth_service = *state.get_client_state().get_auth_service(); + return auth_service.underlying_authorizer().list(auth_service, _permissions, r, _username); + }); }, std::vector(), [](std::vector details, std::vector pd) { diff --git a/cql3/statements/role-management-statements.cc b/cql3/statements/role-management-statements.cc index 520136cb53..4085daecbf 100644 --- a/cql3/statements/role-management-statements.cc +++ b/cql3/statements/role-management-statements.cc @@ -87,7 +87,7 @@ future<> create_role_statement::check_access(const service::client_state& state) state.ensure_not_anonymous(); return async([this, &state] { - state.ensure_has_permission(auth::permission::CREATE, auth::resource(auth::resource_kind::role)).get0(); + state.ensure_has_permission(auth::permission::CREATE, auth::root_role_resource()).get0(); if (*_options.is_superuser) { if (!auth::has_superuser(*state.get_auth_service(), *state.user()).get0()) { @@ -257,9 +257,7 @@ future<> list_roles_statement::check_access(const service::client_state& state) state.ensure_not_anonymous(); return async([this, &state] { - if (state.check_has_permission( - auth::permission::DESCRIBE, - auth::resource(auth::resource_kind::role)).get0()) { + if (state.check_has_permission(auth::permission::DESCRIBE, auth::root_role_resource()).get0()) { return; } @@ -342,7 +340,7 @@ list_roles_statement::execute(distributed&, service::que // only the roles granted to them. return cs.check_has_permission( auth::permission::DESCRIBE, - auth::resource(auth::resource_kind::role)).then([&cs, &rm, query_mode](bool has_describe) { + auth::root_role_resource()).then([&cs, &rm, query_mode](bool has_describe) { if (has_describe) { return rm.query_all().then([&rm](auto&& roles) { return make_results(rm, std::move(roles)); }); } @@ -367,7 +365,10 @@ list_roles_statement::execute(distributed&, service::que future<> grant_role_statement::check_access(const service::client_state& state) { state.ensure_not_anonymous(); - return state.ensure_has_permission(auth::permission::AUTHORIZE, auth::make_role_resource(_role)); + + return do_with(auth::make_role_resource(_role), [this, &state](const auto& r) { + return state.ensure_has_permission(auth::permission::AUTHORIZE, r); + }); } future @@ -389,7 +390,10 @@ grant_role_statement::execute(distributed&, service::que future<> revoke_role_statement::check_access(const service::client_state& state) { state.ensure_not_anonymous(); - return state.ensure_has_permission(auth::permission::AUTHORIZE, auth::make_role_resource(_role)); + + return do_with(auth::make_role_resource(_role), [this, &state](const auto& r) { + return state.ensure_has_permission(auth::permission::AUTHORIZE, r); + }); } future revoke_role_statement::execute( diff --git a/service/client_state.cc b/service/client_state.cc index e377f43fed..da32821332 100644 --- a/service/client_state.cc +++ b/service/client_state.cc @@ -106,25 +106,38 @@ future<> service::client_state::has_all_keyspaces_access( return make_ready_future(); } validate_login(); - return ensure_has_permission(p, auth::resource(auth::resource_kind::data)); + + return do_with(auth::resource(auth::resource_kind::data), [this, p](const auto& r) { + return ensure_has_permission(p, r); + }); } future<> service::client_state::has_keyspace_access(const sstring& ks, auth::permission p) const { - return has_access(ks, p, auth::make_data_resource(ks)); + return do_with(ks, auth::make_data_resource(ks), [this, p](auto const& ks, auto const& r) { + return has_access(ks, p, r); + }); } future<> service::client_state::has_column_family_access(const sstring& ks, const sstring& cf, auth::permission p) const { validation::validate_column_family(ks, cf); - return has_access(ks, p, auth::make_data_resource(ks, cf)); + + return do_with(ks, auth::make_data_resource(ks, cf), [this, p](const auto& ks, const auto& r) { + return has_access(ks, p, r); + }); } future<> service::client_state::has_schema_access(const schema& s, auth::permission p) const { - return has_access(s.ks_name(), p, auth::make_data_resource(s.ks_name(), s.cf_name())); + return do_with( + s.ks_name(), + auth::make_data_resource(s.ks_name(),s.cf_name()), + [this, p](auto const& ks, auto const& r) { + return has_access(ks, p, r); + }); } -future<> service::client_state::has_access(const sstring& ks, auth::permission p, auth::resource resource) const { +future<> service::client_state::has_access(const sstring& ks, auth::permission p, const auth::resource& resource) const { if (ks.empty()) { throw exceptions::invalid_request_exception("You have not set a keyspace for this session"); } @@ -179,34 +192,36 @@ future<> service::client_state::has_access(const sstring& ks, auth::permission p } } - return ensure_has_permission(p, std::move(resource)); + return ensure_has_permission(p, resource); } -future service::client_state::check_has_permission(auth::permission p, auth::resource resource) const { +future service::client_state::check_has_permission(auth::permission p, const auth::resource& r) const { if (_is_internal) { return make_ready_future(true); } - std::optional parent = resource.parent(); - - return _auth_service->get_permissions(*_user->name, resource).then([this, p, parent = std::move(parent)](auth::permission_set set) { - if (set.contains(p)) { - return make_ready_future(true); - } - if (parent) { - return check_has_permission(p, std::move(*parent)); - } - return make_ready_future(false); + return do_with(r.parent(), [this, p, &r](const auto& parent_r) { + return _auth_service->get_permissions(*_user->name, r).then([this, p, &parent_r](auth::permission_set set) { + if (set.contains(p)) { + return make_ready_future(true); + } + if (parent_r) { + return check_has_permission(p, *parent_r); + } + return make_ready_future(false); + }); }); } -future<> service::client_state::ensure_has_permission(auth::permission p, auth::resource resource) const { - return check_has_permission(p, resource).then([this, p, resource](bool ok) { +future<> service::client_state::ensure_has_permission(auth::permission p, const auth::resource& r) const { + return check_has_permission(p, r).then([this, p, &r](bool ok) { if (!ok) { - throw exceptions::unauthorized_exception(sprint("User %s has no %s permission on %s or any of its parents", - *_user, - auth::permissions::to_string(p), - resource)); + throw exceptions::unauthorized_exception( + sprint( + "User %s has no %s permission on %s or any of its parents", + *_user, + auth::permissions::to_string(p), + r)); } }); } diff --git a/service/client_state.hh b/service/client_state.hh index 9375e42996..1369b83c13 100644 --- a/service/client_state.hh +++ b/service/client_state.hh @@ -315,11 +315,11 @@ public: future<> has_schema_access(const schema& s, auth::permission p) const; private: - future<> has_access(const sstring&, auth::permission, auth::resource) const; + future<> has_access(const sstring&, auth::permission, const auth::resource&) const; public: - future check_has_permission(auth::permission, auth::resource) const; - future<> ensure_has_permission(auth::permission, auth::resource) const; + future check_has_permission(auth::permission, const auth::resource&) const; + future<> ensure_has_permission(auth::permission, const auth::resource&) const; void validate_login() const; void ensure_not_anonymous() const; // unauthorized_exception on error diff --git a/tests/auth_resource_test.cc b/tests/auth_resource_test.cc index 6aba18d385..c19d797967 100644 --- a/tests/auth_resource_test.cc +++ b/tests/auth_resource_test.cc @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(from_name) { // const auto dr1 = auth::parse_resource("data"); - BOOST_REQUIRE_EQUAL(dr1, auth::resource(auth::resource_kind::data)); + BOOST_REQUIRE_EQUAL(dr1, auth::root_data_resource()); const auto dr2 = auth::parse_resource("data/my_keyspace"); BOOST_REQUIRE_EQUAL(dr2, auth::make_data_resource("my_keyspace")); @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(from_name) { // const auto rr1 = auth::parse_resource("roles"); - BOOST_REQUIRE_EQUAL(rr1, auth::resource(auth::resource_kind::role)); + BOOST_REQUIRE_EQUAL(rr1, auth::root_role_resource()); const auto rr2 = auth::parse_resource("roles/joe"); BOOST_REQUIRE_EQUAL(rr2, auth::make_role_resource("joe")); @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(name) { // data // - BOOST_REQUIRE_EQUAL(auth::resource(auth::resource_kind::data).name(), "data"); + BOOST_REQUIRE_EQUAL(auth::root_data_resource().name(), "data"); BOOST_REQUIRE_EQUAL(auth::make_data_resource("my_keyspace").name(), "data/my_keyspace"); BOOST_REQUIRE_EQUAL(auth::make_data_resource("my_keyspace", "my_table").name(), "data/my_keyspace/my_table"); @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE(name) { // role // - BOOST_REQUIRE_EQUAL(auth::resource(auth::resource_kind::role).name(), "roles"); + BOOST_REQUIRE_EQUAL(auth::root_role_resource().name(), "roles"); BOOST_REQUIRE_EQUAL(auth::make_role_resource("joe").name(), "roles/joe"); } @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(parent) { const auto r3 = r2->parent(); BOOST_REQUIRE(r3); - BOOST_REQUIRE_EQUAL(*r3, auth::resource(auth::resource_kind::data)); + BOOST_REQUIRE_EQUAL(*r3, auth::root_data_resource()); const auto r4 = r3->parent(); BOOST_REQUIRE(!r4); @@ -147,7 +147,7 @@ BOOST_AUTO_TEST_CASE(output) { // data // - BOOST_REQUIRE_EQUAL(sprint("%s", auth::resource(auth::resource_kind::data)), ""); + BOOST_REQUIRE_EQUAL(sprint("%s", auth::root_data_resource()), ""); BOOST_REQUIRE_EQUAL(sprint("%s", auth::make_data_resource("my_keyspace")), ""); BOOST_REQUIRE_EQUAL( @@ -158,6 +158,6 @@ BOOST_AUTO_TEST_CASE(output) { // role // - BOOST_REQUIRE_EQUAL(sprint("%s", auth::resource(auth::resource_kind::role)), ""); + BOOST_REQUIRE_EQUAL(sprint("%s", auth::root_role_resource()), ""); BOOST_REQUIRE_EQUAL(sprint("%s", auth::make_role_resource("joe")), ""); }