Skip to content

Commit

Permalink
rgw/auth: generalize Identity::get_role_tenant() as get_tenant()
Browse files Browse the repository at this point in the history
all identities can return a tenant. rgw ops should consult the auth
identity for this instead of a rgw_user or rgw::sal::User

Signed-off-by: Casey Bodley <cbodley@redhat.com>
  • Loading branch information
cbodley committed Apr 10, 2024
1 parent efb2a91 commit d82ff16
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 55 deletions.
3 changes: 3 additions & 0 deletions src/rgw/rgw_auth.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ transform_old_authinfo(CephContext* const cct,
string get_subuser() const override {
return {};
}
const std::string& get_tenant() const override {
return id.tenant;
}

void to_str(std::ostream& out) const override {
out << "RGWDummyIdentityApplier(auth_id=" << id
Expand Down
16 changes: 14 additions & 2 deletions src/rgw/rgw_auth.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ class Identity {
/* Subuser of Account */
virtual std::string get_subuser() const = 0;

virtual std::string get_role_tenant() const { return ""; }
/* Identity's tenant namespace */
virtual const std::string& get_tenant() const = 0;

/* write any auth-specific fields that are safe to expose in the ops log */
virtual void write_ops_log_entry(rgw_log_entry& entry) const {};
Expand Down Expand Up @@ -479,6 +480,9 @@ class WebIdentityApplier : public IdentityApplier {
std::string get_subuser() const override {
return {};
}
const std::string& get_tenant() const override {
return role_tenant;
}

struct Factory {
virtual ~Factory() {}
Expand Down Expand Up @@ -626,6 +630,9 @@ class RemoteApplier : public IdentityApplier {
uint32_t get_identity_type() const override { return info.acct_type; }
std::string get_acct_name() const override { return info.acct_name; }
std::string get_subuser() const override { return {}; }
const std::string& get_tenant() const override {
return info.acct_user.tenant;
}

struct Factory {
virtual ~Factory() {}
Expand Down Expand Up @@ -688,6 +695,10 @@ class LocalApplier : public IdentityApplier {
uint32_t get_identity_type() const override { return TYPE_RGW; }
std::string get_acct_name() const override { return {}; }
std::string get_subuser() const override { return subuser; }
const std::string& get_tenant() const override {
return user_info.user_id.tenant;
}

void write_ops_log_entry(rgw_log_entry& entry) const override;

struct Factory {
Expand Down Expand Up @@ -747,8 +758,9 @@ class RoleApplier : public IdentityApplier {
uint32_t get_identity_type() const override { return TYPE_ROLE; }
std::string get_acct_name() const override { return {}; }
std::string get_subuser() const override { return {}; }
const std::string& get_tenant() const override { return role.tenant; }

void modify_request_state(const DoutPrefixProvider* dpp, req_state* s) const override;
std::string get_role_tenant() const override { return role.tenant; }

struct Factory {
virtual ~Factory() {}
Expand Down
4 changes: 2 additions & 2 deletions src/rgw/rgw_auth_filters.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ class DecoratedApplier : public rgw::auth::IdentityApplier {
get_decoratee().to_str(out);
}

std::string get_role_tenant() const override { /* in/out */
return get_decoratee().get_role_tenant();
const std::string& get_tenant() const override {
return get_decoratee().get_tenant();
}

void load_acct_info(const DoutPrefixProvider* dpp, RGWUserInfo& user_info) const override { /* out */
Expand Down
31 changes: 13 additions & 18 deletions src/rgw/rgw_op.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1955,6 +1955,7 @@ int RGWGetObj::handle_user_manifest(const char *prefix, optional_yield y)
return -EINVAL;
}

const std::string& auth_tenant = s->auth.identity->get_tenant();
const std::string bucket_name = url_decode(prefix_view.substr(0, pos));
const std::string obj_prefix = url_decode(prefix_view.substr(pos + 1));

Expand All @@ -1969,7 +1970,7 @@ int RGWGetObj::handle_user_manifest(const char *prefix, optional_yield y)

if (bucket_name.compare(s->bucket->get_name()) != 0) {
map<string, bufferlist> bucket_attrs;
r = driver->load_bucket(this, rgw_bucket(s->user->get_tenant(), bucket_name),
r = driver->load_bucket(this, rgw_bucket(auth_tenant, bucket_name),
&ubucket, y);
if (r < 0) {
ldpp_dout(this, 0) << "could not get bucket info for bucket="
Expand All @@ -1982,7 +1983,7 @@ int RGWGetObj::handle_user_manifest(const char *prefix, optional_yield y)
ldpp_dout(this, 0) << "failed to read bucket policy" << dendl;
return r;
}
_bucket_policy = get_iam_policy_from_attr(s->cct, bucket_attrs, s->user->get_tenant());
_bucket_policy = get_iam_policy_from_attr(s->cct, bucket_attrs, auth_tenant);
bucket_policy = &_bucket_policy;
pbucket = ubucket.get();
} else {
Expand Down Expand Up @@ -2054,6 +2055,7 @@ int RGWGetObj::handle_slo_manifest(bufferlist& bl, optional_yield y)
vector<RGWAccessControlPolicy> allocated_acls;
map<string, pair<RGWAccessControlPolicy *, boost::optional<Policy>>> policies;
map<string, std::unique_ptr<rgw::sal::Bucket>> buckets;
const std::string& auth_tenant = s->auth.identity->get_tenant();

map<uint64_t, rgw_slo_part> slo_parts;

Expand Down Expand Up @@ -2099,8 +2101,7 @@ int RGWGetObj::handle_slo_manifest(bufferlist& bl, optional_yield y)
RGWAccessControlPolicy& _bucket_acl = allocated_acls.emplace_back();

std::unique_ptr<rgw::sal::Bucket> tmp_bucket;
int r = driver->load_bucket(this, rgw_bucket(s->user->get_tenant(),
bucket_name),
int r = driver->load_bucket(this, rgw_bucket(auth_tenant, bucket_name),
&tmp_bucket, y);
if (r < 0) {
ldpp_dout(this, 0) << "could not get bucket info for bucket="
Expand All @@ -2117,7 +2118,7 @@ int RGWGetObj::handle_slo_manifest(bufferlist& bl, optional_yield y)
return r;
}
auto _bucket_policy = get_iam_policy_from_attr(
s->cct, tmp_bucket->get_attrs(), tmp_bucket->get_tenant());
s->cct, tmp_bucket->get_attrs(), auth_tenant);
bucket_policy = _bucket_policy.get_ptr();
buckets[bucket_name].swap(tmp_bucket);
policies[bucket_name] = make_pair(bucket_acl, _bucket_policy);
Expand Down Expand Up @@ -2509,13 +2510,7 @@ int RGWListBuckets::verify_permission(optional_yield y)
rgw::Partition partition = rgw::Partition::aws;
rgw::Service service = rgw::Service::s3;

string tenant;
if (s->auth.identity->get_identity_type() == TYPE_ROLE) {
tenant = s->auth.identity->get_role_tenant();
} else {
tenant = s->user->get_tenant();
}

const std::string& tenant = s->auth.identity->get_tenant();
if (!verify_user_permission(this, s, ARN(partition, service, "", tenant, "*"), rgw::IAM::s3ListAllMyBuckets, false)) {
return -EACCES;
}
Expand Down Expand Up @@ -3177,7 +3172,7 @@ int RGWCreateBucket::verify_permission(optional_yield y)
return -EACCES;
}

if (s->user->get_tenant() != s->bucket_tenant) {
if (s->auth.identity->get_tenant() != s->bucket_tenant) {
//AssumeRole is meant for cross account access
if (s->auth.identity->get_identity_type() != TYPE_ROLE) {
ldpp_dout(this, 10) << "user cannot create a bucket in a different tenant"
Expand Down Expand Up @@ -3773,7 +3768,7 @@ int RGWPutObj::init_processing(optional_yield y) {
pos = copy_source_bucket_name.find(":");
if (pos == std::string::npos) {
// if tenant is not specified in x-amz-copy-source, use tenant of the requester
copy_source_tenant_name = s->user->get_tenant();
copy_source_tenant_name = s->auth.identity->get_tenant();
} else {
copy_source_tenant_name = copy_source_bucket_name.substr(0, pos);
copy_source_bucket_name = copy_source_bucket_name.substr(pos + 1, copy_source_bucket_name.size());
Expand Down Expand Up @@ -7434,7 +7429,7 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie
ACLOwner bowner;
RGWObjVersionTracker ot;

int ret = driver->load_bucket(dpp, rgw_bucket(s->user->get_tenant(),
int ret = driver->load_bucket(dpp, rgw_bucket(s->auth.identity->get_tenant(),
path.bucket_name),
&bucket, y);
if (ret < 0) {
Expand Down Expand Up @@ -7576,9 +7571,9 @@ int RGWBulkUploadOp::verify_permission(optional_yield y)
return -EACCES;
}

if (s->user->get_tenant() != s->bucket_tenant) {
if (s->auth.identity->get_tenant() != s->bucket_tenant) {
ldpp_dout(this, 10) << "user cannot create a bucket in a different tenant"
<< " (user_id.tenant=" << s->user->get_tenant()
<< " (authorized user tenant=" << s->auth.identity->get_tenant()
<< " requested=" << s->bucket_tenant << ")" << dendl;
return -EACCES;
}
Expand Down Expand Up @@ -7828,7 +7823,7 @@ int RGWBulkUploadOp::handle_file(const std::string_view path,
std::unique_ptr<rgw::sal::Bucket> bucket;
ACLOwner bowner;

op_ret = driver->load_bucket(this, rgw_bucket(s->user->get_tenant(),
op_ret = driver->load_bucket(this, rgw_bucket(s->auth.identity->get_tenant(),
bucket_name),
&bucket, y);
if (op_ret < 0) {
Expand Down
4 changes: 0 additions & 4 deletions src/rgw/rgw_pubsub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,6 @@ void rgw_pubsub_dest::decode_json(JSONObj* f) {
: std::stoul(sleep_dur);
}

RGWPubSub::RGWPubSub(rgw::sal::Driver* _driver, const std::string& _tenant)
: driver(_driver), tenant(_tenant)
{}

RGWPubSub::RGWPubSub(rgw::sal::Driver* _driver,
const std::string& _tenant,
const rgw::SiteConfig& site)
Expand Down
8 changes: 3 additions & 5 deletions src/rgw/rgw_pubsub.h
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,9 @@ class RGWPubSub
RGWObjVersionTracker* objv_tracker, optional_yield y) const;

public:
RGWPubSub(rgw::sal::Driver* _driver, const std::string& tenant);

RGWPubSub(rgw::sal::Driver* _driver,
const std::string& _tenant,
const rgw::SiteConfig& site);
RGWPubSub(rgw::sal::Driver* _driver,
const std::string& _tenant,
const rgw::SiteConfig& site);

class Bucket {
friend class RGWPubSub;
Expand Down
26 changes: 13 additions & 13 deletions src/rgw/rgw_rest_pubsub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ std::optional<rgw::IAM::Policy> get_policy_from_text(req_state* const s,
const auto bl = bufferlist::static_from_string(policy_text);
try {
return rgw::IAM::Policy(
s->cct, s->owner.id.tenant, bl,
s->cct, s->auth.identity->get_tenant(), bl,
s->cct->_conf.get_val<bool>("rgw_policy_reject_invalid_principals"));
} catch (rgw::IAM::PolicyParseException& e) {
ldout(s->cct, 1) << "failed to parse policy: '" << policy_text
Expand Down Expand Up @@ -218,7 +218,7 @@ class RGWPSCreateTopicOp : public RGWOp {
return ret;
}

const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
rgw_pubsub_topic result;
ret = ps.get_topic(this, topic_name, result, y, nullptr);
if (ret == -ENOENT) {
Expand Down Expand Up @@ -297,7 +297,7 @@ void RGWPSCreateTopicOp::execute(optional_yield y) {
return;
}
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
op_ret = ps.create_topic(this, topic_name, dest, topic_arn, opaque_data,
s->owner.id, policy_text, y);
if (op_ret < 0) {
Expand Down Expand Up @@ -358,7 +358,7 @@ class RGWPSListTopicsOp : public RGWOp {
void RGWPSListTopicsOp::execute(optional_yield y) {
const std::string start_token = s->info.args.get("NextToken");

const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
constexpr int max_items = 100;
op_ret = ps.get_topics(this, start_token, max_items, result, next_token, y);
// if there are no topics it is not considered an error
Expand Down Expand Up @@ -446,7 +446,7 @@ void RGWPSGetTopicOp::execute(optional_yield y) {
if (op_ret < 0) {
return;
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
op_ret = ps.get_topic(this, topic_name, result, y, nullptr);
if (op_ret < 0) {
ldpp_dout(this, 1) << "failed to get topic '" << topic_name << "', ret=" << op_ret << dendl;
Expand Down Expand Up @@ -530,7 +530,7 @@ void RGWPSGetTopicAttributesOp::execute(optional_yield y) {
if (op_ret < 0) {
return;
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
op_ret = ps.get_topic(this, topic_name, result, y, nullptr);
if (op_ret < 0) {
ldpp_dout(this, 1) << "failed to get topic '" << topic_name << "', ret=" << op_ret << dendl;
Expand Down Expand Up @@ -662,7 +662,7 @@ class RGWPSSetTopicAttributesOp : public RGWOp {
return ret;
}
rgw_pubsub_topic result;
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
ret = ps.get_topic(this, topic_name, result, y, nullptr);
if (ret < 0) {
ldpp_dout(this, 1) << "failed to get topic '" << topic_name
Expand Down Expand Up @@ -739,7 +739,7 @@ void RGWPSSetTopicAttributesOp::execute(optional_yield y) {
return;
}
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
op_ret = ps.create_topic(this, topic_name, dest, topic_arn, opaque_data,
topic_owner, policy_text, y);
if (op_ret < 0) {
Expand Down Expand Up @@ -823,7 +823,7 @@ void RGWPSDeleteTopicOp::execute(optional_yield y) {
return;
}
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);

rgw_pubsub_topic result;
op_ret = ps.get_topic(this, topic_name, result, y, nullptr);
Expand Down Expand Up @@ -1047,7 +1047,7 @@ void RGWPSCreateNotifOp::execute(optional_yield y) {
return;
}

const RGWPubSub ps(driver, s->owner.id.tenant);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
const RGWPubSub::Bucket b(ps, bucket.get());

if(configurations.list.empty()) {
Expand Down Expand Up @@ -1202,7 +1202,7 @@ void RGWPSCreateNotifOp::execute_v2(optional_yield y) {
<< "' , ret = " << op_ret << dendl;
return;
}
const RGWPubSub ps(driver, s->owner.id.tenant, *s->penv.site);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
std::unordered_map<std::string, rgw_pubsub_topic> topics;
for (const auto& c : configurations.list) {
const auto& notif_name = c.id;
Expand Down Expand Up @@ -1352,7 +1352,7 @@ void RGWPSDeleteNotifOp::execute(optional_yield y) {
return;
}

const RGWPubSub ps(driver, s->owner.id.tenant);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
const RGWPubSub::Bucket b(ps, bucket.get());

// get all topics on a bucket
Expand Down Expand Up @@ -1494,7 +1494,7 @@ void RGWPSListNotifsOp::execute(optional_yield y) {
driver->stat_topics_v1(s->bucket_tenant, y, this) == -ENOENT) {
op_ret = get_bucket_notifications(this, bucket.get(), bucket_topics);
} else {
const RGWPubSub ps(driver, s->owner.id.tenant);
const RGWPubSub ps(driver, s->auth.identity->get_tenant(), *s->penv.site);
const RGWPubSub::Bucket b(ps, bucket.get());
op_ret = b.get_topics(this, bucket_topics, y);
}
Expand Down
13 changes: 3 additions & 10 deletions src/rgw/rgw_rest_s3.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4918,14 +4918,13 @@ int RGWHandler_REST_S3::postauth_init(optional_yield y)
{
struct req_init_state *t = &s->init_state;

int ret = rgw_parse_url_bucket(t->url_bucket, s->user->get_tenant(),
const std::string& auth_tenant = s->auth.identity->get_tenant();

int ret = rgw_parse_url_bucket(t->url_bucket, auth_tenant,
s->bucket_tenant, s->bucket_name);
if (ret) {
return ret;
}
if (s->auth.identity->get_identity_type() == TYPE_ROLE) {
s->bucket_tenant = s->auth.identity->get_role_tenant();
}

ldpp_dout(s, 10) << "s->object=" << s->object
<< " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl;
Expand All @@ -4940,12 +4939,6 @@ int RGWHandler_REST_S3::postauth_init(optional_yield y)
}

if (!t->src_bucket.empty()) {
string auth_tenant;
if (s->auth.identity->get_identity_type() == TYPE_ROLE) {
auth_tenant = s->auth.identity->get_role_tenant();
} else {
auth_tenant = s->user->get_tenant();
}
ret = rgw_parse_url_bucket(t->src_bucket, auth_tenant,
s->src_tenant_name, s->src_bucket_name);
if (ret) {
Expand Down
2 changes: 1 addition & 1 deletion src/rgw/rgw_rest_swift.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2931,7 +2931,7 @@ int RGWHandler_REST_SWIFT::postauth_init(optional_yield y)
&& s->user->get_id().id == RGW_USER_ANON_ID) {
s->bucket_tenant = s->account_name;
} else {
s->bucket_tenant = s->user->get_tenant();
s->bucket_tenant = s->auth.identity->get_tenant();
}
s->bucket_name = t->url_bucket;

Expand Down
6 changes: 6 additions & 0 deletions src/test/rgw/test_rgw_iam_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ class FakeIdentity : public Identity {
return 0;
}

const std::string& get_tenant() const override {
ceph_abort();
static std::string empty;
return empty;
}

void to_str(std::ostream& out) const override {
out << id;
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/rgw/test_rgw_lua.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ class FakeIdentity : public Identity {
return "";
}

const std::string& get_tenant() const override {
static std::string empty;
return empty;
}

void to_str(std::ostream& out) const override {
return;
}
Expand Down

0 comments on commit d82ff16

Please sign in to comment.