From 32919c89dab71aabe182007fd74b2bc4f7983a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= <57442769+gjedlicska@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:11:08 +0200 Subject: [PATCH] gergo/web 1746 add resolver for workspace domain policy compliance per user (#2797) * fix(users): verified should be a public limited user field * feat(workspaceSecurity): update security tab copy * feat(workspaces): add limited user domain policy compliance check --- .../settings/workspaces/Security.vue | 9 +- .../lib/common/generated/gql/graphql.ts | 10 ++ .../typedefs/workspaces.graphql | 7 ++ .../modules/core/graph/generated/graphql.ts | 11 ++ .../graph/generated/graphql.ts | 10 ++ .../server/modules/workspaces/domain/logic.ts | 34 ++++++ .../modules/workspaces/errors/workspace.ts | 6 + .../workspaces/graph/resolvers/workspaces.ts | 14 +++ .../modules/workspaces/services/domains.ts | 36 ++++++ .../modules/workspaces/services/invites.ts | 40 +++---- .../tests/unit/domain/logic.spec.ts | 106 ++++++++++++++++++ .../tests/unit/services/domains.spec.ts | 85 ++++++++++++++ .../graph/resolvers/workspacesCore.ts | 3 + .../server/test/graphql/generated/graphql.ts | 10 ++ 14 files changed, 354 insertions(+), 27 deletions(-) create mode 100644 packages/server/modules/workspaces/domain/logic.ts create mode 100644 packages/server/modules/workspaces/services/domains.ts create mode 100644 packages/server/modules/workspaces/tests/unit/domain/logic.spec.ts create mode 100644 packages/server/modules/workspaces/tests/unit/services/domains.spec.ts diff --git a/packages/frontend-2/components/settings/workspaces/Security.vue b/packages/frontend-2/components/settings/workspaces/Security.vue index 14a5ccb81..4b90c9609 100644 --- a/packages/frontend-2/components/settings/workspaces/Security.vue +++ b/packages/frontend-2/components/settings/workspaces/Security.vue @@ -65,8 +65,9 @@

Domain protection

- Members won't be able to add users as members (or admins) to a workspace - unless they are part of a workspace's email domain. + Admins won't be able to add users as members (or admins) to a workspace + unless the one of the users email matches one of the workspace's + verified email domains.

- Makes your workspace discoverable by employees who sign up with your - company's specified email domain. + Makes your workspace discoverable by users who have a verified email + address matching one of the workspace's verified domains.

; + workspaceDomainPolicyCompliant?: Maybe; }; @@ -1036,6 +1037,15 @@ export type LimitedUserTimelineArgs = { limit?: Scalars['Int']['input']; }; + +/** + * Limited user type, for showing public info about a user + * to another user + */ +export type LimitedUserWorkspaceDomainPolicyCompliantArgs = { + workspaceId?: InputMaybe; +}; + export type MarkReceivedVersionInput = { message?: InputMaybe; projectId: Scalars['String']['input']; diff --git a/packages/server/assets/workspacesCore/typedefs/workspaces.graphql b/packages/server/assets/workspacesCore/typedefs/workspaces.graphql index c18aaa761..c09c7f4d6 100644 --- a/packages/server/assets/workspacesCore/typedefs/workspaces.graphql +++ b/packages/server/assets/workspacesCore/typedefs/workspaces.graphql @@ -374,3 +374,10 @@ input UserWorkspacesFilter { extend type Project { workspaceId: String } + +# case of using userSearch, and we alway expose this +extend type LimitedUser { + workspaceDomainPolicyCompliant(workspaceId: String): Boolean + # if workspaceId is undefined | null, just return undefined + # this can be implemented by the workspaceCore resolver too, to avoid frontend component duplication +} diff --git a/packages/server/modules/core/graph/generated/graphql.ts b/packages/server/modules/core/graph/generated/graphql.ts index e1e86dcdd..2b956d388 100644 --- a/packages/server/modules/core/graph/generated/graphql.ts +++ b/packages/server/modules/core/graph/generated/graphql.ts @@ -1003,6 +1003,7 @@ export type LimitedUser = { */ totalOwnedStreamsFavorites: Scalars['Int']['output']; verified?: Maybe; + workspaceDomainPolicyCompliant?: Maybe; }; @@ -1050,6 +1051,15 @@ export type LimitedUserTimelineArgs = { limit?: Scalars['Int']['input']; }; + +/** + * Limited user type, for showing public info about a user + * to another user + */ +export type LimitedUserWorkspaceDomainPolicyCompliantArgs = { + workspaceId?: InputMaybe; +}; + export type MarkReceivedVersionInput = { message?: InputMaybe; projectId: Scalars['String']['input']; @@ -5176,6 +5186,7 @@ export type LimitedUserResolvers, ParentType, ContextType, RequireFields>; totalOwnedStreamsFavorites?: Resolver; verified?: Resolver, ParentType, ContextType>; + workspaceDomainPolicyCompliant?: Resolver, ParentType, ContextType, Partial>; __isTypeOf?: IsTypeOfResolverFn; }; 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 394e656be..e82bbb88a 100644 --- a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts +++ b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts @@ -992,6 +992,7 @@ export type LimitedUser = { */ totalOwnedStreamsFavorites: Scalars['Int']['output']; verified?: Maybe; + workspaceDomainPolicyCompliant?: Maybe; }; @@ -1039,6 +1040,15 @@ export type LimitedUserTimelineArgs = { limit?: Scalars['Int']['input']; }; + +/** + * Limited user type, for showing public info about a user + * to another user + */ +export type LimitedUserWorkspaceDomainPolicyCompliantArgs = { + workspaceId?: InputMaybe; +}; + export type MarkReceivedVersionInput = { message?: InputMaybe; projectId: Scalars['String']['input']; diff --git a/packages/server/modules/workspaces/domain/logic.ts b/packages/server/modules/workspaces/domain/logic.ts new file mode 100644 index 000000000..bf97b6b65 --- /dev/null +++ b/packages/server/modules/workspaces/domain/logic.ts @@ -0,0 +1,34 @@ +import { UserEmail } from '@/modules/core/domain/userEmails/types' +import { WorkspaceDomainsInvalidState } from '@/modules/workspaces/errors/workspace' +import { WorkspaceDomain } from '@/modules/workspacesCore/domain/types' + +export const userEmailsCompliantWithWorkspaceDomains = ({ + userEmails, + workspaceDomains +}: { + userEmails: UserEmail[] + workspaceDomains: WorkspaceDomain[] +}): boolean => + anyEmailCompliantWithWorkspaceDomains({ + emails: userEmails.filter((e) => e.verified).map((e) => e.email), + workspaceDomains + }) + +export const anyEmailCompliantWithWorkspaceDomains = ({ + emails, + workspaceDomains +}: { + emails: string[] + workspaceDomains: WorkspaceDomain[] +}): boolean => { + const validWorkspaceDomains = workspaceDomains.filter((domain) => domain.verified) + + // we must have min 1 domain to validate compliance + if (!validWorkspaceDomains.length) throw new WorkspaceDomainsInvalidState() + + for (const email of emails) { + if (validWorkspaceDomains.some((domain) => email.endsWith(domain.domain))) + return true + } + return false +} diff --git a/packages/server/modules/workspaces/errors/workspace.ts b/packages/server/modules/workspaces/errors/workspace.ts index 691a14577..7a734d45f 100644 --- a/packages/server/modules/workspaces/errors/workspace.ts +++ b/packages/server/modules/workspaces/errors/workspace.ts @@ -78,3 +78,9 @@ export class WorkspaceProtectedError extends BaseError { static code = 'WORKSPACE_PROTECTED' static statusCode = 400 } + +export class WorkspaceDomainsInvalidState extends BaseError { + static defaultMessage = 'Workspace has no verified domains' + static code = 'WORKSPACE_NO_VERIFIED_DOMAINS' + static statusCode = 500 +} diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 22fceff3d..ed84aa519 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -109,6 +109,7 @@ import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userE import { requestNewEmailVerification } from '@/modules/emails/services/verification/request' import { Workspace } from '@/modules/workspacesCore/domain/types' import { WORKSPACE_MAX_PROJECTS_VERSIONS } from '@/modules/gatekeeper/domain/constants' +import { isUserWorkspaceDomainPolicyCompliantFactory } from '@/modules/workspaces/services/domains' const buildCreateAndSendServerOrProjectInvite = () => createAndSendInviteFactory({ @@ -783,6 +784,19 @@ export = FF_WORKSPACES_MODULE_ENABLED workspaceList: async () => { throw new WorkspacesNotYetImplementedError() } + }, + LimitedUser: { + workspaceDomainPolicyCompliant: async (parent, args) => { + const workspaceId = args.workspaceId + if (!workspaceId) return null + + const userId = parent.id + + return await isUserWorkspaceDomainPolicyCompliantFactory({ + findEmailsByUserId: findEmailsByUserIdFactory({ db }), + getWorkspaceWithDomains: getWorkspaceWithDomainsFactory({ db }) + })({ workspaceId, userId }) + } } } as Resolvers) : {} diff --git a/packages/server/modules/workspaces/services/domains.ts b/packages/server/modules/workspaces/services/domains.ts new file mode 100644 index 000000000..677df899d --- /dev/null +++ b/packages/server/modules/workspaces/services/domains.ts @@ -0,0 +1,36 @@ +import { FindEmailsByUserId } from '@/modules/core/domain/userEmails/operations' +import { userEmailsCompliantWithWorkspaceDomains } from '@/modules/workspaces/domain/logic' +import { GetWorkspaceWithDomains } from '@/modules/workspaces/domain/operations' +import { WorkspaceNotFoundError } from '@/modules/workspaces/errors/workspace' + +export const isUserWorkspaceDomainPolicyCompliantFactory = + ({ + getWorkspaceWithDomains, + findEmailsByUserId + }: { + getWorkspaceWithDomains: GetWorkspaceWithDomains + findEmailsByUserId: FindEmailsByUserId + }) => + async ({ + workspaceId, + userId + }: { + workspaceId: string + userId: string + }): Promise => { + const workspace = await getWorkspaceWithDomains({ + id: workspaceId + }) + // maybe we should throw + if (!workspace) throw new WorkspaceNotFoundError() + + // if workspace is not protected, the value is not true, its an empty response + if (!workspace.domainBasedMembershipProtectionEnabled) return null + + const userEmails = await findEmailsByUserId({ userId }) + + return userEmailsCompliantWithWorkspaceDomains({ + userEmails, + workspaceDomains: workspace.domains + }) + } diff --git a/packages/server/modules/workspaces/services/invites.ts b/packages/server/modules/workspaces/services/invites.ts index f25e0dac8..1254b4204 100644 --- a/packages/server/modules/workspaces/services/invites.ts +++ b/packages/server/modules/workspaces/services/invites.ts @@ -62,6 +62,10 @@ import { PendingWorkspaceCollaboratorGraphQLReturn } from '@/modules/workspacesC import { MaybeNullOrUndefined, Nullable, Roles, WorkspaceRoles } from '@speckle/shared' import { WorkspaceProtectedError } from '@/modules/workspaces/errors/workspace' import { FindVerifiedEmailsByUserId } from '@/modules/core/domain/userEmails/operations' +import { + anyEmailCompliantWithWorkspaceDomains, + userEmailsCompliantWithWorkspaceDomains +} from '@/modules/workspaces/domain/logic' const isWorkspaceResourceTarget = ( target: InviteResourceTarget @@ -189,38 +193,28 @@ export const collectAndValidateWorkspaceTargetsFactory = const workspaceDomains = await deps.getWorkspaceDomains({ workspaceIds: [resourceId] }) - const verifiedDomains = workspaceDomains - .filter((domain) => domain?.verified) - .map((domain) => domain.domain) - - const emailMatchesDomain = ({ - emails, - domains - }: { - emails: string[] - domains: string[] - }): boolean => { - for (const email of emails) { - if (domains.some((domain) => email.endsWith(domain))) return true - } - return false - } if (targetUser) { - const verifiedUserEmails = ( - await deps.findVerifiedEmailsByUserId({ - userId: targetUser.id - }) - ).map((userEmail) => userEmail.email) + const userEmails = await deps.findVerifiedEmailsByUserId({ + userId: targetUser.id + }) if ( - !emailMatchesDomain({ emails: verifiedUserEmails, domains: verifiedDomains }) + !userEmailsCompliantWithWorkspaceDomains({ + userEmails, + workspaceDomains + }) ) throw new WorkspaceProtectedError( 'The target user has no verified emails matching the domain policies' ) } else { // its a new server invite, we need to validate the email here too - if (!emailMatchesDomain({ emails: [input.target], domains: verifiedDomains })) + if ( + !anyEmailCompliantWithWorkspaceDomains({ + emails: [input.target], + workspaceDomains + }) + ) throw new WorkspaceProtectedError( 'The target email is not matching the domain policies' ) diff --git a/packages/server/modules/workspaces/tests/unit/domain/logic.spec.ts b/packages/server/modules/workspaces/tests/unit/domain/logic.spec.ts new file mode 100644 index 000000000..eed673900 --- /dev/null +++ b/packages/server/modules/workspaces/tests/unit/domain/logic.spec.ts @@ -0,0 +1,106 @@ +import { UserEmail } from '@/modules/core/domain/userEmails/types' +import { + anyEmailCompliantWithWorkspaceDomains, + userEmailsCompliantWithWorkspaceDomains +} from '@/modules/workspaces/domain/logic' +import { WorkspaceDomainsInvalidState } from '@/modules/workspaces/errors/workspace' +import { WorkspaceDomain } from '@/modules/workspacesCore/domain/types' +import { expectToThrow } from '@/test/assertionHelper' +import { expect } from 'chai' +import cryptoRandomString from 'crypto-random-string' +import { merge } from 'lodash' + +const createTestEmail = ( + emailInput?: Partial +): UserEmail => { + const domain = emailInput?.domain ?? 'example.com' + const defaultEmail = { + createdAt: new Date(), + email: `${cryptoRandomString({ length: 10 })}@${domain}`, + id: cryptoRandomString({ length: 10 }), + primary: true, + updatedAt: new Date(), + userId: cryptoRandomString({ length: 10 }), + verified: false + } + return merge(defaultEmail, emailInput ?? {}) +} + +const createTestDomain = (domainInput?: Partial): WorkspaceDomain => { + const defaultDomain: WorkspaceDomain = { + createdAt: new Date(), + domain: cryptoRandomString({ length: 10 }), + id: cryptoRandomString({ length: 10 }), + workspaceId: cryptoRandomString({ length: 10 }), + updatedAt: new Date(), + createdByUserId: cryptoRandomString({ length: 10 }), + verified: false + } + return merge(defaultDomain, domainInput ?? {}) +} + +describe('workspace domain logic', () => { + describe('anyEmailCompliantWithWorkspaceDomains', () => { + it('returns true for compliant emails', () => { + const domain = 'example.com' + const userEmails: UserEmail[] = [createTestEmail({ domain, verified: true })] + const workspaceDomains: WorkspaceDomain[] = [ + createTestDomain({ domain, verified: true }) + ] + + const isCompliant = userEmailsCompliantWithWorkspaceDomains({ + userEmails, + workspaceDomains + }) + expect(isCompliant).to.be.true + }) + it('filters non verified emails', () => { + const domain = 'example.com' + const userEmails: UserEmail[] = [createTestEmail({ domain, verified: false })] + const workspaceDomains: WorkspaceDomain[] = [ + createTestDomain({ domain, verified: true }) + ] + + const isCompliant = userEmailsCompliantWithWorkspaceDomains({ + userEmails, + workspaceDomains + }) + + expect(isCompliant).to.be.false + }) + }) + describe('anyEmailCompliantWithWorkspaceDomains', () => { + it('throws WorkspaceDomainInvalidState for no verified workspace domains', async () => { + const error = await expectToThrow(() => { + anyEmailCompliantWithWorkspaceDomains({ + emails: [], + workspaceDomains: [createTestDomain({ verified: false })] + }) + }) + expect(error.message).to.be.equal(new WorkspaceDomainsInvalidState().message) + }) + it('returns false if emails is empty', () => { + const isCompliant = anyEmailCompliantWithWorkspaceDomains({ + emails: [], + workspaceDomains: [createTestDomain({ verified: true })] + }) + expect(isCompliant).to.be.false + }) + it('returns false, if no emails match domain', () => { + const isCompliant = anyEmailCompliantWithWorkspaceDomains({ + emails: ['foo@hotmail.com', 'bar@google.com'], + workspaceDomains: [createTestDomain({ verified: true, domain: 'example.com' })] + }) + expect(isCompliant).to.be.false + }) + it('returns true if at least one email matches the domain', () => { + const domain = 'example.com' + + const isCompliant = anyEmailCompliantWithWorkspaceDomains({ + emails: [`foo@${domain}`, 'bar@google.com'], + workspaceDomains: [createTestDomain({ verified: true, domain })] + }) + expect(isCompliant).to.be.true + }) + }) +}) diff --git a/packages/server/modules/workspaces/tests/unit/services/domains.spec.ts b/packages/server/modules/workspaces/tests/unit/services/domains.spec.ts new file mode 100644 index 000000000..ba7013ae6 --- /dev/null +++ b/packages/server/modules/workspaces/tests/unit/services/domains.spec.ts @@ -0,0 +1,85 @@ +import { WorkspaceNotFoundError } from '@/modules/workspaces/errors/workspace' +import { isUserWorkspaceDomainPolicyCompliantFactory } from '@/modules/workspaces/services/domains' +import { expectToThrow } from '@/test/assertionHelper' +import { expect } from 'chai' +import cryptoRandomString from 'crypto-random-string' + +describe('workspace domain services', () => { + describe('isUserWorkspaceDomainPolicyCompliantFactory', () => { + it('throws WorkspaceNotFoundError', async () => { + const error = await expectToThrow(async () => { + await isUserWorkspaceDomainPolicyCompliantFactory({ + getWorkspaceWithDomains: async () => null, + findEmailsByUserId: async () => [] + })({ + workspaceId: cryptoRandomString({ length: 10 }), + userId: cryptoRandomString({ length: 10 }) + }) + }) + expect(error.message).to.be.equal(new WorkspaceNotFoundError().message) + }) + it('returns null if the workspace is not domain protected', async () => { + const isCompliant = await isUserWorkspaceDomainPolicyCompliantFactory({ + getWorkspaceWithDomains: async () => ({ + defaultLogoIndex: 0, + name: cryptoRandomString({ length: 10 }), + logo: null, + createdAt: new Date(), + updatedAt: new Date(), + description: '', + discoverabilityEnabled: false, + domainBasedMembershipProtectionEnabled: false, + domains: [], + id: cryptoRandomString({ length: 10 }) + }), + findEmailsByUserId: async () => [] + })({ + workspaceId: cryptoRandomString({ length: 10 }), + userId: cryptoRandomString({ length: 10 }) + }) + expect(isCompliant).to.be.null + }) + it('returns validation result from compliance check', async () => { + const domain = 'example.com' + const isCompliant = await isUserWorkspaceDomainPolicyCompliantFactory({ + getWorkspaceWithDomains: async () => ({ + defaultLogoIndex: 0, + name: cryptoRandomString({ length: 10 }), + logo: null, + createdAt: new Date(), + updatedAt: new Date(), + description: '', + discoverabilityEnabled: false, + domainBasedMembershipProtectionEnabled: true, + domains: [ + { + createdAt: new Date(), + createdByUserId: cryptoRandomString({ length: 10 }), + domain, + id: cryptoRandomString({ length: 10 }), + updatedAt: new Date(), + verified: true, + workspaceId: cryptoRandomString({ length: 10 }) + } + ], + id: cryptoRandomString({ length: 10 }) + }), + findEmailsByUserId: async () => [ + { + createdAt: new Date(), + email: `foo@${domain}`, + id: cryptoRandomString({ length: 10 }), + primary: false, + updatedAt: new Date(), + userId: cryptoRandomString({ length: 10 }), + verified: true + } + ] + })({ + userId: cryptoRandomString({ length: 10 }), + workspaceId: cryptoRandomString({ length: 10 }) + }) + expect(isCompliant).to.be.true + }) + }) +}) diff --git a/packages/server/modules/workspacesCore/graph/resolvers/workspacesCore.ts b/packages/server/modules/workspacesCore/graph/resolvers/workspacesCore.ts index 86240d870..d362d94da 100644 --- a/packages/server/modules/workspacesCore/graph/resolvers/workspacesCore.ts +++ b/packages/server/modules/workspacesCore/graph/resolvers/workspacesCore.ts @@ -99,6 +99,9 @@ export = !FF_WORKSPACES_MODULE_ENABLED workspaceList: async () => { throw new WorkspacesModuleDisabledError() } + }, + LimitedUser: { + workspaceDomainPolicyCompliant: async () => null } } as Resolvers) : {} diff --git a/packages/server/test/graphql/generated/graphql.ts b/packages/server/test/graphql/generated/graphql.ts index 4db7bce9d..7fc061f00 100644 --- a/packages/server/test/graphql/generated/graphql.ts +++ b/packages/server/test/graphql/generated/graphql.ts @@ -993,6 +993,7 @@ export type LimitedUser = { */ totalOwnedStreamsFavorites: Scalars['Int']['output']; verified?: Maybe; + workspaceDomainPolicyCompliant?: Maybe; }; @@ -1040,6 +1041,15 @@ export type LimitedUserTimelineArgs = { limit?: Scalars['Int']['input']; }; + +/** + * Limited user type, for showing public info about a user + * to another user + */ +export type LimitedUserWorkspaceDomainPolicyCompliantArgs = { + workspaceId?: InputMaybe; +}; + export type MarkReceivedVersionInput = { message?: InputMaybe; projectId: Scalars['String']['input'];