Skip to content

Commit 66f2a35

Browse files
committed
in case of IAM policy not matching throw detailed error
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
1 parent a7a1196 commit 66f2a35

File tree

2 files changed

+40
-4
lines changed

2 files changed

+40
-4
lines changed

src/endpoint/iam/iam_utils.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,32 @@ function get_iam_username(requested_account_name) {
6666
return requested_account_name.split(iam_constants.IAM_SPLIT_CHARACTERS)[0];
6767
}
6868

69+
/**
70+
* _create_detailed_message_for_access_in_s3 returns a detailed message with details needed for user who
71+
* tried to perform S3 operation
72+
* - resource_arn is only relevant for operations related to a bucket
73+
* @param {object} requesting_account
74+
* @param {string|string[]} method
75+
* @param {string} resource_arn
76+
*/
77+
function _create_detailed_message_for_access_in_s3(requesting_account, method, resource_arn) {
78+
const arn_for_requesting_account = create_arn_for_user(requesting_account.owner,
79+
get_iam_username(requesting_account.name.unwrap()), requesting_account.iam_path);
80+
const full_action_name = Array.isArray(method) && method.length > 1 ? method[1] : method; // special case for get_object_attributes
81+
82+
const message_start = `User: ${arn_for_requesting_account} is not authorized to perform: ${full_action_name} `;
83+
const message_resource = `on resource: ${resource_arn} `;
84+
const message_end = `because no identity-based policy allows the ${full_action_name} action`;
85+
86+
let message_with_details;
87+
if (full_action_name === 's3:ListAllMyBuckets') {
88+
message_with_details = message_start + message_end;
89+
} else {
90+
message_with_details = message_start + message_resource + message_end;
91+
}
92+
return message_with_details;
93+
}
94+
6995
/**
7096
* parse_max_items converts the input to the needed type
7197
* assuming that we've got only sting type
@@ -852,4 +878,5 @@ exports.validate_tag_user_params = validate_tag_user_params;
852878
exports.validate_untag_user_params = validate_untag_user_params;
853879
exports.validate_list_user_tags_params = validate_list_user_tags_params;
854880
exports.get_owner_account_id = get_owner_account_id;
881+
exports._create_detailed_message_for_access_in_s3 = _create_detailed_message_for_access_in_s3;
855882

src/endpoint/s3/s3_rest.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const http_utils = require('../../util/http_utils');
1414
const signature_utils = require('../../util/signature_utils');
1515
const config = require('../../../config');
1616
const s3_utils = require('./s3_utils');
17+
const { _create_detailed_message_for_access_in_s3 } = require('../iam/iam_utils'); // authorize_request for IAM policy
1718

1819
const S3_MAX_BODY_LEN = 4 * 1024 * 1024;
1920

@@ -331,7 +332,7 @@ async function authorize_request_policy(req) {
331332
throw new S3Error(S3Error.AccessDenied);
332333
}
333334

334-
// TODO - move the function and throw message error with details
335+
// TODO - move the function
335336
async function authorize_request_iam_policy(req) {
336337
const auth_token = req.object_sdk.get_auth_token();
337338
const is_anonymous = !(auth_token && auth_token.access_key);
@@ -341,11 +342,12 @@ async function authorize_request_iam_policy(req) {
341342
const is_iam_user = account.owner !== undefined;
342343
if (!is_iam_user) return; // IAM policy is only on IAM users (account root user is authorized here)
343344

344-
const resource_arn = _get_arn_from_req_path(req);
345+
const resource_arn = _get_arn_from_req_path(req) || '*'; // special case for list all buckets in an account
345346
const method = _get_method_from_req(req);
346347
const iam_policies = account.iam_user_policies || [];
347348
if (iam_policies.length === 0 && req.object_sdk.nsfs_config_root) return; // We do not have IAM policies in NC yet
348349

350+
const requesting_account = req.object_sdk.requesting_account;
349351
// parallel policy check
350352
const promises = [];
351353
for (const iam_policy of iam_policies) {
@@ -359,14 +361,20 @@ async function authorize_request_iam_policy(req) {
359361
const permission_result = await Promise.all(promises);
360362
let has_allow_permission = false;
361363
for (const permission of permission_result) {
362-
if (permission === "DENY") throw new S3Error(S3Error.AccessDenied);
364+
if (permission === "DENY") _throw_iam_access_denied_error_for_s3_operation(requesting_account, method, resource_arn);
363365
if (permission === "ALLOW") {
364366
has_allow_permission = true;
365367
}
366368
}
367369
if (has_allow_permission) return;
368370
dbg.log1('authorize_request_iam_policy: user have inline policies but none of them matched the method');
369-
throw new S3Error(S3Error.AccessDenied);
371+
_throw_iam_access_denied_error_for_s3_operation(requesting_account, method, resource_arn);
372+
}
373+
374+
function _throw_iam_access_denied_error_for_s3_operation(requesting_account, method, resource_arn) {
375+
const message_with_details = _create_detailed_message_for_access_in_s3(requesting_account, method, resource_arn);
376+
const { code, http_code } = S3Error.AccessDenied;
377+
throw new S3Error({ code, message: message_with_details, http_code});
370378
}
371379

372380
async function authorize_anonymous_access(s3_policy, method, arn_path, req, public_access_block) {
@@ -399,6 +407,7 @@ function _get_method_from_req(req) {
399407
}
400408

401409
function _get_arn_from_req_path(req) {
410+
if (!req.params.bucket) return;
402411
const bucket = req.params.bucket;
403412
const key = req.params.key;
404413
let arn_path = `arn:aws:s3:::${bucket}`;

0 commit comments

Comments
 (0)