From 776f2876852323b9fbc26d196a2f5919cfa4a8c5 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Thu, 29 Sep 2022 14:32:38 -0400 Subject: [PATCH] feat: allow admins to create workspaces for other users in UI (#4247) * added permission for creating a workspace on behalf of a user * committing stashed files * hooked up autocomplete for users * added label * added translations * wrote test * added inputMargin prop * fixed permissions * added inputSTyle prop * ran prettier * fix lint --- .../UserAutocomplete/UserAutocomplete.tsx | 19 ++++- site/src/i18n/en/createWorkspacePage.json | 5 ++ site/src/i18n/en/index.ts | 2 + .../CreateWorkspacePage.test.tsx | 32 ++++++-- .../CreateWorkspacePage.tsx | 23 ++++-- .../CreateWorkspacePageView.tsx | 80 +++++++------------ site/src/testHelpers/entities.ts | 7 ++ .../createWorkspaceXService.ts | 74 +++++++++++++++-- .../src/xServices/users/searchUserXService.ts | 1 + 9 files changed, 169 insertions(+), 74 deletions(-) create mode 100644 site/src/i18n/en/createWorkspacePage.json diff --git a/site/src/components/UserAutocomplete/UserAutocomplete.tsx b/site/src/components/UserAutocomplete/UserAutocomplete.tsx index 2cd30da85c..dbfd473b40 100644 --- a/site/src/components/UserAutocomplete/UserAutocomplete.tsx +++ b/site/src/components/UserAutocomplete/UserAutocomplete.tsx @@ -10,11 +10,20 @@ import { ChangeEvent, useEffect, useState } from "react" import { searchUserMachine } from "xServices/users/searchUserXService" export type UserAutocompleteProps = { - value?: User + value: User | null onChange: (user: User | null) => void + label?: string + inputMargin?: "none" | "dense" | "normal" + inputStyles?: string } -export const UserAutocomplete: React.FC = ({ value, onChange }) => { +export const UserAutocomplete: React.FC = ({ + value, + onChange, + label, + inputMargin, + inputStyles, +}) => { const styles = useStyles() const [isAutocompleteOpen, setIsAutocompleteOpen] = useState(false) const [searchState, sendSearch] = useMachine(searchUserMachine) @@ -77,9 +86,11 @@ export const UserAutocomplete: React.FC = ({ value, onCha renderInput={(params) => ( { }, "& input": { - fontSize: 14, + fontSize: 16, padding: `${theme.spacing(0, 0.5, 0, 0.5)} !important`, }, }, diff --git a/site/src/i18n/en/createWorkspacePage.json b/site/src/i18n/en/createWorkspacePage.json new file mode 100644 index 0000000000..651dadcc72 --- /dev/null +++ b/site/src/i18n/en/createWorkspacePage.json @@ -0,0 +1,5 @@ +{ + "templateLabel": "Template", + "nameLabel": "Name", + "ownerLabel": "Workspace Owner" +} diff --git a/site/src/i18n/en/index.ts b/site/src/i18n/en/index.ts index 32fe288646..7fd2c23363 100644 --- a/site/src/i18n/en/index.ts +++ b/site/src/i18n/en/index.ts @@ -1,5 +1,6 @@ import auditLog from "./auditLog.json" import common from "./common.json" +import createWorkspacePage from "./createWorkspacePage.json" import templatePage from "./templatePage.json" import templatesPage from "./templatesPage.json" import workspacePage from "./workspacePage.json" @@ -10,4 +11,5 @@ export const en = { auditLog, templatePage, templatesPage, + createWorkspacePage, } diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 216b37aaa3..c61ea82b64 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -1,12 +1,16 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ -import { screen } from "@testing-library/react" +import { fireEvent, screen, waitFor } from "@testing-library/react" import userEvent from "@testing-library/user-event" import * as API from "api/api" -import { Language as FooterLanguage } from "../../components/FormFooter/FormFooter" -import { MockTemplate, MockWorkspace } from "../../testHelpers/entities" -import { renderWithAuth } from "../../testHelpers/renderHelpers" +import { Language as FooterLanguage } from "components/FormFooter/FormFooter" +import i18next from "i18next" +import { MockTemplate, MockUser, MockWorkspace, MockWorkspaceRequest } from "testHelpers/entities" +import { renderWithAuth } from "testHelpers/renderHelpers" import CreateWorkspacePage from "./CreateWorkspacePage" -import { Language } from "./CreateWorkspacePageView" + +const { t } = i18next + +const nameLabelText = t("nameLabel", { ns: "createWorkspacePage" }) const renderCreateWorkspacePage = () => { return renderWithAuth(, { @@ -22,14 +26,26 @@ describe("CreateWorkspacePage", () => { expect(element).toBeDefined() }) - it("succeeds", async () => { + it("succeeds with default owner", async () => { + jest.spyOn(API, "getUsers").mockResolvedValueOnce([MockUser]) jest.spyOn(API, "createWorkspace").mockResolvedValueOnce(MockWorkspace) renderCreateWorkspacePage() - const nameField = await screen.findByLabelText(Language.nameLabel) - userEvent.type(nameField, "test") + const nameField = await screen.findByLabelText(nameLabelText) + + // have to use fireEvent b/c userEvent isn't cleaning up properly between tests + fireEvent.change(nameField, { + target: { value: "test" }, + }) + const submitButton = screen.getByText(FooterLanguage.defaultSubmitLabel) userEvent.click(submitButton) + + await waitFor(() => + expect(API.createWorkspace).toBeCalledWith(MockUser.organization_ids[0], MockUser.id, { + ...MockWorkspaceRequest, + }), + ) }) }) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 9e2ef6bba4..94ff2fabbc 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -1,10 +1,12 @@ -import { useMachine } from "@xstate/react" -import { FC } from "react" +import { useActor, useMachine } from "@xstate/react" +import { User } from "api/typesGenerated" +import { useOrganizationId } from "hooks/useOrganizationId" +import { FC, useContext, useState } from "react" import { Helmet } from "react-helmet-async" import { useNavigate, useParams } from "react-router-dom" -import { useOrganizationId } from "../../hooks/useOrganizationId" -import { pageTitle } from "../../util/page" -import { createWorkspaceMachine } from "../../xServices/createWorkspace/createWorkspaceXService" +import { pageTitle } from "util/page" +import { createWorkspaceMachine } from "xServices/createWorkspace/createWorkspaceXService" +import { XServiceContext } from "xServices/StateContext" import { CreateWorkspaceErrors, CreateWorkspacePageView } from "./CreateWorkspacePageView" const CreateWorkspacePage: FC = () => { @@ -28,8 +30,15 @@ const CreateWorkspacePage: FC = () => { getTemplateSchemaError, getTemplatesError, createWorkspaceError, + permissions, } = createWorkspaceState.context + const xServices = useContext(XServiceContext) + const [authState] = useActor(xServices.authXService) + const { me } = authState.context + + const [owner, setOwner] = useState(me ?? null) + return ( <> @@ -49,6 +58,9 @@ const CreateWorkspacePage: FC = () => { [CreateWorkspaceErrors.GET_TEMPLATE_SCHEMA_ERROR]: getTemplateSchemaError, [CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]: createWorkspaceError, }} + canCreateForUser={permissions?.createWorkspaceForUser} + defaultWorkspaceOwner={me ?? null} + setOwner={setOwner} onCancel={() => { navigate("/templates") }} @@ -56,6 +68,7 @@ const CreateWorkspacePage: FC = () => { send({ type: "CREATE_WORKSPACE", request, + owner, }) }} /> diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 03ef6e9a38..1372f3517f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -1,21 +1,18 @@ -import { makeStyles } from "@material-ui/core/styles" import TextField from "@material-ui/core/TextField" +import * as TypesGen from "api/typesGenerated" import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" +import { FormFooter } from "components/FormFooter/FormFooter" +import { FullPageForm } from "components/FullPageForm/FullPageForm" +import { Loader } from "components/Loader/Loader" +import { ParameterInput } from "components/ParameterInput/ParameterInput" +import { Stack } from "components/Stack/Stack" +import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete" import { FormikContextType, FormikTouched, useFormik } from "formik" +import { i18n } from "i18n" import { FC, useState } from "react" +import { useTranslation } from "react-i18next" +import { getFormHelpers, nameValidator, onChangeTrimmed } from "util/formUtils" import * as Yup from "yup" -import * as TypesGen from "../../api/typesGenerated" -import { FormFooter } from "../../components/FormFooter/FormFooter" -import { FullPageForm } from "../../components/FullPageForm/FullPageForm" -import { Loader } from "../../components/Loader/Loader" -import { ParameterInput } from "../../components/ParameterInput/ParameterInput" -import { Stack } from "../../components/Stack/Stack" -import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils" - -export const Language = { - templateLabel: "Template", - nameLabel: "Name", -} export enum CreateWorkspaceErrors { GET_TEMPLATES_ERROR = "getTemplatesError", @@ -33,21 +30,27 @@ export interface CreateWorkspacePageViewProps { selectedTemplate?: TypesGen.Template templateSchema?: TypesGen.ParameterSchema[] createWorkspaceErrors: Partial> + canCreateForUser?: boolean + defaultWorkspaceOwner: TypesGen.User | null + setOwner: (arg0: TypesGen.User | null) => void onCancel: () => void onSubmit: (req: TypesGen.CreateWorkspaceRequest) => void // initialTouched is only used for testing the error state of the form. initialTouched?: FormikTouched } +const { t } = i18n + export const validationSchema = Yup.object({ - name: nameValidator(Language.nameLabel), + name: nameValidator(t("nameLabel", { ns: "createWorkspacePage" })), }) export const CreateWorkspacePageView: FC> = ( props, ) => { + const { t } = useTranslation("createWorkspacePage") + const [parameterValues, setParameterValues] = useState>({}) - useStyles() const form: FormikContextType = useFormik({ @@ -114,17 +117,15 @@ export const CreateWorkspacePageView: FC
- {props.createWorkspaceErrors[CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR] ? ( + {Boolean(props.createWorkspaceErrors[CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]) && ( - ) : ( - <> )} @@ -138,10 +139,19 @@ export const CreateWorkspacePageView: FC + {props.canCreateForUser && ( + props.setOwner(user)} + label={t("ownerLabel")} + inputMargin="dense" + /> + )} + {props.templateSchema.length > 0 && ( {props.templateSchema.map((schema) => ( @@ -168,33 +178,3 @@ export const CreateWorkspacePageView: FC ) } - -const useStyles = makeStyles((theme) => ({ - readMoreLink: { - display: "flex", - alignItems: "center", - - "& svg": { - width: 12, - height: 12, - marginLeft: theme.spacing(0.5), - }, - }, - emptyState: { - padding: 0, - fontFamily: "inherit", - textAlign: "left", - minHeight: "auto", - alignItems: "flex-start", - }, - emptyStateDescription: { - lineHeight: "160%", - }, - code: { - background: theme.palette.background.paper, - width: "100%", - }, - codeButton: { - background: theme.palette.background.paper, - }, -})) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 177e79842e..fa889484de 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -324,6 +324,13 @@ export const MockQueuedWorkspace: TypesGen.Workspace = { }, } +// requests the MockWorkspace +export const MockWorkspaceRequest: TypesGen.CreateWorkspaceRequest = { + name: "test", + parameter_values: [], + template_id: "test-template", +} + export const MockWorkspaceApp: TypesGen.WorkspaceApp = { id: "test-app", name: "test-app", diff --git a/site/src/xServices/createWorkspace/createWorkspaceXService.ts b/site/src/xServices/createWorkspace/createWorkspaceXService.ts index 2b81709ceb..d7f5a58a32 100644 --- a/site/src/xServices/createWorkspace/createWorkspaceXService.ts +++ b/site/src/xServices/createWorkspace/createWorkspaceXService.ts @@ -1,14 +1,21 @@ -import { assign, createMachine } from "xstate" -import { createWorkspace, getTemplates, getTemplateVersionSchema } from "../../api/api" +import { + checkAuthorization, + createWorkspace, + getTemplates, + getTemplateVersionSchema, +} from "api/api" import { CreateWorkspaceRequest, ParameterSchema, Template, + User, Workspace, -} from "../../api/typesGenerated" +} from "api/typesGenerated" +import { assign, createMachine } from "xstate" type CreateWorkspaceContext = { organizationId: string + owner: User | null templateName: string templates?: Template[] selectedTemplate?: Template @@ -18,11 +25,14 @@ type CreateWorkspaceContext = { createWorkspaceError?: Error | unknown getTemplatesError?: Error | unknown getTemplateSchemaError?: Error | unknown + permissions?: Record + checkPermissionsError?: Error | unknown } type CreateWorkspaceEvent = { type: "CREATE_WORKSPACE" request: CreateWorkspaceRequest + owner: User | null } export const createWorkspaceMachine = createMachine( @@ -73,7 +83,7 @@ export const createWorkspaceMachine = createMachine( src: "getTemplateSchema", onDone: { actions: ["assignTemplateSchema"], - target: "fillingParams", + target: "checkingPermissions", }, onError: { actions: ["assignGetTemplateSchemaError"], @@ -81,10 +91,25 @@ export const createWorkspaceMachine = createMachine( }, }, }, + checkingPermissions: { + entry: "clearCheckPermissionsError", + invoke: { + src: "checkPermissions", + id: "checkPermissions", + onDone: { + actions: ["assignPermissions"], + target: "fillingParams", + }, + onError: { + actions: ["assignCheckPermissionsError"], + target: "error", + }, + }, + }, fillingParams: { on: { CREATE_WORKSPACE: { - actions: ["assignCreateWorkspaceRequest"], + actions: ["assignCreateWorkspaceRequest", "assignOwner"], target: "creatingWorkspace", }, }, @@ -121,14 +146,37 @@ export const createWorkspaceMachine = createMachine( return getTemplateVersionSchema(selectedTemplate.active_version_id) }, + checkPermissions: async (context) => { + if (!context.organizationId) { + throw new Error("No organization ID") + } + + // HACK: below, we pass in * for the owner_id, which is a hacky way of checking if the + // current user can create a workspace on behalf of anyone within the org (only org owners should be able to do this). + // This pattern should not be replicated outside of this narrow use case. + const permissionsToCheck = { + createWorkspaceForUser: { + object: { + resource_type: "workspace", + organization_id: `${context.organizationId}`, + owner_id: "*", + }, + action: "create", + }, + } + + return checkAuthorization({ + checks: permissionsToCheck, + }) + }, createWorkspace: (context) => { - const { createWorkspaceRequest, organizationId } = context + const { createWorkspaceRequest, organizationId, owner } = context if (!createWorkspaceRequest) { throw new Error("No create workspace request") } - return createWorkspace(organizationId, "me", createWorkspaceRequest) + return createWorkspace(organizationId, owner?.id ?? "me", createWorkspaceRequest) }, }, guards: { @@ -149,9 +197,21 @@ export const createWorkspaceMachine = createMachine( // CLI code: https://github.com/coder/coder/blob/main/cli/create.go#L152-L155 templateSchema: (_, event) => event.data.filter((param) => param.allow_override_source), }), + assignPermissions: assign({ + permissions: (_, event) => event.data as Record, + }), + assignCheckPermissionsError: assign({ + checkPermissionsError: (_, event) => event.data, + }), + clearCheckPermissionsError: assign({ + checkPermissionsError: (_) => undefined, + }), assignCreateWorkspaceRequest: assign({ createWorkspaceRequest: (_, event) => event.request, }), + assignOwner: assign({ + owner: (_, event) => event.owner, + }), assignCreateWorkspaceError: assign({ createWorkspaceError: (_, event) => event.data, }), diff --git a/site/src/xServices/users/searchUserXService.ts b/site/src/xServices/users/searchUserXService.ts index 28fcb043d8..cee8757a98 100644 --- a/site/src/xServices/users/searchUserXService.ts +++ b/site/src/xServices/users/searchUserXService.ts @@ -8,6 +8,7 @@ export type AutocompleteEvent = { type: "SEARCH"; query: string } | { type: "CLE export const searchUserMachine = createMachine( { id: "searchUserMachine", + predictableActionArguments: true, schema: { context: {} as { searchResults?: User[]