From d0b86de1288df090d94838c690c5e427df5e7f71 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Fri, 3 Nov 2023 15:56:58 -0400 Subject: [PATCH] rgw/auth: Identity::is_owner/admin_of(rgw_owner) is_owner_of() and is_admin_of() take rgw_owner instead of rgw_user so that identities associated with an account share ownership of that account's resources LocalApplier is the only Identity type that supports accounts, based on comparison with RGWUserInfo::account_id Signed-off-by: Casey Bodley (cherry picked from commit a1c675da7cf571457898d799206e911e23cdc711) --- src/rgw/driver/rados/rgw_data_sync.cc | 1 + src/rgw/rgw_auth.cc | 58 ++++++++++++++++++--------- src/rgw/rgw_auth.h | 41 +++++++++---------- src/rgw/rgw_auth_filters.h | 8 ++-- src/rgw/rgw_swift_auth.h | 7 +++- src/test/rgw/test_rgw_iam_policy.cc | 4 +- src/test/rgw/test_rgw_lua.cc | 4 +- 7 files changed, 72 insertions(+), 51 deletions(-) diff --git a/src/rgw/driver/rados/rgw_data_sync.cc b/src/rgw/driver/rados/rgw_data_sync.cc index 88ac7e8629d06..804ffdb876184 100644 --- a/src/rgw/driver/rados/rgw_data_sync.cc +++ b/src/rgw/driver/rados/rgw_data_sync.cc @@ -2647,6 +2647,7 @@ class RGWUserPermHandler { info->identity = rgw::auth::transform_old_authinfo(sync_env->cct, uid, info->user_info.display_name, + info->user_info.account_id, RGW_PERM_FULL_CONTROL, false, /* system_request? */ TYPE_RGW); diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index 1daa709aa5620..6e9029f4c537b 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -3,6 +3,7 @@ #include #include +#include #include "rgw_common.h" #include "rgw_auth.h" @@ -13,6 +14,7 @@ #include "rgw_sal.h" #include "rgw_log.h" +#include "include/function2.hpp" #include "include/str_list.h" #define dout_context g_ceph_context @@ -23,10 +25,20 @@ using namespace std; namespace rgw { namespace auth { +static bool match_owner(const rgw_owner& owner, const rgw_user& uid, + std::string_view account_id) +{ + return std::visit(fu2::overload( + [&uid] (const rgw_user& u) { return u == uid; }, + [&account_id] (const rgw_account_id& a) { return a == account_id; } + ), owner); +} + std::unique_ptr transform_old_authinfo(CephContext* const cct, const rgw_user& auth_id, const std::string& display_name, + const rgw_account_id& account_id, const int perm_mask, const bool is_admin, const uint32_t type) @@ -42,6 +54,7 @@ transform_old_authinfo(CephContext* const cct, * new auth. */ const rgw_user id; const std::string display_name; + const rgw_account_id account_id; const int perm_mask; const bool is_admin; const uint32_t type; @@ -49,12 +62,14 @@ transform_old_authinfo(CephContext* const cct, DummyIdentityApplier(CephContext* const cct, const rgw_user& auth_id, const std::string display_name, + const rgw_account_id& account_id, const int perm_mask, const bool is_admin, const uint32_t type) : cct(cct), id(auth_id), display_name(display_name), + account_id(account_id), perm_mask(perm_mask), is_admin(is_admin), type(type) { @@ -71,12 +86,12 @@ transform_old_authinfo(CephContext* const cct, return rgw_perms_from_aclspec_default_strategy(id.to_str(), aclspec, dpp); } - bool is_admin_of(const rgw_user& acct_id) const override { + bool is_admin_of(const rgw_owner& o) const override { return is_admin; } - bool is_owner_of(const rgw_user& acct_id) const override { - return id == acct_id; + bool is_owner_of(const rgw_owner& o) const override { + return match_owner(o, id, account_id); } bool is_identity(const idset_t& ids) const override { @@ -120,26 +135,24 @@ transform_old_authinfo(CephContext* const cct, } }; - return std::unique_ptr( - new DummyIdentityApplier(cct, - auth_id, - display_name, - perm_mask, - is_admin, - type)); + return std::make_unique( + cct, auth_id, display_name, account_id, + perm_mask, is_admin, type); } std::unique_ptr transform_old_authinfo(const req_state* const s) { + const RGWUserInfo& info = s->user->get_info(); return transform_old_authinfo(s->cct, - s->user->get_id(), - s->user->get_display_name(), + info.user_id, + info.display_name, + info.account_id, s->perm_mask, /* System user has admin permissions by default - it's supposed to pass * through any security check. */ s->system_request, - s->user->get_type()); + info.type); } } /* namespace auth */ @@ -582,22 +595,27 @@ uint32_t rgw::auth::RemoteApplier::get_perms_from_aclspec(const DoutPrefixProvid return perm; } -bool rgw::auth::RemoteApplier::is_admin_of(const rgw_user& uid) const +bool rgw::auth::RemoteApplier::is_admin_of(const rgw_owner& o) const { return info.is_admin; } -bool rgw::auth::RemoteApplier::is_owner_of(const rgw_user& uid) const +bool rgw::auth::RemoteApplier::is_owner_of(const rgw_owner& o) const { + auto* uid = std::get_if(&o); + if (!uid) { + return false; + } + if (info.acct_user.tenant.empty()) { const rgw_user tenanted_acct_user(info.acct_user.id, info.acct_user.id); - if (tenanted_acct_user == uid) { + if (tenanted_acct_user == *uid) { return true; } } - return info.acct_user == uid; + return info.acct_user == *uid; } bool rgw::auth::RemoteApplier::is_identity(const idset_t& ids) const { @@ -797,14 +815,14 @@ uint32_t rgw::auth::LocalApplier::get_perms_from_aclspec(const DoutPrefixProvide return mask; } -bool rgw::auth::LocalApplier::is_admin_of(const rgw_user& uid) const +bool rgw::auth::LocalApplier::is_admin_of(const rgw_owner& o) const { return user_info.admin || user_info.system; } -bool rgw::auth::LocalApplier::is_owner_of(const rgw_user& uid) const +bool rgw::auth::LocalApplier::is_owner_of(const rgw_owner& o) const { - return uid == user_info.user_id; + return match_owner(o, user_info.user_id, user_info.account_id); } bool rgw::auth::LocalApplier::is_identity(const idset_t& ids) const { diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index cf2d3583e8769..aa95ae154962d 100644 --- a/src/rgw/rgw_auth.h +++ b/src/rgw/rgw_auth.h @@ -48,15 +48,13 @@ public: * applier that is being used. */ virtual uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const = 0; - /* Verify whether a given identity *can be treated as* an admin of rgw_user - * (account in Swift's terminology) specified in @uid. On error throws - * rgw::auth::Exception storing the reason. */ - virtual bool is_admin_of(const rgw_user& uid) const = 0; + /* Verify whether a given identity *can be treated as* an admin of rgw_owner + * specified in @o. On error throws rgw::auth::Exception storing the reason. */ + virtual bool is_admin_of(const rgw_owner& o) const = 0; - /* Verify whether a given identity *is* the owner of the rgw_user (account - * in the Swift's terminology) specified in @uid. On internal error throws - * rgw::auth::Exception storing the reason. */ - virtual bool is_owner_of(const rgw_user& uid) const = 0; + /* Verify whether a given identity is the rgw_owner specified in @o. + * On internal error throws rgw::auth::Exception storing the reason. */ + virtual bool is_owner_of(const rgw_owner& o) const = 0; /* Return the permission mask that is used to narrow down the set of * operations allowed for a given identity. This method reflects the idea @@ -104,6 +102,7 @@ std::unique_ptr transform_old_authinfo(CephContext* const cct, const rgw_user& auth_id, const std::string& display_name, + const rgw_account_id& account_id, const int perm_mask, const bool is_admin, const uint32_t type); @@ -459,15 +458,13 @@ public: return RGW_PERM_NONE; } - bool is_admin_of(const rgw_user& uid) const override { + bool is_admin_of(const rgw_owner& o) const override { return false; } - bool is_owner_of(const rgw_user& uid) const override { - if (uid.id == this->sub && uid.tenant == role_tenant && uid.ns == "oidc") { - return true; - } - return false; + bool is_owner_of(const rgw_owner& o) const override { + auto* uid = std::get_if(&o); + return uid && uid->id == sub && uid->tenant == role_tenant && uid->ns == "oidc"; } uint32_t get_perm_mask() const override { @@ -631,8 +628,8 @@ public: ACLOwner get_aclowner() const override; uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const override; - bool is_admin_of(const rgw_user& uid) const override; - bool is_owner_of(const rgw_user& uid) const override; + bool is_admin_of(const rgw_owner& o) const override; + bool is_owner_of(const rgw_owner& o) const override; bool is_identity(const idset_t& ids) const override; uint32_t get_perm_mask() const override { return info.perm_mask; } @@ -692,8 +689,8 @@ public: ACLOwner get_aclowner() const override; uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const override; - bool is_admin_of(const rgw_user& uid) const override; - bool is_owner_of(const rgw_user& uid) const override; + bool is_admin_of(const rgw_owner& o) const override; + bool is_owner_of(const rgw_owner& o) const override; bool is_identity(const idset_t& ids) const override; uint32_t get_perm_mask() const override { if (this->perm_mask == RGW_PERM_INVALID) { @@ -756,11 +753,13 @@ public: uint32_t get_perms_from_aclspec(const DoutPrefixProvider* dpp, const aclspec_t& aclspec) const override { return 0; } - bool is_admin_of(const rgw_user& uid) const override { + bool is_admin_of(const rgw_owner& o) const override { return false; } - bool is_owner_of(const rgw_user& uid) const override { - return (this->token_attrs.user_id.id == uid.id && this->token_attrs.user_id.tenant == uid.tenant && this->token_attrs.user_id.ns == uid.ns); + bool is_owner_of(const rgw_owner& o) const override { + auto* uid = std::get_if(&o); + // TODO: handle account roles + return uid && *uid == token_attrs.user_id; } bool is_identity(const idset_t& ids) const override; uint32_t get_perm_mask() const override { diff --git a/src/rgw/rgw_auth_filters.h b/src/rgw/rgw_auth_filters.h index d02772487e953..39b647c909a0c 100644 --- a/src/rgw/rgw_auth_filters.h +++ b/src/rgw/rgw_auth_filters.h @@ -73,12 +73,12 @@ public: return get_decoratee().get_perms_from_aclspec(dpp, aclspec); } - bool is_admin_of(const rgw_user& uid) const override { - return get_decoratee().is_admin_of(uid); + bool is_admin_of(const rgw_owner& o) const override { + return get_decoratee().is_admin_of(o); } - bool is_owner_of(const rgw_user& uid) const override { - return get_decoratee().is_owner_of(uid); + bool is_owner_of(const rgw_owner& o) const override { + return get_decoratee().is_owner_of(o); } bool is_anonymous() const override { diff --git a/src/rgw/rgw_swift_auth.h b/src/rgw/rgw_swift_auth.h index 3564a6b39b5c6..1c72a9472f65f 100644 --- a/src/rgw/rgw_swift_auth.h +++ b/src/rgw/rgw_swift_auth.h @@ -157,8 +157,11 @@ class SwiftAnonymousApplier : public rgw::auth::LocalApplier { const RGWUserInfo& user_info) : LocalApplier(cct, user_info, LocalApplier::NO_SUBUSER, std::nullopt, LocalApplier::NO_ACCESS_KEY) { } - bool is_admin_of(const rgw_user& uid) const {return false;} - bool is_owner_of(const rgw_user& uid) const {return uid.id.compare(RGW_USER_ANON_ID) == 0;} + bool is_admin_of(const rgw_owner& o) const {return false;} + bool is_owner_of(const rgw_owner& o) const { + auto* uid = std::get_if(&o); + return uid && uid->id == RGW_USER_ANON_ID; + } }; class SwiftAnonymousEngine : public rgw::auth::AnonymousEngine { diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index faf7dabf9eb41..ea86de91f64a4 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/src/test/rgw/test_rgw_iam_policy.cc @@ -113,12 +113,12 @@ public: return 0; }; - bool is_admin_of(const rgw_user& uid) const override { + bool is_admin_of(const rgw_owner& o) const override { ceph_abort(); return false; } - bool is_owner_of(const rgw_user& uid) const override { + bool is_owner_of(const rgw_owner& owner) const override { ceph_abort(); return false; } diff --git a/src/test/rgw/test_rgw_lua.cc b/src/test/rgw/test_rgw_lua.cc index 0fe87a4cd47b0..ed60fb9ec1f44 100644 --- a/src/test/rgw/test_rgw_lua.cc +++ b/src/test/rgw/test_rgw_lua.cc @@ -41,11 +41,11 @@ public: return 0; }; - bool is_admin_of(const rgw_user& uid) const override { + bool is_admin_of(const rgw_owner& o) const override { return false; } - bool is_owner_of(const rgw_user& uid) const override { + bool is_owner_of(const rgw_owner& uid) const override { return false; } -- 2.39.5