Skip to content

Commit 3d7a476

Browse files
committed
Account with NotPrincipal should explicitly provide permission
This is the bucket policy with below combination ``` Effect : DENY, Action: $OPERATION, NotPrincipal: $ACCOUNT ``` The permission on the "Action" for the $ACCOUNT mentioned in the "NotPrincipal" should be explicitly given. Example: For "Action: get_object", if we want to DENY permission for * (all accounts) and we want to give explicit permission to any one or few accounts, then we can include that account as part of part of "NotPrincipal" Check DFBUGS-1519 for steps to reproduce the issue Fixes: https://issues.redhat.com/browse/DFBUGS-1519 Signed-off-by: Vinayakswami Hariharmath <[email protected]>
1 parent a5d0e0c commit 3d7a476

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

src/endpoint/s3/s3_rest.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -293,16 +293,19 @@ async function authorize_request_policy(req) {
293293
s3_policy, account_identifier_id, method, arn_path, req, public_access_block_cfg?.public_access_block?.restrict_public_buckets);
294294
dbg.log3('authorize_request_policy: permission_by_id', permission_by_id);
295295
}
296-
if (permission_by_id === "DENY") throw new S3Error(S3Error.AccessDenied);
297296

298-
if ((!account_identifier_id || permission_by_id !== "DENY") && account.owner === undefined) {
297+
if (permission_by_id === "ALLOW") return;
298+
299+
if ((!account_identifier_id || permission_by_id === "DENY" || permission_by_id === "IMPLICIT_DENY") && account.owner === undefined) {
299300
permission_by_name = await s3_bucket_policy_utils.has_bucket_policy_permission(
300301
s3_policy, account_identifier_name, method, arn_path, req, public_access_block_cfg?.public_access_block?.restrict_public_buckets
301302
);
302303
dbg.log3('authorize_request_policy: permission_by_name', permission_by_name);
303304
}
305+
304306
if (permission_by_name === "DENY") throw new S3Error(S3Error.AccessDenied);
305-
if ((permission_by_id === "ALLOW" || permission_by_name === "ALLOW") || is_owner) return;
307+
// Allow access if either ID or Name has ALLOW permission, or if the user is the owner
308+
if (permission_by_id === "ALLOW" || permission_by_name === "ALLOW" || is_owner) return;
306309

307310
throw new S3Error(S3Error.AccessDenied);
308311
}

0 commit comments

Comments
 (0)