From: Ali Maredia Date: Fri, 5 Jan 2024 19:21:17 +0000 (+0000) Subject: rgw: add new cap user-info-without-keys X-Git-Tag: v19.1.0~314^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f0987e5151b43b70f62e823abd65aa4fa8306e3f;p=ceph.git 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 --- diff --git a/doc/radosgw/admin.rst b/doc/radosgw/admin.rst index 57d38c97ab2ff..501f161c1df99 100644 --- 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 f1faac6b1c2f4..f9e501515d0bd 100644 --- 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 3de4d6bc92587..4b07ad330d342 100644 --- 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 200f1c03d2e19..89712970fb2da 100644 --- 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 b5569e481c53e..f13e087c43648 100644 --- 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 a0cd7ed84fee7..91c859feafca5 100644 --- 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 f75ed2aca016b..125d278018387 100644 --- 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) {