Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

object/put: Process session token of the original request in ACL checks #2461

Merged
merged 1 commit into from
Jul 28, 2023
Merged

object/put: Process session token of the original request in ACL checks #2461

merged 1 commit into from
Jul 28, 2023

Conversation

cthulhu-rider
Copy link
Contributor

@codecov
Copy link

codecov bot commented Jul 26, 2023 •
edited
Loading

Codecov Report

Merging #2461 (44a4f14) into master (14ad097) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 44a4f14 differs from pull request most recent head cf3d615. Consider uploading reports for the commit cf3d615 to get more accurate results

@@           Coverage Diff           @@
##           master    #2461   +/-   ##
=======================================
  Coverage   29.47%   29.48%           
=======================================
  Files         399      399           
  Lines       30432    30430    -2     
=======================================
  Hits         8971     8971           
+ Misses      20717    20715    -2     
  Partials      744      744
Files Changed Coverage Δ
pkg/services/object/acl/v2/service.go 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@roman-khimov
Copy link
Member

Related to #548/#561.

@roman-khimov
Copy link
Member

See also #881/#897.

@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Jul 26, 2023 •
edited
Loading

guess we just forgot to get original token in #897 as was done in ee37662. And the problem is not reproduced with dynamic sessions, static sessions haven't been used yet

@cthulhu-rider cthulhu-rider mentioned this pull request Jul 26, 2023
Closed
@cthulhu-rider
Copy link
Contributor Author

need to re-check for >1 replica container

@cthulhu-rider cthulhu-rider marked this pull request as draft July 26, 2023 12:57
@cthulhu-rider
Copy link
Contributor Author

need to re-check for >1 replica container

checked, works fine. #881 doesn't reproduce.

@cthulhu-rider cthulhu-rider marked this pull request as ready for review July 26, 2023 13:46
@cthulhu-rider
object/put: Process session token of the original request in ACL checks
cf3d615 
Previously, when checking write access to an object, the storage node
used only the session token from the last request sent. In case of a
chain of more than one request, the original session token was ignored.
As a result, it did not allow to correctly interpret on whose behalf
the action was performed. In particular, in private containers,
gateways were deprived of the right to write data on behalf of the
user.

From now storage node always processes the session token from the
original request.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@roman-khimov roman-khimov merged commit 5c54e27 into nspcc-dev:master Jul 28, 2023
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carpawell carpawell carpawell approved these changes

@roman-khimov roman-khimov roman-khimov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Broken relay of objects prepared on the client with session
3 participants
@cthulhu-rider @roman-khimov @carpawell