feat: let ordinary users watch tracked commands - #137
Conversation
Change tracked commands from admin-only to available to arbitrary logged in users. A tracked command now records a TrackedCommandOwner, and a second route streams user-owned commands to the user who started them. The two routes share the server-sent-event body, which moves to trackedCommandStream.ts. Components now have a `scope` prop which can be 'admin' or 'user'.
There was a problem hiding this comment.
I think the interface here makes sense, but I'm worried about the implementation — if user command keys don't incorporate the user's id, then the assumption that you can still view the stream for a given key if you could view that key's stream at some point in the past before breaks down.
My proposal would be to change the implementation so that each user has their own user-level key -> commandstate mapping, and have the admin commands exist in their own administrative key -> commandstate map. (Two users should be able to run a publish command simultaneously, but I think we'd rather not let two administrators install a toolchain simultaneously.)
(Aside: the robots that pester me about my own PRs are generally very vocal about untrusted users being able to create unbounded logs, and that's maybe also a concern here — user tracked command streams may need to get truncated or cleaned up somehow, that doesn't need to happen right away but seemed worth noting.)
|
This proposal sounds good to me. Out of band you were questioning whether the user/admin distinction should exist as command intent vs. it being autodetected. Do you still have feelings about that? My default sense is to prefer capturing "this is a command that I mean to run as admin, [and with your proposal therefore] in the admin running-task-name-space, do confirm that I have permission to do this" rather than silently, like, escalate commands to admin-ness just because I have admin rights at the moment. I think there may exist situations where the admin user wants to take actions that don't necessarily carry all admin capabilities with them. |
I agree with your default stance — you are correct I briefly disagreed with this stance, but then I talked myself out of that disagreement in favor of the "request to put stuff in admin namespace" interface. If there's a separate "admin namespace" of tracked commands, it makes sense to say "I'm requesting to put this task in the admin namespace". Users can generally only put stuff in their own namespace. That's all very in line with the design of project namespacing which is nice. |
Change tracked commands from admin-only to available to arbitrary logged in users.
A tracked command now records a TrackedCommandOwner, and a second route streams user-owned commands to the user who started them. The two routes share the server-sent-event body, which moves to trackedCommandStream.ts. Components now have a
scopeprop which can be 'admin' or 'user'.