From d50ef542372f541ac9411f655cddd5fcab4dceac Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Wed, 19 Jul 2017 17:06:24 +0200 Subject: [PATCH 1/3] rgw_user: validate tenant names when creating user Also moved `validate_tenant_name` function from RGWHandler_REST to rgw_user, a new function called `rgw_validate_tenant_name` validates tenant names. There is special handling in rgw_admin to reraise -ERR_INVALID_TENANT to -EINVAL Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_admin.cc | 3 +++ src/rgw/rgw_rest.cc | 12 ------------ src/rgw/rgw_rest.h | 1 - src/rgw/rgw_rest_s3.cc | 8 ++++---- src/rgw/rgw_rest_swift.cc | 2 +- src/rgw/rgw_user.cc | 19 +++++++++++++++++++ src/rgw/rgw_user.h | 14 ++++++-------- 7 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 1be1d1594ed..3c4c8bfeb8d 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -4375,6 +4375,9 @@ int main(int argc, const char **argv) ret = user.add(user_op, &err_msg); if (ret < 0) { cerr << "could not create user: " << err_msg << std::endl; + if (ret == -ERR_INVALID_TENANT_NAME) + ret = -EINVAL; + return -ret; } if (!subuser.empty()) { diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index 505596242aa..f67a076c622 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1834,18 +1834,6 @@ int RGWHandler_REST::allocate_formatter(struct req_state *s, return 0; } -int RGWHandler_REST::validate_tenant_name(string const& t) -{ - struct tench { - static bool is_good(char ch) { - return isalnum(ch) || ch == '_'; - } - }; - std::string::const_iterator it = - std::find_if_not(t.begin(), t.end(), tench::is_good); - return (it == t.end())? 0: -ERR_INVALID_TENANT_NAME; -} - // This function enforces Amazon's spec for bucket names. // (The requirements, not the recommendations.) int RGWHandler_REST::validate_bucket_name(const string& bucket) diff --git a/src/rgw/rgw_rest.h b/src/rgw/rgw_rest.h index 615ececc595..f780ab4abac 100644 --- a/src/rgw/rgw_rest.h +++ b/src/rgw/rgw_rest.h @@ -501,7 +501,6 @@ public: RGWHandler_REST() {} ~RGWHandler_REST() override {} - static int validate_tenant_name(const string& bucket); static int validate_bucket_name(const string& bucket); static int validate_object_name(const string& object); diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 51044e9a8f3..8141edfd0ee 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -3185,7 +3185,7 @@ int RGWHandler_REST_S3::postauth_init() << " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl; int ret; - ret = validate_tenant_name(s->bucket_tenant); + ret = rgw_validate_tenant_name(s->bucket_tenant); if (ret) return ret; if (!s->bucket_name.empty()) { @@ -3200,7 +3200,7 @@ int RGWHandler_REST_S3::postauth_init() if (!t->src_bucket.empty()) { rgw_parse_url_bucket(t->src_bucket, s->user->user_id.tenant, s->src_tenant_name, s->src_bucket_name); - ret = validate_tenant_name(s->src_tenant_name); + ret = rgw_validate_tenant_name(s->src_tenant_name); if (ret) return ret; ret = valid_s3_bucket_name(s->src_bucket_name, relaxed_names); @@ -3216,8 +3216,8 @@ int RGWHandler_REST_S3::init(RGWRados *store, struct req_state *s, int ret; s->dialect = "s3"; - - ret = validate_tenant_name(s->bucket_tenant); + + ret = rgw_validate_tenant_name(s->bucket_tenant); if (ret) return ret; bool relaxed_names = s->cct->_conf->rgw_relaxed_s3_bucket_names; diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index 90a829da5b9..220c349cbc1 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -2557,7 +2557,7 @@ int RGWHandler_REST_SWIFT::postauth_init() << dendl; int ret; - ret = validate_tenant_name(s->bucket_tenant); + ret = rgw_validate_tenant_name(s->bucket_tenant); if (ret) return ret; ret = validate_bucket_name(s->bucket_name); diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 57f2dc227c4..e2ab8bf4321 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -542,6 +542,18 @@ uint32_t rgw_str_to_perm(const char *str) return RGW_PERM_INVALID; } +int rgw_validate_tenant_name(const string& t) +{ + struct tench { + static bool is_good(char ch) { + return isalnum(ch) || ch == '_'; + } + }; + std::string::const_iterator it = + std::find_if_not(t.begin(), t.end(), tench::is_good); + return (it == t.end())? 0: -ERR_INVALID_TENANT_NAME; +} + static bool validate_access_key(string& key) { const char *p = key.c_str(); @@ -1850,6 +1862,13 @@ int RGWUser::check_op(RGWUserAdminOpState& op_state, std::string *err_msg) return -EINVAL; } + int ret = rgw_validate_tenant_name(op_id.tenant); + if (ret) { + set_err_msg(err_msg, + "invalid tenant only alphanumeric and _ characters are allowed"); + return ret; + } + //set key type when it not set or set by context if ((op_state.get_key_type() < 0) || op_state.key_type_setbycontext) { op_state.set_key_type(KEY_TYPE_S3); diff --git a/src/rgw/rgw_user.h b/src/rgw/rgw_user.h index 7a27d0e7007..c6f15d0a46d 100644 --- a/src/rgw/rgw_user.h +++ b/src/rgw/rgw_user.h @@ -113,9 +113,6 @@ extern int rgw_get_user_attrs_by_uid(RGWRados *store, * Given an RGWUserInfo, deletes the user and its bucket ACLs. */ extern int rgw_delete_user(RGWRados *store, RGWUserInfo& user, RGWObjVersionTracker& objv_tracker); -/** - * Store a list of the user's buckets, with associated functinos. - */ /* * remove the different indexes @@ -125,14 +122,11 @@ extern int rgw_remove_uid_index(RGWRados *store, rgw_user& uid); extern int rgw_remove_email_index(RGWRados *store, string& email); extern int rgw_remove_swift_name_index(RGWRados *store, string& swift_name); -/* - * An RGWUser class along with supporting classes created - * to support the creation of an RESTful administrative API - */ - extern void rgw_perm_to_str(uint32_t mask, char *buf, int len); extern uint32_t rgw_str_to_perm(const char *str); +extern int rgw_validate_tenant_name(const string& t); + enum ObjectKeyType { KEY_TYPE_SWIFT, KEY_TYPE_S3, @@ -151,6 +145,10 @@ enum RGWUserId { RGW_ACCESS_KEY, }; +/* + * An RGWUser class along with supporting classes created + * to support the creation of an RESTful administrative API + */ struct RGWUserAdminOpState { // user attributes RGWUserInfo info; From 5c8d27363cec3a33ae4eea8065eff76b4f302ab1 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Thu, 20 Jul 2017 11:05:24 +0200 Subject: [PATCH 2/3] doc: mention that tenant names may be specified in adminops api We already allow for tenant names to be specified as a part of uid in the adminops api, mention this. Also mention about ``tenant`` in the json response we send. Signed-off-by: Abhishek Lekshmanan --- doc/radosgw/adminops.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/radosgw/adminops.rst b/doc/radosgw/adminops.rst index 28c02164b0d..597f4fe5d26 100644 --- a/doc/radosgw/adminops.rst +++ b/doc/radosgw/adminops.rst @@ -342,6 +342,7 @@ Request Parameters :Type: String :Example: ``foo_user`` :Required: Yes +A tenant name may also specified as a part of ``uid``, by following the syntax ``tenant$user``, refer to `Multitenancy`_ for more details. ``display-name`` @@ -418,6 +419,11 @@ If successful, the response contains the user information. :Description: A container for the user data information. :Type: Container +``tenant`` +:Description: The tenant which user is a part of +:Type: String +:Parent: ``user`` + ``user_id`` :Description: The user id. @@ -1924,3 +1930,4 @@ Standard Error Responses .. _Admin Guide: ../admin .. _Quota Management: ../admin#quota-management +.. _Multitenancy: ./multitenancy From af0e307aae679cbaede6979b6ea9d531721c4ce1 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Thu, 20 Jul 2017 11:08:30 +0200 Subject: [PATCH 3/3] rgw: adminops API now supports tenant param for user creation Allow `tenant` as a param for user creation API, also document this. Currently we still return a -ENOENT when an invalid tenant name is specified, while we could make it return -ERR_INVALID_TENANT, this would make rgw admin cli not return -ENOENT when an invalid tenant name is specified. Signed-off-by: Abhishek Lekshmanan --- doc/radosgw/adminops.rst | 13 +++++++++++++ src/rgw/rgw_rest_user.cc | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/doc/radosgw/adminops.rst b/doc/radosgw/adminops.rst index 597f4fe5d26..422dd16527a 100644 --- a/doc/radosgw/adminops.rst +++ b/doc/radosgw/adminops.rst @@ -321,6 +321,11 @@ generated key is added to the keyring without replacing an existing key pair. If ``access-key`` is specified and refers to an existing key owned by the user then it will be modified. +.. versionadded:: Luminous + +A ``tenant`` may either be specified as a part of uid or as an additional +request param. + :caps: users=write Syntax @@ -409,6 +414,14 @@ A tenant name may also specified as a part of ``uid``, by following the syntax ` :Example: False [False] :Required: No +.. versionadded:: Jewel +``tenant`` + +:Description: the Tenant under which a user is a part of. +:Type: string +:Example: tenant1 +:Required: No + Response Entities ~~~~~~~~~~~~~~~~~ diff --git a/src/rgw/rgw_rest_user.cc b/src/rgw/rgw_rest_user.cc index 711fd3a0c1d..8539c3ed766 100644 --- a/src/rgw/rgw_rest_user.cc +++ b/src/rgw/rgw_rest_user.cc @@ -76,6 +76,7 @@ void RGWOp_User_Create::execute() std::string secret_key; std::string key_type_str; std::string caps; + std::string tenant_name; bool gen_key; bool suspended; @@ -96,6 +97,7 @@ void RGWOp_User_Create::execute() RESTArgs::get_string(s, "secret-key", secret_key, &secret_key); RESTArgs::get_string(s, "key-type", key_type_str, &key_type_str); RESTArgs::get_string(s, "user-caps", caps, &caps); + RESTArgs::get_string(s, "tenant", tenant_name, &tenant_name); RESTArgs::get_bool(s, "generate-key", true, &gen_key); RESTArgs::get_bool(s, "suspended", false, &suspended); RESTArgs::get_int32(s, "max-buckets", default_max_buckets, &max_buckets); @@ -108,6 +110,10 @@ void RGWOp_User_Create::execute() return; } + if (!tenant_name.empty()) { + uid.tenant = tenant_name; + } + // 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);