diff --git a/coderd/coderd.go b/coderd/coderd.go index ca0b9cd47f..6e88759fa3 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -292,6 +292,7 @@ func New(options *Options) *API { r.Use( apiKeyMiddleware, httpmw.ExtractWorkspaceAgentParam(options.Database), + httpmw.ExtractWorkspaceParam(options.Database), ) r.Get("/", api.workspaceAgent) r.Get("/dial", api.workspaceAgentDial) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index dbf3a5cede..02fba90966 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -153,10 +153,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/workspaceagents/me/listen": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true}, - "GET:/api/v2/workspaceagents/{workspaceagent}": {NoAuthorize: true}, - "GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true}, - "GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true}, // 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) { AssertAction: rbac.ActionRead, AssertObject: workspaceRBACObj, }, + "GET:/api/v2/workspaceagents/{workspaceagent}": { + AssertAction: rbac.ActionRead, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspaceagents/{workspaceagent}/dial": { + AssertAction: rbac.ActionUpdate, + AssertObject: workspaceRBACObj, + }, + "GET:/api/v2/workspaceagents/{workspaceagent}/pty": { + AssertAction: rbac.ActionUpdate, + AssertObject: workspaceRBACObj, + }, "GET:/api/v2/workspaces/": { StatusCode: http.StatusOK, AssertAction: rbac.ActionRead, @@ -378,6 +387,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { route = strings.ReplaceAll(route, "{workspacebuild}", workspace.LatestBuild.ID.String()) route = strings.ReplaceAll(route, "{workspacename}", workspace.Name) route = strings.ReplaceAll(route, "{workspacebuildname}", workspace.LatestBuild.Name) + route = strings.ReplaceAll(route, "{workspaceagent}", workspaceResources[0].Agents[0].ID.String()) route = strings.ReplaceAll(route, "{template}", template.ID.String()) route = strings.ReplaceAll(route, "{hash}", file.Hash) route = strings.ReplaceAll(route, "{workspaceresource}", workspaceResources[0].ID.String()) diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index e2f9af9440..acbcc9dee2 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -37,11 +37,11 @@ type userRolesKey struct{} // AuthorizationUserRoles returns the roles used for authorization. // Comes from the ExtractAPIKey handler. func AuthorizationUserRoles(r *http.Request) database.GetAuthorizationUserRolesRow { - apiKey, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow) + userRoles, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow) if !ok { panic("developer error: user roles middleware not provided") } - return apiKey + return userRoles } // OAuth2Configs is a collection of configurations for OAuth-based authentication. diff --git a/coderd/httpmw/workspaceagentparam.go b/coderd/httpmw/workspaceagentparam.go index d9facdf605..b958f4cd9f 100644 --- a/coderd/httpmw/workspaceagentparam.go +++ b/coderd/httpmw/workspaceagentparam.go @@ -6,6 +6,8 @@ import ( "errors" "net/http" + "github.com/go-chi/chi/v5" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" ) @@ -74,24 +76,9 @@ func ExtractWorkspaceAgentParam(db database.Store) func(http.Handler) http.Handl }) return } - workspace, err := db.GetWorkspaceByID(r.Context(), build.WorkspaceID) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: "Internal error fetching workspace.", - Detail: err.Error(), - }) - return - } - - apiKey := APIKey(r) - if apiKey.UserID != workspace.OwnerID { - httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: "Getting non-personal agents isn't supported.", - }) - return - } ctx := context.WithValue(r.Context(), workspaceAgentParamContextKey{}, agent) + chi.RouteContext(ctx).URLParams.Add("workspace", build.WorkspaceID.String()) next.ServeHTTP(rw, r.WithContext(ctx)) }) } diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 8ba6a355e5..a7de85c234 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/turnconn" "github.com/coder/coder/codersdk" "github.com/coder/coder/peer" @@ -31,6 +32,10 @@ import ( func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) { workspaceAgent := httpmw.WorkspaceAgentParam(r) + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionRead, workspace) { + return + } dbApps, err := api.Database.GetWorkspaceAppsByAgentID(r.Context(), workspaceAgent.ID) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -58,6 +63,10 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) { defer api.websocketWaitGroup.Done() workspaceAgent := httpmw.WorkspaceAgentParam(r) + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) { + return + } apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -369,6 +378,10 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { defer api.websocketWaitGroup.Done() workspaceAgent := httpmw.WorkspaceAgentParam(r) + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) { + return + } apiAgent, err := convertWorkspaceAgent(workspaceAgent, nil, api.AgentConnectionUpdateFrequency) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ diff --git a/site/src/components/Resources/Resources.tsx b/site/src/components/Resources/Resources.tsx index 9ee4581af6..4e36fc17e2 100644 --- a/site/src/components/Resources/Resources.tsx +++ b/site/src/components/Resources/Resources.tsx @@ -63,9 +63,10 @@ interface ResourcesProps { resources?: WorkspaceResource[] getResourcesError?: Error workspace: Workspace + canUpdateWorkspace: boolean } -export const Resources: FC = ({ resources, getResourcesError, workspace }) => { +export const Resources: FC = ({ resources, getResourcesError, workspace, canUpdateWorkspace }) => { const styles = useStyles() const theme: Theme = useTheme() @@ -89,7 +90,7 @@ export const Resources: FC = ({ resources, getResourcesError, wo - {Language.accessLabel} + {canUpdateWorkspace && {Language.accessLabel}} {Language.statusLabel} @@ -130,28 +131,30 @@ export const Resources: FC = ({ resources, getResourcesError, wo {agent.name} {agent.operating_system} - - - {agent.status === "connected" && ( - - )} - {agent.status === "connected" && - agent.apps.map((app) => ( - + + {agent.status === "connected" && ( + - ))} - - + )} + {agent.status === "connected" && + agent.apps.map((app) => ( + + ))} + + + )} {getDisplayAgentStatus(theme, agent).status} diff --git a/site/src/components/Workspace/Workspace.stories.tsx b/site/src/components/Workspace/Workspace.stories.tsx index 2b2cb317bd..0ed600e104 100644 --- a/site/src/components/Workspace/Workspace.stories.tsx +++ b/site/src/components/Workspace/Workspace.stories.tsx @@ -22,6 +22,13 @@ Started.args = { handleStop: action("stop"), resources: [Mocks.MockWorkspaceResource, Mocks.MockWorkspaceResource2], builds: [Mocks.MockWorkspaceBuild], + canUpdateWorkspace: true, +} + +export const WithoutUpdateAccess = Template.bind({}) +WithoutUpdateAccess.args = { + ...Started.args, + canUpdateWorkspace: false, } export const Starting = Template.bind({}) diff --git a/site/src/components/Workspace/Workspace.tsx b/site/src/components/Workspace/Workspace.tsx index e607017341..2e7338c5f7 100644 --- a/site/src/components/Workspace/Workspace.tsx +++ b/site/src/components/Workspace/Workspace.tsx @@ -28,6 +28,7 @@ export interface WorkspaceProps { resources?: TypesGen.WorkspaceResource[] getResourcesError?: Error builds?: TypesGen.WorkspaceBuild[] + canUpdateWorkspace: boolean } /** @@ -44,6 +45,7 @@ export const Workspace: FC = ({ resources, getResourcesError, builds, + canUpdateWorkspace, }) => { const styles = useStyles() const navigate = useNavigate() @@ -80,7 +82,12 @@ export const Workspace: FC = ({ {!!resources && !!resources.length && ( - + )} diff --git a/site/src/pages/WorkspacePage/WorkspacePage.tsx b/site/src/pages/WorkspacePage/WorkspacePage.tsx index 7584c8d193..cbc4929646 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.tsx @@ -1,5 +1,5 @@ -import { useMachine } from "@xstate/react" -import React, { useEffect } from "react" +import { useMachine, useSelector } from "@xstate/react" +import React, { useContext, useEffect } from "react" import { Helmet } from "react-helmet" import { useParams } from "react-router-dom" import { DeleteWorkspaceDialog } from "../../components/DeleteWorkspaceDialog/DeleteWorkspaceDialog" @@ -8,6 +8,8 @@ import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" import { Workspace } from "../../components/Workspace/Workspace" import { firstOrItem } from "../../util/array" import { pageTitle } from "../../util/page" +import { selectUser } from "../../xServices/auth/authSelectors" +import { XServiceContext } from "../../xServices/StateContext" import { workspaceMachine } from "../../xServices/workspace/workspaceXService" import { workspaceScheduleBannerMachine } from "../../xServices/workspaceSchedule/workspaceScheduleBannerXService" @@ -16,8 +18,17 @@ export const WorkspacePage: React.FC = () => { const username = firstOrItem(usernameQueryParam, null) const workspaceName = firstOrItem(workspaceQueryParam, null) - const [workspaceState, workspaceSend] = useMachine(workspaceMachine) - const { workspace, resources, getWorkspaceError, getResourcesError, builds } = workspaceState.context + const xServices = useContext(XServiceContext) + const me = useSelector(xServices.authXService, selectUser) + + const [workspaceState, workspaceSend] = useMachine(workspaceMachine, { + context: { + userId: me?.id, + }, + }) + const { workspace, resources, getWorkspaceError, getResourcesError, builds, permissions } = workspaceState.context + + const canUpdateWorkspace = !!permissions?.updateWorkspace const [bannerState, bannerSend] = useMachine(workspaceScheduleBannerMachine) @@ -56,6 +67,7 @@ export const WorkspacePage: React.FC = () => { resources={resources} getResourcesError={getResourcesError instanceof Error ? getResourcesError : undefined} builds={builds} + canUpdateWorkspace={canUpdateWorkspace} /> { export const selectPermissions = (state: AuthState): AuthContext["permissions"] => { return state.context.permissions } + +export const selectUser = (state: AuthState): AuthContext["me"] => { + return state.context.me +} diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index c24c5b968c..1a8d94395b 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -17,6 +17,8 @@ const Language = { buildError: "Workspace action failed.", } +type Permissions = Record, boolean> + export interface WorkspaceContext { workspace?: TypesGen.Workspace template?: TypesGen.Template @@ -34,6 +36,10 @@ export interface WorkspaceContext { getBuildsError?: Error | unknown loadMoreBuildsError?: Error | unknown cancellationMessage: string + // permissions + permissions?: Permissions + checkPermissionsError?: Error | unknown + userId?: string } export type WorkspaceEvent = @@ -48,6 +54,30 @@ export type WorkspaceEvent = | { type: "LOAD_MORE_BUILDS" } | { type: "REFRESH_TIMELINE" } +export const checks = { + readWorkspace: "readWorkspace", + updateWorkspace: "updateWorkspace", +} as const + +const permissionsToCheck = (workspace: TypesGen.Workspace) => ({ + [checks.readWorkspace]: { + object: { + resource_type: "workspace", + resource_id: workspace.id, + owner_id: workspace.owner_id, + }, + action: "read", + }, + [checks.updateWorkspace]: { + object: { + resource_type: "workspace", + resource_id: workspace.id, + owner_id: workspace.owner_id, + }, + action: "update", + }, +}) + export const workspaceMachine = createMachine( { tsTypes: {} as import("./workspaceXService.typegen").Typegen0, @@ -82,6 +112,9 @@ export const workspaceMachine = createMachine( loadMoreBuilds: { data: TypesGen.WorkspaceBuild[] } + checkPermissions: { + data: TypesGen.UserAuthorizationResponse + } }, }, id: "workspaceState", @@ -99,7 +132,7 @@ export const workspaceMachine = createMachine( src: "getWorkspace", id: "getWorkspace", onDone: { - target: "ready", + target: "gettingPermissions", actions: ["assignWorkspace"], }, onError: { @@ -109,6 +142,25 @@ export const workspaceMachine = createMachine( }, tags: "loading", }, + gettingPermissions: { + entry: "clearGetPermissionsError", + invoke: { + src: "checkPermissions", + id: "checkPermissions", + onDone: [ + { + actions: ["assignPermissions"], + target: "ready", + }, + ], + onError: [ + { + actions: "assignGetPermissionsError", + target: "error", + }, + ], + }, + }, ready: { type: "parallel", states: { @@ -312,6 +364,7 @@ export const workspaceMachine = createMachine( workspace: undefined, template: undefined, build: undefined, + permissions: undefined, }), assignWorkspace: assign({ workspace: (_, event) => event.data, @@ -323,6 +376,17 @@ export const workspaceMachine = createMachine( assignTemplate: assign({ template: (_, event) => event.data, }), + assignPermissions: assign({ + // Setting event.data as Permissions to be more stricted. So we know + // what permissions we asked for. + permissions: (_, event) => event.data as Permissions, + }), + assignGetPermissionsError: assign({ + checkPermissionsError: (_, event) => event.data, + }), + clearGetPermissionsError: assign({ + checkPermissionsError: (_) => undefined, + }), assignBuild: (_, event) => assign({ build: event.data, @@ -489,14 +553,23 @@ export const workspaceMachine = createMachine( if (context.workspace) { return await API.getWorkspaceBuilds(context.workspace.id) } else { - throw Error("Cannot refresh workspace without id") + throw Error("Cannot get builds without id") } }, loadMoreBuilds: async (context) => { if (context.workspace) { return await API.getWorkspaceBuilds(context.workspace.id) } else { - throw Error("Cannot refresh workspace without id") + throw Error("Cannot load more builds without id") + } + }, + checkPermissions: async (context) => { + if (context.workspace && context.userId) { + return await API.checkUserPermissions(context.userId, { + checks: permissionsToCheck(context.workspace), + }) + } else { + throw Error("Cannot check permissions without both workspace and user id") } }, },