fix: don't allow sharing admins to change own role#21634
Conversation
2b491cc to
7193d1c
Compare
|
We now have an interesting loophole: ws owners can't share with themselves, but if they admin-share with someone, that someone will be able to share with the ws owner (and the ws owner won't be able to remove the share) :) |
| }) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder if we should add a check to close the loophole?
| if _, ok := req.UserRoles[workspace.OwnerID.String()]; ok { | |
| httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | |
| Message: "You cannot change the workspace owner's sharing role.", | |
| }) | |
| return | |
| } |
There was a problem hiding this comment.
Is this necessary since, changing the workspace owner's role would have no effect?
There was a problem hiding this comment.
Not strictly necessary. But without this, workspace owners can be added to the ACL by users they shared with:
- Alice shares with Bob using
adminrole - Bob shares "back" with Alice (just for funzies :))
- Alice is stuck with her name in the ACL list that she can't remove (due to this PR).
It won't affect the actual access permissions, but the workspace ACL (and the related UI) will be in a state that the ws owner can't change
|
If another admin shares the workspace with the workspace owner does it even matter? I'm assuming the ws owner will always be the owner and the share is irrelevant? |
Yes, the share will be irrelevant for all practical reasons. |
resolve coder/internal#1280