Merge pull request #66170 from kchheda3/wip-fix-account-acls-backward-compatbility

rgw/account: Support backward compatibility for s3:PutAcls calls for users migrated to account.

Reviewed-by: Casey Bodley <cbodley@redhat.com>

doc/radosgw/s3/bucketops.rst

@@ -288,6 +288,8 @@ Request Entities
 | ``Permission``            | String      | The permission given to the ``Grantee`` bucket.                                              |
 +---------------------------+-------------+----------------------------------------------------------------------------------------------+
 
+.. note:: For accounts users, the ``Owner`` and ``ID`` in ``AccessControlPolicy`` may be set to either the legacy user ID or the new account ID. Both are accepted for backward compatibility.
+
 List Bucket Multipart Uploads
 -----------------------------
 

doc/radosgw/s3/objectops.rst

@@ -234,6 +234,7 @@ Request Entities
 | ``Permission``            | String      | The permission given to the ``Grantee`` object.                                              |
 +---------------------------+-------------+----------------------------------------------------------------------------------------------+
 
+.. note:: For accounts users, the ``Owner`` and ``ID`` in ``AccessControlPolicy`` may be set to either the legacy user ID or the new account ID. Both are accepted for backward compatibility.
 
 
 Initiate Multi-part Upload

qa/workunits/rgw/test_account_migration.sh

@@ -34,6 +34,10 @@ export AWS_SECRET_ACCESS_KEY=$(echo $userinfo | jq -r .keys[0].secret_key)
 aws s3 mb s3://testmigrate
 aws s3api put-object --bucket testmigrate --key obj
 
+# put bucket and object acls before migration
+aws s3api put-bucket-acl --bucket testmigrate --acl private
+aws s3api put-object-acl --bucket testmigrate --key obj --acl private
+
 # create an account and migrate the user as account root
 accountid=$(radosgw-admin account create | jq -r .id)
 radosgw-admin user modify --uid test-account-migration --account-root --account-id=$accountid
@@ -41,6 +45,14 @@ radosgw-admin user modify --uid test-account-migration --account-root --account-
 # verify the migrated user still has access
 aws s3api head-object --bucket testmigrate --key obj
 
+# verify get/put acl backward compatibility after migration.
+# the bucket/object acl owner is still the old user id, but the
+# requester now authenticates as the account id. both should work.
+aws s3api get-bucket-acl --bucket testmigrate
+aws s3api get-object-acl --bucket testmigrate --key obj
+aws s3api put-bucket-acl --bucket testmigrate --acl private
+aws s3api put-object-acl --bucket testmigrate --key obj --acl private
+
 # replace account-root flag with managed policy
 aws iam attach-user-policy --region us-east-1 --user-name MigratedUser \
 	--policy-arn arn:aws:iam::aws:policy/AmazonS3FullAccess

src/rgw/rgw_auth.cc

@@ -74,6 +74,22 @@ static bool match_owner(const rgw_owner& owner, const rgw_user& uid,
       }), owner);
 }
 
+static bool match_owner(const rgw_owner& owner, const rgw_user& uid)
+{
+  return std::visit(fu2::overload(
+      [&uid] (const rgw_user& u) { return u == uid; },
+      [] (const rgw_account_id&) { return false; }), owner);
+}
+
+static bool match_owner(const rgw_owner& owner, const RGWAccountInfo& account)
+{
+  return std::visit(fu2::overload(
+      [] (const rgw_user&) { return false; },
+      [&account] (const rgw_account_id& a) {
+        return a == account.id;
+      }), owner);
+}
+
 static bool match_account_or_tenant(const std::optional<RGWAccountInfo>& account,
                                     std::string_view tenant,
                                     std::string_view expected)
@@ -595,7 +611,11 @@ ACLOwner rgw::auth::WebIdentityApplier::get_aclowner() const
 
 bool rgw::auth::WebIdentityApplier::is_owner_of(const rgw_owner& o) const
 {
-  return match_owner(o, rgw_user{role_tenant, sub, "oidc"}, account);
+  if (account) {
+    return match_owner(o, *account);
+  } else {
+    return match_owner(o, rgw_user{role_tenant, sub, "oidc"});
+  }
 }
 
 void rgw::auth::WebIdentityApplier::to_str(std::ostream& out) const
@@ -1206,7 +1226,11 @@ ACLOwner rgw::auth::RoleApplier::get_aclowner() const
 
 bool rgw::auth::RoleApplier::is_owner_of(const rgw_owner& o) const
 {
-  return match_owner(o, token_attrs.user_id, role.account);
+  if (role.account) {
+    return match_owner(o, *role.account);
+  } else {
+    return match_owner(o, token_attrs.user_id);
+  }
 }
 
 void rgw::auth::RoleApplier::to_str(std::ostream& out) const {

src/rgw/rgw_op.cc

@@ -6692,8 +6692,12 @@ void RGWPutACLs::execute(optional_yield y)
   if (op_ret < 0)
     return;
 
+  // only allow acl owner to change if the requester views them as equivalent.
+  // the requester may change between their user id and account id.
   if (!existing_owner.empty() &&
-      existing_owner.id != new_policy.get_owner().id) {
+      existing_owner.id != new_policy.get_owner().id &&
+      !(s->auth.identity->is_owner_of(existing_owner.id) &&
+        s->auth.identity->is_owner_of(new_policy.get_owner().id))) {
     s->err.message = "Cannot modify ACL Owner";
     op_ret = -EPERM;
     return;