rgw: rest_admin/user avoid double checking input args

Since all the op_state functions already check for string emptiness, the
check in the execute function becomes redundant.

A TODO to knockdown is validate that the required args are passed in, we
currently return 403s instead of 400 even when required args for eg. uid
are not passed in

Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

diff --git a/src/rgw/rgw_rest_user.cc b/src/rgw/rgw_rest_user.cc
index ec869d6638d13b1eeaa5841ba106a017779d325d..21e27cf7900b630656109851ae1f031f6dbb27ec 100644 (file)
--- a/src/rgw/rgw_rest_user.cc
+++ b/src/rgw/rgw_rest_user.cc
@@ -108,24 +108,13 @@ void RGWOp_User_Create::execute()
     return;
   }
 
-  // FIXME: don't do double argument checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!display_name.empty())
-    op_state.set_display_name(display_name);
-
-  if (!email.empty())
-    op_state.set_user_email(email);
-
-  if (!caps.empty())
-    op_state.set_caps(caps);
-
-  if (!access_key.empty())
-    op_state.set_access_key(access_key);
-
-  if (!secret_key.empty())
-    op_state.set_secret_key(secret_key);
+  // TODO: validate required args are passed in. (for eg. uid and display_name here)
+  op_state.set_user_id(uid);
+  op_state.set_display_name(display_name);
+  op_state.set_user_email(email);
+  op_state.set_caps(caps);
+  op_state.set_access_key(access_key);
+  op_state.set_secret_key(secret_key);
 
   if (!key_type_str.empty()) {
     int32_t key_type = KEY_TYPE_UNDEFINED;
@@ -239,23 +228,12 @@ void RGWOp_User_Modify::execute()
     return;
   }
 
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!display_name.empty())
-    op_state.set_display_name(display_name);
-
-  if (!email.empty())
-    op_state.set_user_email(email);
-
-  if (!caps.empty())
-    op_state.set_caps(caps);
-
-  if (!access_key.empty())
-    op_state.set_access_key(access_key);
-
-  if (!secret_key.empty())
-    op_state.set_secret_key(secret_key);
+  op_state.set_user_id(uid);
+  op_state.set_display_name(display_name);
+  op_state.set_user_email(email);
+  op_state.set_caps(caps);
+  op_state.set_access_key(access_key);
+  op_state.set_secret_key(secret_key);
 
   if (max_buckets != RGW_DEFAULT_MAX_BUCKETS)
     op_state.set_max_buckets(max_buckets);
@@ -364,19 +342,10 @@ void RGWOp_Subuser_Create::execute()
   perm_mask = rgw_str_to_perm(perm_str.c_str());
   op_state.set_perm(perm_mask);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!subuser.empty())
-    op_state.set_subuser(subuser);
-
-  if (!access_key.empty())
-    op_state.set_access_key(access_key);
-  
-  if (!secret_key.empty())
-    op_state.set_secret_key(secret_key);
-
+  op_state.set_user_id(uid);
+  op_state.set_subuser(subuser);
+  op_state.set_access_key(access_key);
+  op_state.set_secret_key(secret_key);
   op_state.set_generate_subuser(gen_subuser);
 
   if (gen_access)
@@ -437,18 +406,10 @@ void RGWOp_Subuser_Modify::execute()
   perm_mask = rgw_str_to_perm(perm_str.c_str());
   op_state.set_perm(perm_mask);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!subuser.empty())
-    op_state.set_subuser(subuser);
-
-  if (!secret_key.empty())
-    op_state.set_secret_key(secret_key);
-
-  if (gen_secret)
-    op_state.set_gen_secret();
+  op_state.set_user_id(uid);
+  op_state.set_subuser(subuser);
+  op_state.set_secret_key(secret_key);
+  op_state.set_gen_secret();
 
   if (!key_type_str.empty()) {
     if (key_type_str.compare("swift") == 0)
@@ -489,12 +450,8 @@ void RGWOp_Subuser_Remove::execute()
   RESTArgs::get_string(s, "subuser", subuser, &subuser);
   RESTArgs::get_bool(s, "purge-keys", true, &purge_keys);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!subuser.empty())
-    op_state.set_subuser(subuser);
+  op_state.set_user_id(uid);
+  op_state.set_subuser(subuser);
 
   if (purge_keys)
     op_state.set_purge_keys();
@@ -537,18 +494,10 @@ void RGWOp_Key_Create::execute()
   RESTArgs::get_string(s, "key-type", key_type_str, &key_type_str);
   RESTArgs::get_bool(s, "generate-key", true, &gen_key);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!subuser.empty())
-    op_state.set_subuser(subuser);
-
-  if (!access_key.empty())
-    op_state.set_access_key(access_key);
-
-  if (!secret_key.empty())
-    op_state.set_secret_key(secret_key);
+  op_state.set_user_id(uid);
+  op_state.set_subuser(subuser);
+  op_state.set_access_key(access_key);
+  op_state.set_secret_key(secret_key);
 
   if (gen_key)
     op_state.set_generate_key();
@@ -596,15 +545,9 @@ void RGWOp_Key_Remove::execute()
   RESTArgs::get_string(s, "access-key", access_key, &access_key);
   RESTArgs::get_string(s, "key-type", key_type_str, &key_type_str);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!subuser.empty())
-    op_state.set_subuser(subuser);
-
-  if (!access_key.empty())
-    op_state.set_access_key(access_key);
+  op_state.set_user_id(uid);
+  op_state.set_subuser(subuser);
+  op_state.set_access_key(access_key);
 
   if (!key_type_str.empty()) {
     int32_t key_type = KEY_TYPE_UNDEFINED;
@@ -645,12 +588,8 @@ void RGWOp_Caps_Add::execute()
 
   RESTArgs::get_string(s, "user-caps", caps, &caps);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!caps.empty())
-    op_state.set_caps(caps);
+  op_state.set_user_id(uid);
+  op_state.set_caps(caps);
 
   http_ret = RGWUserAdminOp_Caps::add(store, op_state, flusher);
 }
@@ -681,12 +620,8 @@ void RGWOp_Caps_Remove::execute()
 
   RESTArgs::get_string(s, "user-caps", caps, &caps);
 
-  // FIXME: no double checking
-  if (!uid.empty())
-    op_state.set_user_id(uid);
-
-  if (!caps.empty())
-    op_state.set_caps(caps);
+  op_state.set_user_id(uid);
+  op_state.set_caps(caps);
 
   http_ret = RGWUserAdminOp_Caps::remove(store, op_state, flusher);
 }