From 9bf24c08259f1164fe158e6028557bd1e1d9193c Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Thu, 16 Nov 2023 10:46:35 -0500 Subject: [PATCH] rgw/acl: add_grant() takes const ref that also required fixing some const-correctness issues Signed-off-by: Casey Bodley --- src/rgw/driver/rados/rgw_sal_rados.cc | 2 +- src/rgw/rgw_acl.cc | 34 ++++++++++++--------------- src/rgw/rgw_acl.h | 20 +++++++--------- src/rgw/rgw_acl_s3.cc | 29 +++++++++++------------ src/rgw/rgw_acl_swift.cc | 10 ++++---- src/rgw/rgw_rest_client.cc | 19 +++++++-------- src/rgw/rgw_rest_client.h | 2 +- 7 files changed, 53 insertions(+), 63 deletions(-) diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 94c6001983c..5ede8d44fa9 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -1670,7 +1670,7 @@ int RadosObject::chown(User& new_user, const DoutPrefixProvider* dpp, optional_y //Create a grant and add grant ACLGrant grant; grant.set_canon(new_user.get_id(), new_user.get_display_name(), RGW_PERM_FULL_CONTROL); - acl.add_grant(&grant); + acl.add_grant(grant); //Update the ACL owner to the new user owner.id = new_user.get_id(); diff --git a/src/rgw/rgw_acl.cc b/src/rgw/rgw_acl.cc index 18824f86fe9..6e3aafe762a 100644 --- a/src/rgw/rgw_acl.cc +++ b/src/rgw/rgw_acl.cc @@ -70,38 +70,38 @@ bool operator!=(const RGWAccessControlPolicy& lhs, return !(lhs == rhs); } -void RGWAccessControlList::_add_grant(ACLGrant *grant) +void RGWAccessControlList::register_grant(const ACLGrant& grant) { - ACLPermission& perm = grant->get_permission(); - ACLGranteeType& type = grant->get_type(); + ACLPermission perm = grant.get_permission(); + ACLGranteeType type = grant.get_type(); switch (type.get_type()) { case ACL_TYPE_REFERER: - referer_list.emplace_back(grant->get_referer(), perm.get_permissions()); + referer_list.emplace_back(grant.get_referer(), perm.get_permissions()); /* We're specially handling the Swift's .r:* as the S3 API has a similar * concept and thus we can have a small portion of compatibility here. */ - if (grant->get_referer() == RGW_REFERER_WILDCARD) { + if (grant.get_referer() == RGW_REFERER_WILDCARD) { acl_group_map[ACL_GROUP_ALL_USERS] |= perm.get_permissions(); } break; case ACL_TYPE_GROUP: - acl_group_map[grant->get_group()] |= perm.get_permissions(); + acl_group_map[grant.get_group()] |= perm.get_permissions(); break; default: { rgw_user id; - grant->get_id(id); + grant.get_id(id); acl_user_map[id.to_str()] |= perm.get_permissions(); } } } -void RGWAccessControlList::add_grant(ACLGrant *grant) +void RGWAccessControlList::add_grant(const ACLGrant& grant) { rgw_user id; - grant->get_id(id); // not that this will return false for groups, but that's ok, we won't search groups - grant_map.insert(pair(id.to_str(), *grant)); - _add_grant(grant); + grant.get_id(id); // note that this will return false for groups, but that's ok, we won't search groups + grant_map.emplace(id.to_str(), grant); + register_grant(grant); } void RGWAccessControlList::remove_canon_user_grant(rgw_user& user_id) @@ -323,14 +323,10 @@ void RGWAccessControlList::generate_test_instances(list& { RGWAccessControlList *acl = new RGWAccessControlList; - list glist; - list::iterator iter; - - ACLGrant::generate_test_instances(glist); - for (iter = glist.begin(); iter != glist.end(); ++iter) { - ACLGrant *grant = *iter; - acl->add_grant(grant); - + list grants; + ACLGrant::generate_test_instances(grants); + for (ACLGrant* grant : grants) { + acl->add_grant(*grant); delete grant; } o.push_back(acl); diff --git a/src/rgw/rgw_acl.h b/src/rgw/rgw_acl.h index 13141279e8d..f60295d90ab 100644 --- a/src/rgw/rgw_acl.h +++ b/src/rgw/rgw_acl.h @@ -60,10 +60,8 @@ public: } } - ACLGranteeType& get_type() { return type; } - const ACLGranteeType& get_type() const { return type; } - ACLPermission& get_permission() { return permission; } - const ACLPermission& get_permission() const { return permission; } + ACLGranteeType get_type() const { return type; } + ACLPermission get_permission() const { return permission; } ACLGroupTypeEnum get_group() const { return group; } const std::string& get_referer() const { return url_spec; } @@ -225,7 +223,8 @@ protected: std::map acl_group_map; std::list referer_list; ACLGrantMap grant_map; - void _add_grant(ACLGrant *grant); + // register a grant in the correspoding acl_user/group_map + void register_grant(const ACLGrant& grant); public: uint32_t get_perm(const DoutPrefixProvider* dpp, const rgw::auth::Identity& auth_identity, @@ -253,10 +252,9 @@ public: if (struct_v >= 2) { decode(acl_group_map, bl); } else if (!maps_initialized) { - ACLGrantMap::iterator iter; - for (iter = grant_map.begin(); iter != grant_map.end(); ++iter) { - ACLGrant& grant = iter->second; - _add_grant(&grant); + // register everything in the grant_map + for (const auto& [id, grant] : grant_map) { + register_grant(grant); } } if (struct_v >= 4) { @@ -267,7 +265,7 @@ public: void dump(Formatter *f) const; static void generate_test_instances(std::list& o); - void add_grant(ACLGrant *grant); + void add_grant(const ACLGrant& grant); void remove_canon_user_grant(rgw_user& user_id); ACLGrantMap& get_grant_map() { return grant_map; } @@ -280,7 +278,7 @@ public: ACLGrant grant; grant.set_canon(id, name, RGW_PERM_FULL_CONTROL); - add_grant(&grant); + add_grant(grant); } friend bool operator==(const RGWAccessControlList& lhs, const RGWAccessControlList& rhs); diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc index 2641c19ec17..52eeb44f243 100644 --- a/src/rgw/rgw_acl_s3.cc +++ b/src/rgw/rgw_acl_s3.cc @@ -66,7 +66,7 @@ xml_end(const char *el) class ACLGranteeType_S3 { public: - static const char *to_string(ACLGranteeType& type) { + static const char *to_string(ACLGranteeType type) { switch (type.get_type()) { case ACL_TYPE_CANON_USER: return "CanonicalUser"; @@ -264,7 +264,7 @@ bool RGWAccessControlList_S3::xml_end(const char *el) { XMLObjIter iter = find("Grant"); ACLGrant_S3 *grant = static_cast(iter.get_next()); while (grant) { - add_grant(grant); + add_grant(*grant); grant = static_cast(iter.get_next()); } return true; @@ -371,7 +371,7 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own /* owner gets full control */ owner_grant.set_canon(owner.id, owner.display_name, RGW_PERM_FULL_CONTROL); - add_grant(&owner_grant); + add_grant(owner_grant); if (canned_acl.size() == 0 || canned_acl.compare("private") == 0) { return 0; @@ -381,24 +381,24 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own ACLGrant group_grant; if (canned_acl.compare("public-read") == 0) { group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_READ); - add_grant(&group_grant); + add_grant(group_grant); } else if (canned_acl.compare("public-read-write") == 0) { group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_READ); - add_grant(&group_grant); + add_grant(group_grant); group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_WRITE); - add_grant(&group_grant); + add_grant(group_grant); } else if (canned_acl.compare("authenticated-read") == 0) { group_grant.set_group(ACL_GROUP_AUTHENTICATED_USERS, RGW_PERM_READ); - add_grant(&group_grant); + add_grant(group_grant); } else if (canned_acl.compare("bucket-owner-read") == 0) { bucket_owner_grant.set_canon(bid, bname, RGW_PERM_READ); if (bid != owner.id) { - add_grant(&bucket_owner_grant); + add_grant(bucket_owner_grant); } } else if (canned_acl.compare("bucket-owner-full-control") == 0) { bucket_owner_grant.set_canon(bid, bname, RGW_PERM_FULL_CONTROL); if (bid != owner.id) { - add_grant(&bucket_owner_grant); + add_grant(bucket_owner_grant); } } else { return -EINVAL; @@ -415,9 +415,8 @@ int RGWAccessControlList_S3::create_from_grants(std::list& grants) acl_user_map.clear(); grant_map.clear(); - for (std::list::iterator it = grants.begin(); it != grants.end(); ++it) { - ACLGrant g = *it; - add_grant(&g); + for (const auto& g : grants) { + add_grant(g); } return 0; @@ -513,7 +512,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp, multimap::iterator iter; for (iter = grant_map.begin(); iter != grant_map.end(); ++iter) { ACLGrant& src_grant = iter->second; - ACLGranteeType& type = src_grant.get_type(); + ACLGranteeType type = src_grant.get_type(); ACLGrant new_grant; bool grant_ok = false; rgw_user uid; @@ -557,7 +556,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp, grant_user = user->get_info(); } } - ACLPermission& perm = src_grant.get_permission(); + ACLPermission perm = src_grant.get_permission(); new_grant.set_canon(uid, grant_user.display_name, perm.get_permissions()); grant_ok = true; rgw_user new_id; @@ -582,7 +581,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp, break; } if (grant_ok) { - dst_acl.add_grant(&new_grant); + dst_acl.add_grant(new_grant); } } diff --git a/src/rgw/rgw_acl_swift.cc b/src/rgw/rgw_acl_swift.cc index e829788c6d5..70fa793dd9a 100644 --- a/src/rgw/rgw_acl_swift.cc +++ b/src/rgw/rgw_acl_swift.cc @@ -168,7 +168,7 @@ int RGWAccessControlPolicy_SWIFT::add_grants(const DoutPrefixProvider *dpp, } if (grant) { - acl.add_grant(&*grant); + acl.add_grant(*grant); } else { return -EINVAL; } @@ -257,7 +257,7 @@ void RGWAccessControlPolicy_SWIFT::filter_merge(uint32_t rw_mask, } } if (perm & rw_mask) { - acl.add_grant(&grant); + acl.add_grant(grant); } } } @@ -313,7 +313,7 @@ void RGWAccessControlPolicy_SWIFTAcct::add_grants(const DoutPrefixProvider *dpp, if (uid_is_public(uid)) { grant.set_group(ACL_GROUP_ALL_USERS, perm); - acl.add_grant(&grant); + acl.add_grant(grant); } else { std::unique_ptr user = driver->get_user(rgw_user(uid)); @@ -321,10 +321,10 @@ void RGWAccessControlPolicy_SWIFTAcct::add_grants(const DoutPrefixProvider *dpp, ldpp_dout(dpp, 10) << "grant user does not exist:" << uid << dendl; /* skipping silently */ grant.set_canon(user->get_id(), std::string(), perm); - acl.add_grant(&grant); + acl.add_grant(grant); } else { grant.set_canon(user->get_id(), user->get_display_name(), perm); - acl.add_grant(&grant); + acl.add_grant(grant); } } } diff --git a/src/rgw/rgw_rest_client.cc b/src/rgw/rgw_rest_client.cc index 1ccb813a109..71ead6a8b89 100644 --- a/src/rgw/rgw_rest_client.cc +++ b/src/rgw/rgw_rest_client.cc @@ -497,7 +497,7 @@ RGWRESTStreamS3PutObj::~RGWRESTStreamS3PutObj() delete out_cb; } -static void grants_by_type_add_one_grant(map& grants_by_type, int perm, ACLGrant& grant) +static void grants_by_type_add_one_grant(map& grants_by_type, int perm, const ACLGrant& grant) { string& s = grants_by_type[perm]; @@ -505,7 +505,7 @@ static void grants_by_type_add_one_grant(map& grants_by_type, int p s.append(", "); string id_type_str; - ACLGranteeType& type = grant.get_type(); + ACLGranteeType type = grant.get_type(); switch (type.get_type()) { case ACL_TYPE_GROUP: id_type_str = "uri"; @@ -535,7 +535,7 @@ struct grant_type_to_header grants_headers_def[] = { { 0, NULL} }; -static bool grants_by_type_check_perm(map& grants_by_type, int perm, ACLGrant& grant, int check_perm) +static bool grants_by_type_check_perm(map& grants_by_type, int perm, const ACLGrant& grant, int check_perm) { if ((perm & check_perm) == check_perm) { grants_by_type_add_one_grant(grants_by_type, check_perm, grant); @@ -544,7 +544,7 @@ static bool grants_by_type_check_perm(map& grants_by_type, int perm return false; } -static void grants_by_type_add_perm(map& grants_by_type, int perm, ACLGrant& grant) +static void grants_by_type_add_perm(map& grants_by_type, int perm, const ACLGrant& grant) { struct grant_type_to_header *t; @@ -669,16 +669,13 @@ void RGWRESTGenerateHTTPHeaders::set_http_attrs(const map& http_ } } -void RGWRESTGenerateHTTPHeaders::set_policy(RGWAccessControlPolicy& policy) +void RGWRESTGenerateHTTPHeaders::set_policy(const RGWAccessControlPolicy& policy) { /* update acl headers */ - RGWAccessControlList& acl = policy.get_acl(); - multimap& grant_map = acl.get_grant_map(); - multimap::iterator giter; + const RGWAccessControlList& acl = policy.get_acl(); map grants_by_type; - for (giter = grant_map.begin(); giter != grant_map.end(); ++giter) { - ACLGrant& grant = giter->second; - ACLPermission& perm = grant.get_permission(); + for (const auto& [id, grant] : acl.get_grant_map()) { + ACLPermission perm = grant.get_permission(); grants_by_type_add_perm(grants_by_type, perm.get_permissions(), grant); } add_grants_headers(grants_by_type, *new_env, new_info->x_meta_map); diff --git a/src/rgw/rgw_rest_client.h b/src/rgw/rgw_rest_client.h index 6e27576c676..923f8cc1783 100644 --- a/src/rgw/rgw_rest_client.h +++ b/src/rgw/rgw_rest_client.h @@ -94,7 +94,7 @@ public: void set_extra_headers(const std::map& extra_headers); int set_obj_attrs(const DoutPrefixProvider *dpp, std::map& rgw_attrs); void set_http_attrs(const std::map& http_attrs); - void set_policy(RGWAccessControlPolicy& policy); + void set_policy(const RGWAccessControlPolicy& policy); int sign(const DoutPrefixProvider *dpp, RGWAccessKey& key, const bufferlist *opt_content); const std::string& get_url() { return url; } -- 2.39.5