chore(coderd/database/dbauthz): update RBAC for InsertWorkspaceApp #18223
Conversation
Instead of using `ResourceSystem` as the resource for `InsertWorkspaceApp`, we instead use the associated workspace (if it exists), with the action `ActionCreateAgent`. The decision to use this action is because everywhere where `InsertWorkspaceApp` is called is also where `InsertWorkspaceAgent` is called. I don't think adding an extra action makes sense, and adding an extra resource also doesn't make sense.
| workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.AgentID) | ||
| if err != nil && !errors.Is(err, sql.ErrNoRows) { | ||
| return database.WorkspaceApp{}, err | ||
| } |
There was a problem hiding this comment.
Doesn't this mean you can insert a workspace app for a non-existent agent ID?
There was a problem hiding this comment.
If you used a non-existent Agent ID, you would get past this check, correct. However, the agent_id field on the app is both NOT NULL and has a foreign key mapping to a workspace agent so the insertion would always fail.
If you think we should tweak the logic slightly to ensure there is a valid agent then I'm happy to make that change 👍.
https://github.com/coder/coder/blob/9995a098d5f0816128d00f5ec01767bba0b76164/coderd/database/dump.sql#L2035
https://github.com/coder/coder/blob/9995a098d5f0816128d00f5ec01767bba0b76164/coderd/database/dump.sql#L3060-L3061
There was a problem hiding this comment.
That's fair 👍 We could add an extra check for an agent with the given ID, but it does introduce an extra DB lookup. I'm not sure the extra round-trip gives us anything here.
Closes #18210
Instead of using
ResourceSystemas the resource forInsertWorkspaceApp, we instead use the associated workspace (if itexists), with the action
ActionUpdate.