From: Pritha Srivastava <prsrivas@redhat.com>
Date: Tue, 20 Nov 2018 05:32:27 +0000 (+0530)
Subject: rgw: Correcting permission evaluation for Roles.
X-Git-Tag: v14.1.0~510^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7f67813701e618f8815ffdbc25f0ffee590fca9c;p=ceph.git

rgw: Correcting permission evaluation for Roles.

User Policy and Policies attached to roles are disjoint, hence
user policies for a user shouldn't be taken into account when
assuming a role.

Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>
---

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 552e3894ff5d..788e78c6b5c8 100644
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -570,12 +570,20 @@ int rgw_build_bucket_policies(RGWRados* store, struct req_state* s)
       return ret;
     }
   }
-
-  if (! s->user->user_id.empty()) {
+  // We don't need user policies in case of STS token returned by AssumeRole,
+  // hence the check for user type
+  if (! s->user->user_id.empty() && s->user->type != TYPE_NONE) {
     try {
       map<string, bufferlist> uattrs;
       if (ret = rgw_get_user_attrs_by_uid(store, s->user->user_id, uattrs); ! ret) {
-        s->iam_user_policies = get_iam_user_policy_from_attr(s->cct, store, uattrs, s->user->user_id.tenant);
+        if (s->iam_user_policies.empty()) {
+          s->iam_user_policies = get_iam_user_policy_from_attr(s->cct, store, uattrs, s->user->user_id.tenant);
+        } else {
+          // This scenario can happen when a STS token has a policy, then we need to append other user policies
+          // to the existing ones. (e.g. token returned by GetSessionToken)
+          auto user_policies = get_iam_user_policy_from_attr(s->cct, store, uattrs, s->user->user_id.tenant);
+          s->iam_user_policies.insert(s->iam_user_policies.end(), user_policies.begin(), user_policies.end());
+        }
       } else {
         if (ret == -ENOENT)
           ret = 0;
diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index 97cbf9a4fc0d..e4671660c02d 100644
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -4478,9 +4478,13 @@ rgw::auth::s3::STSEngine::authenticate(
         role_policies.push_back(std::move(perm_policy));
       }
     }
-    role_policies.push_back(std::move(token.policy));
+    if (! token.policy.empty()) {
+      role_policies.push_back(std::move(token.policy));
+    }
+    // This is mostly needed to assign the owner of a bucket during its creation
+    user_info.user_id = token.user;
   }
-  if (! token.user.empty()) {
+  if (! token.user.empty() && token.acct_type != TYPE_NONE) {
     // get user info
     int ret = rgw_get_user_info_by_uid(store, token.user, user_info, NULL);
     if (ret < 0) {
diff --git a/src/rgw/rgw_sts.cc b/src/rgw/rgw_sts.cc
index 3abae7f3343d..4158179dfa60 100644
--- a/src/rgw/rgw_sts.cc
+++ b/src/rgw/rgw_sts.cc
@@ -301,7 +301,7 @@ AssumeRoleResponse STSService::assumeRole(AssumeRoleRequest& req)
   //Role and Policy provide the authorization info, user id and applier info are not needed
   if (ret = cred.generateCredentials(cct, req.getDuration(),
                                       req.getPolicy(), roleId,
-                                      boost::none, nullptr); ret < 0) {
+                                      user_id, nullptr); ret < 0) {
     return make_tuple(ret, user, cred, packedPolicySize);
   }