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.

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

Support NeoFSID bound keys in access control checks #767

Open
alexvanin opened this issue Aug 25, 2021 · 4 comments
Open

Support NeoFSID bound keys in access control checks #767

alexvanin opened this issue Aug 25, 2021 · 4 comments
Labels
enhancement Improving existing functionality I2 Regular impact S2 Regular significance U4 Nothing urgent

Comments

@alexvanin
Copy link
Contributor

alexvanin commented Aug 25, 2021

We have plenty of code with such pattern:

if !isOwnerFromKey(tokenOwner, tokenIssuerKey) {
// todo: in this case we can issue all owner keys from neofs.id and check once again
return nil, nil, fmt.Errorf("%w: invalid session token owner", ErrMalformedRequest)
}

All these control checks should consider access to NeoFSID contract for further checks. In this issue first define all places where NeoFSID lookup should be implemented.

@cthulhu-rider
Copy link
Contributor

Maybe we also need to support external auth service like AuthCenter in S3.

@alexvanin
Copy link
Contributor Author

alexvanin commented Jun 1, 2022

Related to nspcc-dev/neofs-s3-gw#483

Consider owner with key X which produces Foo user ID and bound key Y which produces Bar user ID.
To create new container, user may produce session token and sign it with Y key. Then, at container create stage, user must specify container owner. Do we expect container owner Foo or Bar in this case?

  1. We can forbid using bound keys to create containers, so this won't be valid by definition.
  2. If container owner will be Foo, then Alphabet nodes at container creation stage must check that Y key is related to Foo.
  3. If container owner will be Bar, then ACL checks may be a bit more complex, but I am not 100% sure (ACL checks will be more complex with bound keys anyway).

@cthulhu-rider
Copy link
Contributor

2nd is already implemented here

if verificationKeys == nil {
.

@alexvanin
Copy link
Contributor Author

Great, then, I suppose, we go with option 2. Does it mean that container owner must always be Foo when session token signed by Y is attached. Or it can be either Foo or Bar, depending on the client's need?

@roman-khimov roman-khimov added enhancement Improving existing functionality U4 Nothing urgent S2 Regular significance I2 Regular impact labels Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality I2 Regular impact S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

4 participants