From 99d4aba767d43e0477249aebf2397649daabb00e Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Thu, 27 Feb 2025 11:06:31 +0100 Subject: [PATCH 01/28] feat(workspaces): create workspace seat function --- .../server/modules/gatekeeper/domain/operations.ts | 5 +++++ .../modules/gatekeeper/repositories/workspaceSeat.ts | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/packages/server/modules/gatekeeper/domain/operations.ts b/packages/server/modules/gatekeeper/domain/operations.ts index 80139ead6..0ee85228d 100644 --- a/packages/server/modules/gatekeeper/domain/operations.ts +++ b/packages/server/modules/gatekeeper/domain/operations.ts @@ -1,3 +1,4 @@ +import { WorkspaceSeat } from '@/modules/gatekeeper/domain/billing' import { WorkspaceFeatureName } from '@/modules/gatekeeper/domain/workspacePricing' import { PlanStatuses, @@ -30,3 +31,7 @@ export type GetWorkspacePlanByProjectId = ({ }: { projectId: string }) => Promise + +export type CreateWorkspaceSeat = ( + args: Pick +) => Promise diff --git a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts index b33fa200a..a60c160d4 100644 --- a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts +++ b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts @@ -1,4 +1,5 @@ import { WorkspaceSeat } from '@/modules/gatekeeper/domain/billing' +import { CreateWorkspaceSeat } from '@/modules/gatekeeper/domain/operations' import { Knex } from 'knex' const tables = { @@ -17,3 +18,13 @@ export const countSeatsByTypeInWorkspaceFactory = .count('id') return parseInt(count.toString()) } + +export const createWorkspaceSeatFactory = + ({ db }: { db: Knex }): CreateWorkspaceSeat => + async ({ userId, workspaceId, type }) => { + return await tables.workspaceSeats(db).insert({ + workspaceId, + userId, + type + }) + } From 9275ea1ee90232aff668f2da99c0b1721f3ffe34 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Thu, 27 Feb 2025 16:20:33 +0100 Subject: [PATCH 02/28] feat(workspaces): assign workspace seat logic --- .../modules/workspaces/domain/operations.ts | 5 + .../workspaces/errors/workspaceSeat.ts | 7 + .../workspaces/services/workspaceSeat.ts | 101 ++++++++++ .../tests/integration/workspaceSeat.spec.ts | 183 ++++++++++++++++++ .../tests/unit/services/workspaceSeat.spec.ts | 50 +++++ 5 files changed, 346 insertions(+) create mode 100644 packages/server/modules/workspaces/errors/workspaceSeat.ts create mode 100644 packages/server/modules/workspaces/services/workspaceSeat.ts create mode 100644 packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts create mode 100644 packages/server/modules/workspaces/tests/unit/services/workspaceSeat.spec.ts diff --git a/packages/server/modules/workspaces/domain/operations.ts b/packages/server/modules/workspaces/domain/operations.ts index 0e330d04b..bdbe69994 100644 --- a/packages/server/modules/workspaces/domain/operations.ts +++ b/packages/server/modules/workspaces/domain/operations.ts @@ -29,6 +29,7 @@ import { Stream } from '@/modules/core/domain/streams/types' import { TokenResourceIdentifier } from '@/modules/core/domain/tokens/types' import { ServerRegion } from '@/modules/multiregion/domain/types' import { SetOptional } from 'type-fest' +import { WorkspaceSeat, WorkspaceSeatType } from '@/modules/gatekeeper/domain/billing' /** Workspace */ @@ -369,3 +370,7 @@ export type CopyProjectObjects = (params: { export type CopyProjectAutomations = (params: { projectIds: string[] }) => Promise> + +export type AssignWorkspaceSeat = ( + params: Pick & { type?: WorkspaceSeatType } +) => Promise diff --git a/packages/server/modules/workspaces/errors/workspaceSeat.ts b/packages/server/modules/workspaces/errors/workspaceSeat.ts new file mode 100644 index 000000000..3c4ecc11a --- /dev/null +++ b/packages/server/modules/workspaces/errors/workspaceSeat.ts @@ -0,0 +1,7 @@ +import { BaseError } from '@/modules/shared/errors' + +export class InvalidWorkspaceSeatType extends BaseError { + static defaultMessage = 'Workspace seat type is invalid' + static code = 'INDALID_WORKSPACE_SEAT_TYPE' + static statusCode = 400 +} diff --git a/packages/server/modules/workspaces/services/workspaceSeat.ts b/packages/server/modules/workspaces/services/workspaceSeat.ts new file mode 100644 index 000000000..82be5226e --- /dev/null +++ b/packages/server/modules/workspaces/services/workspaceSeat.ts @@ -0,0 +1,101 @@ +import { WorkspaceSeatType } from '@/modules/gatekeeper/domain/billing' +import { CreateWorkspaceSeat } from '@/modules/gatekeeper/domain/operations' +import { NotFoundError } from '@/modules/shared/errors' +import { + AssignWorkspaceSeat, + GetWorkspaceRoleForUser +} from '@/modules/workspaces/domain/operations' +import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { Roles, WorkspaceRoles } from '@speckle/shared' +import { z } from 'zod' + +const getDefaultWorkspaceSeatTypeByWorkspaceRole = ({ + workspaceRole +}: { + workspaceRole: WorkspaceRoles +}): WorkspaceSeatType => { + if (workspaceRole === Roles.Workspace.Admin) { + return 'editor' + } + return 'viewer' +} + +const WorkspaceRoleWorkspaceSeatTypeMapping = z.union([ + z.object({ + workspaceRole: z.literal(Roles.Workspace.Admin), + workspaceSeatType: z.literal('editor') + }), + z.object({ + workspaceRole: z.literal(Roles.Workspace.Member), + workspaceSeatType: z.union([z.literal('editor'), z.literal('viewer')]) + }), + z.object({ + workspaceRole: z.literal(Roles.Workspace.Guest), + workspaceSeatType: z.union([z.literal('editor'), z.literal('viewer')]) + }) +]) + +type WorkspaceRoleWorkspaceSeatTypeMapping = z.infer< + typeof WorkspaceRoleWorkspaceSeatTypeMapping +> + +export const isWorkspaceRoleWorkspaceSeatTypeValid = ({ + workspaceRole, + workspaceSeatType +}: { + workspaceRole: WorkspaceRoles + workspaceSeatType: WorkspaceSeatType +}): boolean => { + return WorkspaceRoleWorkspaceSeatTypeMapping.safeParse({ + workspaceRole, + workspaceSeatType + }).success +} + +export const assignWorkspaceSeatFactory = + ({ + createWorkspaceSeat, + getWorkspaceRoleForUser + }: { + createWorkspaceSeat: CreateWorkspaceSeat + getWorkspaceRoleForUser: GetWorkspaceRoleForUser + }): AssignWorkspaceSeat => + async ({ workspaceId, userId, type }) => { + const workspaceAcl = await getWorkspaceRoleForUser({ workspaceId, userId }) + if (!workspaceAcl) { + throw new NotFoundError('User does not have a role in the workspace') + } + if (!type) { + return await createWorkspaceSeat({ + workspaceId, + userId, + type: type + ? type + : getDefaultWorkspaceSeatTypeByWorkspaceRole({ + workspaceRole: workspaceAcl.role + }) + }) + } + if ( + !isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: workspaceAcl.role, + workspaceSeatType: type + }) + ) { + throw new InvalidWorkspaceSeatType( + `User with workspace role ${workspaceAcl.role} cannot have a seat of type ${type}`, + { + info: { + workspaceId, + userId + } + } + ) + } + + return await createWorkspaceSeat({ + workspaceId, + userId, + type + }) + } diff --git a/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts new file mode 100644 index 000000000..86bd30757 --- /dev/null +++ b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts @@ -0,0 +1,183 @@ +import { db } from '@/db/knex' +import { + createRandomEmail, + createRandomString +} from '@/modules/core/helpers/testHelpers' +import { createWorkspaceSeatFactory } from '@/modules/gatekeeper/repositories/workspaceSeat' +import { NotFoundError } from '@/modules/shared/errors' +import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { getWorkspaceRoleForUserFactory } from '@/modules/workspaces/repositories/workspaces' +import { assignWorkspaceSeatFactory } from '@/modules/workspaces/services/workspaceSeat' +import { + assignToWorkspace, + BasicTestWorkspace, + createTestWorkspace +} from '@/modules/workspaces/tests/helpers/creation' +import { expectToThrow } from '@/test/assertionHelper' +import { BasicTestUser, createTestUser } from '@/test/authHelper' +import { Roles } from '@speckle/shared' +import { expect } from 'chai' +import cryptoRandomString from 'crypto-random-string' + +describe('Workspace workspaceSeat services', () => { + describe('assignWorkspaceSeatFactory', () => { + it('should throw an error if user is not a member of the workspace', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + const err = await expectToThrow(() => + assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db }) + })({ userId: user.id, workspaceId: workspace.id, type: 'editor' }) + ) + + expect(err.name).to.eq(NotFoundError.name) + }) + it('should assign a workspace seat with the default type if none is provided', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + await assignToWorkspace(workspace, user, Roles.Workspace.Member) + + await assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db }) + })({ userId: user.id, workspaceId: workspace.id }) + + const workspaceSeat = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeat.type).to.eq('viewer') + }) + it('should assign a workspace seat with the provided type', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + await assignToWorkspace(workspace, user, Roles.Workspace.Member) + + await assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db }) + })({ userId: user.id, workspaceId: workspace.id, type: 'editor' }) + + const workspaceSeat = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeat.type).to.eq('editor') + }) + it('should throw an error if seat type is not compatible with workspace role', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + await assignToWorkspace(workspace, user, Roles.Workspace.Admin) + + const err = await expectToThrow(() => + assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db }) + })({ userId: user.id, workspaceId: workspace.id, type: 'viewer' }) + ) + + expect(err.name).to.eq(InvalidWorkspaceSeatType.name) + }) + }) +}) diff --git a/packages/server/modules/workspaces/tests/unit/services/workspaceSeat.spec.ts b/packages/server/modules/workspaces/tests/unit/services/workspaceSeat.spec.ts new file mode 100644 index 000000000..c184e4165 --- /dev/null +++ b/packages/server/modules/workspaces/tests/unit/services/workspaceSeat.spec.ts @@ -0,0 +1,50 @@ +import { isWorkspaceRoleWorkspaceSeatTypeValid } from '@/modules/workspaces/services/workspaceSeat' +import { Roles } from '@speckle/shared' +import { expect } from 'chai' + +describe('Workspace workspaceSeat services', () => { + describe('isWorkspaceRoleWorkspaceSeatTypeValid', () => { + it('should return true if the role is admin and seat type is editor', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Admin, + workspaceSeatType: 'editor' + }) + expect(result).to.be.true + }) + it('should return false if the role is admin and seat type is viewer', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Admin, + workspaceSeatType: 'viewer' as 'editor' + }) + expect(result).to.be.false + }) + it('should return true if the role is member and seat type is editor', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Member, + workspaceSeatType: 'editor' + }) + expect(result).to.be.true + }) + it('should return true if the role is member and seat type is viewer', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Member, + workspaceSeatType: 'viewer' + }) + expect(result).to.be.true + }) + it('should return true if the role is guest and seat type is editor', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Guest, + workspaceSeatType: 'editor' + }) + expect(result).to.be.true + }) + it('should return true if the role is member and seat type is viewer', () => { + const result = isWorkspaceRoleWorkspaceSeatTypeValid({ + workspaceRole: Roles.Workspace.Guest, + workspaceSeatType: 'viewer' + }) + expect(result).to.be.true + }) + }) +}) From 938127bf4b6f89ad71ea15d36cf939419e8323db Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Thu, 27 Feb 2025 16:21:44 +0100 Subject: [PATCH 03/28] feat(workspaces): add listeners to assign workspace seats --- .../workspaces/events/eventListener.ts | 50 +++++++++++++++++-- .../workspaces/tests/helpers/creation.ts | 4 -- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/packages/server/modules/workspaces/events/eventListener.ts b/packages/server/modules/workspaces/events/eventListener.ts index 5b8914012..e96b71e01 100644 --- a/packages/server/modules/workspaces/events/eventListener.ts +++ b/packages/server/modules/workspaces/events/eventListener.ts @@ -5,6 +5,7 @@ import { upsertProjectRoleFactory } from '@/modules/core/repositories/streams' import { + AssignWorkspaceSeat, CountWorkspaceRoleWithOptionalProjectRole, GetDefaultRegion, GetWorkspace, @@ -71,7 +72,8 @@ import { getBaseTrackingProperties, getClient } from '@/modules/shared/utils/mix import { calculateSubscriptionSeats, GetWorkspacePlan, - GetWorkspaceSubscription + GetWorkspaceSubscription, + WorkspaceSeatType } from '@/modules/gatekeeper/domain/billing' import { getWorkspacePlanProductId } from '@/modules/gatekeeper/stripe' import { Workspace } from '@/modules/workspacesCore/domain/types' @@ -81,6 +83,11 @@ import { getWorkspacePlanFactory, getWorkspaceSubscriptionFactory } from '@/modules/gatekeeper/repositories/billing' +import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' +import { assignWorkspaceSeatFactory } from '@/modules/workspaces/services/workspaceSeat' +import { createWorkspaceSeatFactory } from '@/modules/gatekeeper/repositories/workspaceSeat' + +const { FF_WORKSPACES_NEW_PLANS_ENABLED } = getFeatureFlags() export const onProjectCreatedFactory = ({ @@ -222,22 +229,26 @@ export const onWorkspaceRoleUpdatedFactory = getWorkspaceRoleToDefaultProjectRoleMapping, queryAllWorkspaceProjects, deleteProjectRole, - upsertProjectRole + upsertProjectRole, + assignWorkspaceSeat }: { getWorkspaceRoleToDefaultProjectRoleMapping: GetWorkspaceRoleToDefaultProjectRoleMapping queryAllWorkspaceProjects: QueryAllWorkspaceProjects deleteProjectRole: DeleteProjectRole upsertProjectRole: UpsertProjectRole + assignWorkspaceSeat: AssignWorkspaceSeat }) => async ({ userId, role, workspaceId, + seatType, flags }: { userId: string role: WorkspaceRoles workspaceId: string + seatType?: WorkspaceSeatType flags?: { skipProjectRoleUpdatesFor: string[] } @@ -276,6 +287,10 @@ export const onWorkspaceRoleUpdatedFactory = }) ) } + + if (FF_WORKSPACES_NEW_PLANS_ENABLED) { + await assignWorkspaceSeat({ userId, workspaceId, type: seatType }) + } } export const workspaceTrackingFactory = @@ -452,6 +467,21 @@ const emitWorkspaceGraphqlSubscriptionsFactory = } } +const onWorkspaceCreatedFactory = + ({ assignWorkspaceSeat }: { assignWorkspaceSeat: AssignWorkspaceSeat }) => + async ({ + workspace, + createdByUserId + }: { + workspace: Workspace + createdByUserId: string + }) => { + if (!FF_WORKSPACES_NEW_PLANS_ENABLED) { + return + } + await assignWorkspaceSeat({ userId: createdByUserId, workspaceId: workspace.id }) + } + export const initializeEventListenersFactory = ({ db }: { db: Knex }) => () => { @@ -534,10 +564,24 @@ export const initializeEventListenersFactory = }), queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), deleteProjectRole: deleteProjectRoleFactory({ db: trx }), - upsertProjectRole: upsertProjectRoleFactory({ db: trx }) + upsertProjectRole: upsertProjectRoleFactory({ db: trx }), + assignWorkspaceSeat: assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db: trx }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db: trx }) + }) }) await withTransaction(onWorkspaceRoleUpdated(payload), trx) }), + eventBus.listen(WorkspaceEvents.Created, async ({ payload }) => { + const trx = await db.transaction() + const onWorkspaceCreated = onWorkspaceCreatedFactory({ + assignWorkspaceSeat: assignWorkspaceSeatFactory({ + createWorkspaceSeat: createWorkspaceSeatFactory({ db: trx }), + getWorkspaceRoleForUser: getWorkspaceRoleForUserFactory({ db: trx }) + }) + }) + await withTransaction(onWorkspaceCreated(payload), trx) + }), eventBus.listen('**', emitWorkspaceGraphqlSubscriptions) ] diff --git a/packages/server/modules/workspaces/tests/helpers/creation.ts b/packages/server/modules/workspaces/tests/helpers/creation.ts index b7e0ef94c..d3fc30f29 100644 --- a/packages/server/modules/workspaces/tests/helpers/creation.ts +++ b/packages/server/modules/workspaces/tests/helpers/creation.ts @@ -246,10 +246,6 @@ export const assignToWorkspace = async ( user: BasicTestUser, role?: WorkspaceRoles ) => { - if (!FF_WORKSPACES_MODULE_ENABLED) { - return // Just skip - } - const updateWorkspaceRole = updateWorkspaceRoleFactory({ getWorkspaceWithDomains: getWorkspaceWithDomainsFactory({ db }), findVerifiedEmailsByUserId: findVerifiedEmailsByUserIdFactory({ db }), From b6f269b8ea2dee3eebada09b67a77ac3c4f863ec Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Fri, 28 Feb 2025 11:14:28 +0100 Subject: [PATCH 04/28] feat(workspaces): code review changes --- .../modules/gatekeeper/domain/operations.ts | 2 +- .../gatekeeper/repositories/workspaceSeat.ts | 14 ++++-- .../modules/workspaces/domain/operations.ts | 2 +- .../workspaces/errors/workspaceSeat.ts | 4 +- .../workspaces/services/workspaceSeat.ts | 4 +- .../tests/integration/workspaceSeat.spec.ts | 48 ++++++++++++++++++- 6 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/server/modules/gatekeeper/domain/operations.ts b/packages/server/modules/gatekeeper/domain/operations.ts index 981056d6b..c6ee84fcd 100644 --- a/packages/server/modules/gatekeeper/domain/operations.ts +++ b/packages/server/modules/gatekeeper/domain/operations.ts @@ -34,4 +34,4 @@ export type GetWorkspacePlanByProjectId = ({ export type CreateWorkspaceSeat = ( args: Pick -) => Promise +) => Promise diff --git a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts index a60c160d4..d22b2233f 100644 --- a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts +++ b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts @@ -22,9 +22,13 @@ export const countSeatsByTypeInWorkspaceFactory = export const createWorkspaceSeatFactory = ({ db }: { db: Knex }): CreateWorkspaceSeat => async ({ userId, workspaceId, type }) => { - return await tables.workspaceSeats(db).insert({ - workspaceId, - userId, - type - }) + await tables + .workspaceSeats(db) + .insert({ + workspaceId, + userId, + type + }) + .onConflict(['workspaceId', 'userId']) + .merge() } diff --git a/packages/server/modules/workspaces/domain/operations.ts b/packages/server/modules/workspaces/domain/operations.ts index 83680afe8..1835511fe 100644 --- a/packages/server/modules/workspaces/domain/operations.ts +++ b/packages/server/modules/workspaces/domain/operations.ts @@ -389,7 +389,7 @@ export type CopyProjectAutomations = (params: { export type AssignWorkspaceSeat = ( params: Pick & { type?: WorkspaceSeatType } -) => Promise +) => Promise export type CopyProjectComments = (params: { projectIds: string[] }) => Promise> diff --git a/packages/server/modules/workspaces/errors/workspaceSeat.ts b/packages/server/modules/workspaces/errors/workspaceSeat.ts index 3c4ecc11a..10aeeff88 100644 --- a/packages/server/modules/workspaces/errors/workspaceSeat.ts +++ b/packages/server/modules/workspaces/errors/workspaceSeat.ts @@ -1,7 +1,7 @@ import { BaseError } from '@/modules/shared/errors' -export class InvalidWorkspaceSeatType extends BaseError { +export class InvalidWorkspaceSeatTypeError extends BaseError { static defaultMessage = 'Workspace seat type is invalid' - static code = 'INDALID_WORKSPACE_SEAT_TYPE' + static code = 'INDALID_WORKSPACE_SEAT_TYPE_ERROR' static statusCode = 400 } diff --git a/packages/server/modules/workspaces/services/workspaceSeat.ts b/packages/server/modules/workspaces/services/workspaceSeat.ts index 82be5226e..6d1605912 100644 --- a/packages/server/modules/workspaces/services/workspaceSeat.ts +++ b/packages/server/modules/workspaces/services/workspaceSeat.ts @@ -5,7 +5,7 @@ import { AssignWorkspaceSeat, GetWorkspaceRoleForUser } from '@/modules/workspaces/domain/operations' -import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { InvalidWorkspaceSeatTypeError } from '@/modules/workspaces/errors/workspaceSeat' import { Roles, WorkspaceRoles } from '@speckle/shared' import { z } from 'zod' @@ -82,7 +82,7 @@ export const assignWorkspaceSeatFactory = workspaceSeatType: type }) ) { - throw new InvalidWorkspaceSeatType( + throw new InvalidWorkspaceSeatTypeError( `User with workspace role ${workspaceAcl.role} cannot have a seat of type ${type}`, { info: { diff --git a/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts index 86bd30757..02afd5d51 100644 --- a/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts @@ -5,7 +5,7 @@ import { } from '@/modules/core/helpers/testHelpers' import { createWorkspaceSeatFactory } from '@/modules/gatekeeper/repositories/workspaceSeat' import { NotFoundError } from '@/modules/shared/errors' -import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { InvalidWorkspaceSeatTypeError } from '@/modules/workspaces/errors/workspaceSeat' import { getWorkspaceRoleForUserFactory } from '@/modules/workspaces/repositories/workspaces' import { assignWorkspaceSeatFactory } from '@/modules/workspaces/services/workspaceSeat' import { @@ -177,7 +177,51 @@ describe('Workspace workspaceSeat services', () => { })({ userId: user.id, workspaceId: workspace.id, type: 'viewer' }) ) - expect(err.name).to.eq(InvalidWorkspaceSeatType.name) + expect(err.name).to.eq(InvalidWorkspaceSeatTypeError.name) + }) + it('should update seat type on role change', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + await assignToWorkspace(workspace, user, Roles.Workspace.Member) + const workspaceSeat = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeat.type).to.eq('viewer') + + // Change workspace role + await assignToWorkspace(workspace, user, Roles.Workspace.Admin) + + const workspaceSeatUpdated = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeatUpdated.type).to.eq('editor') }) }) }) From cae5b36681ef39bd496b4a1feabdfba6738c5837 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Fri, 28 Feb 2025 11:47:05 +0100 Subject: [PATCH 05/28] test(workspaces): fix tests --- .../workspaces/tests/unit/events/eventListener.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts b/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts index c7bfc1add..e2b08c763 100644 --- a/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts +++ b/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts @@ -84,7 +84,8 @@ describe('Event handlers', () => { }, upsertProjectRole: async () => { expect.fail() - } + }, + assignWorkspaceSeat: async () => undefined })({ role: Roles.Workspace.Guest, userId: cryptoRandomString({ length: 10 }), @@ -123,7 +124,8 @@ describe('Event handlers', () => { storedRoles.push(args) trackProjectUpdate = trackProjectUpdate || options?.trackProjectUpdate return {} as StreamRecord - } + }, + assignWorkspaceSeat: async () => undefined })({ role: Roles.Workspace.Member, userId, From 6f0133a39bcdee081687555e1aca42aa5ccf7e80 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Fri, 28 Feb 2025 15:42:12 +0000 Subject: [PATCH 06/28] chore(server/observability): logging of resolver to create checkout session (#4067) --- .../gatekeeper/graph/resolvers/index.ts | 61 +++++++++++++------ .../modules/gatekeeper/services/checkout.ts | 6 +- .../components/apollo/apolloSubscriptions.ts | 13 +--- .../server/observability/domain/fields.ts | 31 ++++++++++ .../server/observability/utils/logLevels.ts | 23 ++++++- 5 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 packages/server/observability/domain/fields.ts diff --git a/packages/server/modules/gatekeeper/graph/resolvers/index.ts b/packages/server/modules/gatekeeper/graph/resolvers/index.ts index 8c2ae2430..708220a7a 100644 --- a/packages/server/modules/gatekeeper/graph/resolvers/index.ts +++ b/packages/server/modules/gatekeeper/graph/resolvers/index.ts @@ -1,7 +1,7 @@ import { getFeatureFlags, getFrontendOrigin } from '@/modules/shared/helpers/envHelper' -import { Resolvers } from '@/modules/core/graph/generated/graphql' +import type { Resolvers } from '@/modules/core/graph/generated/graphql' import { authorizeResolver } from '@/modules/shared' -import { Roles, throwUncoveredError } from '@speckle/shared' +import { ensureError, Roles, throwUncoveredError } from '@speckle/shared' import { countWorkspaceRoleWithOptionalProjectRoleFactory, getWorkspaceFactory @@ -35,6 +35,9 @@ import { calculateSubscriptionSeats } from '@/modules/gatekeeper/domain/billing' import { WorkspacePaymentMethod } from '@/test/graphql/generated/graphql' import { LogicError, NotImplementedError } from '@/modules/shared/errors' import { isNewPlanType } from '@/modules/gatekeeper/helpers/plans' +import { extendLoggerComponent } from '@/observability/logging' +import { OperationName, OperationStatus } from '@/observability/domain/fields' +import { logWithErr } from '@/observability/utils/logLevels' const { FF_GATEKEEPER_MODULE_ENABLED, FF_BILLING_INTEGRATION_ENABLED } = getFeatureFlags() @@ -138,8 +141,15 @@ export = FF_GATEKEEPER_MODULE_ENABLED return true }, createCheckoutSession: async (parent, args, ctx) => { + let logger = extendLoggerComponent( + ctx.log, + 'gatekeeper', + 'resolvers', + 'createCheckoutSession' + ).child(OperationName('createCheckoutSession')) const { workspaceId, workspacePlan, billingInterval, isCreateFlow } = args.input + logger = logger.child({ workspaceId, workspacePlan }) const workspace = await getWorkspaceFactory({ db })({ workspaceId }) if (!workspace) throw new WorkspaceNotFoundError() @@ -159,22 +169,37 @@ export = FF_GATEKEEPER_MODULE_ENABLED const countRole = countWorkspaceRoleWithOptionalProjectRoleFactory({ db }) - const session = await startCheckoutSessionFactory({ - getWorkspaceCheckoutSession: getWorkspaceCheckoutSessionFactory({ db }), - getWorkspacePlan: getWorkspacePlanFactory({ db }), - countRole, - createCheckoutSession, - saveCheckoutSession: saveCheckoutSessionFactory({ db }), - deleteCheckoutSession: deleteCheckoutSessionFactory({ db }) - })({ - workspacePlan, - workspaceId, - workspaceSlug: workspace.slug, - isCreateFlow: isCreateFlow || false, - billingInterval - }) - - return session + try { + logger.info(OperationStatus.start, '[{operationName} ({operationStatus})]') + const session = await startCheckoutSessionFactory({ + getWorkspaceCheckoutSession: getWorkspaceCheckoutSessionFactory({ db }), + getWorkspacePlan: getWorkspacePlanFactory({ db }), + countRole, + createCheckoutSession, + saveCheckoutSession: saveCheckoutSessionFactory({ db }), + deleteCheckoutSession: deleteCheckoutSessionFactory({ db }) + })({ + workspacePlan, + workspaceId, + workspaceSlug: workspace.slug, + isCreateFlow: isCreateFlow || false, + billingInterval + }) + logger.info( + { ...OperationStatus.success, sessionId: session.id }, + '[{operationName} ({operationStatus})]' + ) + return session + } catch (err) { + const e = ensureError(err, 'Unknown error creating checkout session') + logWithErr( + logger, + e, + { ...OperationStatus.failure }, + '[{operationName} ({operationStatus})]' + ) + throw e + } }, upgradePlan: async (_parent, args, ctx) => { const { workspaceId, workspacePlan, billingInterval } = args.input diff --git a/packages/server/modules/gatekeeper/services/checkout.ts b/packages/server/modules/gatekeeper/services/checkout.ts index 3a84009f0..73221a7c3 100644 --- a/packages/server/modules/gatekeeper/services/checkout.ts +++ b/packages/server/modules/gatekeeper/services/checkout.ts @@ -97,11 +97,7 @@ export const startCheckoutSessionFactory = if (workspaceCheckoutSession.paymentStatus === 'paid') // this is should not be possible, but its better to be checking it here, than double charging the customer throw new WorkspaceAlreadyPaidError() - if ( - new Date().getTime() - workspaceCheckoutSession.createdAt.getTime() > - 1000 - // 10 * 60 * 1000 - ) { + if (new Date().getTime() - workspaceCheckoutSession.createdAt.getTime() > 1000) { await deleteCheckoutSession({ checkoutSessionId: workspaceCheckoutSession.id }) diff --git a/packages/server/observability/components/apollo/apolloSubscriptions.ts b/packages/server/observability/components/apollo/apolloSubscriptions.ts index c06724673..7c497e6f0 100644 --- a/packages/server/observability/components/apollo/apolloSubscriptions.ts +++ b/packages/server/observability/components/apollo/apolloSubscriptions.ts @@ -1,10 +1,7 @@ /* eslint-disable camelcase */ import type { GraphQLContext } from '@/modules/shared/helpers/typeHelper' import type { ExecutionParams } from 'subscriptions-transport-ws' -import { - shouldLogAsInfoLevel, - shouldLogAsWarnLevel -} from '@/observability/utils/logLevels' +import { logWithErr } from '@/observability/utils/logLevels' import { BaseError } from '@/modules/shared/errors' import { GraphQLError } from 'graphql' import { redactSensitiveVariables } from '@/observability/utils/redact' @@ -108,13 +105,7 @@ export function logSubscriptionOperation(params: { if (error instanceof BaseError) { errorLogger = errorLogger.child({ ...error.info() }) } - if (shouldLogAsInfoLevel(error)) { - errorLogger.info({ err: error }, errMsg) - } else if (shouldLogAsWarnLevel(error)) { - errorLogger.warn({ err: error }, errMsg) - } else { - errorLogger.error({ err: error }, errMsg) - } + logWithErr(errorLogger, error, errMsg) } } else if (response?.data) { logger.info('GQL subscription event {graphql_operation_name} emitted') diff --git a/packages/server/observability/domain/fields.ts b/packages/server/observability/domain/fields.ts new file mode 100644 index 000000000..845a8a171 --- /dev/null +++ b/packages/server/observability/domain/fields.ts @@ -0,0 +1,31 @@ +/** + * Helper constants for log fields. + * Intended to be used as values when logging. + */ + +/** + * Operation status values. + * Intended to be used with the `operationStatus` field when logging. + * Helps to avoid typos and ensure consistency. + */ +const STATUS = { + START: 'start', + SUCCESS: 'success', + FAILURE: 'failure' +} as const + +export const OperationStatus = { + start: { + operationStatus: STATUS.START + }, + success: { + operationStatus: STATUS.SUCCESS + }, + failure: { + operationStatus: STATUS.FAILURE + } +} as const + +export const OperationName = (name: string) => ({ + operationName: name +}) diff --git a/packages/server/observability/utils/logLevels.ts b/packages/server/observability/utils/logLevels.ts index 723a07d0e..e74e067ba 100644 --- a/packages/server/observability/utils/logLevels.ts +++ b/packages/server/observability/utils/logLevels.ts @@ -1,7 +1,28 @@ import { BaseError } from '@/modules/shared/errors' import { isUserGraphqlError } from '@/modules/shared/helpers/graphqlHelper' import { ApolloError } from '@apollo/client/core' +import { ensureError } from '@speckle/shared' import { GraphQLError } from 'graphql' +import type { Logger } from 'pino' + +interface LogFn { + (logger: Logger, e: unknown, obj?: unknown, msg?: string, ...args: unknown[]): void +} + +/** + * Uses the provided error to determine which log level to use, and adds the error to the logger instance. + * @param logger The logger instance + * @param e The error which will be used to determine the log level. It will be added to the logger instance. + * @param obj The object providing additional context to the log message (see Pino documentation https://github.com/pinojs/pino/blob/main/docs/api.md#logging-method-parameters) + * @param msg The message to log (see Pino documentation https://github.com/pinojs/pino/blob/main/docs/api.md#logging-method-parameters) + * @param args Additional arguments to log (see Pino documentation https://github.com/pinojs/pino/blob/main/docs/api.md#logging-method-parameters) + */ +export const logWithErr: LogFn = (logger, e, obj, msg?, ...args) => { + const err = ensureError(e) + if (shouldLogAsInfoLevel(err)) return logger.child({ err }).info(obj, msg, ...args) + if (shouldLogAsWarnLevel(err)) return logger.child({ err }).warn(obj, msg, ...args) + return logger.child({ err }).error(obj, msg, ...args) +} export const shouldLogAsInfoLevel = (err: unknown): boolean => { if (err instanceof GraphQLError) { @@ -22,7 +43,7 @@ export const shouldLogAsInfoLevel = (err: unknown): boolean => { return err instanceof ApolloError } -export const shouldLogAsWarnLevel = (err: unknown): boolean => { +const shouldLogAsWarnLevel = (err: unknown): boolean => { if (!(err instanceof GraphQLError)) return false if (err.message.startsWith('Cannot return null for non-nullable field')) return true From 3cc68bb0e2ba5074c01de5dfc23bad8b0f85b7f4 Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Fri, 28 Feb 2025 16:03:34 +0000 Subject: [PATCH 07/28] feat(automate): use updated hybrid function search (#4085) * fix(automate): use new function query for workspace functions * fix(automate): include workspace resource claims in auth flow * chore(automate): do not use btoa --- .../assets/automate/typedefs/automate.graphql | 12 ++++- .../automate/clients/executionEngine.ts | 54 +++++++++++++++++-- .../automate/graph/resolvers/automate.ts | 8 ++- .../automate/helpers/executionEngine.ts | 2 + .../modules/automate/services/authCode.ts | 14 +++-- .../automate/services/functionManagement.ts | 10 +++- .../modules/core/graph/generated/graphql.ts | 8 +++ .../graph/generated/graphql.ts | 6 +++ .../workspaces/graph/resolvers/workspaces.ts | 47 +++++++++------- .../server/test/graphql/generated/graphql.ts | 6 +++ 10 files changed, 135 insertions(+), 32 deletions(-) diff --git a/packages/server/assets/automate/typedefs/automate.graphql b/packages/server/assets/automate/typedefs/automate.graphql index 7698362da..a75e61660 100644 --- a/packages/server/assets/automate/typedefs/automate.graphql +++ b/packages/server/assets/automate/typedefs/automate.graphql @@ -352,6 +352,13 @@ input AutomateAuthCodePayloadTest { action: String! } +""" +Additional resources to validate user access to. +""" +input AutomateAuthCodeResources { + workspaceId: String +} + extend type Query { automateFunctions( filter: AutomateFunctionsFilter @@ -366,7 +373,10 @@ extend type Query { """ Part of the automation/function creation handshake mechanism """ - automateValidateAuthCode(payload: AutomateAuthCodePayloadTest!): Boolean! + automateValidateAuthCode( + payload: AutomateAuthCodePayloadTest! + resources: AutomateAuthCodeResources + ): Boolean! } extend type Mutation { diff --git a/packages/server/modules/automate/clients/executionEngine.ts b/packages/server/modules/automate/clients/executionEngine.ts index c60cdd908..d29219d2a 100644 --- a/packages/server/modules/automate/clients/executionEngine.ts +++ b/packages/server/modules/automate/clients/executionEngine.ts @@ -28,7 +28,7 @@ import { retry, timeoutAt } from '@speckle/shared' -import { has, isObjectLike } from 'lodash' +import { has, isObjectLike, isEmpty } from 'lodash' export type AuthCodePayloadWithOrigin = AuthCodePayload & { origin: string } @@ -48,7 +48,7 @@ export type AutomationCreateResponse = { const getApiUrl = ( path?: string, options?: Partial<{ - query: Record + query: Record }> ) => { const automateUrl = speckleAutomateUrl() @@ -62,9 +62,10 @@ const getApiUrl = ( const url = new URL(path, automateUrl) if (options?.query) { Object.entries(options.query).forEach(([key, val]) => { - if (isNullOrUndefined(val)) return + if (isEmpty(val) || isNullOrUndefined(val)) return try { - url.searchParams.append(key, val.toString()) + const urlValue = typeof val === 'object' ? val.join(',') : val.toString() + url.searchParams.append(key, urlValue) } catch { console.log({ val }) } @@ -460,7 +461,50 @@ export const getFunctionRelease = async (params: { : null } +export type GetFunctionsParams = { + auth?: AuthCodePayload + filters: { + query?: string + cursor?: string + limit?: number + requireRelease?: boolean + includeFeatured?: boolean + includeWorkspaces?: string[] + includeUsers?: string[] + } +} + export type GetFunctionsResponse = { + items: FunctionSchemaType[] + cursor: Nullable + totalCount: number +} + +export const getFunctions = async (params: GetFunctionsParams) => { + const url = getApiUrl(`/api/v2/functions`, { + query: { + requireRelease: true, + ...params.filters + } + }) + + const authToken = params.auth + ? Buffer.from( + JSON.stringify({ + ...params.auth, + origin: getServerOrigin() + }) + ).toString('base64') + : undefined + + return await invokeSafeJsonRequest({ + url, + method: 'get', + token: authToken + }) +} + +export type GetPublicFunctionsResponse = { totalCount: number cursor: Nullable items: FunctionWithVersionsSchemaType[] @@ -482,7 +526,7 @@ export const getPublicFunctions = async (params: { } }) - return await invokeSafeJsonRequest({ + return await invokeSafeJsonRequest({ url, method: 'get' }) diff --git a/packages/server/modules/automate/graph/resolvers/automate.ts b/packages/server/modules/automate/graph/resolvers/automate.ts index e4dc15f9a..0eec4ba48 100644 --- a/packages/server/modules/automate/graph/resolvers/automate.ts +++ b/packages/server/modules/automate/graph/resolvers/automate.ts @@ -736,9 +736,13 @@ export = (FF_AUTOMATE_MODULE_ENABLED emit: getEventBus().emit }) const payload = removeNullOrUndefinedKeys(args.payload) + const resources = removeNullOrUndefinedKeys(args.resources ?? {}) return await validate({ - ...payload, - action: args.payload.action as AuthCodePayloadAction + payload: { + ...payload, + action: args.payload.action as AuthCodePayloadAction + }, + resources }) }, async automateFunction(_parent, { id }, ctx) { diff --git a/packages/server/modules/automate/helpers/executionEngine.ts b/packages/server/modules/automate/helpers/executionEngine.ts index 5e5a0db14..97b4a3195 100644 --- a/packages/server/modules/automate/helpers/executionEngine.ts +++ b/packages/server/modules/automate/helpers/executionEngine.ts @@ -20,6 +20,8 @@ export type FunctionSchemaType = { speckleUserId: string speckleServerOrigin: string }> + functionCreatorSpeckleUserId: Nullable + functionCreatorSpeckleServerOrigin: Nullable workspaceIds: string[] } diff --git a/packages/server/modules/automate/services/authCode.ts b/packages/server/modules/automate/services/authCode.ts index ac2bf3233..8c89bc783 100644 --- a/packages/server/modules/automate/services/authCode.ts +++ b/packages/server/modules/automate/services/authCode.ts @@ -19,7 +19,6 @@ export enum AuthCodePayloadAction { export type AuthCodePayload = { code: string userId: string - workspaceId?: string action: AuthCodePayloadAction } @@ -49,8 +48,14 @@ export const createStoredAuthCodeFactory = export const validateStoredAuthCodeFactory = (deps: { redis: Redis; emit: EventBus['emit'] }) => - async (payload: AuthCodePayload) => { + async (params: { + payload: AuthCodePayload + resources?: { + workspaceId?: string + } + }) => { const { redis, emit } = deps + const { payload, resources } = params const potentialPayloadString = await redis.get(payload.code) const potentialPayload: unknown = potentialPayloadString @@ -67,10 +72,11 @@ export const validateStoredAuthCodeFactory = throw new AutomateAuthCodeHandshakeError('Invalid automate auth payload') } - if (payload.workspaceId) { + // Token is valid, confirm user is authorized to access specified resources. + if (resources?.workspaceId) { emit({ eventName: 'workspace.authorized', - payload: { userId: payload.userId, workspaceId: payload.workspaceId } + payload: { userId: payload.userId, workspaceId: resources?.workspaceId } }) } diff --git a/packages/server/modules/automate/services/functionManagement.ts b/packages/server/modules/automate/services/functionManagement.ts index 45ff90edd..e71ad02ac 100644 --- a/packages/server/modules/automate/services/functionManagement.ts +++ b/packages/server/modules/automate/services/functionManagement.ts @@ -89,6 +89,14 @@ const cleanFunctionLogo = (logo: MaybeNullOrUndefined): Nullable export const convertFunctionToGraphQLReturn = ( fn: FunctionSchemaType ): AutomateFunctionGraphQLReturn => { + const functionCreator: FunctionSchemaType['functionCreator'] = + fn.functionCreatorSpeckleUserId && fn.functionCreatorSpeckleServerOrigin + ? { + speckleUserId: fn.functionCreatorSpeckleUserId, + speckleServerOrigin: fn.functionCreatorSpeckleServerOrigin + } + : fn.functionCreator + const ret: AutomateFunctionGraphQLReturn = { id: fn.functionId, name: fn.functionName, @@ -98,7 +106,7 @@ export const convertFunctionToGraphQLReturn = ( logo: cleanFunctionLogo(fn.logo), tags: fn.tags, supportedSourceApps: fn.supportedSourceApps, - functionCreator: fn.functionCreator, + functionCreator, workspaceIds: fn.workspaceIds } diff --git a/packages/server/modules/core/graph/generated/graphql.ts b/packages/server/modules/core/graph/generated/graphql.ts index e3d2f8f47..b2f276023 100644 --- a/packages/server/modules/core/graph/generated/graphql.ts +++ b/packages/server/modules/core/graph/generated/graphql.ts @@ -265,6 +265,11 @@ export type AutomateAuthCodePayloadTest = { workspaceId?: InputMaybe; }; +/** Additional resources to validate user access to. */ +export type AutomateAuthCodeResources = { + workspaceId?: InputMaybe; +}; + export type AutomateFunction = { __typename?: 'AutomateFunction'; /** Only returned if user is a part of this speckle server */ @@ -2747,6 +2752,7 @@ export type QueryAutomateFunctionsArgs = { export type QueryAutomateValidateAuthCodeArgs = { payload: AutomateAuthCodePayloadTest; + resources?: InputMaybe; }; @@ -4938,6 +4944,7 @@ export type ResolversTypes = { ArchiveCommentInput: ArchiveCommentInput; AuthStrategy: ResolverTypeWrapper; AutomateAuthCodePayloadTest: AutomateAuthCodePayloadTest; + AutomateAuthCodeResources: AutomateAuthCodeResources; AutomateFunction: ResolverTypeWrapper; AutomateFunctionCollection: ResolverTypeWrapper & { items: Array }>; AutomateFunctionRelease: ResolverTypeWrapper; @@ -5251,6 +5258,7 @@ export type ResolversParentTypes = { ArchiveCommentInput: ArchiveCommentInput; AuthStrategy: AuthStrategy; AutomateAuthCodePayloadTest: AutomateAuthCodePayloadTest; + AutomateAuthCodeResources: AutomateAuthCodeResources; AutomateFunction: AutomateFunctionGraphQLReturn; AutomateFunctionCollection: Omit & { items: Array }; AutomateFunctionRelease: AutomateFunctionReleaseGraphQLReturn; diff --git a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts index 472244db8..11ed499d1 100644 --- a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts +++ b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts @@ -246,6 +246,11 @@ export type AutomateAuthCodePayloadTest = { workspaceId?: InputMaybe; }; +/** Additional resources to validate user access to. */ +export type AutomateAuthCodeResources = { + workspaceId?: InputMaybe; +}; + export type AutomateFunction = { __typename?: 'AutomateFunction'; /** Only returned if user is a part of this speckle server */ @@ -2728,6 +2733,7 @@ export type QueryAutomateFunctionsArgs = { export type QueryAutomateValidateAuthCodeArgs = { payload: AutomateAuthCodePayloadTest; + resources?: InputMaybe; }; diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index a35dde9d4..3090fbf10 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -41,7 +41,7 @@ import { import { createProjectInviteFactory } from '@/modules/serverinvites/services/projectInviteManagement' import { getInvitationTargetUsersFactory } from '@/modules/serverinvites/services/retrieval' import { authorizeResolver } from '@/modules/shared' -import { getFeatureFlags, getServerOrigin } from '@/modules/shared/helpers/envHelper' +import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getEventBus } from '@/modules/shared/services/eventBus' import { WorkspaceInviteResourceType } from '@/modules/workspacesCore/domain/constants' import { @@ -169,17 +169,12 @@ import { listWorkspaceSsoMembershipsFactory } from '@/modules/workspaces/repositories/sso' import { getDecryptor } from '@/modules/workspaces/helpers/sso' -import { getWorkspaceFunctions } from '@/modules/automate/clients/executionEngine' +import { getFunctions } from '@/modules/automate/clients/executionEngine' import { ExecutionEngineFailedResponseError, ExecutionEngineNetworkError } from '@/modules/automate/errors/executionEngine' import { getDefaultRegionFactory } from '@/modules/workspaces/repositories/regions' -import { - AuthCodePayloadAction, - createStoredAuthCodeFactory -} from '@/modules/automate/services/authCode' -import { getGenericRedis } from '@/modules/shared/redis/redis' import { convertFunctionToGraphQLReturn } from '@/modules/automate/services/functionManagement' import { getWorkspacePlanFactory, @@ -202,6 +197,11 @@ import { OperationTypeNode } from 'graphql' import { updateWorkspacePlanFactory } from '@/modules/gatekeeper/services/workspacePlans' import { GetWorkspaceCollaboratorsArgs } from '@/modules/workspaces/domain/operations' import { WorkspaceTeamMember } from '@/modules/workspaces/domain/types' +import { getGenericRedis } from '@/modules/shared/redis/redis' +import { + AuthCodePayloadAction, + createStoredAuthCodeFactory +} from '@/modules/automate/services/authCode' const eventBus = getEventBus() const getServerInfo = getServerInfoFactory({ db }) @@ -1079,6 +1079,13 @@ export = FF_WORKSPACES_MODULE_ENABLED }, automateFunctions: async (parent, args, context) => { try { + await authorizeResolver( + context.userId, + parent.id, + Roles.Workspace.Member, + context.resourceAccessRules + ) + const authCode = await createStoredAuthCodeFactory({ redis: getGenericRedis() })({ @@ -1086,14 +1093,16 @@ export = FF_WORKSPACES_MODULE_ENABLED action: AuthCodePayloadAction.ListWorkspaceFunctions }) - const res = await getWorkspaceFunctions({ - workspaceId: parent.id, - query: removeNullOrUndefinedKeys(args), - body: { - speckleServerAuthenticationPayload: { - ...authCode, - origin: getServerOrigin() - } + const res = await getFunctions({ + auth: authCode, + filters: { + query: args.filter?.search ?? undefined, + cursor: args.cursor ?? undefined, + limit: args.limit, + requireRelease: true, + includeFeatured: true, + includeWorkspaces: [parent.id], + includeUsers: [] } }) @@ -1105,12 +1114,12 @@ export = FF_WORKSPACES_MODULE_ENABLED } } - const items = res.functions.map(convertFunctionToGraphQLReturn) + const items = res.items.map(convertFunctionToGraphQLReturn) return { - cursor: undefined, - totalCount: res.functions.length, - items + items, + cursor: res.cursor, + totalCount: res.totalCount } } catch (e) { const isNotFound = diff --git a/packages/server/test/graphql/generated/graphql.ts b/packages/server/test/graphql/generated/graphql.ts index 80068b083..6f0e1c765 100644 --- a/packages/server/test/graphql/generated/graphql.ts +++ b/packages/server/test/graphql/generated/graphql.ts @@ -247,6 +247,11 @@ export type AutomateAuthCodePayloadTest = { workspaceId?: InputMaybe; }; +/** Additional resources to validate user access to. */ +export type AutomateAuthCodeResources = { + workspaceId?: InputMaybe; +}; + export type AutomateFunction = { __typename?: 'AutomateFunction'; /** Only returned if user is a part of this speckle server */ @@ -2729,6 +2734,7 @@ export type QueryAutomateFunctionsArgs = { export type QueryAutomateValidateAuthCodeArgs = { payload: AutomateAuthCodePayloadTest; + resources?: InputMaybe; }; From 04f84c31f36da3d8eb828c7262a040e4c5ecee2e Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Fri, 28 Feb 2025 18:22:00 +0000 Subject: [PATCH 08/28] fix(server/automate): logger should have request context and request ID sent to execution engine (#4092) * fix(server/automate): logger should have request context * WIP - pass in request Id * WIP * fix(automate): better logging for automate processes * chore(automate): slight log improvement * fix(automate): just in case --------- Co-authored-by: Charles Driesler --- .../automate/clients/executionEngine.ts | 350 ++++++++++-------- .../automate/graph/resolvers/automate.ts | 42 ++- packages/server/modules/automate/index.ts | 4 +- .../modules/automate/services/authCode.ts | 16 +- .../automate/services/automationManagement.ts | 12 +- .../automate/services/functionManagement.ts | 11 +- .../modules/core/graph/dataloaders/index.ts | 9 +- .../workspaces/graph/resolvers/workspaces.ts | 6 +- 8 files changed, 255 insertions(+), 195 deletions(-) diff --git a/packages/server/modules/automate/clients/executionEngine.ts b/packages/server/modules/automate/clients/executionEngine.ts index d29219d2a..3f0349894 100644 --- a/packages/server/modules/automate/clients/executionEngine.ts +++ b/packages/server/modules/automate/clients/executionEngine.ts @@ -28,6 +28,8 @@ import { retry, timeoutAt } from '@speckle/shared' +import { randomUUID } from 'crypto' +import { Logger } from 'pino' import { has, isObjectLike, isEmpty } from 'lodash' export type AuthCodePayloadWithOrigin = AuthCodePayload & { origin: string } @@ -75,22 +77,23 @@ const getApiUrl = ( return url.toString() } -const invokeSafeJsonRequest = async < - Response extends Record = Record ->( - ...args: Parameters -): Promise => { - const [{ url, method }] = args - try { - return await invokeJsonRequest(...args) - } catch (e) { - automateLogger.error( - { url, method, err: e }, - 'Automate API request error suppressed.' - ) - return null +const invokeSafeJsonRequestFactory = + = Record>(deps: { + logger: Logger + }) => + async (...args: Parameters): Promise => { + const { logger } = deps + const [{ url, method }] = args + try { + return await invokeJsonRequest({ + ...args[0], + requestId: logger.bindings?.()?.req?.id + }) + } catch (e) { + logger.error({ url, method, err: e }, 'Automate API request error suppressed.') + return null + } } -} const invokeJsonRequest = async >( ...args: Parameters @@ -110,10 +113,11 @@ const invokeRequest = async (params: { url: string method?: RequestInit['method'] body?: Record + requestId?: string token?: string retry?: boolean }) => { - const { url, method = 'get', body, token } = params + const { url, method = 'get', body, token, requestId } = params const response = await retry( async () => @@ -122,6 +126,7 @@ const invokeRequest = async (params: { method, headers: { 'Content-Type': 'application/json', + 'X-Request-Id': requestId ?? randomUUID(), ...(token?.length ? { Authorization: `Bearer ${token}` } : {}) }, body: body && isObjectLike(body) ? JSON.stringify(body) : undefined @@ -388,78 +393,91 @@ export type GetFunctionResponse = FunctionWithVersionsSchemaType & { versionCursor: Nullable } -export const getFunction = async (params: { - functionId: string - token?: string - releases?: { cursor?: string; limit?: number; versionsFilter?: string } -}) => { - const { functionId, token } = params - const query = Object.values(params.releases || {}).filter(isNonNullable).length - ? params.releases - : undefined +export const getFunctionFactory = + (deps: { logger: Logger }) => + async (params: { + functionId: string + token?: string + releases?: { cursor?: string; limit?: number; versionsFilter?: string } + }) => { + const { logger } = deps + const { functionId, token } = params + const query = Object.values(params.releases || {}).filter(isNonNullable).length + ? params.releases + : undefined - const url = getApiUrl(`/api/v1/functions/${functionId}`, { - query - }) + const url = getApiUrl(`/api/v1/functions/${functionId}`, { + query + }) - return await invokeSafeJsonRequest({ - url, - method: 'get', - token - }) -} + return await invokeSafeJsonRequestFactory({ + logger + })({ + url, + method: 'get', + token + }) + } export type GetFunctionReleaseResponse = FunctionReleaseSchemaType /** * TODO: Build optimized exec engine endpoint for this */ -export const getFunctionReleases = async (params: { - ids: Array<{ functionId: string; functionReleaseId: string }> -}) => { - const { ids } = params - const results = await Promise.all( - ids.map(async ({ functionId, functionReleaseId }) => { - try { - return await getFunctionRelease({ functionId, functionReleaseId }) - } catch (e) { - if (e instanceof ExecutionEngineNetworkError) { - return null - } - if ( - e instanceof ExecutionEngineFailedResponseError && - e.response.statusMessage === 'FunctionNotFound' - ) { - return null - } +export const getFunctionReleasesFactory = + (deps: { logger: Logger }) => + async (params: { ids: Array<{ functionId: string; functionReleaseId: string }> }) => { + const { logger } = deps + const { ids } = params + const results = await Promise.all( + ids.map(async ({ functionId, functionReleaseId }) => { + try { + return await getFunctionReleaseFactory({ logger })({ + functionId, + functionReleaseId + }) + } catch (e) { + if (e instanceof ExecutionEngineNetworkError) { + return null + } + if ( + e instanceof ExecutionEngineFailedResponseError && + e.response.statusMessage === 'FunctionNotFound' + ) { + return null + } - throw e - } + throw e + } + }) + ) + + return results.filter(isNonNullable) + } + +export const getFunctionReleaseFactory = + (deps: { logger: Logger }) => + async (params: { functionId: string; functionReleaseId: string }) => { + const { logger } = deps + const { functionId, functionReleaseId } = params + const url = getApiUrl( + `/api/v1/functions/${functionId}/versions/${functionReleaseId}` + ) + + const result = await invokeSafeJsonRequestFactory({ + logger + })({ + url, + method: 'get' }) - ) - return results.filter(isNonNullable) -} - -export const getFunctionRelease = async (params: { - functionId: string - functionReleaseId: string -}) => { - const { functionId, functionReleaseId } = params - const url = getApiUrl(`/api/v1/functions/${functionId}/versions/${functionReleaseId}`) - - const result = await invokeSafeJsonRequest({ - url, - method: 'get' - }) - - return result - ? { - ...result, - functionId - } - : null -} + return result + ? { + ...result, + functionId + } + : null + } export type GetFunctionsParams = { auth?: AuthCodePayload @@ -480,29 +498,32 @@ export type GetFunctionsResponse = { totalCount: number } -export const getFunctions = async (params: GetFunctionsParams) => { - const url = getApiUrl(`/api/v2/functions`, { - query: { - requireRelease: true, - ...params.filters - } - }) +export const getFunctionsFactory = + (deps: { logger: Logger }) => async (params: GetFunctionsParams) => { + const { logger } = deps - const authToken = params.auth - ? Buffer.from( - JSON.stringify({ - ...params.auth, - origin: getServerOrigin() - }) - ).toString('base64') - : undefined + const url = getApiUrl(`/api/v2/functions`, { + query: { + requireRelease: true, + ...params.filters + } + }) - return await invokeSafeJsonRequest({ - url, - method: 'get', - token: authToken - }) -} + const authToken = params.auth + ? Buffer.from( + JSON.stringify({ + ...params.auth, + origin: getServerOrigin() + }) + ).toString('base64') + : undefined + + return await invokeSafeJsonRequestFactory({ logger })({ + url, + method: 'get', + token: authToken + }) + } export type GetPublicFunctionsResponse = { totalCount: number @@ -510,79 +531,94 @@ export type GetPublicFunctionsResponse = { items: FunctionWithVersionsSchemaType[] } -export const getPublicFunctions = async (params: { - query?: { - query?: string - cursor?: string - limit?: number - functionsWithoutVersions?: boolean - } -}) => { - const { query } = params - const url = getApiUrl(`/api/v1/functions`, { - query: { - ...query, - featuredFunctionsOnly: true +export const getPublicFunctionsFactory = + (deps: { logger: Logger }) => + async (params: { + query?: { + query?: string + cursor?: string + limit?: number + functionsWithoutVersions?: boolean } - }) + }) => { + const { logger } = deps + const { query } = params + const url = getApiUrl(`/api/v1/functions`, { + query: { + ...query, + featuredFunctionsOnly: true + } + }) - return await invokeSafeJsonRequest({ - url, - method: 'get' - }) -} + return await invokeSafeJsonRequestFactory({ + logger + })({ + url, + method: 'get' + }) + } type GetUserFunctionsResponse = { functions: FunctionWithVersionsSchemaType[] } -export const getUserFunctions = async (params: { - userId: string - query?: { - query?: string - cursor?: string - limit?: number - } - body: { - speckleServerAuthenticationPayload: AuthCodePayloadWithOrigin - } -}) => { - const { userId, query, body } = params - const url = getApiUrl(`/api/v2/users/${userId}/functions`, { query }) +export const getUserFunctionsFactory = + (deps: { logger: Logger }) => + async (params: { + userId: string + query?: { + query?: string + cursor?: string + limit?: number + } + body: { + speckleServerAuthenticationPayload: AuthCodePayloadWithOrigin + } + }) => { + const { logger } = deps + const { userId, query, body } = params + const url = getApiUrl(`/api/v2/users/${userId}/functions`, { query }) - return await invokeSafeJsonRequest({ - url, - method: 'POST', - body, - retry: false - }) -} + return await invokeSafeJsonRequestFactory({ + logger + })({ + url, + method: 'POST', + body, + retry: false + }) + } type GetWorkspaceFunctionsResponse = { functions: FunctionWithVersionsSchemaType[] } -export const getWorkspaceFunctions = async (params: { - workspaceId: string - query?: { - query?: string - cursor?: string - limit?: number - } - body: { - speckleServerAuthenticationPayload: AuthCodePayloadWithOrigin - } -}) => { - const { workspaceId, query, body } = params - const url = getApiUrl(`/api/v2/workspaces/${workspaceId}/functions`, { query }) +export const getWorkspaceFunctionsFactory = + (deps: { logger: Logger }) => + async (params: { + workspaceId: string + query?: { + query?: string + cursor?: string + limit?: number + } + body: { + speckleServerAuthenticationPayload: AuthCodePayloadWithOrigin + } + }) => { + const { logger } = deps + const { workspaceId, query, body } = params + const url = getApiUrl(`/api/v2/workspaces/${workspaceId}/functions`, { query }) - return await invokeSafeJsonRequest({ - url, - method: 'POST', - body, - retry: false - }) -} + return await invokeSafeJsonRequestFactory({ + logger + })({ + url, + method: 'POST', + body, + retry: false + }) + } type UserGithubAuthStateResponse = { userHasAuthorizedGitHubApp: boolean diff --git a/packages/server/modules/automate/graph/resolvers/automate.ts b/packages/server/modules/automate/graph/resolvers/automate.ts index 0eec4ba48..ea9fb53b6 100644 --- a/packages/server/modules/automate/graph/resolvers/automate.ts +++ b/packages/server/modules/automate/graph/resolvers/automate.ts @@ -3,13 +3,13 @@ import { createFunctionWithoutVersion, triggerAutomationRun, updateFunction as execEngineUpdateFunction, - getFunction, - getFunctionRelease, - getPublicFunctions, - getFunctionReleases, + getFunctionFactory, + getFunctionReleaseFactory, + getPublicFunctionsFactory, + getFunctionReleasesFactory, getUserGithubAuthState, getUserGithubOrganizations, - getUserFunctions + getUserFunctionsFactory } from '@/modules/automate/clients/executionEngine' import { GetProjectAutomationsParams, @@ -459,10 +459,12 @@ export = (FF_AUTOMATE_MODULE_ENABLED } }, AutomateFunction: { - async releases(parent, args) { + async releases(parent, args, context) { try { // TODO: Replace w/ dataloader batch call, when/if possible - const fn = await getFunction({ + const fn = await getFunctionFactory({ + logger: context.log + })({ functionId: parent.id, releases: args?.cursor || args?.filter?.search || args?.limit @@ -567,10 +569,11 @@ export = (FF_AUTOMATE_MODULE_ENABLED async updateFunction(_parent, args, ctx) { const update = updateFunctionFactory({ updateFunction: execEngineUpdateFunction, - getFunction, + getFunction: getFunctionFactory({ logger: ctx.log }), createStoredAuthCode: createStoredAuthCodeFactory({ redis: getGenericRedis() - }) + }), + logger: ctx.log }) return await update({ input: args.input, userId: ctx.userId! }) } @@ -621,12 +624,12 @@ export = (FF_AUTOMATE_MODULE_ENABLED getAutomation: getAutomationFactory({ db: projectDb }), storeAutomationRevision: storeAutomationRevisionFactory({ db: projectDb }), getBranchesByIds: getBranchesByIdsFactory({ db: projectDb }), - getFunctionRelease, + getFunctionRelease: getFunctionReleaseFactory({ logger: ctx.log }), getEncryptionKeyPair, getFunctionInputDecryptor: getFunctionInputDecryptorFactory({ buildDecryptor }), - getFunctionReleases, + getFunctionReleases: getFunctionReleasesFactory({ logger: ctx.log }), eventEmit: getEventBus().emit, validateStreamAccess }) @@ -679,7 +682,7 @@ export = (FF_AUTOMATE_MODULE_ENABLED const create = createTestAutomationFactory({ getEncryptionKeyPair, - getFunction, + getFunction: getFunctionFactory({ logger: ctx.log }), storeAutomation: storeAutomationFactory({ db: projectDb }), storeAutomationRevision: storeAutomationRevisionFactory({ db: projectDb }), validateStreamAccess, @@ -730,10 +733,11 @@ export = (FF_AUTOMATE_MODULE_ENABLED } }, Query: { - async automateValidateAuthCode(_parent, args) { + async automateValidateAuthCode(_parent, args, ctx) { const validate = validateStoredAuthCodeFactory({ redis: getGenericRedis(), - emit: getEventBus().emit + emit: getEventBus().emit, + logger: ctx.log }) const payload = removeNullOrUndefinedKeys(args.payload) const resources = removeNullOrUndefinedKeys(args.resources ?? {}) @@ -755,9 +759,11 @@ export = (FF_AUTOMATE_MODULE_ENABLED return convertFunctionToGraphQLReturn(fn) }, - async automateFunctions(_parent, args) { + async automateFunctions(_parent, args, ctx) { try { - const res = await getPublicFunctions({ + const res = await getPublicFunctionsFactory({ + logger: ctx.log + })({ query: { query: args.filter?.search || undefined, cursor: args.cursor || undefined, @@ -809,7 +815,9 @@ export = (FF_AUTOMATE_MODULE_ENABLED action: AuthCodePayloadAction.ListUserFunctions }) - const res = await getUserFunctions({ + const res = await getUserFunctionsFactory({ + logger: context.log + })({ userId: context.userId!, query: { query: args.filter?.search || undefined, diff --git a/packages/server/modules/automate/index.ts b/packages/server/modules/automate/index.ts index 5ed5c1bb9..0ce22f83a 100644 --- a/packages/server/modules/automate/index.ts +++ b/packages/server/modules/automate/index.ts @@ -17,7 +17,7 @@ import { import { isNonNullable, Scopes } from '@speckle/shared' import { registerOrUpdateScopeFactory } from '@/modules/shared/repositories/scopes' import { - getFunction, + getFunctionFactory, triggerAutomationRun } from '@/modules/automate/clients/executionEngine' import logStreamRest from '@/modules/automate/rest/logStream' @@ -289,7 +289,7 @@ const initializeEventListeners = () => { const fn = isTestEnv() ? null - : await getFunction({ functionId: functionRun.functionId }) + : await getFunctionFactory({ logger })({ functionId: functionRun.functionId }) const userEmail = await getUserEmailFromAutomationRunFactory({ getFullAutomationRevisionMetadata: getFullAutomationRevisionMetadataFactory({ diff --git a/packages/server/modules/automate/services/authCode.ts b/packages/server/modules/automate/services/authCode.ts index 8c89bc783..cab3950bd 100644 --- a/packages/server/modules/automate/services/authCode.ts +++ b/packages/server/modules/automate/services/authCode.ts @@ -5,6 +5,7 @@ import { EventBus } from '@/modules/shared/services/eventBus' import cryptoRandomString from 'crypto-random-string' import Redis from 'ioredis' import { get, has, isObjectLike } from 'lodash' +import { Logger } from 'pino' export enum AuthCodePayloadAction { CreateAutomation = 'createAutomation', @@ -47,14 +48,14 @@ export const createStoredAuthCodeFactory = } export const validateStoredAuthCodeFactory = - (deps: { redis: Redis; emit: EventBus['emit'] }) => + (deps: { redis: Redis; logger: Logger; emit: EventBus['emit'] }) => async (params: { payload: AuthCodePayload resources?: { workspaceId?: string } }) => { - const { redis, emit } = deps + const { redis, logger, emit } = deps const { payload, resources } = params const potentialPayloadString = await redis.get(payload.code) @@ -63,6 +64,17 @@ export const validateStoredAuthCodeFactory = : null const formattedPayload = isPayload(potentialPayload) ? potentialPayload : null + logger.info( + { + payloadString: potentialPayloadString, + payload: { + ...formattedPayload, + code: null + } + }, + 'Validating execution engine request with provided auth payload.' + ) + if ( !formattedPayload || formattedPayload.code !== payload.code || diff --git a/packages/server/modules/automate/services/automationManagement.ts b/packages/server/modules/automate/services/automationManagement.ts index fe8a8c703..9b11e64ee 100644 --- a/packages/server/modules/automate/services/automationManagement.ts +++ b/packages/server/modules/automate/services/automationManagement.ts @@ -7,9 +7,9 @@ import { getServerOrigin } from '@/modules/shared/helpers/envHelper' import cryptoRandomString from 'crypto-random-string' import { createAutomation as clientCreateAutomation, - getFunction, - getFunctionRelease, - getFunctionReleases + getFunctionFactory, + getFunctionReleaseFactory, + getFunctionReleasesFactory } from '@/modules/automate/clients/executionEngine' import { Automate, Roles, removeNullOrUndefinedKeys } from '@speckle/shared' import { AuthCodePayloadAction } from '@/modules/automate/services/authCode' @@ -139,7 +139,7 @@ export const createAutomationFactory = export type CreateTestAutomationDeps = { getEncryptionKeyPair: GetEncryptionKeyPair - getFunction: typeof getFunction + getFunction: ReturnType storeAutomation: StoreAutomation storeAutomationRevision: StoreAutomationRevision validateStreamAccess: ValidateStreamAccess @@ -356,7 +356,7 @@ const validateNewTriggerDefinitions = } type ValidateNewRevisionFunctionsDeps = { - getFunctionRelease: typeof getFunctionRelease + getFunctionRelease: ReturnType } const validateNewRevisionFunctions = @@ -399,7 +399,7 @@ export type CreateAutomationRevisionDeps = { storeAutomationRevision: StoreAutomationRevision getEncryptionKeyPair: GetEncryptionKeyPair getFunctionInputDecryptor: FunctionInputDecryptor - getFunctionReleases: typeof getFunctionReleases + getFunctionReleases: ReturnType validateStreamAccess: ValidateStreamAccess eventEmit: EventBusEmit } & ValidateNewTriggerDefinitionsDeps & diff --git a/packages/server/modules/automate/services/functionManagement.ts b/packages/server/modules/automate/services/functionManagement.ts index e71ad02ac..907c4255a 100644 --- a/packages/server/modules/automate/services/functionManagement.ts +++ b/packages/server/modules/automate/services/functionManagement.ts @@ -2,7 +2,7 @@ import { CreateFunctionBody, ExecutionEngineFunctionTemplateId, createFunction, - getFunction, + getFunctionFactory, updateFunction as updateExecEngineFunction } from '@/modules/automate/clients/executionEngine' import { @@ -43,7 +43,7 @@ import { speckleAutomateUrl } from '@/modules/shared/helpers/envHelper' import { getFunctionsMarketplaceUrl } from '@/modules/core/helpers/routeHelper' -import { automateLogger } from '@/observability/logging' +import { automateLogger, Logger } from '@/observability/logging' import { CreateStoredAuthCode } from '@/modules/automate/domain/operations' import { GetUser } from '@/modules/core/domain/users/operations' import { noop } from 'lodash' @@ -195,17 +195,18 @@ export const createFunctionFromTemplateFactory = export type UpdateFunctionDeps = { updateFunction: typeof updateExecEngineFunction - getFunction: typeof getFunction + getFunction: ReturnType createStoredAuthCode: CreateStoredAuthCode + logger: Logger } export const updateFunctionFactory = (deps: UpdateFunctionDeps) => async (params: { input: UpdateAutomateFunctionInput; userId: string }) => { - const { updateFunction, createStoredAuthCode } = deps + const { updateFunction, createStoredAuthCode, logger } = deps const { input, userId } = params - const existingFn = await getFunction({ functionId: input.id }) + const existingFn = await getFunctionFactory({ logger })({ functionId: input.id }) if (!existingFn) { throw new AutomateFunctionUpdateError('Function not found') } diff --git a/packages/server/modules/core/graph/dataloaders/index.ts b/packages/server/modules/core/graph/dataloaders/index.ts index f6ed23fd1..d98183a9b 100644 --- a/packages/server/modules/core/graph/dataloaders/index.ts +++ b/packages/server/modules/core/graph/dataloaders/index.ts @@ -71,8 +71,8 @@ import { getRevisionsTriggerDefinitionsFactory } from '@/modules/automate/repositories/automations' import { - getFunction, - getFunctionReleases + getFunctionFactory, + getFunctionReleasesFactory } from '@/modules/automate/clients/executionEngine' import { FunctionReleaseSchemaType, @@ -96,6 +96,7 @@ import { CommitWithStreamBranchId, CommitWithStreamBranchMetadata } from '@/modules/core/domain/commits/types' +import { logger } from '@/observability/logging' declare module '@/modules/core/loaders' { interface ModularizedDataLoaders extends ReturnType {} @@ -618,7 +619,7 @@ const dataLoadersDefinition = defineRequestDataloaders( const results = await Promise.all( fnIds.map(async (fnId) => { try { - return await getFunction({ functionId: fnId }) + return await getFunctionFactory({ logger })({ functionId: fnId }) } catch (e) { const isNotFound = e instanceof ExecutionEngineFailedResponseError && @@ -642,7 +643,7 @@ const dataLoadersDefinition = defineRequestDataloaders( >( async (keys) => { const results = keyBy( - await getFunctionReleases({ + await getFunctionReleasesFactory({ logger })({ ids: keys.map(([fnId, fnReleaseId]) => ({ functionId: fnId, functionReleaseId: fnReleaseId diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 3090fbf10..2204097d5 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -169,7 +169,7 @@ import { listWorkspaceSsoMembershipsFactory } from '@/modules/workspaces/repositories/sso' import { getDecryptor } from '@/modules/workspaces/helpers/sso' -import { getFunctions } from '@/modules/automate/clients/executionEngine' +import { getFunctionsFactory } from '@/modules/automate/clients/executionEngine' import { ExecutionEngineFailedResponseError, ExecutionEngineNetworkError @@ -1093,7 +1093,9 @@ export = FF_WORKSPACES_MODULE_ENABLED action: AuthCodePayloadAction.ListWorkspaceFunctions }) - const res = await getFunctions({ + const res = await getFunctionsFactory({ + logger: context.log + })({ auth: authCode, filters: { query: args.filter?.search ?? undefined, From d0d9d22fe2e1a4a9abea331c4fc954b0b3d86642 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:17:55 +0000 Subject: [PATCH 09/28] chore(server): tidy up maybeLoggerWithContext (#4095) --- packages/server/modules/auth/services/postAuth.ts | 4 ++-- packages/server/modules/automate/index.ts | 8 ++++---- .../components/express/requestContext.ts | 13 +++++-------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/server/modules/auth/services/postAuth.ts b/packages/server/modules/auth/services/postAuth.ts index d903e1013..755e3888c 100644 --- a/packages/server/modules/auth/services/postAuth.ts +++ b/packages/server/modules/auth/services/postAuth.ts @@ -1,5 +1,5 @@ import { authLogger, type Logger } from '@/observability/logging' -import { maybeLoggerWithContext } from '@/observability/components/express/requestContext' +import { loggerWithMaybeContext } from '@/observability/components/express/requestContext' import { addToMailchimpAudience, triggerMailchimpCustomerJourney @@ -16,7 +16,7 @@ import { mixpanel } from '@/modules/shared/utils/mixpanel' const onUserCreatedFactory = () => async (payload: EventPayload) => { - const logger = maybeLoggerWithContext({ logger: authLogger })! + const logger = loggerWithMaybeContext({ logger: authLogger }) const { user, signUpCtx } = payload.payload try { diff --git a/packages/server/modules/automate/index.ts b/packages/server/modules/automate/index.ts index 0ce22f83a..18ce5b0fe 100644 --- a/packages/server/modules/automate/index.ts +++ b/packages/server/modules/automate/index.ts @@ -58,7 +58,7 @@ import { getEventBus } from '@/modules/shared/services/eventBus' import { VersionEvents } from '@/modules/core/domain/commits/events' import { AutomationEvents, AutomationRunEvents } from '@/modules/automate/domain/events' import { LogicError } from '@/modules/shared/errors' -import { maybeLoggerWithContext } from '@/observability/components/express/requestContext' +import { loggerWithMaybeContext } from '@/observability/components/express/requestContext' const { FF_AUTOMATE_MODULE_ENABLED } = getFeatureFlags() let quitListeners: Optional<() => void> = undefined @@ -177,7 +177,7 @@ const initializeEventListeners = () => { getEventBus().listen( AutomationRunEvents.Created, async ({ payload: { manifests, run, automation } }) => { - const logger = maybeLoggerWithContext({ logger: automateLogger })! + const logger = loggerWithMaybeContext({ logger: automateLogger }) const validatedManifests = manifests .map((manifest) => { if (isVersionCreatedTriggerManifest(manifest)) { @@ -267,7 +267,7 @@ const initializeEventListeners = () => { AutomationRunEvents.StatusUpdated, async ({ payload: { run, functionRun, automationId, projectId } }) => { if (!isFinished(run.status)) return - const logger = maybeLoggerWithContext({ logger: automateLogger })! + const logger = loggerWithMaybeContext({ logger: automateLogger }) const projectDb = await getProjectDbClient({ projectId }) const project = await getProjectFactory({ db: projectDb })({ projectId }) @@ -321,7 +321,7 @@ const initializeEventListeners = () => { getEventBus().listen( AutomationRunEvents.Created, async ({ payload: { automation, run: automationRun, source, manifests } }) => { - const logger = maybeLoggerWithContext({ logger: automateLogger })! + const logger = loggerWithMaybeContext({ logger: automateLogger }) const manifest = manifests.at(0) if (!manifest || !isVersionCreatedTriggerManifest(manifest)) { logger.error( diff --git a/packages/server/observability/components/express/requestContext.ts b/packages/server/observability/components/express/requestContext.ts index 93b175445..e87c63691 100644 --- a/packages/server/observability/components/express/requestContext.ts +++ b/packages/server/observability/components/express/requestContext.ts @@ -40,14 +40,11 @@ export const enterNewRequestContext = (params: { reqId: string }) => { export const getRequestContext = () => storage?.getStore() -export const maybeLoggerWithContext = ({ logger }: { logger?: Logger }) => { +export const loggerWithMaybeContext = ({ logger }: { logger: Logger }) => { const reqCtx = getRequestContext() - return logger?.child({ - ...(reqCtx - ? { - req: { id: reqCtx.requestId }, - dbMetrics: reqCtx.dbMetrics - } - : {}) + if (!reqCtx) return logger + return logger.child({ + req: { id: reqCtx.requestId }, + dbMetrics: reqCtx.dbMetrics }) } From 76623529a5d83b88f68ba0ff0897aef49921321a Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:18:33 +0000 Subject: [PATCH 10/28] tests(server/tests): convert modules/core/tests/graph.spec to typescript (#4096) --- .../tests/{graph.spec.js => graph.spec.ts} | 244 +++++++++++------- packages/server/test/helpers.js | 88 ------- packages/server/test/helpers.ts | 112 ++++++++ 3 files changed, 264 insertions(+), 180 deletions(-) rename packages/server/modules/core/tests/{graph.spec.js => graph.spec.ts} (93%) delete mode 100644 packages/server/test/helpers.js create mode 100644 packages/server/test/helpers.ts diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.ts similarity index 93% rename from packages/server/modules/core/tests/graph.spec.js rename to packages/server/modules/core/tests/graph.spec.ts index 249e2bc86..b7a9b7600 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/packages/server/modules/core/tests/graph.spec.ts @@ -1,26 +1,26 @@ /* istanbul ignore file */ -const expect = require('chai').expect -const request = require('supertest') +import { expect } from 'chai' +import request from 'supertest' -const { beforeEachContext, initializeTestServer } = require(`@/test/hooks`) -const { generateManyObjects } = require(`@/test/helpers`) +import { beforeEachContext, initializeTestServer } from '@/test/hooks' +import { generateManyObjects } from '@/test/helpers' -const { Roles, Scopes } = require('@speckle/shared') -const cryptoRandomString = require('crypto-random-string') -const { db } = require('@/db/knex') -const { +import { Roles, Scopes } from '@speckle/shared' +import cryptoRandomString from 'crypto-random-string' +import { db } from '@/db/knex' +import { validateStreamAccessFactory, isStreamCollaboratorFactory, removeStreamCollaboratorFactory, addOrUpdateStreamCollaboratorFactory -} = require('@/modules/core/services/streams/access') -const { authorizeResolver } = require('@/modules/shared') -const { +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' +import { getStreamFactory, revokeStreamPermissionsFactory, grantStreamPermissionsFactory -} = require('@/modules/core/repositories/streams') -const { +} from '@/modules/core/repositories/streams' +import { getUserFactory, legacyGetPaginatedUsersFactory, storeUserFactory, @@ -28,43 +28,38 @@ const { storeUserAclFactory, isLastAdminUserFactory, updateUserServerRoleFactory -} = require('@/modules/core/repositories/users') -const { +} from '@/modules/core/repositories/users' +import { findEmailFactory, createUserEmailFactory, ensureNoPrimaryEmailForUserFactory -} = require('@/modules/core/repositories/userEmails') -const { - requestNewEmailVerificationFactory -} = require('@/modules/emails/services/verification/request') -const { - deleteOldAndInsertNewVerificationFactory -} = require('@/modules/emails/repositories') -const { renderEmail } = require('@/modules/emails/services/emailRendering') -const { sendEmail } = require('@/modules/emails/services/sending') -const { +} from '@/modules/core/repositories/userEmails' +import { requestNewEmailVerificationFactory } from '@/modules/emails/services/verification/request' +import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' +import { renderEmail } from '@/modules/emails/services/emailRendering' +import { sendEmail } from '@/modules/emails/services/sending' +import { createUserFactory, changeUserRoleFactory -} = require('@/modules/core/services/users/management') -const { - validateAndCreateUserEmailFactory -} = require('@/modules/core/services/userEmails') -const { - finalizeInvitedServerRegistrationFactory -} = require('@/modules/serverinvites/services/processing') -const { +} from '@/modules/core/services/users/management' +import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' +import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' +import { deleteServerOnlyInvitesFactory, updateAllInviteTargetsFactory -} = require('@/modules/serverinvites/repositories/serverInvites') -const { createPersonalAccessTokenFactory } = require('@/modules/core/services/tokens') -const { +} from '@/modules/serverinvites/repositories/serverInvites' +import { createPersonalAccessTokenFactory } from '@/modules/core/services/tokens' +import { storeApiTokenFactory, storeTokenScopesFactory, storeTokenResourceAccessDefinitionsFactory, storePersonalApiTokenFactory -} = require('@/modules/core/repositories/tokens') -const { getServerInfoFactory } = require('@/modules/core/repositories/server') -const { getEventBus } = require('@/modules/shared/services/eventBus') +} from '@/modules/core/repositories/tokens' +import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { getEventBus } from '@/modules/shared/services/eventBus' +import { Express } from 'express' +import { Server } from 'http' +import { omit } from 'lodash' const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) @@ -125,9 +120,9 @@ const createPersonalAccessToken = createPersonalAccessTokenFactory({ storePersonalApiToken: storePersonalApiTokenFactory({ db }) }) -let app -let server -let sendRequest +let app: Express +let server: Server +let sendRequest: Awaited>['sendRequest'] const changeUserRole = changeUserRoleFactory({ getServerInfo, @@ -137,16 +132,22 @@ const changeUserRole = changeUserRoleFactory({ describe('GraphQL API Core @core-api', () => { const userA = { + id: '', + token: '', name: 'd1', email: 'd.1@speckle.systems', password: 'wowwowwowwowwow' } const userB = { + id: '', + token: '', name: 'd2', email: 'd.2@speckle.systems', password: 'wowwowwowwowwow' } const userC = { + id: '', + token: '', name: 'd3', email: 'd.3@speckle.systems', password: 'wowwowwowwowwow' @@ -263,30 +264,64 @@ describe('GraphQL API Core @core-api', () => { }) // the stream ids - let ts1 - let ts2 - let ts3 - let ts4 - let ts5 - let ts6 + let ts1: string + let ts2: string + let ts3: string + let ts4: string + let ts5: string + let ts6: string // some api tokens - let token1 - let token2 - let token3 + let token1: string + let token2: string + let token3: string // object ids - let objIds + let objIds: string[] // some commits - const c1 = {} - const c2 = {} + const c1 = { + id: '', + message: '', + streamId: '', + objectId: '', + branchName: '', + previousCommitIds: [] + } + const c2 = { + id: '', + message: '', + streamId: '', + objectId: '', + branchName: '', + previousCommitIds: new Array() + } // some branches - let b1 = {} - let b2 = {} - let b3 = {} - let b4 = {} + let b1 = { + id: '', + streamId: '', + name: '', + description: '' + } + let b2 = { + id: '', + streamId: '', + name: '', + description: '' + } + let b3 = { + id: '', + streamId: '', + name: '', + description: '' + } + let b4 = { + id: '', + streamId: '', + name: '', + description: '' + } describe('Mutations', () => { describe('Users & Api tokens', () => { @@ -358,6 +393,8 @@ describe('GraphQL API Core @core-api', () => { it('Should delete my account', async () => { const userDelete = { + id: '', + token: '', name: 'delete', email: `${cryptoRandomString({ length: 10 })}@example.org`, password: 'wowwowwowwowwow' @@ -468,6 +505,7 @@ describe('GraphQL API Core @core-api', () => { describe('User deletion', () => { it('Only admins can delete user', async () => { const userDelete = { + id: '', name: 'delete', email: `${cryptoRandomString({ length: 10 })}@example.org`, password: 'wowwowwowwowwow' @@ -484,6 +522,7 @@ describe('GraphQL API Core @core-api', () => { it('Admin can delete user', async () => { const userDelete = { + id: '', name: 'delete', email: 'd3l3t3@speckle.systems', password: 'wowwowwowwowwow' @@ -572,7 +611,7 @@ describe('GraphQL API Core @core-api', () => { expect(res).to.be.json expect(res.body.errors).to.be.ok expect(res.body.data.streamUpdatePermission).to.be.not.ok - expect(res.body.errors.map((e) => e.message).join('|')).to.contain( + expect(res.body.errors.map((e: Error) => e.message).join('|')).to.contain( "Cannot grant permissions to users who aren't collaborators already" ) }) @@ -763,16 +802,18 @@ describe('GraphQL API Core @core-api', () => { query: '{ adminStreams( visibility: "private" ) { totalCount items { id name isPublic } } }' }) - expect(streamResults.body.data.adminStreams.items).to.satisfy((streams) => - streams.every((stream) => !stream.isPublic) + expect(streamResults.body.data.adminStreams.items).to.satisfy( + (streams: { isPublic: boolean }[]) => + streams.every((stream) => !stream.isPublic) ) streamResults = await sendRequest(userA.token, { query: '{ adminStreams( visibility: "public" ) { totalCount items { id name isPublic } } }' }) - expect(streamResults.body.data.adminStreams.items).to.satisfy((streams) => - streams.every((stream) => stream.isPublic) + expect(streamResults.body.data.adminStreams.items).to.satisfy( + (streams: { isPublic: boolean }[]) => + streams.every((stream) => stream.isPublic) ) }) @@ -782,7 +823,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(streamResults.body.data.adminStreams.totalCount).to.equal(5) const streamIds = streamResults.body.data.adminStreams.items.map( - (stream) => stream.id + (stream: { id: string }) => stream.id ) const res = await sendRequest(userA.token, { query: 'mutation ( $ids: [String!] ){ streamsDelete( ids: $ids )}', @@ -848,11 +889,11 @@ describe('GraphQL API Core @core-api', () => { let res = await sendRequest(userA.token, { query: 'mutation( $myCommit: CommitCreateInput! ) { commitCreate( commit: $myCommit ) }', - variables: { myCommit: c1 } + variables: { myCommit: omit(c1, 'id') } }) expect(res).to.be.json - expect(res.body.errors).to.not.exist + expect(res.body.errors, JSON.stringify(res.body.errors)).to.not.exist expect(res.body.data).to.have.property('commitCreate') expect(res.body.data.commitCreate).to.be.a('string') c1.id = res.body.data.commitCreate @@ -866,10 +907,10 @@ describe('GraphQL API Core @core-api', () => { res = await sendRequest(userA.token, { query: 'mutation( $myCommit: CommitCreateInput! ) { commitCreate( commit: $myCommit ) }', - variables: { myCommit: c2 } + variables: { myCommit: omit(c2, 'id') } }) expect(res).to.be.json - expect(res.body.errors).to.not.exist + expect(res.body.errors, JSON.stringify(res.body.errors)).to.not.exist expect(res.body.data).to.have.property('commitCreate') expect(res.body.data.commitCreate).to.be.a('string') @@ -961,6 +1002,7 @@ describe('GraphQL API Core @core-api', () => { it('Should create several branches', async () => { b1 = { + id: '', streamId: ts1, name: 'dim/dev', description: 'dimitries development branch' @@ -969,15 +1011,16 @@ describe('GraphQL API Core @core-api', () => { const res1 = await sendRequest(userA.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b1 } + variables: { branch: omit(b1, 'id') } }) expect(res1).to.be.json - expect(res1.body.errors).to.not.exist + expect(res1.body.errors, JSON.stringify(res1.body.errors)).to.not.exist expect(res1.body.data).to.have.property('branchCreate') expect(res1.body.data.branchCreate).to.be.a('string') b1.id = res1.body.data.branchCreate b2 = { + id: '', streamId: ts1, name: 'dim/dev/api-surgery', description: 'another branch' @@ -992,12 +1035,13 @@ describe('GraphQL API Core @core-api', () => { const res2 = await sendRequest(userB.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b2 } + variables: { branch: omit(b2, 'id') } }) - expect(res2.body.errors).to.not.exist + expect(res2.body.errors, JSON.stringify(res2.body.errors)).to.not.exist b2.id = res2.body.data.branchCreate b3 = { + id: '', streamId: ts1, name: 'userB/dev/api', description: 'more branches branch' @@ -1005,14 +1049,15 @@ describe('GraphQL API Core @core-api', () => { const res3 = await sendRequest(userB.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b3 } + variables: { branch: omit(b3, 'id') } }) - expect(res3.body.errors).to.not.exist + expect(res3.body.errors, JSON.stringify(res3.body.errors)).to.not.exist b3.id = res3.body.data.branchCreate }) it('Should update a branch', async () => { const b1 = { + id: '', streamId: ts1, name: 'randomupdateablebranch', description: 'dimitries development branch' @@ -1020,8 +1065,10 @@ describe('GraphQL API Core @core-api', () => { const b1Res = await sendRequest(userA.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b1 } + variables: { branch: omit(b1, 'id') } }) + expect(b1Res).to.be.json + expect(b1Res.body.errors, JSON.stringify(b1Res.body)).to.not.exist b1.id = b1Res.body.data.branchCreate const payload = { @@ -1043,6 +1090,7 @@ describe('GraphQL API Core @core-api', () => { it('Should delete a branch', async () => { const b1 = { + id: '', streamId: ts1, name: 'randomudeletablebranch', description: 'dimitries development branch' @@ -1050,8 +1098,10 @@ describe('GraphQL API Core @core-api', () => { const b1Res = await sendRequest(userA.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b1 } + variables: { branch: omit(b1, 'id') } }) + expect(b1Res).to.be.json + expect(b1Res.body.errors, JSON.stringify(b1Res.body.errors)).to.not.exist b1.id = b1Res.body.data.branchCreate // give C some access permissions @@ -1109,11 +1159,12 @@ describe('GraphQL API Core @core-api', () => { }) it('Should commit to a non-main branch as well...', async () => { - const cc = {} - cc.message = 'what a message for a second commit' - cc.streamId = ts1 - cc.objectId = objIds[3] - cc.branchName = 'userB/dev/api' + const cc = { + message: 'what a message for a second commit', + streamId: ts1, + objectId: objIds[3], + branchName: 'userB/dev/api' + } const res = await sendRequest(userB.token, { query: @@ -1121,7 +1172,7 @@ describe('GraphQL API Core @core-api', () => { variables: { myCommit: cc } }) expect(res).to.be.json - expect(res.body.errors).to.not.exist + expect(res.body.errors, JSON.stringify(res.body.errors)).to.not.exist expect(res.body.data).to.have.property('commitCreate') expect(res.body.data.commitCreate).to.be.a('string') }) @@ -1132,10 +1183,13 @@ describe('GraphQL API Core @core-api', () => { query: 'mutation { streamCreate(stream: { name: "TS (u C) private", description: "sup my dudes", isPublic:false } ) }' }) + expect(res).to.be.json + expect(res.body.errors, JSON.stringify(res.body.errors)).to.not.exist ts6 = res.body.data.streamCreate // user B creates branch on private stream b4 = { + id: '', streamId: ts3, name: 'izz/secret', description: 'a private branch on a private stream' @@ -1143,10 +1197,10 @@ describe('GraphQL API Core @core-api', () => { const res1 = await sendRequest(userB.token, { query: 'mutation( $branch:BranchCreateInput! ) { branchCreate( branch:$branch ) }', - variables: { branch: b4 } + variables: { branch: omit(b4, 'id') } }) expect(res1).to.be.json - expect(res1.body.errors).to.not.exist + expect(res1.body.errors, JSON.stringify(res1.body.errors)).to.not.exist expect(res1.body.data).to.have.property('branchCreate') expect(res1.body.data.branchCreate).to.be.a('string') b4.id = res1.body.data.branchCreate @@ -1241,7 +1295,9 @@ describe('GraphQL API Core @core-api', () => { expect(res2.body.data.user.streams.items.length).to.equal(3) const streams = res2.body.data.user.streams.items - const s1 = streams.find((s) => s.name === 'TS1 (u A) Private UPDATED') + const s1 = streams.find( + (s: { name: string }) => s.name === 'TS1 (u A) Private UPDATED' + ) expect(s1).to.exist }) @@ -1427,9 +1483,11 @@ describe('GraphQL API Core @core-api', () => { expect(stream.name).to.equal('TS1 (u A) Private UPDATED') expect(stream.collaborators).to.have.lengthOf(2) - const d2User = stream.collaborators.find((c) => c.name === 'd2') + const d2User = stream.collaborators.find( + (c: { name: string }) => c.name === 'd2' + ) const testUserUpdated = stream.collaborators.find( - (c) => c.name === 'test user updated' + (c: { name: string }) => c.name === 'test user updated' ) expect(d2User).to.be.ok expect(testUserUpdated).to.be.ok @@ -1556,7 +1614,7 @@ describe('GraphQL API Core @core-api', () => { ) }) - let commitList + let commitList: { id: string }[] it('should retrieve all stream commits', async () => { const query = ` @@ -1633,8 +1691,8 @@ describe('GraphQL API Core @core-api', () => { }) describe('Objects', () => { - let myCommit - let myObjs + let myCommit: { id: string; name: string } + let myObjs: { name: string }[] before(async () => { const { commit, objs } = generateManyObjects(100, 'noise__') @@ -1656,7 +1714,7 @@ describe('GraphQL API Core @core-api', () => { expect(objIds.length).to.equal(101) // +1 for the actual "commit" object }) - it("should get an object's subojects objects", async () => { + it("should get an object's sub-objects' objects", async () => { const first = await sendRequest(userA.token, { query: ` query { @@ -1846,11 +1904,13 @@ describe('GraphQL API Core @core-api', () => { describe('Archived role access validation', () => { const archivedUser = { + id: '', + token: '', name: 'Mark von Archival', email: 'archi@speckle.systems', password: 'i"ll be back, just wait' } - let streamId + let streamId: string before(async () => { archivedUser.id = await createUser(archivedUser) archivedUser.token = `Bearer ${await createPersonalAccessToken( diff --git a/packages/server/test/helpers.js b/packages/server/test/helpers.js deleted file mode 100644 index f89f4c536..000000000 --- a/packages/server/test/helpers.js +++ /dev/null @@ -1,88 +0,0 @@ -/* istanbul ignore file */ -// const { logger } = require('@/observability/logging') -const crypto = require('crypto') - -function generateManyObjects(shitTon, noise) { - shitTon = shitTon || 10000 - noise = noise || Math.random() * 100 - - const objs = [] - - const base = { name: 'base bastard 2', noise, __closure: {} } - // objs.push( base ) - let k = 0 - - for (let i = 0; i < shitTon; i++) { - const baby = { - name: `mr. ${i}`, - nest: { duck: i % 2 === 0, mallard: 'falsey', arr: [i + 42, i, i] }, - test: { value: i, secondValue: 'mallard ' + (i % 10) }, - similar: k, - even: i % 2 === 0, - objArr: [{ a: i }, { b: i * i }, { c: true }], - noise, - sortValueA: i, - sortValueB: i * 0.42 * i - } - if (i % 3 === 0) k++ - - getAnIdForThisOnePlease(baby) - - base.__closure[baby.id] = 1 - - objs.push(baby) - } - - getAnIdForThisOnePlease(base) - return { commit: base, objs } -} - -function createManyObjects(num, noise) { - num = num || 10000 - noise = noise || Math.random() * 100 - - const objs = [] - - const base = { name: 'base bastard 2', noise, __closure: {} } - objs.push(base) - - for (let i = 0; i < num; i++) { - const baby = { - name: `mr. ${i}`, - nest: { duck: i % 2 === 0, mallard: 'falsey', arr: [i + 42, i, i] } - } - getAnIdForThisOnePlease(baby) - base.__closure[baby.id] = 1 - objs.push(baby) - } - getAnIdForThisOnePlease(base) - return objs -} - -exports.createManyObjects = createManyObjects - -function getAnIdForThisOnePlease(obj) { - obj.id = obj.id || crypto.createHash('md5').update(JSON.stringify(obj)).digest('hex') -} - -exports.generateManyObjects = generateManyObjects -exports.getAnIdForThisOnePlease = getAnIdForThisOnePlease - -exports.sleep = (ms) => { - // logger.debug( `\t Sleeping ${ms}ms ` ) - return new Promise((resolve) => { - setTimeout(resolve, ms) - }) -} - -/** - * Checks the response body for errors. To be used in expect assertions. - * Will throw an error if 'errors' exist. - * @param {*} res - */ -function noErrors(res) { - if (res.error) throw new Error(`Failed GraphQL request: ${JSON.stringify(res.error)}`) - if ('errors' in res.body) - throw new Error(`Failed GraphQL request: ${JSON.stringify(res.body.errors)}`) -} -exports.noErrors = noErrors diff --git a/packages/server/test/helpers.ts b/packages/server/test/helpers.ts new file mode 100644 index 000000000..03bc7d369 --- /dev/null +++ b/packages/server/test/helpers.ts @@ -0,0 +1,112 @@ +import crypto from 'crypto' +import { get } from 'lodash' + +/** + * Generates an object containing the base object and an array of objects with an id. The base object will have a closure property which references all the other objects. + * @description Differs from createManyObjects in that it returns an object with a 'commit' property (the base object) and a separate 'objs' property (an array of children objects). It also adds more properties to the objects. + * @param shitTon the number of objects to generate + * @param noise Any data to be added to the objects. Defaults to a random number between 0 and 100, inclusive + * @returns An object. The 'commit' property is the base object with a closure property which references all the other ('children') objects. The 'objs' property is an array of children objects. + */ +export function generateManyObjects(shitTon: number, noise?: unknown) { + shitTon = shitTon || 10000 + noise = noise || Math.random() * 100 + + const objs = [] + + const base: { + id?: string + name: string + noise: unknown + __closure: Record + } = { name: 'base bastard 2', noise, __closure: {} } + let k = 0 + + for (let i = 0; i < shitTon; i++) { + const baby = { + name: `mr. ${i}`, + nest: { duck: i % 2 === 0, mallard: 'falsey', arr: [i + 42, i, i] }, + test: { value: i, secondValue: 'mallard ' + (i % 10) }, + similar: k, + even: i % 2 === 0, + objArr: [{ a: i }, { b: i * i }, { c: true }], + noise, + sortValueA: i, + sortValueB: i * 0.42 * i + } + if (i % 3 === 0) k++ + + if (!getAnIdForThisOnePlease(baby)) continue //this will never be true, but typescript now knows baby definitely has an id + + base.__closure[baby.id] = 1 + + objs.push(baby) + } + + if (!getAnIdForThisOnePlease(base)) throw new Error('base object has no id') //this will never be true, but typescript now knows base definitely has an id + return { commit: base, objs } +} + +/** + * Generates a bunch of objects with an id. The first object in the array will have a closure property which references all the other objects. + * @description Differs from generateManyObjects in that it returns an array of objects, including a base object (at index 0). + * @param num The number of objects to create. + * @param noise Any arbitrary data which will be added to the objects. Defaults to a random number between 0 and 100, inclusive. + * @returns An array of objects, including a base object (at index 0) with a closure property which references all the other objects. + */ +export function createManyObjects(num: number, noise?: unknown) { + num = num || 10000 + noise = noise || Math.random() * 100 + + const objs = [] + + const base: { + __closure: Record + } & Record = { name: 'base bastard 2', noise, __closure: {} } + + for (let i = 0; i < num; i++) { + const baby: Record = { + name: `mr. ${i}`, + nest: { duck: i % 2 === 0, mallard: 'falsey', arr: [i + 42, i, i] } + } + + if (!getAnIdForThisOnePlease(baby)) continue //this will never be true, but typescript now knows baby definitely has an id + base.__closure[baby.id] = 1 + objs.push(baby) + } + if (!getAnIdForThisOnePlease(base)) return objs //this will never be true, but typescript now knows base definitely has an id + objs.unshift(base) + return objs +} + +/** + * Adds an 'id' property to an object if it doesn't already have one. The 'id' is a hash (md5) of the object. + * @param obj This object is passed by reference and will be modified + * @returns true. This is a hack to make typescript happy and eliminate the 'undefined' type from 'id' + */ +export function getAnIdForThisOnePlease( + obj: Record +): obj is Record<'id', string> & Record { + obj.id = obj.id || crypto.createHash('md5').update(JSON.stringify(obj)).digest('hex') + return true //HACK to make typescript happy and eliminate the 'undefined' type from 'id' +} + +export const sleep = (ms: number) => { + // logger.debug( `\t Sleeping ${ms}ms ` ) + return new Promise((resolve) => { + setTimeout(resolve, ms) + }) +} + +/** + * Checks the response body for errors. To be used in expect assertions. + * Will throw an error if 'errors' exist. + * @param {*} res + */ +export function noErrors(res: unknown) { + const e = get(res, 'error') + if (e) throw new Error(`Failed GraphQL request: ${JSON.stringify(e)}`) + const bodyErrors = get(res, 'body.errors') + if (bodyErrors) + throw new Error(`Failed GraphQL request: ${JSON.stringify(bodyErrors)}`) +} From 9b7c56de9f907993e56c038e9004ec0680cc47f9 Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Mon, 3 Mar 2025 09:19:22 +0000 Subject: [PATCH 11/28] fix(workspaces): emit role updated on join request approved (#4100) --- .../modules/workspaces/services/workspaceJoinRequests.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/server/modules/workspaces/services/workspaceJoinRequests.ts b/packages/server/modules/workspaces/services/workspaceJoinRequests.ts index f6d4f84cc..7b792d108 100644 --- a/packages/server/modules/workspaces/services/workspaceJoinRequests.ts +++ b/packages/server/modules/workspaces/services/workspaceJoinRequests.ts @@ -147,6 +147,10 @@ export const approveWorkspaceJoinRequestFactory = await upsertWorkspaceRole({ userId, workspaceId, role, createdAt: new Date() }) await emit({ eventName: WorkspaceEvents.Updated, payload: { workspace } }) + await emit({ + eventName: WorkspaceEvents.RoleUpdated, + payload: { workspaceId, userId, role } + }) await sendWorkspaceJoinRequestApprovedEmail({ workspace, From be556c76d9f7c215c2bb088169b615475d47949b Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 3 Mar 2025 09:19:43 +0000 Subject: [PATCH 12/28] chore(devcontainers): Move post create command to a script (#4090) - it can be linted and has IDE code-completion - we can add a lot more things to the script without sacrificing readability --- .devcontainer/devcontainer.json | 2 +- .devcontainer/postCreateCommand.sh | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100755 .devcontainer/postCreateCommand.sh diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 5f65b7580..418edea78 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -31,7 +31,7 @@ ], // Use 'postCreateCommand' to run commands after the container is created. - "postCreateCommand": "yarn && yarn build:public && cp -n /workspaces/${localWorkspaceFolderBasename}/packages/server/.env-example /workspaces/${localWorkspaceFolderBasename}/packages/server/.env && cp -n /workspaces/${localWorkspaceFolderBasename}/packages/frontend-2/.env.example /workspaces/${localWorkspaceFolderBasename}/packages/frontend-2/.env", + "postCreateCommand": "/workspaces/${localWorkspaceFolderBasename}/.devcontainer/postCreateCommand.sh", // Configure tool-specific properties. // "customizations": {}, diff --git a/.devcontainer/postCreateCommand.sh b/.devcontainer/postCreateCommand.sh new file mode 100755 index 000000000..7b8817014 --- /dev/null +++ b/.devcontainer/postCreateCommand.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -eox pipefail + +echo "Running postCreateCommand.sh" + +# determine where the script is located, navigate into that directory, then find the root of the git repo in which it is located +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) +cd "${SCRIPT_DIR}" +GIT_ROOT="$(git rev-parse --show-toplevel)" + +echo "Setting up environment variables by copying .env files" +cp -n "${GIT_ROOT}/packages/server/.env-example" "${GIT_ROOT}/packages/server/.env" || true +cp -n "${GIT_ROOT}/packages/frontend-2/.env.example" "${GIT_ROOT}/packages/frontend-2/.env" || true + +echo "Installing nodejs dependencies and building shared packages" +yarn +yarn build:public From 0a77270da1a63d8a4f2eb7c6bde68645571dc435 Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Mon, 3 Mar 2025 09:19:59 +0000 Subject: [PATCH 13/28] fix(automate): fix search param parsing (#4099) --- packages/server/modules/automate/clients/executionEngine.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/server/modules/automate/clients/executionEngine.ts b/packages/server/modules/automate/clients/executionEngine.ts index 3f0349894..1248629ec 100644 --- a/packages/server/modules/automate/clients/executionEngine.ts +++ b/packages/server/modules/automate/clients/executionEngine.ts @@ -64,7 +64,8 @@ const getApiUrl = ( const url = new URL(path, automateUrl) if (options?.query) { Object.entries(options.query).forEach(([key, val]) => { - if (isEmpty(val) || isNullOrUndefined(val)) return + if (isNullOrUndefined(val)) return + if (typeof val === 'object' && isEmpty(val)) return try { const urlValue = typeof val === 'object' ? val.join(',') : val.toString() url.searchParams.append(key, urlValue) From 4d60e5e42b2d4b7bc29081efecde31d02ba662b5 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:50:43 +0000 Subject: [PATCH 14/28] feat(feature flag): add workspacesNewPlanEnabled (#4101) --- utils/helm/speckle-server/templates/_helpers.tpl | 3 +++ .../helm/speckle-server/templates/frontend_2/deployment.yml | 2 ++ utils/helm/speckle-server/values.schema.json | 5 +++++ utils/helm/speckle-server/values.yaml | 2 ++ 4 files changed, 12 insertions(+) diff --git a/utils/helm/speckle-server/templates/_helpers.tpl b/utils/helm/speckle-server/templates/_helpers.tpl index 989f3d76a..3e2f02a3e 100644 --- a/utils/helm/speckle-server/templates/_helpers.tpl +++ b/utils/helm/speckle-server/templates/_helpers.tpl @@ -569,6 +569,9 @@ Generate the environment variables for Speckle server and Speckle objects deploy - name: FF_WORKSPACES_SSO_ENABLED value: {{ .Values.featureFlags.workspacesSSOEnabled | quote }} +- name: FF_WORKSPACES_NEW_PLAN_ENABLED + value: {{ .Values.featureFlags.workspacesNewPlanEnabled | quote }} + {{- if .Values.featureFlags.workspacesModuleEnabled }} - name: LICENSE_TOKEN valueFrom: diff --git a/utils/helm/speckle-server/templates/frontend_2/deployment.yml b/utils/helm/speckle-server/templates/frontend_2/deployment.yml index 638412dd1..21845f964 100644 --- a/utils/helm/speckle-server/templates/frontend_2/deployment.yml +++ b/utils/helm/speckle-server/templates/frontend_2/deployment.yml @@ -140,6 +140,8 @@ spec: value: {{ .Values.featureFlags.forceOnboarding | quote }} - name: NUXT_PUBLIC_FF_NO_PERSONAL_EMAILS_ENABLED value: {{ .Values.featureFlags.noPersonalEmailsEnabled | quote }} + - name: NUXT_PUBLIC_FF_WORKSPACES_NEW_PLAN_ENABLED + value: {{ .Values.featureFlags.workspacesNewPlanEnabled | quote }} {{- if .Values.analytics.survicate_workspace_key }} - name: NUXT_PUBLIC_SURVICATE_WORKSPACE_KEY value: {{ .Values.analytics.survicate_workspace_key | quote }} diff --git a/utils/helm/speckle-server/values.schema.json b/utils/helm/speckle-server/values.schema.json index 914d0f6a2..f11ef137f 100644 --- a/utils/helm/speckle-server/values.schema.json +++ b/utils/helm/speckle-server/values.schema.json @@ -94,6 +94,11 @@ "type": "boolean", "description": "Disables the ability sign up with personal email addresses", "default": false + }, + "workspacesNewPlanEnabled": { + "type": "boolean", + "description": "Toggles whether the new (Q1 2025) plans for workspaces are available. workspacesModuleEnabled must also be enabled for this to take effect.", + "default": false } } }, diff --git a/utils/helm/speckle-server/values.yaml b/utils/helm/speckle-server/values.yaml index 8593c99d1..e2a598542 100644 --- a/utils/helm/speckle-server/values.yaml +++ b/utils/helm/speckle-server/values.yaml @@ -59,6 +59,8 @@ featureFlags: forceOnboarding: false ## @param featureFlags.noPersonalEmailsEnabled Disables the ability sign up with personal email addresses noPersonalEmailsEnabled: false + ## @param featureFlags.workspacesNewPlanEnabled Toggles whether the new (Q1 2025) plans for workspaces are available. workspacesModuleEnabled must also be enabled for this to take effect. + workspacesNewPlanEnabled: false analytics: ## @param analytics.enabled Enable or disable analytics From 876a0ee217a90195113a28be7c8ac1cad33e2689 Mon Sep 17 00:00:00 2001 From: andrewwallacespeckle <139135120+andrewwallacespeckle@users.noreply.github.com> Date: Mon, 3 Mar 2025 13:05:07 +0000 Subject: [PATCH 15/28] feat(fe2): Create/Join Workspace as part of signup flow (#3997) * New middleware. New page structure * Changes from designs * New workspace creation flow * FF Hide SSO * No middleware with no FF * When to show join * Update Join description text based on count * Use new FF * Major changes * Update join text * New FF in middleware * Discoverable Banners * Fix cache warning * Undo merge conflict * Revert merge conflicts * Remove unneeded change * Rename * Revert merge issues * Fix error * Remove FF * Check workspaces is enabled * Use FF to show old onboarding flow * Remove unused FF * Fixes from PR * Remove Region & SSO * Revert workspace wizard changes * WorkspaceDiscoverableWorkspacesCard * Remove old code that was hidden with FF * Fix * Changes from call with Mike * Fix typo * Fix typo * Update JoinPage.vue --------- Co-authored-by: Mike Tasset --- .../components/onboarding/JoinTeammates.vue | 100 ---------- .../components/onboarding/questions/Form.vue | 10 +- .../components/projects/DashboardHeader.vue | 24 ++- .../settings/workspaces/regions/Select.vue | 2 +- .../components/workspace/CreatePage.vue | 29 ++- .../components/workspace/JoinPage.vue | 84 +++++++++ .../workspace/discoverableWorkspaces/Card.vue | 54 ++++++ .../invite/DiscoverableWorkspaceBanner.vue | 80 ++------ .../workspace/wizard/step/Details.vue | 5 +- .../components/workspace/wizard/step/Step.vue | 6 +- .../lib/auth/composables/activeUser.ts | 28 ++- .../lib/common/generated/gql/gql.ts | 54 +++--- .../lib/common/generated/gql/graphql.ts | 46 +++-- .../frontend-2/lib/common/helpers/route.ts | 5 +- .../lib/onboarding/graphql/queries.ts | 10 - .../composables/discoverableWorkspaces.ts | 175 ++++++++++++++++++ .../lib/workspaces/graphql/queries.ts | 18 ++ .../middleware/005-onboarding.global.ts | 18 +- .../middleware/006-workspace.global.ts | 59 ++++++ packages/frontend-2/pages/onboarding.vue | 70 +------ packages/frontend-2/pages/verify-email.vue | 4 +- packages/frontend-2/pages/workspaces/join.vue | 15 ++ .../src/components/form/RadioGroup.stories.ts | 10 + .../src/components/form/RadioGroup.vue | 76 ++++---- 24 files changed, 634 insertions(+), 348 deletions(-) delete mode 100644 packages/frontend-2/components/onboarding/JoinTeammates.vue create mode 100644 packages/frontend-2/components/workspace/JoinPage.vue create mode 100644 packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue delete mode 100644 packages/frontend-2/lib/onboarding/graphql/queries.ts create mode 100644 packages/frontend-2/lib/workspaces/composables/discoverableWorkspaces.ts create mode 100644 packages/frontend-2/middleware/006-workspace.global.ts create mode 100644 packages/frontend-2/pages/workspaces/join.vue diff --git a/packages/frontend-2/components/onboarding/JoinTeammates.vue b/packages/frontend-2/components/onboarding/JoinTeammates.vue deleted file mode 100644 index c0b65e96b..000000000 --- a/packages/frontend-2/components/onboarding/JoinTeammates.vue +++ /dev/null @@ -1,100 +0,0 @@ - - - diff --git a/packages/frontend-2/components/onboarding/questions/Form.vue b/packages/frontend-2/components/onboarding/questions/Form.vue index 0b181a8ca..0d5e939bb 100644 --- a/packages/frontend-2/components/onboarding/questions/Form.vue +++ b/packages/frontend-2/components/onboarding/questions/Form.vue @@ -28,9 +28,11 @@ import { useForm } from 'vee-validate' import type { OnboardingRole, OnboardingPlan, OnboardingSource } from '@speckle/shared' import { useProcessOnboarding } from '~~/lib/auth/composables/onboarding' -import { homeRoute } from '~/lib/common/helpers/route' +import { homeRoute, workspaceJoinRoute } from '~/lib/common/helpers/route' const isOnboardingForced = useIsOnboardingForced() +const isWorkspacesEnabled = useIsWorkspacesEnabled() +const isWorkspaceNewPlansEnabled = useWorkspaceNewPlansEnabled() const { setUserOnboardingComplete, setMixpanelSegments } = useProcessOnboarding() @@ -51,6 +53,10 @@ const onSubmit = handleSubmit(async () => { plans: values.plan, source: values.source }) - navigateTo(homeRoute) + if (!isWorkspaceNewPlansEnabled.value && isWorkspacesEnabled.value) { + navigateTo(workspaceJoinRoute) + } else { + navigateTo(homeRoute) + } }) diff --git a/packages/frontend-2/components/projects/DashboardHeader.vue b/packages/frontend-2/components/projects/DashboardHeader.vue index 4288cc10e..f7ca7b0de 100644 --- a/packages/frontend-2/components/projects/DashboardHeader.vue +++ b/packages/frontend-2/components/projects/DashboardHeader.vue @@ -13,14 +13,16 @@ :invite="invite" /> diff --git a/packages/frontend-2/components/settings/workspaces/regions/Select.vue b/packages/frontend-2/components/settings/workspaces/regions/Select.vue index 858d2bca3..afa724296 100644 --- a/packages/frontend-2/components/settings/workspaces/regions/Select.vue +++ b/packages/frontend-2/components/settings/workspaces/regions/Select.vue @@ -14,7 +14,7 @@ + +
+ + + + +
+ + + + + +
+

+ Join teammates +

+

+ {{ description }} +

+ +
+ + Continue + + + Create a new workspace + + + Skip for now + +
+
+ + + + diff --git a/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue b/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue new file mode 100644 index 000000000..4b3144eca --- /dev/null +++ b/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue @@ -0,0 +1,54 @@ + + + diff --git a/packages/frontend-2/components/workspace/invite/DiscoverableWorkspaceBanner.vue b/packages/frontend-2/components/workspace/invite/DiscoverableWorkspaceBanner.vue index c71d74aa0..23790662c 100644 --- a/packages/frontend-2/components/workspace/invite/DiscoverableWorkspaceBanner.vue +++ b/packages/frontend-2/components/workspace/invite/DiscoverableWorkspaceBanner.vue @@ -1,5 +1,5 @@
-

+

{{ isPrimaryEmail ? 'Verify your email' : 'Verify additional email' }}

@@ -61,7 +61,7 @@

- +