There are cases for both. For now, I think it is best to keep this option enabled for admins. In Coder classic, many admins have built internal tooling to exec into workspaces and reproduce bugs. Having this as a first-class feature (when the agent works, of course) is a helpful utility.

Down the road, fine-grained permissions can be used to restrict this to 1 or 2 users or disable it entirely.

@AbhineetJain we may want to adjust the reconnecting PTY route to allow access in that case, then it all should just work.

@AbhineetJain we may want to adjust the reconnecting PTY route to allow access in that case, then it all should just work.

@kylecarbs sounds good, I'll update the PR to do that.

Do we want to implement this as admins connecting to a user's workspace, or have admins "impersonate" the user, then access?

We talked about masquerading/impersonation as a feature before. That avenue seems to accomplish the same thing in a more general way? Keeping the codepath as "the owner" accessing. Once we get audit logs, it will show who actually is connecting.

@kylecarbs: @Emyrk and I worked together to set up the RBAC check for allowing terminal (or workspace agent) access to users with workspace update permissions. I am planning to block the access links in the UI for users without the workspace update permission, if this looks like a good approach. Alternatively, we might need to create a new resource for workspace agents and its own set of permissions.

This seems highly related to #2135 issue.

I don't understand why we would restrict it to the owner instead of relying on RBAC. RBAC is a lot more configurable. Also, we want to minimize assumptions about access in the frontend to avoid future bugs due to misalignment between backend and frontend. (Edit: I see we're aligned here)

I'm in favor of keeping the behavior as it is in main. The superadmin/root has full access to everyone's workspace, and we create a lesser admin role that is RBAC-restricted from workspace exec.

Another reason to have a user that can literally do anything is limitless automation.

@ammario The PR evolved and the original description was outdated. I've now updated the description.

In main, we are showing Open terminal links to admin users for member workspaces, but clicking on the link opens the terminal in a failed/disconnected state due to their access being forbidden.

We're now allowing users with the workspace update access as per RBAC to see the link and use the terminal, and hide the link for users without access.

@ammario The PR evolved and the original description was outdated. I've now updated the description.

In main, we are showing Open terminal links to admin users for member workspaces, but clicking on the link opens the terminal in a failed/disconnected state due to their access being forbidden.

We're now allowing users with the workspace update access as per RBAC to see the link and use the terminal, and hide the link for users without access.

Ah ok. Makes a lot of sense.

I want to mention the RBAC perm of "update workspace" is being used for now. If we have reason too, we can break this out into a new resource. Keeping the number of RBAC unique resources as few as possible at the moment.

This should be almost ready. I am seeing this error in the console on the Workspace page though:

Machine given to `useMachine` has changed between renders. This is not supported and might lead to unexpected results.
Please make sure that you pass the same Machine as argument each time.

I am not sure what caused it, so can use some help there. @coder/frontend Any ideas?

This should be almost ready. I am seeing this error in the console on the Workspace page though:

Machine given to `useMachine` has changed between renders. This is not supported and might lead to unexpected results.
Please make sure that you pass the same Machine as argument each time.

I am not sure what caused it, so can use some help there. @coder/frontend Any ideas?

@AbhineetJain I think maybe it is complaining because we are modifying workspace as we pass it in to the useMachine hook.
When we use that hook, could we try:

  const [workspaceState, workspaceSend] = useMachine(workspaceMachine, {
    context: {
      userId: me?.id,
    },
  })

That seems to clear out the warning on my end while still passing in the userId.

This should be almost ready. I am seeing this error in the console on the Workspace page though:

Machine given to `useMachine` has changed between renders. This is not supported and might lead to unexpected results.
Please make sure that you pass the same Machine as argument each time.

I am not sure what caused it, so can use some help there. @coder/frontend Any ideas?

@AbhineetJain I think maybe it is complaining because we are modifying workspace as we pass it in to the useMachine hook. When we use that hook, could we try:

  const [workspaceState, workspaceSend] = useMachine(workspaceMachine, {
    context: {
      userId: me?.id,
    },
  })

That seems to clear out the warning on my end while still passing in the userId.

@Kira-Pilot It does indeed! Thank you so much. How'd you figure this out? 😄

@AbhineetJain I found this example in the XState docs. They should really update the context section you and I looked at earlier. Glad it worked out!