-
Notifications
You must be signed in to change notification settings - Fork 15
session: add a new session token v2 #350
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
base: master
Are you sure you want to change the base?
Conversation
e65a42a to
11f58a6
Compare
session/types.proto
Outdated
| // Not valid before epoch, the first epoch when token is valid. | ||
| uint64 nbf = 2 [json_name = "nbf"]; | ||
|
|
||
| // Issued at Epoch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Issued at Epoch | |
| // Issued at epoch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, it was just copied, ok. but still dot at the end would look better according to other fields
session/types.proto
Outdated
| // Account represents an identity in NeoFS. | ||
| // It can be either direct (OwnerID) or indirect (NNS domain). | ||
| message Account { | ||
| // Account identifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most of the time, we add dots at the end of comments in this repo
session/types.proto
Outdated
|
|
||
| // Account represents an identity in NeoFS. | ||
| // It can be either direct (OwnerID) or indirect (NNS domain). | ||
| message Account { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we sure such a general entity should be described in the session package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would look better there for me, yes
11f58a6 to
694d30d
Compare
b967342 to
7a51333
Compare
session/types.proto
Outdated
| ObjectSessionContextV2 object = 6 [json_name = "object"]; | ||
|
|
||
| // ContainerService authorization context. | ||
| ContainerSessionContextV2 container = 7 [json_name = "container"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can have a single set of object/container operations? I'm not sure what this separation buys us.
7a51333 to
45af0fd
Compare
d863037 to
f7f489d
Compare
f7f489d to
a4dfb59
Compare
Session Token v2 solves the delegation, power of attorney, and chain-of-trust problems. It enables: - Account-based authority (direct or NNS-based indirect) - Multi-account subjects (multiple entities can use same token) - Multi-verb operations (GET, PUT, DELETE in single token) - Delegation chains (verifiable like X.509 certificates) - Indirect accounts (NeoFS Name Service resolution) Refs #241. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
roman-khimov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it looks good, pretty simple but powerful structure.
| // Lifetime parameters of the token. Field names taken from rfc7519. | ||
| message TokenLifetime { | ||
| // Expiration epoch, the last epoch when token is valid. | ||
| // For SessionTokenV2 this is the last valid Unix timestamp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It's very different in semantics.
- Compatibility warning fires for the removal of old
TokenLifetime, so we can keep it as is and add this new one for v2.
| // NeoFS Name Service. The name must be a valid DNS-like domain name | ||
| // (e.g., "example.neofs") that is registered in the NNS contract on | ||
| // the Neo blockchain. The NNS record should contain a string record with | ||
| // the corresponding OwnerID value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention NEP-18?
| // Account that issued this token (who signed it). | ||
| Target issuer = 3 [json_name = "issuer"]; | ||
|
|
||
| // Accounts authorized by this token (who can use it). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some limits for all repeated fields.
Refs #241.