Skip to content

Conversation

@fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Mar 1, 2022

Close #1191 .
Signed-off-by: Evgenii Stratonikov [email protected]

@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #1203 (423e219) into master (03b601b) will increase coverage by 0.03%.
The diff coverage is 52.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1203      +/-   ##
==========================================
+ Coverage   35.64%   35.68%   +0.03%     
==========================================
  Files         288      288              
  Lines       18131    18144      +13     
==========================================
+ Hits         6463     6474      +11     
- Misses      11177    11179       +2     
  Partials      491      491              
Impacted Files Coverage Δ
pkg/services/object/acl/v2/errors.go 0.00% <ø> (ø)
pkg/services/object/acl/v2/service.go 0.00% <0.00%> (ø)
pkg/services/object/acl/v2/util.go 16.23% <61.11%> (+8.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03b601b...423e219. Read the comment docs.

// Check if session token verb is equal to the current operation.
// Unknown means everything is allowed.
verb := sourceVerbOfRequest(req.token, op)
if verb != op {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, we can't simply compare op and value from token. One request type can produce requests of other type. During the tests, I logged such errors:

token: DELETE  op: PUT
token: DELETE  op: HEAD
token: GETRANGEHASH  op: GETRANGE

We also have this table in spec of produced request types.

Base/Gen PUT DELETE HEAD RANGE GET HASH SEARCH
PUT + - - - - - -
DELETE + - + - - - +
HEAD - - + - - - -
RANGE - - + + - - -
GET - - + - + - -
HASH - - + + - - -
SEARCH - - - - - - +

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

// find verb from token if it is present
verb := sourceVerbOfRequest(req, op)
// Check if session token verb is equal to the current operation.
// Unknown means everything is allowed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, can you fix API spec and mention such behavior there?

@alexvanin alexvanin merged commit 0bf5952 into nspcc-dev:master Mar 5, 2022
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.

Verb of object session token is not checked

2 participants