Skip to content

Commit 953e8c8

Browse files
AbhineetJainAbhineetJain
authored
feat: Allow admins to access member workspace terminals (#2114)
* allow workspace update permissions to access agents * do not show app links to users without workspace update access * address CR comments * initialize machine context in the hook * revert scoped connected status check
1 parent 0260e39 commit 953e8c8

File tree

11 files changed

+168
-51
lines changed

11 files changed

+168
-51
lines changed

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ func New(options *Options) *API {
292292
r.Use(
293293
apiKeyMiddleware,
294294
httpmw.ExtractWorkspaceAgentParam(options.Database),
295+
httpmw.ExtractWorkspaceParam(options.Database),
295296
)
296297
r.Get("/", api.workspaceAgent)
297298
r.Get("/dial", api.workspaceAgentDial)

‎coderd/coderd_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
153153
"GET:/api/v2/workspaceagents/me/listen": {NoAuthorize: true},
154154
"GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true},
155155
"GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true},
156-
"GET:/api/v2/workspaceagents/{workspaceagent}": {NoAuthorize: true},
157-
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true},
158156
"GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true},
159-
"GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true},
160157
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true},
161158

162159
// These endpoints have more assertions. This is good, add more endpoints to assert if you can!
@@ -210,6 +207,18 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
210207
AssertAction: rbac.ActionRead,
211208
AssertObject: workspaceRBACObj,
212209
},
210+
"GET:/api/v2/workspaceagents/{workspaceagent}": {
211+
AssertAction: rbac.ActionRead,
212+
AssertObject: workspaceRBACObj,
213+
},
214+
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {
215+
AssertAction: rbac.ActionUpdate,
216+
AssertObject: workspaceRBACObj,
217+
},
218+
"GET:/api/v2/workspaceagents/{workspaceagent}/pty": {
219+
AssertAction: rbac.ActionUpdate,
220+
AssertObject: workspaceRBACObj,
221+
},
213222
"GET:/api/v2/workspaces/": {
214223
StatusCode: http.StatusOK,
215224
AssertAction: rbac.ActionRead,
@@ -378,6 +387,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
378387
route = strings.ReplaceAll(route, "{workspacebuild}", workspace.LatestBuild.ID.String())
379388
route = strings.ReplaceAll(route, "{workspacename}", workspace.Name)
380389
route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name)
390+
route = strings.ReplaceAll(route, "{workspaceagent}", workspaceResources[0].Agents[0].ID.String())
381391
route = strings.ReplaceAll(route, "{template}", template.ID.String())
382392
route = strings.ReplaceAll(route, "{hash}", file.Hash)
383393
route = strings.ReplaceAll(route, "{workspaceresource}", workspaceResources[0].ID.String())

‎coderd/httpmw/apikey.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ type userRolesKey struct{}
3737
// AuthorizationUserRoles returns the roles used for authorization.
3838
// Comes from the ExtractAPIKey handler.
3939
func AuthorizationUserRoles(r *http.Request) database.GetAuthorizationUserRolesRow {
40-
apiKey, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow)
40+
userRoles, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow)
4141
if !ok {
4242
panic("developer error: user roles middleware not provided")
4343
}
44-
return apiKey
44+
return userRoles
4545
}
4646

4747
// OAuth2Configs is a collection of configurations for OAuth-based authentication.

‎coderd/httpmw/workspaceagentparam.go

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"errors"
77
"net/http"
88

9+
"github.com/go-chi/chi/v5"
10+
911
"github.com/coder/coder/coderd/database"
1012
"github.com/coder/coder/coderd/httpapi"
1113
)
@@ -74,24 +76,9 @@ func ExtractWorkspaceAgentParam(db database.Store) func(http.Handler) http.Handl
7476
})
7577
return
7678
}
77-
workspace, err := db.GetWorkspaceByID(r.Context(), build.WorkspaceID)
78-
if err != nil {
79-
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
80-
Message: "Internal error fetching workspace.",
81-
Detail: err.Error(),
82-
})
83-
return
84-
}
85-
86-
apiKey := APIKey(r)
87-
if apiKey.UserID != workspace.OwnerID {
88-
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
89-
Message: "Getting non-personal agents isn't supported.",
90-
})
91-
return
92-
}
9379

9480
ctx := context.WithValue(r.Context(), workspaceAgentParamContextKey{}, agent)
81+
chi.RouteContext(ctx).URLParams.Add("workspace", build.WorkspaceID.String())
9582
next.ServeHTTP(rw, r.WithContext(ctx))
9683
})
9784
}

‎coderd/workspaceagents.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/coder/coder/coderd/database"
2222
"github.com/coder/coder/coderd/httpapi"
2323
"github.com/coder/coder/coderd/httpmw"
24+
"github.com/coder/coder/coderd/rbac"
2425
"github.com/coder/coder/coderd/turnconn"
2526
"github.com/coder/coder/codersdk"
2627
"github.com/coder/coder/peer"
@@ -31,6 +32,10 @@ import (
3132

3233
func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) {
3334
workspaceAgent := httpmw.WorkspaceAgentParam(r)
35+
workspace := httpmw.WorkspaceParam(r)
36+
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
37+
return
38+
}
3439
dbApps, err := api.Database.GetWorkspaceAppsByAgentID(r.Context(), workspaceAgent.ID)
3540
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
3641
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
@@ -58,6 +63,10 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) {
5863
defer api.websocketWaitGroup.Done()
5964

6065
workspaceAgent := httpmw.WorkspaceAgentParam(r)
66+
workspace := httpmw.WorkspaceParam(r)
67+
if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) {
68+
return
69+
}
6170
apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency)
6271
if err != nil {
6372
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
@@ -369,6 +378,10 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
369378
defer api.websocketWaitGroup.Done()
370379

371380
workspaceAgent := httpmw.WorkspaceAgentParam(r)
381+
workspace := httpmw.WorkspaceParam(r)
382+
if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) {
383+
return
384+
}
372385
apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency)
373386
if err != nil {
374387
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

‎site/src/components/Resources/Resources.tsx

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ interface ResourcesProps {
6363
resources?: WorkspaceResource[]
6464
getResourcesError?: Error
6565
workspace: Workspace
66+
canUpdateWorkspace: boolean
6667
}
6768

68-
export const Resources: FC<ResourcesProps> = ({ resources, getResourcesError, workspace }) => {
69+
export const Resources: FC<ResourcesProps> = ({ resources, getResourcesError, workspace, canUpdateWorkspace }) => {
6970
const styles = useStyles()
7071
const theme: Theme = useTheme()
7172

@@ -89,7 +90,7 @@ export const Resources: FC<ResourcesProps> = ({ resources, getResourcesError, wo
8990
<AgentHelpTooltip />
9091
</Stack>
9192
</TableCell>
92-
<TableCell>{Language.accessLabel}</TableCell>
93+
{canUpdateWorkspace && <TableCell>{Language.accessLabel}</TableCell>}
9394
<TableCell>{Language.statusLabel}</TableCell>
9495
</TableHeaderRow>
9596
</TableHead>
@@ -130,28 +131,30 @@ export const Resources: FC<ResourcesProps> = ({ resources, getResourcesError, wo
130131
{agent.name}
131132
<span className={styles.operatingSystem}>{agent.operating_system}</span>
132133
</TableCell>
133-
<TableCell>
134-
<Stack>
135-
{agent.status === "connected" && (
136-
<TerminalLink
137-
className={styles.accessLink}
138-
workspaceName={workspace.name}
139-
agentName={agent.name}
140-
userName={workspace.owner_name}
141-
/>
142-
)}
143-
{agent.status === "connected" &&
144-
agent.apps.map((app) => (
145-
<AppLink
146-
key={app.name}
147-
appIcon={app.icon}
148-
appName={app.name}
149-
userName={workspace.owner_name}
134+
{canUpdateWorkspace && (
135+
<TableCell>
136+
<Stack>
137+
{agent.status === "connected" && (
138+
<TerminalLink
139+
className={styles.accessLink}
150140
workspaceName={workspace.name}
141+
agentName={agent.name}
142+
userName={workspace.owner_name}
151143
/>
152-
))}
153-
</Stack>
154-
</TableCell>
144+
)}
145+
{agent.status === "connected" &&
146+
agent.apps.map((app) => (
147+
<AppLink
148+
key={app.name}
149+
appIcon={app.icon}
150+
appName={app.name}
151+
userName={workspace.owner_name}
152+
workspaceName={workspace.name}
153+
/>
154+
))}
155+
</Stack>
156+
</TableCell>
157+
)}
155158
<TableCell>
156159
<span style={{ color: getDisplayAgentStatus(theme, agent).color }}>
157160
{getDisplayAgentStatus(theme, agent).status}

‎site/src/components/Workspace/Workspace.stories.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ Started.args = {
2222
handleStop: action("stop"),
2323
resources: [Mocks.MockWorkspaceResource, Mocks.MockWorkspaceResource2],
2424
builds: [Mocks.MockWorkspaceBuild],
25+
canUpdateWorkspace: true,
26+
}
27+
28+
export const WithoutUpdateAccess = Template.bind({})
29+
WithoutUpdateAccess.args = {
30+
...Started.args,
31+
canUpdateWorkspace: false,
2532
}
2633

2734
export const Starting = Template.bind({})

‎site/src/components/Workspace/Workspace.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export interface WorkspaceProps {
2828
resources?: TypesGen.WorkspaceResource[]
2929
getResourcesError?: Error
3030
builds?: TypesGen.WorkspaceBuild[]
31+
canUpdateWorkspace: boolean
3132
}
3233

3334
/**
@@ -44,6 +45,7 @@ export const Workspace: FC<WorkspaceProps> = ({
4445
resources,
4546
getResourcesError,
4647
builds,
48+
canUpdateWorkspace,
4749
}) => {
4850
const styles = useStyles()
4951
const navigate = useNavigate()
@@ -80,7 +82,12 @@ export const Workspace: FC<WorkspaceProps> = ({
8082
<WorkspaceStats workspace={workspace} />
8183

8284
{!!resources && !!resources.length && (
83-
<Resources resources={resources} getResourcesError={getResourcesError} workspace={workspace} />
85+
<Resources
86+
resources={resources}
87+
getResourcesError={getResourcesError}
88+
workspace={workspace}
89+
canUpdateWorkspace={canUpdateWorkspace}
90+
/>
8491
)}
8592

8693
<WorkspaceSection title="Timeline" contentsProps={{ className: styles.timelineContents }}>

‎site/src/pages/WorkspacePage/WorkspacePage.tsx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { useMachine } from "@xstate/react"
2-
import React, { useEffect } from "react"
1+
import { useMachine, useSelector } from "@xstate/react"
2+
import React, { useContext, useEffect } from "react"
33
import { Helmet } from "react-helmet"
44
import { useParams } from "react-router-dom"
55
import { DeleteWorkspaceDialog } from "../../components/DeleteWorkspaceDialog/DeleteWorkspaceDialog"
@@ -8,6 +8,8 @@ import { FullScreenLoader } from "../../components/Loader/FullScreenLoader"
88
import { Workspace } from "../../components/Workspace/Workspace"
99
import { firstOrItem } from "../../util/array"
1010
import { pageTitle } from "../../util/page"
11+
import { selectUser } from "../../xServices/auth/authSelectors"
12+
import { XServiceContext } from "../../xServices/StateContext"
1113
import { workspaceMachine } from "../../xServices/workspace/workspaceXService"
1214
import { workspaceScheduleBannerMachine } from "../../xServices/workspaceSchedule/workspaceScheduleBannerXService"
1315

@@ -16,8 +18,17 @@ export const WorkspacePage: React.FC = () => {
1618
const username = firstOrItem(usernameQueryParam, null)
1719
const workspaceName = firstOrItem(workspaceQueryParam, null)
1820

19-
const [workspaceState, workspaceSend] = useMachine(workspaceMachine)
20-
const { workspace, resources, getWorkspaceError, getResourcesError, builds } = workspaceState.context
21+
const xServices = useContext(XServiceContext)
22+
const me = useSelector(xServices.authXService, selectUser)
23+
24+
const [workspaceState, workspaceSend] = useMachine(workspaceMachine, {
25+
context: {
26+
userId: me?.id,
27+
},
28+
})
29+
const { workspace, resources, getWorkspaceError, getResourcesError, builds, permissions } = workspaceState.context
30+
31+
const canUpdateWorkspace = !!permissions?.updateWorkspace
2132

2233
const [bannerState, bannerSend] = useMachine(workspaceScheduleBannerMachine)
2334

@@ -56,6 +67,7 @@ export const WorkspacePage: React.FC = () => {
5667
resources={resources}
5768
getResourcesError={getResourcesError instanceof Error ? getResourcesError : undefined}
5869
builds={builds}
70+
canUpdateWorkspace={canUpdateWorkspace}
5971
/>
6072
<DeleteWorkspaceDialog
6173
isOpen={workspaceState.matches({ ready: { build: "askingDelete" } })}

‎site/src/xServices/auth/authSelectors.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@ export const selectOrgId = (state: AuthState): string | undefined => {
1010
export const selectPermissions = (state: AuthState): AuthContext["permissions"] => {
1111
return state.context.permissions
1212
}
13+
14+
export const selectUser = (state: AuthState): AuthContext["me"] => {
15+
return state.context.me
16+
}

0 commit comments

Comments
 (0)