fix: restrict edit schedule access (#2698)

This commit is contained in:
Abhineet Jain
2022-07-01 16:43:51 -04:00
committed by GitHub
parent ea5c2cd09b
commit c1b3080162
7 changed files with 164 additions and 29 deletions

View File

@ -64,6 +64,7 @@ export const Workspace: FC<WorkspaceProps> = ({
workspace={workspace}
onDeadlineMinus={scheduleProps.onDeadlineMinus}
onDeadlinePlus={scheduleProps.onDeadlinePlus}
canUpdateWorkspace={canUpdateWorkspace}
/>
<WorkspaceActions
workspace={workspace}

View File

@ -16,6 +16,11 @@ const THIRTY = 30
export default {
title: "components/WorkspaceSchedule",
component: WorkspaceSchedule,
argTypes: {
canUpdateWorkspace: {
defaultValue: true,
},
},
}
const Template: Story<WorkspaceScheduleProps> = (args) => <WorkspaceSchedule {...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,
}

View File

@ -33,9 +33,13 @@ export const Language = {
export interface WorkspaceScheduleProps {
workspace: Workspace
canUpdateWorkspace: boolean
}
export const WorkspaceSchedule: FC<WorkspaceScheduleProps> = ({ workspace }) => {
export const WorkspaceSchedule: FC<WorkspaceScheduleProps> = ({
workspace,
canUpdateWorkspace,
}) => {
const styles = useStyles()
const timezone = workspace.autostart_schedule
? extractTimezone(workspace.autostart_schedule)
@ -62,15 +66,17 @@ export const WorkspaceSchedule: FC<WorkspaceScheduleProps> = ({ workspace }) =>
</span>
</Stack>
</div>
<div>
<Link
className={styles.scheduleAction}
component={RouterLink}
to={`/@${workspace.owner_name}/${workspace.name}/schedule`}
>
{Language.editScheduleLink}
</Link>
</div>
{canUpdateWorkspace && (
<div>
<Link
className={styles.scheduleAction}
component={RouterLink}
to={`/@${workspace.owner_name}/${workspace.name}/schedule`}
>
{Language.editScheduleLink}
</Link>
</div>
)}
</Stack>
</div>
)

View File

@ -16,6 +16,11 @@ const THIRTY = 30
export default {
title: "components/WorkspaceScheduleButton",
component: WorkspaceScheduleButton,
argTypes: {
canUpdateWorkspace: {
defaultValue: true,
},
},
}
const Template: Story<WorkspaceScheduleButtonProps> = (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,
}

View File

@ -54,12 +54,14 @@ export interface WorkspaceScheduleButtonProps {
workspace: Workspace
onDeadlinePlus: () => void
onDeadlineMinus: () => void
canUpdateWorkspace: boolean
}
export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = ({
workspace,
onDeadlinePlus,
onDeadlineMinus,
canUpdateWorkspace,
}) => {
const anchorRef = useRef<HTMLButtonElement>(null)
const [isOpen, setIsOpen] = useState(false)
@ -74,7 +76,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
<div className={styles.wrapper}>
<div className={styles.label}>
<WorkspaceScheduleLabel workspace={workspace} />
{shouldDisplayPlusMinus(workspace) && (
{canUpdateWorkspace && shouldDisplayPlusMinus(workspace) && (
<Stack direction="row" spacing={0}>
<IconButton
className={styles.iconButton}
@ -124,7 +126,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
horizontal: "right",
}}
>
<WorkspaceSchedule workspace={workspace} />
<WorkspaceSchedule workspace={workspace} canUpdateWorkspace={canUpdateWorkspace} />
</Popover>
</div>
</div>

View File

@ -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 <FullScreenLoader />
} else if (scheduleState.matches("error")) {
}
if (scheduleState.matches("error")) {
return (
<ErrorSummary
error={getWorkspaceError}
error={getWorkspaceError || checkPermissionsError}
retry={() => scheduleSend({ type: "GET_WORKSPACE", username, workspaceName })}
/>
)
} else if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) {
}
if (!permissions?.updateWorkspace) {
return <ErrorSummary error={Error(Language.forbiddenError)} />
}
if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) {
return (
<WorkspaceScheduleForm
fieldErrors={formErrors}
@ -184,13 +210,15 @@ export const WorkspaceSchedulePage: React.FC = () => {
}}
/>
)
} else if (scheduleState.matches("submitSuccess")) {
}
if (scheduleState.matches("submitSuccess")) {
navigate(`/@${username}/${workspaceName}`)
return <FullScreenLoader />
} 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
}

View File

@ -14,6 +14,8 @@ export const Language = {
successMessage: "Successfully updated workspace schedule.",
}
type Permissions = Record<keyof ReturnType<typeof permissionsToCheck>, 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