-
Notifications
You must be signed in to change notification settings - Fork 153
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
Authorization Setup for Write Access on RPC Calls #620
Conversation
utils/auth/src/lib.rs
Outdated
lazy_static! { | ||
/// Constants of all Levels of permissions | ||
pub static ref ADMIN: Vec<String> = vec![ | ||
"read".to_string(), | ||
"write".to_string(), | ||
"sign".to_string(), | ||
"admin".to_string() | ||
]; | ||
pub static ref SIGN: Vec<String> = | ||
vec!["read".to_string(), "write".to_string(), "sign".to_string()]; | ||
pub static ref WRITE: Vec<String> = vec!["read".to_string(), "write".to_string()]; | ||
pub static ref READ: Vec<String> = vec!["read".to_string()]; | ||
} |
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.
Same here, none of this needs to be allocated, and why are they kept seperate
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.
yeah, they don't need to be separate, was just matching lotus
// TODO temp use of bls key as placeholder, need to update keyinfo to use string instead of keyinfo | ||
// for key type |
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.
Why would string be needed? I don't see how that helps this
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.
In lotus the key_info is not necesarily either BLS or SECP256K1, so switching to a string key_type would allow storing more types of keys (like the JWT secret key) rather than just BLS or SECP256K1.
I definitely want to think about this more in depth soon because I don't see how this helps us or even has support for multiple tokens (did I miss that part in this PR?) Also, can you please try running our node and testing functionally, I am getting this error when running and stopping the node:
|
This PR basically just lays the ground work for adding privileges on certain RPC calls, just like how lotus has set up. as for multiple tokens, the format is the same as with lotus, each token is identical for each permission level, as long as the private JWT secret key is deleted. |
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.
utack
pub const ADMIN: [&str; 4] = ["read", "write", "sign", "admin"]; | ||
/// Signing permissions | ||
pub const SIGN: [&str; 3] = ["read", "write", "sign"]; | ||
/// Writing permissions | ||
pub const WRITE: [&str; 2] = ["read", "write"]; | ||
/// Reading permissions | ||
pub const READ: [&str; 1] = ["read"]; |
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.
This could be a lot better (enums with a value of priority or something) but can always change later I guess
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.
Yeah I think this was from Jaden's existing implementation. I think can be just changed later for now
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #592