From c1b30801624a118b57464b1000ddda39ca94e8f6 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 1 Jul 2022 16:43:51 -0400 Subject: [PATCH] fix: restrict edit schedule access (#2698) --- site/src/components/Workspace/Workspace.tsx | 1 + .../WorkspaceSchedule.stories.tsx | 21 +++++- .../WorkspaceSchedule/WorkspaceSchedule.tsx | 26 +++++--- .../WorkspaceScheduleButton.stories.tsx | 19 ++++++ .../WorkspaceScheduleButton.tsx | 6 +- .../WorkspaceSchedulePage.tsx | 56 ++++++++++++---- .../workspaceScheduleXService.ts | 64 ++++++++++++++++++- 7 files changed, 164 insertions(+), 29 deletions(-) diff --git a/site/src/components/Workspace/Workspace.tsx b/site/src/components/Workspace/Workspace.tsx index e1f706d3fc..8c0ab30548 100644 --- a/site/src/components/Workspace/Workspace.tsx +++ b/site/src/components/Workspace/Workspace.tsx @@ -64,6 +64,7 @@ export const Workspace: FC = ({ workspace={workspace} onDeadlineMinus={scheduleProps.onDeadlineMinus} onDeadlinePlus={scheduleProps.onDeadlinePlus} + canUpdateWorkspace={canUpdateWorkspace} /> = (args) => @@ -40,7 +45,7 @@ NoTTL.args = { ...Mocks.MockWorkspace, latest_build: { ...Mocks.MockWorkspaceBuild, - // a mannual shutdown has a deadline of '"0001-01-01T00:00:00Z"' + // a manual shutdown has a deadline of '"0001-01-01T00:00:00Z"' // SEE: #1834 deadline: "0001-01-01T00:00:00Z", }, @@ -113,3 +118,17 @@ WorkspaceOffLong.args = { ttl_ms: 2 * 365 * 24 * 60 * 60 * 1000, // 2 years }, } + +export const CannotEdit = Template.bind({}) +CannotEdit.args = { + workspace: { + ...Mocks.MockWorkspace, + + latest_build: { + ...Mocks.MockWorkspaceBuild, + transition: "stop", + }, + ttl_ms: 2 * 60 * 60 * 1000, // 2 hours + }, + canUpdateWorkspace: false, +} diff --git a/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx b/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx index b6730733fe..8878f1b33b 100644 --- a/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx +++ b/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx @@ -33,9 +33,13 @@ export const Language = { export interface WorkspaceScheduleProps { workspace: Workspace + canUpdateWorkspace: boolean } -export const WorkspaceSchedule: FC = ({ workspace }) => { +export const WorkspaceSchedule: FC = ({ + workspace, + canUpdateWorkspace, +}) => { const styles = useStyles() const timezone = workspace.autostart_schedule ? extractTimezone(workspace.autostart_schedule) @@ -62,15 +66,17 @@ export const WorkspaceSchedule: FC = ({ workspace }) => -
- - {Language.editScheduleLink} - -
+ {canUpdateWorkspace && ( +
+ + {Language.editScheduleLink} + +
+ )} ) diff --git a/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.stories.tsx b/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.stories.tsx index ec5a54fd17..32ff566878 100644 --- a/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.stories.tsx +++ b/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.stories.tsx @@ -16,6 +16,11 @@ const THIRTY = 30 export default { title: "components/WorkspaceScheduleButton", component: WorkspaceScheduleButton, + argTypes: { + canUpdateWorkspace: { + defaultValue: true, + }, + }, } const Template: Story = (args) => ( @@ -115,3 +120,17 @@ WorkspaceOffLong.args = { ttl_ms: 2 * 365 * 24 * 60 * 60 * 1000, // 2 years }, } + +export const CannotEdit = Template.bind({}) +CannotEdit.args = { + workspace: { + ...Mocks.MockWorkspace, + + latest_build: { + ...Mocks.MockWorkspaceBuild, + transition: "stop", + }, + ttl_ms: 2 * 60 * 60 * 1000, // 2 hours + }, + canUpdateWorkspace: false, +} diff --git a/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx b/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx index 37dff6205e..d03c0ad357 100644 --- a/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx +++ b/site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx @@ -54,12 +54,14 @@ export interface WorkspaceScheduleButtonProps { workspace: Workspace onDeadlinePlus: () => void onDeadlineMinus: () => void + canUpdateWorkspace: boolean } export const WorkspaceScheduleButton: React.FC = ({ workspace, onDeadlinePlus, onDeadlineMinus, + canUpdateWorkspace, }) => { const anchorRef = useRef(null) const [isOpen, setIsOpen] = useState(false) @@ -74,7 +76,7 @@ export const WorkspaceScheduleButton: React.FC = (
- {shouldDisplayPlusMinus(workspace) && ( + {canUpdateWorkspace && shouldDisplayPlusMinus(workspace) && ( = ( horizontal: "right", }} > - +
diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index 5aa9da8527..fca1607135 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -1,9 +1,9 @@ -import { useMachine } from "@xstate/react" +import { useMachine, useSelector } from "@xstate/react" import * as cronParser from "cron-parser" import dayjs from "dayjs" import timezone from "dayjs/plugin/timezone" import utc from "dayjs/plugin/utc" -import React, { useEffect } from "react" +import React, { useContext, useEffect } from "react" import { useNavigate, useParams } from "react-router-dom" import * as TypesGen from "../../api/typesGenerated" import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" @@ -16,6 +16,8 @@ import { } from "../../components/WorkspaceScheduleForm/WorkspaceScheduleForm" import { firstOrItem } from "../../util/array" import { extractTimezone, stripTimezone } from "../../util/schedule" +import { selectUser } from "../../xServices/auth/authSelectors" +import { XServiceContext } from "../../xServices/StateContext" import { workspaceSchedule } from "../../xServices/workspaceSchedule/workspaceScheduleXService" // REMARK: timezone plugin depends on UTC @@ -24,6 +26,10 @@ import { workspaceSchedule } from "../../xServices/workspaceSchedule/workspaceSc dayjs.extend(utc) dayjs.extend(timezone) +const Language = { + forbiddenError: "You don't have permissions to update the schedule for this workspace.", +} + export const formValuesToAutoStartRequest = ( values: WorkspaceScheduleFormValues, ): TypesGen.UpdateWorkspaceAutostartRequest => { @@ -141,8 +147,17 @@ export const WorkspaceSchedulePage: React.FC = () => { const navigate = useNavigate() const username = firstOrItem(usernameQueryParam, null) const workspaceName = firstOrItem(workspaceQueryParam, null) - const [scheduleState, scheduleSend] = useMachine(workspaceSchedule) - const { formErrors, getWorkspaceError, workspace } = scheduleState.context + + const xServices = useContext(XServiceContext) + const me = useSelector(xServices.authXService, selectUser) + + const [scheduleState, scheduleSend] = useMachine(workspaceSchedule, { + context: { + userId: me?.id, + }, + }) + const { checkPermissionsError, formErrors, getWorkspaceError, permissions, workspace } = + scheduleState.context // Get workspace on mount and whenever the args for getting a workspace change. // scheduleSend should not change. @@ -153,20 +168,31 @@ export const WorkspaceSchedulePage: React.FC = () => { if (!username || !workspaceName) { navigate("/workspaces") return null - } else if ( + } + + if ( scheduleState.matches("idle") || scheduleState.matches("gettingWorkspace") || + scheduleState.matches("gettingPermissions") || !workspace ) { return - } else if (scheduleState.matches("error")) { + } + + if (scheduleState.matches("error")) { return ( scheduleSend({ type: "GET_WORKSPACE", username, workspaceName })} /> ) - } else if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) { + } + + if (!permissions?.updateWorkspace) { + return + } + + if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) { return ( { }} /> ) - } else if (scheduleState.matches("submitSuccess")) { + } + + if (scheduleState.matches("submitSuccess")) { navigate(`/@${username}/${workspaceName}`) return - } else { - // Theoretically impossible - log and bail - console.error("WorkspaceSchedulePage: unknown state :: ", scheduleState) - navigate("/") - return null } + + // Theoretically impossible - log and bail + console.error("WorkspaceSchedulePage: unknown state :: ", scheduleState) + navigate("/") + return null } diff --git a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts index 64f6ca3af3..fec8356f80 100644 --- a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts +++ b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts @@ -14,6 +14,8 @@ export const Language = { successMessage: "Successfully updated workspace schedule.", } +type Permissions = Record, boolean> + export interface WorkspaceScheduleContext { formErrors?: FieldErrors getWorkspaceError?: Error | unknown @@ -23,8 +25,27 @@ export interface WorkspaceScheduleContext { * machine is partially influenced by workspaceXService. */ workspace?: TypesGen.Workspace + // permissions + userId?: string + permissions?: Permissions + checkPermissionsError?: Error | unknown } +export const checks = { + updateWorkspace: "updateWorkspace", +} as const + +const permissionsToCheck = (workspace: TypesGen.Workspace) => ({ + [checks.updateWorkspace]: { + object: { + resource_type: "workspace", + resource_id: workspace.id, + owner_id: workspace.owner_id, + }, + action: "update", + }, +}) + export type WorkspaceScheduleEvent = | { type: "GET_WORKSPACE"; username: string; workspaceName: string } | { @@ -60,7 +81,7 @@ export const workspaceSchedule = createMachine( src: "getWorkspace", id: "getWorkspace", onDone: { - target: "presentForm", + target: "gettingPermissions", actions: ["assignWorkspace"], }, onError: { @@ -70,6 +91,25 @@ export const workspaceSchedule = createMachine( }, tags: "loading", }, + gettingPermissions: { + entry: "clearGetPermissionsError", + invoke: { + src: "checkPermissions", + id: "checkPermissions", + onDone: [ + { + actions: ["assignPermissions"], + target: "presentForm", + }, + ], + onError: [ + { + actions: "assignGetPermissionsError", + target: "error", + }, + ], + }, + }, presentForm: { on: { SUBMIT_SCHEDULE: "submittingSchedule", @@ -113,8 +153,19 @@ export const workspaceSchedule = createMachine( assignGetWorkspaceError: assign({ getWorkspaceError: (_, 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, + }), clearContext: () => { - assign({ workspace: undefined }) + assign({ workspace: undefined, permissions: undefined }) }, clearGetWorkspaceError: (context) => { assign({ ...context, getWorkspaceError: undefined }) @@ -134,6 +185,15 @@ export const workspaceSchedule = createMachine( getWorkspace: async (_, event) => { return await API.getWorkspaceByOwnerAndName(event.username, event.workspaceName) }, + 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") + } + }, submitSchedule: async (context, event) => { if (!context.workspace?.id) { // This state is theoretically impossible, but helps TS