-
Notifications
You must be signed in to change notification settings - Fork 956
feat: implement acl for workspaces #19094
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
Conversation
I'm concerned about the security implications of exposing this value too prominently, and we have a lot queries that currently select Edit: I guess it doesn't really matter if the db types have this column. We'd have to expose them through the codersdk types manually still. |
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.
Wait, not done reviewing! Sorry, finishing up. Misclicked
// "left" will be a map of group names to actions in rego. | ||
// { | ||
// "all_users": ["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 is not 100% correct anymore. Maybe just note the SubField
is ""
for this example.
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.
I think this is still correct tho. by the time it gets to rego it will just be a list of actions because the subfield gets handled in sql right? the rego policy doesn't even know about the indirection
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.
The rego just gets a slice of actions iirc. I will need to double check, we can keep this for now
ACLMappingVar
to support permissions lists stored in a field rather than directly in the mappingworkspaces
table