Skip to content

Improvement/cldsrv 428 put apis implicit deny#5449

Closed
benzekrimaha wants to merge 6 commits intodevelopment/7.10from
improvement/CLDSRV-428-put-apis-implicit-Deny
Closed

Improvement/cldsrv 428 put apis implicit deny#5449
benzekrimaha wants to merge 6 commits intodevelopment/7.10from
improvement/CLDSRV-428-put-apis-implicit-Deny

Conversation

@benzekrimaha
Copy link
Contributor

PR opened after closing : #5325

Bucket policies are not correctly interpreted, this is part of the following epic to fix that: scality/Arsenal#2181

This PR is aiming to update put apis since object and bucket authorisations are made at API level and need to support implicit denies, ticket linked to this issue here : https://scality.atlassian.net/browse/CLDSRV-428

PRs providing implicit Deny logic to CS for processing in this PR
scality/Arsenal#2181
https://github.com/scality/Vault/pull/2135
#5322
#5420
#5432

I'll be bumping a new CLDSRV once the reviews done as these are changes that will be tested against Integration

@bert-e
Copy link
Contributor

bert-e commented Nov 17, 2023

Hello benzekrimaha,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Nov 17, 2023

Incorrect fix version

The Fix Version/s in issue CLDSRV-428 contains:

  • 7.10.34

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

  • 7.10.34

  • 7.70.31

  • 8.6.13

  • 8.7.32

  • 8.8.6

Please check the Fix Version/s of CLDSRV-428, or the target
branch of this pull request.

Copy link

@KazToozs KazToozs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing stood out to me that needs particular improvmeent, but a lot of linter changes made this quite a difficult review.

}
// if requester is not bucket owner, bucket policy actions should be denied with
// MethodNotAllowed error
const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy'];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this is not the case anymore and is worth going over

@benzekrimaha benzekrimaha force-pushed the improvement/CLDSRV-428-put-apis-implicit-Deny branch from 430b54c to 4fbe1cd Compare November 20, 2023 10:52
@benzekrimaha benzekrimaha force-pushed the improvement/CLDSRV-428-put-apis-implicit-Deny branch from d8d4982 to 0131eab Compare November 21, 2023 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants