-
Notifications
You must be signed in to change notification settings - Fork 881
feat: Add template-admin + user-admin role for managing templates + users #3490
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
Might want to make this an org role in the future. Also also shows assignable roles in the ui.
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.
@mafredri I made a |
Name: "MyWorkspaceInOrgExecution", | ||
// When creating the WithID won't be set, but it does not change the result. | ||
Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, | ||
Resource: rbac.ResourceWorkspaceExecution.InOrg(orgID).WithOwner(currentUser.String()), | ||
AuthorizeMap: map[bool][]authSubject{ | ||
true: {admin, orgAdmin, orgMemberMe}, | ||
false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, |
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.
Workspace execution can only be done by admin
, orgadmin
and the owner of the workspace.
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 new role was a nice addition!
@@ -70,7 +70,7 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) { | |||
|
|||
workspaceAgent := httpmw.WorkspaceAgentParam(r) | |||
workspace := httpmw.WorkspaceParam(r) | |||
if !api.Authorize(r, rbac.ActionUpdate, workspace) { | |||
if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { |
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 didn't quite catch why this changed from Update to Create. My intuition says it doesn't really matter. So it's probably fine but just checking.
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 doesn't. It's arbitrary.
But what I did change was I added ResourceWorkspaceExecution
as a unique resource. So giving CRUD to a workspace does not grant execution via a tunnel. These are now distinct permissions.
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
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.
nicely done
What this does
Adds a
template-admin
role that has full CRUD to templates and files.Adds a
user-admin
role that has full CRUD on usersThoughts? Trying to partially solve #2135 by removing the need to promote everyone to admin.
Bonus
UI only shows assignable roles. So if you cannot assign a role, it won't appear in the dropdown. (Except for self assigning roles)
TODO:
Next PR will migrate "Admin" -> "Template Admin, User Admin"
And migrate the first user from "Admin" -> "Owner"