From: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Date: Tue, 10 May 2016 12:31:40 +0000 (+0200)
Subject: rgw: let RGWPutMetadataAccount extract params before verify_permission.
X-Git-Tag: v11.0.0~349^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=86941f8e2b13a0ba78065ad61005ff2dddff1468;p=ceph.git

rgw: let RGWPutMetadataAccount extract params before verify_permission.

Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
---

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 8e55b7635e0f..7e44d3ae8231 100644
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -2797,16 +2797,6 @@ done:
   dispose_processor(processor);
 }
 
-int RGWPutMetadataAccount::verify_permission()
-{
-  if (!rgw_user_is_authenticated(*(s->user))) {
-    return -EACCES;
-  }
-  // if ((s->perm_mask & RGW_PERM_WRITE) == 0) {
-  //   return -EACCES;
-  // }
-  return 0;
-}
 
 void RGWPutMetadataAccount::filter_out_temp_url(map<string, bufferlist>& add_attrs,
                                                 const set<string>& rmattr_names,
@@ -2826,10 +2816,7 @@ void RGWPutMetadataAccount::filter_out_temp_url(map<string, bufferlist>& add_att
     add_attrs.erase(iter);
   }
 
-  set<string>::const_iterator riter;
-  for(riter = rmattr_names.begin(); riter != rmattr_names.end(); ++riter) {
-    const string& name = *riter;
-
+  for (const string& name : rmattr_names) {
     if (name.compare(RGW_ATTR_TEMPURL_KEY1) == 0) {
       temp_url_keys[0] = string();
     }
@@ -2839,26 +2826,60 @@ void RGWPutMetadataAccount::filter_out_temp_url(map<string, bufferlist>& add_att
   }
 }
 
-void RGWPutMetadataAccount::execute()
+int RGWPutMetadataAccount::init_processing()
 {
-  map<string, bufferlist> attrs, orig_attrs, rmattrs;
-  RGWObjVersionTracker acct_op_tracker;
+  /* First, go to the base class. At the time of writing the method was
+   * responsible only for initializing the quota. This isn't necessary
+   * here as we are touching metadata only. I'm putting this call only
+   * for the future. */
+  op_ret = RGWOp::init_processing();
+  if (op_ret < 0) {
+    return op_ret;
+  }
 
   op_ret = get_params();
   if (op_ret < 0) {
-    return;
+    return op_ret;
   }
 
   op_ret = rgw_get_user_attrs_by_uid(store, s->user->user_id, orig_attrs,
                                      &acct_op_tracker);
   if (op_ret < 0) {
-    return;
+    return op_ret;
   }
 
   rgw_get_request_metadata(s->cct, s->info, attrs, false);
   prepare_add_del_attrs(orig_attrs, rmattr_names, attrs);
   populate_with_generic_attrs(s, attrs);
 
+  /* Try extract the TempURL-related stuff now to allow verify_permission
+   * evaluate whether we need FULL_CONTROL or not. */
+  filter_out_temp_url(attrs, rmattr_names, temp_url_keys);
+
+  return 0;
+}
+
+int RGWPutMetadataAccount::verify_permission()
+{
+  if (!rgw_user_is_authenticated(*(s->user))) {
+    return -EACCES;
+  }
+
+  // if ((s->perm_mask & RGW_PERM_WRITE) == 0) {
+  //   return -EACCES;
+  // }
+
+  /* Altering TempURL keys requires FULL_CONTROL. */
+  if (!temp_url_keys.empty() && s->perm_mask != RGW_PERM_FULL_CONTROL) {
+    return -EPERM;
+  }
+
+  return 0;
+}
+
+void RGWPutMetadataAccount::execute()
+{
+  /* Params have been extracted earlier. See init_processing(). */
   RGWUserInfo new_uinfo;
   op_ret = rgw_get_user_info_by_uid(store, s->user->user_id, new_uinfo,
                                     &acct_op_tracker);
@@ -2867,14 +2888,7 @@ void RGWPutMetadataAccount::execute()
   }
 
   /* Handle the TempURL-related stuff. */
-  std::map<int, std::string> temp_url_keys;
-  filter_out_temp_url(attrs, rmattr_names, temp_url_keys);
   if (!temp_url_keys.empty()) {
-    if (s->perm_mask != RGW_PERM_FULL_CONTROL) {
-      op_ret = -EPERM;
-      return;
-    }
-
     for (auto& pair : temp_url_keys) {
       new_uinfo.temp_url_keys[pair.first] = std::move(pair.second);
     }
diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index 249e2f180577..abed5a556de6 100644
--- a/src/rgw/rgw_op.h
+++ b/src/rgw/rgw_op.h
@@ -745,7 +745,12 @@ public:
 
 class RGWPutMetadataAccount : public RGWOp {
 protected:
-  set<string> rmattr_names;
+  std::set<std::string> rmattr_names;
+  std::map<std::string, bufferlist> attrs, orig_attrs;
+  std::map<int, std::string> temp_url_keys;
+
+  RGWObjVersionTracker acct_op_tracker;
+
   RGWAccessControlPolicy policy;
 
 public:
@@ -755,6 +760,7 @@ public:
     RGWOp::init(store, s, h);
     policy.set_ctx(s->cct);
   }
+  int init_processing();
   int verify_permission();
   void pre_exec() { }
   void execute();