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

acl, container: Add negative tests #582

Merged
merged 2 commits into from
Aug 25, 2023
Merged

acl, container: Add negative tests #582

merged 2 commits into from
Aug 25, 2023

Conversation

vvarg229
Copy link
Collaborator

Added two negative tests that verify that the container cannot be removed or set container EACL by anyone other than the owner (creator) or trusted trusted party proved by the container owner via session token.

@roman-khimov
Copy link
Member

These tests are OK (when fixed), but in #579 we wanted to check for session-based actions as well.

@vvarg229
Copy link
Collaborator Author

These tests are OK (when fixed), but in #579 we wanted to check for session-based actions as well.

OK, I'll add session-based tests

@vvarg229 vvarg229 marked this pull request as draft July 28, 2023 10:56
@vvarg229 vvarg229 force-pushed the non-owner branch 4 times, most recently from 7c9e126 to d0f4433 Compare July 31, 2023 17:29
@vvarg229 vvarg229 marked this pull request as ready for review July 31, 2023 17:55
@vvarg229
Copy link
Collaborator Author

OK, I'll add session-based tests

Done

roman-khimov
roman-khimov reviewed Jul 31, 2023
View reviewed changes
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

It's nice, but I've noticed that generate_session_token() actually fills in ownerID from owner_wallet which then signs the token in sign_session_token(). What also need is a correct ownerID in the token, but a token that is signed by a different wallet.

@vvarg229
Copy link
Collaborator Author

It's nice, but I've noticed that generate_session_token() actually fills in ownerID from owner_wallet which then signs the token in sign_session_token(). What also need is a correct ownerID in the token, but a token that is signed by a different wallet.

Thank you for the comment. I'll also add a related case.

@vvarg229 vvarg229 force-pushed the non-owner branch 2 times, most recently from 3d1b0cd to 0b9638f Compare August 1, 2023 17:51
@vvarg229 vvarg229 marked this pull request as draft August 1, 2023 17:56
@vvarg229 vvarg229 marked this pull request as ready for review August 1, 2023 19:20
@vvarg229
Copy link
Collaborator Author

vvarg229 commented Aug 1, 2023

These tests are OK (when fixed), but in #579 we wanted to check for session-based actions as well.

Done.

I have a question in the process of writing tests: nspcc-dev/neofs-node#2466

@vvarg229
Copy link
Collaborator Author

These tests are OK (when fixed), but in #579 we wanted to check for session-based actions as well.

Done.

I have a question in the process of writing tests: nspcc-dev/neofs-node#2466

Question resolved

@vvarg229 vvarg229 marked this pull request as draft August 24, 2023 09:20
@vvarg229 vvarg229 force-pushed the non-owner branch 9 times, most recently from e215021 to 3b903af Compare August 24, 2023 18:57
@vvarg229
Copy link
Collaborator Author

After the PR nspcc-dev/neofs-node#2501 the error messages have changed.
I changed it and in the test, here are the new results:
https://http.t5.fs.neo.org/86C4P6uJC7gb5n3KkwEGpXRfdczubXyRNW5N9KeJRW73/323-1692901993/index.html#

@vvarg229 vvarg229 marked this pull request as ready for review August 24, 2023 19:05
@vvarg229
acl, container: Add negative tests
7ecae02 
Added two negative tests that verify that the container cannot be removed or
set container EACL by anyone other than the owner (creator).

Closes nspcc-dev#579

Signed-off-by: Oleg Kulachenko <[email protected]>
@vvarg229
session_token: Add negative tests
0086ed9 
Added negative tests that verify that the container can be removed or set
container EACL only by trusted party proved by the container owner via
session token.

Closes nspcc-dev#579

Signed-off-by: Oleg Kulachenko <[email protected]>
@vvarg229 vvarg229 changed the title acl, container: Add negative tests #579 acl, container: Add negative tests Aug 25, 2023
@vvarg229 vvarg229 merged commit 0226721 into nspcc-dev:master Aug 25, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@evgeniiz321 evgeniiz321 evgeniiz321 approved these changes

@MaxGelbakhiani MaxGelbakhiani Awaiting requested review from MaxGelbakhiani

@cthulhu-rider cthulhu-rider Awaiting requested review from cthulhu-rider cthulhu-rider is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@vvarg229 @roman-khimov @evgeniiz321