fix(workspaces): backend validation on workspace settings fields (#2584)

* fix(workspaces): backend validation on workspace settings fields

* chore(workspaces): move authz to resolver level

packages/server/modules/workspaces/graph/resolvers/workspaces.ts

@@ -153,6 +153,13 @@ export = FF_WORKSPACES_MODULE_ENABLED
         update: async (_parent, args, context) => {
           const { id: workspaceId, ...workspaceInput } = args.input
 
+          await authorizeResolver(
+            context.userId!,
+            workspaceId,
+            Roles.Workspace.Admin,
+            context.resourceAccessRules
+          )
+
           const updateWorkspace = updateWorkspaceFactory({
             getWorkspace: getWorkspaceFactory({ db }),
             upsertWorkspace: upsertWorkspaceFactory({ db }),
@@ -161,9 +168,7 @@ export = FF_WORKSPACES_MODULE_ENABLED
 
           const workspace = await updateWorkspace({
             workspaceId,
-            workspaceInput,
-            workspaceUpdaterId: context.userId!,
-            updaterResourceAccessLimits: context.resourceAccessRules
+            workspaceInput
           })
 
           return workspace

packages/server/modules/workspaces/services/management.ts

@@ -29,7 +29,6 @@ import {
 import { queryAllWorkspaceProjectsFactory } from '@/modules/workspaces/services/projects'
 import { EventBus } from '@/modules/shared/services/eventBus'
 import { removeNullOrUndefinedKeys } from '@speckle/shared'
-import { authorizeResolver } from '@/modules/shared'
 import { isNewResourceAllowed } from '@/modules/core/helpers/token'
 import {
   TokenResourceIdentifier,
@@ -37,6 +36,7 @@ import {
 } from '@/modules/core/domain/tokens/types'
 import { ForbiddenError } from '@/modules/shared/errors'
 import { validateImageString } from '@/modules/workspaces/helpers/images'
+import { isEmpty } from 'lodash'
 
 type WorkspaceCreateArgs = {
   userId: string
@@ -96,15 +96,12 @@ export const createWorkspaceFactory =
   }
 
 type WorkspaceUpdateArgs = {
-  /** Id of user performing the operation */
-  workspaceUpdaterId: string
   workspaceId: string
   workspaceInput: {
     name?: string | null
     description?: string | null
     logo?: string | null
   }
-  updaterResourceAccessLimits: MaybeNullOrUndefined<TokenResourceIdentifier[]>
 }
 
 export const updateWorkspaceFactory =
@@ -117,27 +114,20 @@ export const updateWorkspaceFactory =
     upsertWorkspace: UpsertWorkspace
     emitWorkspaceEvent: EventBus['emit']
   }) =>
-  async ({
-    workspaceUpdaterId,
-    workspaceId,
-    workspaceInput,
-    updaterResourceAccessLimits
-  }: WorkspaceUpdateArgs): Promise<Workspace> => {
-    await authorizeResolver(
-      workspaceUpdaterId,
-      workspaceId,
-      Roles.Workspace.Admin,
-      updaterResourceAccessLimits
-    )
+  async ({ workspaceId, workspaceInput }: WorkspaceUpdateArgs): Promise<Workspace> => {
+    // Get existing workspace to merge with incoming changes
+    const currentWorkspace = await getWorkspace({ workspaceId })
+    if (!currentWorkspace) {
+      throw new WorkspaceNotFoundError()
+    }
 
+    // Validate incoming changes
     if (!!workspaceInput.logo) {
       validateImageString(workspaceInput.logo)
     }
-
-    const currentWorkspace = await getWorkspace({ workspaceId })
-
-    if (!currentWorkspace) {
-      throw new WorkspaceNotFoundError()
+    if (isEmpty(workspaceInput.name)) {
+      // Do not allow setting an empty name (empty descriptions allowed)
+      delete workspaceInput.name
     }
 
     const workspace = {

packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts

@@ -20,6 +20,10 @@ import {
 import { Workspace } from '@/modules/workspacesCore/domain/types'
 import { beforeEachContext } from '@/test/hooks'
 import { AllScopes } from '@/modules/core/helpers/mainConstants'
+import {
+  BasicTestWorkspace,
+  createTestWorkspace
+} from '@/modules/workspaces/tests/helpers/creation'
 
 describe('Workspaces GQL CRUD', () => {
   let apollo: TestApolloServer
@@ -114,27 +118,65 @@ describe('Workspaces GQL CRUD', () => {
     })
 
     describe('mutation workspaceMutations.update', () => {
-      it('should update a workspace', async () => {
-        const createRes = await apollo.execute(CreateWorkspaceDocument, {
-          input: { name: cryptoRandomString({ length: 6 }) }
-        })
+      const workspace: BasicTestWorkspace = {
+        id: '',
+        ownerId: '',
+        name: cryptoRandomString({ length: 6 }),
+        description: cryptoRandomString({ length: 12 })
+      }
 
+      beforeEach(async () => {
+        await createTestWorkspace(workspace, testUser)
+      })
+
+      it('should update a workspace', async () => {
         const workspaceName = cryptoRandomString({ length: 6 })
 
-        await apollo.execute(UpdateWorkspaceDocument, {
+        const updateRes = await apollo.execute(UpdateWorkspaceDocument, {
           input: {
-            id: createRes.data!.workspaceMutations.create.id,
+            id: workspace.id,
             name: workspaceName
           }
         })
 
-        const getRes = await apollo.execute(GetWorkspaceDocument, {
-          workspaceId: createRes.data!.workspaceMutations.create.id
+        const { data } = await apollo.execute(GetWorkspaceDocument, {
+          workspaceId: workspace.id
         })
 
-        expect(createRes).to.not.haveGraphQLErrors()
-        expect(getRes).to.not.haveGraphQLErrors()
-        expect(getRes.data?.workspace.name).to.equal(workspaceName)
+        expect(updateRes).to.not.haveGraphQLErrors()
+        expect(data?.workspace.name).to.equal(workspaceName)
+      })
+
+      it('should not allow workspace name to be empty', async () => {
+        const updateRes = await apollo.execute(UpdateWorkspaceDocument, {
+          input: {
+            id: workspace.id,
+            name: ''
+          }
+        })
+
+        const { data } = await apollo.execute(GetWorkspaceDocument, {
+          workspaceId: workspace.id
+        })
+
+        expect(updateRes).to.not.haveGraphQLErrors()
+        expect(data?.workspace.name).to.equal(workspace.name)
+      })
+
+      it('should allow workspace description to be empty', async () => {
+        const updateRes = await apollo.execute(UpdateWorkspaceDocument, {
+          input: {
+            id: workspace.id,
+            description: ''
+          }
+        })
+
+        const { data } = await apollo.execute(GetWorkspaceDocument, {
+          workspaceId: workspace.id
+        })
+
+        expect(updateRes).to.not.haveGraphQLErrors()
+        expect(data?.workspace.description).to.equal('')
       })
     })
   })