rgw: add new cap user-info-without-keys

This new cap allows users to run the admin api op
`get user info` without the S3 keys and Swift keys
in the response.

Signed-off-by: Ali Maredia <amaredia@redhat.com>

diff --git a/doc/radosgw/admin.rst b/doc/radosgw/admin.rst
index 57d38c97ab2fffbd4a22e8117d35c771097846f7..501f161c1df99904d6a3ef3f3b719388e98c3c98 100644 (file)
--- a/doc/radosgw/admin.rst
+++ b/doc/radosgw/admin.rst
@@ -375,7 +375,7 @@ form:
 
 .. prompt:: bash
 
-   --caps="[users|buckets|metadata|usage|zone|amz-cache|info|bilog|mdlog|datalog|user-policy|oidc-provider|roles|ratelimit]=[\*|read|write|read, write]"
+   --caps="[users|buckets|metadata|usage|zone|amz-cache|info|bilog|mdlog|datalog|user-policy|oidc-provider|roles|ratelimit|user-info-without-keys]=[\*|read|write|read, write]"
 
 For example:
 

diff --git a/doc/radosgw/adminops.rst b/doc/radosgw/adminops.rst
index f1faac6b1c2f4d991c8db02f91a49e5447c8f9c2..f9e501515d0bd52901b8a48d47a7c522b68c98c9 100644 (file)
--- a/doc/radosgw/adminops.rst
+++ b/doc/radosgw/adminops.rst
@@ -273,13 +273,14 @@ TBD.
 Get User Info
 =============
 
-Get user information.
+Get user information. Cap ``users`` or ``user-info-without-keys`` must be set to ``read`` to run this operation.
+If cap ``user-info-without-keys`` is set to ``read`` or ``*``, S3 keys and Swift keys will not be
+included in the response unless the user running this operation is the system user, an admin user, or the cap ``users`` is set to ``read``.
 
 Either a ``uid`` or ``access-key`` must be supplied as a request parameter. We recommend supplying uid.
 If both are provided but correspond to different users, the info for the user specified with ``uid`` will be returned.
 
-:caps: users=read
-
+:caps: users=read or user-info-without-keys=read
 
 Syntax
 ~~~~~~

diff --git a/qa/tasks/radosgw_admin_rest.py b/qa/tasks/radosgw_admin_rest.py
index 3de4d6bc92587890dbe6c2999a8830565f96d88e..4b07ad330d342d5a87c7b62ed8304073b0b592fe 100644 (file)
--- a/qa/tasks/radosgw_admin_rest.py
+++ b/qa/tasks/radosgw_admin_rest.py
@@ -28,13 +28,13 @@ def rgwadmin_rest(connection, cmd, params=None, headers=None, raw=False):
     perform a rest command
     """
     log.info('radosgw-admin-rest: %s %s' % (cmd, params))
-    put_cmds = ['create', 'link', 'add']
+    put_cmds = ['create', 'link', 'add', 'set']
     post_cmds = ['unlink', 'modify']
     delete_cmds = ['trim', 'rm', 'process']
-    get_cmds = ['check', 'info', 'show', 'list', '']
+    get_cmds = ['check', 'info', 'show', 'list', 'get', '']
 
     bucket_sub_resources = ['object', 'policy', 'index']
-    user_sub_resources = ['subuser', 'key', 'caps']
+    user_sub_resources = ['subuser', 'key', 'caps', 'quota']
     zone_sub_resources = ['pool', 'log', 'garbage']
 
     def get_cmd_method_and_handler(cmd):
@@ -117,6 +117,167 @@ def rgwadmin_rest(connection, cmd, params=None, headers=None, raw=False):
         log.info(' json result: %s' % result.json())
         return result.status_code, result.json()
 
+def test_cap_user_info_without_keys_get_user_info_privileged_users(ctx, client, op, op_args, uid, display_name, access_key, secret_key, user_type):
+    user_caps = 'user-info-without-keys=read'
+
+    (err, out) = rgwadmin(ctx, client, [
+            'user', 'create',
+            '--uid', uid,
+            '--display-name', display_name,
+            '--access-key', access_key,
+            '--secret', secret_key,
+            '--caps', user_caps,
+            user_type
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    endpoint = ctx.rgw.role_endpoints.get(client)
+
+    privileged_user_conn = boto.s3.connection.S3Connection(
+        aws_access_key_id=access_key,
+        aws_secret_access_key=secret_key,
+        is_secure=True if endpoint.cert else False,
+        port=endpoint.port,
+        host=endpoint.hostname,
+        calling_format=boto.s3.connection.OrdinaryCallingFormat(),
+        )
+
+    (ret, out) = rgwadmin_rest(privileged_user_conn, op, op_args)
+    # show that even though the cap is set, since the user is privileged the user can still see keys
+    assert len(out['keys']) == 1
+    assert out['swift_keys'] == []
+
+    (err, out) = rgwadmin(ctx, client, [
+            'user', 'rm',
+            '--uid', uid,
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+def test_cap_user_info_without_keys_get_user_info(ctx, client, admin_conn, admin_user, op, op_args):
+    true_admin_uid = 'a_user'
+    true_admin_display_name = 'True Admin User'
+    true_admin_access_key = 'true_admin_akey'
+    true_admin_secret_key = 'true_admin_skey'
+
+    system_uid = 'system_user'
+    system_display_name = 'System User'
+    system_access_key = 'system_akey'
+    system_secret_key = 'system_skey'
+
+    test_cap_user_info_without_keys_get_user_info_privileged_users(ctx, client, op, op_args, system_uid, system_display_name, system_access_key, system_secret_key, '--system')
+    test_cap_user_info_without_keys_get_user_info_privileged_users(ctx, client, op, op_args, true_admin_uid, true_admin_display_name, true_admin_access_key, true_admin_secret_key, '--admin')
+
+    # TESTCASE 'info-existing','user','info','existing user','returns no keys with user-info-without-keys cap set to read'
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'user-info-without-keys=read'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    (ret, out) = rgwadmin_rest(admin_conn, op, op_args)
+    assert 'keys' not in out
+    assert 'swift_keys' not in out
+
+    # TESTCASE 'info-existing','user','info','existing user','returns no keys with user-info-without-keys cap set to read'
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'user-info-without-keys=*'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    (ret, out) = rgwadmin_rest(admin_conn, op, op_args)
+    assert 'keys' not in out
+    assert 'swift_keys' not in out
+
+    # TESTCASE 'info-existing','user','info','existing user','returns keys with user-info-without-keys cap set to read but cap users is set to read'
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'users=read, write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    (ret, out) = rgwadmin_rest(admin_conn, op, op_args)
+    assert 'keys' in out
+    assert 'swift_keys' in out
+
+    # TESTCASE 'info-existing','user','info','existing user','returns 403 with user-info-without-keys cap set to write'
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'rm',
+            '--uid', admin_user,
+            '--caps', 'users=read, write; user-info-without-keys=*'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'user-info-without-keys=write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    (ret, out) = rgwadmin_rest(admin_conn, op, op_args)
+    assert ret == 403
+
+    # remove cap user-info-without-keys permenantly for future testing
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'rm',
+            '--uid', admin_user,
+            '--caps', 'user-info-without-keys=write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    # reset cap users permenantly for future testing
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'users=read, write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+def test_cap_user_info_without_keys(ctx, client, admin_conn, admin_user, user1):
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'rm',
+            '--uid', admin_user,
+            '--caps', 'users=read, write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
+
+    op = ['user', 'info']
+    op_args = {'uid' : user1}
+    test_cap_user_info_without_keys_get_user_info(ctx, client, admin_conn, admin_user, op, op_args)
+
+    # add caps that were removed earlier in the function back in
+    (err, out) = rgwadmin(ctx, client, [
+            'caps', 'add',
+            '--uid', admin_user,
+            '--caps', 'users=read, write'
+            ])
+    logging.error(out)
+    logging.error(err)
+    assert not err
 
 def task(ctx, config):
     """
@@ -291,6 +452,9 @@ def task(ctx, config):
     assert out['type'] == 'rgw'
     assert out['mfa_ids'] == []
 
+    # TESTCASES for cap user-info-without-keys
+    test_cap_user_info_without_keys(ctx, client, admin_conn, admin_user, user1)
+
     # TESTCASE 'suspend-ok','user','suspend','active user','succeeds'
     (ret, out) = rgwadmin_rest(admin_conn, ['user', 'modify'], {'uid' : user1, 'suspended' : True})
     assert ret == 200
@@ -812,4 +976,4 @@ def task(ctx, config):
 
     # TESTCASE 'ratelimit' 'global' 'modify' 'anonymous' 'enabled' 'succeeds'
     (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'enabled' : 'true'})
-    assert ret == 200
\ No newline at end of file
+    assert ret == 200

diff --git a/src/rgw/driver/rados/rgw_rest_user.cc b/src/rgw/driver/rados/rgw_rest_user.cc
index 200f1c03d2e195bf1335c2581d89422d2ffa5d34..89712970fb2da92cb6550d556bd39a694ad72aeb 100644 (file)
--- a/src/rgw/driver/rados/rgw_rest_user.cc
+++ b/src/rgw/driver/rados/rgw_rest_user.cc
@@ -73,6 +73,10 @@ public:
   RGWOp_User_Info() {}
 
   int check_caps(const RGWUserCaps& caps) override {
+    int r = caps.check_cap("user-info-without-keys", RGW_CAP_READ);
+    if (r == 0) {
+      return r;
+    }
     return caps.check_cap("users", RGW_CAP_READ);
   }
 
@@ -84,10 +88,12 @@ public:
 void RGWOp_User_Info::execute(optional_yield y)
 {
   RGWUserAdminOpState op_state(driver);
+  op_state.set_system(s->system_request);
 
   std::string uid_str, access_key_str;
   bool fetch_stats;
   bool sync_stats;
+  bool dump_keys = false;
 
   RESTArgs::get_string(s, "uid", uid_str, &uid_str);
   RESTArgs::get_string(s, "access-key", access_key_str, &access_key_str);
@@ -111,7 +117,15 @@ void RGWOp_User_Info::execute(optional_yield y)
   op_state.set_fetch_stats(fetch_stats);
   op_state.set_sync_stats(sync_stats);
 
-  op_ret = RGWUserAdminOp_User::info(s, driver, op_state, flusher, y);
+  // dump_keys is false if user-info-without-keys is 'read' and
+  // the user is not the system user or an admin user
+  int keys_perm = s->user->get_info().caps.check_cap("users", RGW_CAP_READ);
+  if (keys_perm == 0 || op_state.system || s->auth.identity->is_admin_of(uid)) {
+    dump_keys = true;
+    ldpp_dout(s, 20) << "dump_keys is set to true" << dendl;
+  }
+
+  op_ret = RGWUserAdminOp_User::info(s, driver, op_state, flusher, dump_keys, y);
 }
 
 class RGWOp_User_Create : public RGWRESTOp {

diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc
index b5569e481c53e04cd584e5ffae4e56550e6b40af..f13e087c436484faa58aba550c20fad9b1c84997 100644 (file)
--- a/src/rgw/driver/rados/rgw_user.cc
+++ b/src/rgw/driver/rados/rgw_user.cc
@@ -126,7 +126,7 @@ static void dump_swift_keys_info(Formatter *f, RGWUserInfo &info)
 }
 
 static void dump_user_info(Formatter *f, RGWUserInfo &info,
-                           RGWStorageStats *stats = NULL)
+                           bool dump_keys, RGWStorageStats *stats = NULL)
 {
   f->open_object_section("user_info");
   encode_json("tenant", info.user_id.tenant, f);
@@ -137,8 +137,11 @@ static void dump_user_info(Formatter *f, RGWUserInfo &info,
   encode_json("max_buckets", (int)info.max_buckets, f);
 
   dump_subusers_info(f, info);
-  dump_access_keys_info(f, info);
-  dump_swift_keys_info(f, info);
+
+  if (dump_keys) {
+    dump_access_keys_info(f, info);
+    dump_swift_keys_info(f, info);
+  }
 
   encode_json("caps", info.caps, f);
 
@@ -2098,6 +2101,7 @@ int RGWUserAdminOp_User::list(const DoutPrefixProvider *dpp, rgw::sal::Driver* d
 int RGWUserAdminOp_User::info(const DoutPrefixProvider *dpp,
                              rgw::sal::Driver* driver, RGWUserAdminOpState& op_state,
                              RGWFormatterFlusher& flusher,
+                              bool dump_keys,
                              optional_yield y)
 {
   RGWUserInfo info;
@@ -2140,7 +2144,7 @@ int RGWUserAdminOp_User::info(const DoutPrefixProvider *dpp,
   if (formatter) {
     flusher.start(0);
 
-    dump_user_info(formatter, info, arg_stats);
+    dump_user_info(formatter, info, dump_keys, arg_stats);
     flusher.flush();
   }
 
@@ -2174,7 +2178,7 @@ int RGWUserAdminOp_User::create(const DoutPrefixProvider *dpp,
   if (formatter) {
     flusher.start(0);
 
-    dump_user_info(formatter, info);
+    dump_user_info(formatter, info, true);
     flusher.flush();
   }
 
@@ -2207,7 +2211,7 @@ int RGWUserAdminOp_User::modify(const DoutPrefixProvider *dpp,
   if (formatter) {
     flusher.start(0);
 
-    dump_user_info(formatter, info);
+    dump_user_info(formatter, info, true);
     flusher.flush();
   }
 

diff --git a/src/rgw/driver/rados/rgw_user.h b/src/rgw/driver/rados/rgw_user.h
index a0cd7ed84fee7b295336c8c0c3957040ef410d1c..91c859feafca512dd7883d7e11b1733da63105f6 100644 (file)
--- a/src/rgw/driver/rados/rgw_user.h
+++ b/src/rgw/driver/rados/rgw_user.h
@@ -645,7 +645,7 @@ public:
   static int info(const DoutPrefixProvider *dpp,
                  rgw::sal::Driver* driver,
                   RGWUserAdminOpState& op_state, RGWFormatterFlusher& flusher,
-                 optional_yield y);
+                 bool dump_keys, optional_yield y);
 
   static int create(const DoutPrefixProvider *dpp,
                    rgw::sal::Driver* driver,

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index f75ed2aca016b53b895a2aed5259b3a955da7eee..125d2780183870d4c4716b1e53f5587007487aae 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -2091,6 +2091,7 @@ bool RGWUserCaps::is_valid_cap_type(const string& tp)
                                     "user-policy",
                                     "amz-cache",
                                     "oidc-provider",
+                                    "user-info-without-keys",
                                                            "ratelimit"};
 
   for (unsigned int i = 0; i < sizeof(cap_type) / sizeof(char *); ++i) {