From 71a2185d852890ca3681092feadd2989a5177a87 Mon Sep 17 00:00:00 2001 From: Charles Driesler Date: Tue, 17 Dec 2024 19:58:21 +0000 Subject: [PATCH 1/3] feat(users): bulk lookup users by email --- .../server/assets/core/typedefs/user.graphql | 13 +++ .../modules/core/domain/users/operations.ts | 12 +++ .../modules/core/graph/generated/graphql.ts | 16 ++++ .../modules/core/graph/resolvers/users.ts | 15 +++- .../server/modules/core/repositories/users.ts | 86 +++++++++++++------ .../graph/generated/graphql.ts | 13 +++ .../server/test/graphql/generated/graphql.ts | 13 +++ 7 files changed, 140 insertions(+), 28 deletions(-) diff --git a/packages/server/assets/core/typedefs/user.graphql b/packages/server/assets/core/typedefs/user.graphql index d8911f09e1..0ad5c21d35 100644 --- a/packages/server/assets/core/typedefs/user.graphql +++ b/packages/server/assets/core/typedefs/user.graphql @@ -51,6 +51,13 @@ extend type Query { @hasServerRole(role: SERVER_GUEST) @hasScopes(scopes: ["users:read", "profile:read"]) + """ + Look up server users with a collection of emails + """ + usersByEmail(input: BulkUsersRetrievalInput!): UserSearchResultCollection! + @hasServerRole(role: SERVER_GUEST) + @hasScopes(scopes: ["users:read", "profile:read"]) + """ Validate password strength """ @@ -80,6 +87,12 @@ input UsersRetrievalInput { projectId: String } +input BulkUsersRetrievalInput { + emails: [String!]! + cursor: String + limit: Int +} + type PasswordStrengthCheckResults { """ Integer from 0-4 (useful for implementing a strength bar): diff --git a/packages/server/modules/core/domain/users/operations.ts b/packages/server/modules/core/domain/users/operations.ts index 8d35419d0e..b371995d4b 100644 --- a/packages/server/modules/core/domain/users/operations.ts +++ b/packages/server/modules/core/domain/users/operations.ts @@ -195,6 +195,18 @@ export type LookupUsers = (filter: { cursor: Nullable }> +export type BulkLookupUsers = (filter: { + emails: string[] + /** + * Defaults to 10 + */ + limit?: MaybeNullOrUndefined + cursor?: MaybeNullOrUndefined +}) => Promise<{ + users: User[] + cursor: Nullable +}> + type AdminUserListArgs = { cursor: string | null query: string | null diff --git a/packages/server/modules/core/graph/generated/graphql.ts b/packages/server/modules/core/graph/generated/graphql.ts index 772658e784..ad5fae14f2 100644 --- a/packages/server/modules/core/graph/generated/graphql.ts +++ b/packages/server/modules/core/graph/generated/graphql.ts @@ -555,6 +555,12 @@ export type BranchUpdateInput = { streamId: Scalars['String']['input']; }; +export type BulkUsersRetrievalInput = { + cursor?: InputMaybe; + emails: Array; + limit?: InputMaybe; +}; + export type CancelCheckoutSessionInput = { sessionId: Scalars['ID']['input']; workspaceId: Scalars['ID']['input']; @@ -2600,6 +2606,8 @@ export type Query = { userSearch: UserSearchResultCollection; /** Look up server users */ users: UserSearchResultCollection; + /** Look up server users with a collection of emails */ + usersByEmail: UserSearchResultCollection; /** Validates the slug, to make sure it contains only valid characters and its not taken. */ validateWorkspaceSlug: Scalars['Boolean']['output']; workspace: Workspace; @@ -2746,6 +2754,11 @@ export type QueryUsersArgs = { }; +export type QueryUsersByEmailArgs = { + input: BulkUsersRetrievalInput; +}; + + export type QueryValidateWorkspaceSlugArgs = { slug: Scalars['String']['input']; }; @@ -4738,6 +4751,7 @@ export type ResolversTypes = { BranchCreateInput: BranchCreateInput; BranchDeleteInput: BranchDeleteInput; BranchUpdateInput: BranchUpdateInput; + BulkUsersRetrievalInput: BulkUsersRetrievalInput; CancelCheckoutSessionInput: CancelCheckoutSessionInput; CheckoutSession: ResolverTypeWrapper; CheckoutSessionInput: CheckoutSessionInput; @@ -5029,6 +5043,7 @@ export type ResolversParentTypes = { BranchCreateInput: BranchCreateInput; BranchDeleteInput: BranchDeleteInput; BranchUpdateInput: BranchUpdateInput; + BulkUsersRetrievalInput: BulkUsersRetrievalInput; CancelCheckoutSessionInput: CancelCheckoutSessionInput; CheckoutSession: CheckoutSession; CheckoutSessionInput: CheckoutSessionInput; @@ -6166,6 +6181,7 @@ export type QueryResolvers>; userSearch?: Resolver>; users?: Resolver>; + usersByEmail?: Resolver>; validateWorkspaceSlug?: Resolver>; workspace?: Resolver>; workspaceBySlug?: Resolver>; diff --git a/packages/server/modules/core/graph/resolvers/users.ts b/packages/server/modules/core/graph/resolvers/users.ts index 8fbf3e5182..26e5e6cd75 100644 --- a/packages/server/modules/core/graph/resolvers/users.ts +++ b/packages/server/modules/core/graph/resolvers/users.ts @@ -15,7 +15,8 @@ import { markOnboardingCompleteFactory, legacyGetPaginatedUsersCountFactory, legacyGetPaginatedUsersFactory, - lookupUsersFactory + lookupUsersFactory, + bulkLookupUsersFactory } from '@/modules/core/repositories/users' import { UsersMeta } from '@/modules/core/dbSchema' import { throwForNotHavingServerRole } from '@/modules/shared/authz' @@ -69,6 +70,7 @@ const changeUserRole = changeUserRoleFactory({ updateUserServerRole: updateUserServerRoleFactory({ db }) }) const searchUsers = searchUsersFactory({ db }) +const bulkLookupUsers = bulkLookupUsersFactory({ db }) const lookupUsers = lookupUsersFactory({ db }) const markOnboardingComplete = markOnboardingCompleteFactory({ db }) const getAdminUsersListCollection = getAdminUsersListCollectionFactory({ @@ -150,7 +152,18 @@ export = { const { cursor, users } = await lookupUsers(args.input) return { cursor, items: users } }, + async usersByEmail(_parent, args) { + if (args.input.emails.length < 1) + throw new BadRequestError('Must provide at least one email to search for.') + if ((args.input.limit || 0) > 100) + throw new BadRequestError( + 'Cannot return more than 100 items, please use pagination.' + ) + + const { cursor, users } = await bulkLookupUsers(args.input) + return { cursor, items: users } + }, async userPwdStrength(_parent, args) { const res = zxcvbn(args.pwd) return { score: res.score, feedback: res.feedback } diff --git a/packages/server/modules/core/repositories/users.ts b/packages/server/modules/core/repositories/users.ts index abf8c4af16..d3775f8c61 100644 --- a/packages/server/modules/core/repositories/users.ts +++ b/packages/server/modules/core/repositories/users.ts @@ -10,6 +10,7 @@ import { updateUserEmailFactory } from '@/modules/core/repositories/userEmails' import { markUserEmailAsVerifiedFactory } from '@/modules/core/services/users/emailVerification' import { UserWithOptionalRole } from '@/modules/core/domain/users/types' import { + BulkLookupUsers, CountAdminUsers, CountUsers, DeleteUserRecord, @@ -449,6 +450,36 @@ export const getUserRoleFactory = return role as Nullable } +type LookupUsersBaseQueryFilter = { + cursor?: string | null + limit?: number | null +} + +export const lookupUsersBaseQuery = ( + db: Knex, + filter: LookupUsersBaseQueryFilter = {} +) => { + const query = tables + .users(db) + .join(ServerAcl.name, Users.col.id, ServerAcl.col.userId) + .leftJoin(UserEmails.name, UserEmails.col.userId, Users.col.id) + .columns([ + ...Object.values( + omit(Users.col, [Users.col.email, Users.col.verified, Users.col.passwordDigest]) + ), + knex.raw(`(array_agg(??))[1] as "verified"`, [UserEmails.col.verified]), + knex.raw(`(array_agg(??))[1] as "email"`, [UserEmails.col.email]) + ]) + .groupBy(Users.col.id) + + if (filter.cursor) query.andWhere(Users.col.createdAt, '<', filter.cursor) + + const finalLimit = clamp(filter.limit || 10, 1, 100) + query.orderBy(Users.col.createdAt, 'desc').limit(finalLimit) + + return query +} + /** * Used for (Limited)User search. No need to convert users to Limited here, because non-limited fields * cannot be leaked out from the GQL API. @@ -465,43 +496,44 @@ export const lookupUsersFactory = projectId } = filter - const query = tables - .users(deps.db) - .join(ServerAcl.name, Users.col.id, ServerAcl.col.userId) - .leftJoin(UserEmails.name, UserEmails.col.userId, Users.col.id) - .columns([ - ...Object.values( - omit(Users.col, [ - Users.col.email, - Users.col.verified, - Users.col.passwordDigest - ]) - ), - knex.raw(`(array_agg(??))[1] as "verified"`, [UserEmails.col.verified]), - knex.raw(`(array_agg(??))[1] as "email"`, [UserEmails.col.email]) - ]) - .groupBy(Users.col.id) - .where((queryBuilder) => { - queryBuilder.where({ [UserEmails.col.email]: searchQuery }) //match full email or partial name - if (!emailOnly) - queryBuilder.orWhere(Users.col.name, 'ILIKE', `%${searchQuery}%`) - if (!archived) - queryBuilder.andWhere(ServerAcl.col.role, '!=', Roles.Server.ArchivedUser) - }) + const query = lookupUsersBaseQuery(deps.db, { limit, cursor }) + // match full email or partial name + query.where((queryBuilder) => { + queryBuilder.where({ [UserEmails.col.email]: searchQuery }) + if (!emailOnly) queryBuilder.orWhere(Users.col.name, 'ILIKE', `%${searchQuery}%`) + if (!archived) + queryBuilder.andWhere(ServerAcl.col.role, '!=', Roles.Server.ArchivedUser) + }) + + // limit to given project if (projectId) { query .innerJoin(StreamAcl.name, StreamAcl.col.userId, Users.col.id) .andWhere(StreamAcl.col.resourceId, projectId) } - if (cursor) query.andWhere(Users.col.createdAt, '<', cursor) + const rows = (await query) as UserRecord[] + const users = rows.map((u) => sanitizeUserRecord(u)) // pw shouldnt be there, but just making sure + + return { + users, + cursor: users.length > 0 ? users[users.length - 1].createdAt.toISOString() : null + } + } + +export const bulkLookupUsersFactory = + (deps: { db: Knex }): BulkLookupUsers => + async (filter) => { + const { emails, limit, cursor } = filter + + const query = lookupUsersBaseQuery(deps.db, { limit, cursor }) - const finalLimit = clamp(limit || 10, 1, 100) - query.orderBy(Users.col.createdAt, 'desc').limit(finalLimit) + // limit to exact matches on provided emails + query.whereIn(UserEmails.col.email, emails) const rows = (await query) as UserRecord[] - const users = rows.map((u) => sanitizeUserRecord(u)) // pw shouldnt be there, but just making sure + const users = rows.map((u) => sanitizeUserRecord(u)) return { users, 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 93d4f6e96b..c71bfc99a1 100644 --- a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts +++ b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts @@ -536,6 +536,12 @@ export type BranchUpdateInput = { streamId: Scalars['String']['input']; }; +export type BulkUsersRetrievalInput = { + cursor?: InputMaybe; + emails: Array; + limit?: InputMaybe; +}; + export type CancelCheckoutSessionInput = { sessionId: Scalars['ID']['input']; workspaceId: Scalars['ID']['input']; @@ -2581,6 +2587,8 @@ export type Query = { userSearch: UserSearchResultCollection; /** Look up server users */ users: UserSearchResultCollection; + /** Look up server users with a collection of emails */ + usersByEmail: UserSearchResultCollection; /** Validates the slug, to make sure it contains only valid characters and its not taken. */ validateWorkspaceSlug: Scalars['Boolean']['output']; workspace: Workspace; @@ -2727,6 +2735,11 @@ export type QueryUsersArgs = { }; +export type QueryUsersByEmailArgs = { + input: BulkUsersRetrievalInput; +}; + + export type QueryValidateWorkspaceSlugArgs = { slug: Scalars['String']['input']; }; diff --git a/packages/server/test/graphql/generated/graphql.ts b/packages/server/test/graphql/generated/graphql.ts index 2b975f7220..51e9237d23 100644 --- a/packages/server/test/graphql/generated/graphql.ts +++ b/packages/server/test/graphql/generated/graphql.ts @@ -537,6 +537,12 @@ export type BranchUpdateInput = { streamId: Scalars['String']['input']; }; +export type BulkUsersRetrievalInput = { + cursor?: InputMaybe; + emails: Array; + limit?: InputMaybe; +}; + export type CancelCheckoutSessionInput = { sessionId: Scalars['ID']['input']; workspaceId: Scalars['ID']['input']; @@ -2582,6 +2588,8 @@ export type Query = { userSearch: UserSearchResultCollection; /** Look up server users */ users: UserSearchResultCollection; + /** Look up server users with a collection of emails */ + usersByEmail: UserSearchResultCollection; /** Validates the slug, to make sure it contains only valid characters and its not taken. */ validateWorkspaceSlug: Scalars['Boolean']['output']; workspace: Workspace; @@ -2728,6 +2736,11 @@ export type QueryUsersArgs = { }; +export type QueryUsersByEmailArgs = { + input: BulkUsersRetrievalInput; +}; + + export type QueryValidateWorkspaceSlugArgs = { slug: Scalars['String']['input']; }; From a7e20dfbe6915416a0a879bebf6d7d3b146828ce Mon Sep 17 00:00:00 2001 From: Charles Driesler Date: Wed, 18 Dec 2024 18:18:36 +0000 Subject: [PATCH 2/3] chore(users): add tests for lookups --- .../server/modules/core/repositories/users.ts | 12 +- .../core/tests/integration/findUsers.spec.ts | 110 ++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/packages/server/modules/core/repositories/users.ts b/packages/server/modules/core/repositories/users.ts index d3775f8c61..a033200fe9 100644 --- a/packages/server/modules/core/repositories/users.ts +++ b/packages/server/modules/core/repositories/users.ts @@ -500,7 +500,7 @@ export const lookupUsersFactory = // match full email or partial name query.where((queryBuilder) => { - queryBuilder.where({ [UserEmails.col.email]: searchQuery }) + queryBuilder.where({ [UserEmails.col.email]: searchQuery.toLowerCase() }) if (!emailOnly) queryBuilder.orWhere(Users.col.name, 'ILIKE', `%${searchQuery}%`) if (!archived) queryBuilder.andWhere(ServerAcl.col.role, '!=', Roles.Server.ArchivedUser) @@ -522,6 +522,11 @@ export const lookupUsersFactory = } } +/** + * Used for (Limited)User search when multiple potential emails are known + * @param deps + * @returns + */ export const bulkLookupUsersFactory = (deps: { db: Knex }): BulkLookupUsers => async (filter) => { @@ -530,7 +535,10 @@ export const bulkLookupUsersFactory = const query = lookupUsersBaseQuery(deps.db, { limit, cursor }) // limit to exact matches on provided emails - query.whereIn(UserEmails.col.email, emails) + query.whereIn( + UserEmails.col.email, + emails.map((email) => email.toLowerCase()) + ) const rows = (await query) as UserRecord[] const users = rows.map((u) => sanitizeUserRecord(u)) diff --git a/packages/server/modules/core/tests/integration/findUsers.spec.ts b/packages/server/modules/core/tests/integration/findUsers.spec.ts index 7de1a36537..c571d91e12 100644 --- a/packages/server/modules/core/tests/integration/findUsers.spec.ts +++ b/packages/server/modules/core/tests/integration/findUsers.spec.ts @@ -12,11 +12,13 @@ import { import { db } from '@/db/knex' import { expect } from 'chai' import { + bulkLookupUsersFactory, countAdminUsersFactory, getUserByEmailFactory, getUserFactory, getUsersFactory, listUsersFactory, + lookupUsersFactory, storeUserAclFactory, storeUserFactory } from '@/modules/core/repositories/users' @@ -33,6 +35,8 @@ import { } from '@/modules/serverinvites/repositories/serverInvites' import { UsersEmitter } from '@/modules/core/events/usersEmitter' import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { BasicTestStream, createTestStream } from '@/test/speckle-helpers/streamHelper' +import { BasicTestUser, createTestUser } from '@/test/authHelper' const getServerInfo = getServerInfoFactory({ db }) const getUsers = getUsersFactory({ db }) @@ -65,6 +69,8 @@ const createUser = createUserFactory({ }) const getUserByEmail = getUserByEmailFactory({ db }) const listUsers = listUsersFactory({ db }) +const lookupUsers = lookupUsersFactory({ db }) +const bulkLookupUsers = bulkLookupUsersFactory({ db }) describe('Find users @core', () => { describe('getUsers', () => { @@ -157,4 +163,108 @@ describe('Find users @core', () => { expect(user!.email).to.equal(email.toLowerCase()) }) }) + + describe('lookupUsers', () => { + it('should find matches by name', async () => { + const email = createRandomEmail() + const userId = await createUser({ + email, + name: 'John Spackle', + password: createRandomPassword() + }) + const { users } = await lookupUsers({ query: 'Spack' }) + expect(users.some((user) => user.id === userId)).to.equal(true) + }) + it('should not find matches by name if filtered to emails only', async () => { + const email = createRandomEmail() + const userId = await createUser({ + email, + name: 'John Spackle', + password: createRandomPassword() + }) + const { users } = await lookupUsers({ query: 'Spack', emailOnly: true }) + expect(users.some((user) => user.id === userId)).to.equal(false) + }) + it('should find matches by email', async () => { + const email = createRandomEmail() + const userId = await createUser({ + email, + name: 'John Spackle', + password: createRandomPassword() + }) + const { users } = await lookupUsers({ query: email }) + expect(users.some((user) => user.id === userId)).to.equal(true) + }) + it('should find matches by email, case insensitive', async () => { + const email = 'fooBAR@example.org' + const userId = await createUser({ + email, + name: 'John Spackle', + password: createRandomPassword() + }) + const { users } = await lookupUsers({ query: 'FoObAr@example.org' }) + expect(users.some((user) => user.id === userId)).to.equal(true) + }) + it('should find matches limited to the given project', async () => { + const userA: BasicTestUser = { + id: '', + email: createRandomEmail(), + name: 'Beatrice' + } + const userB: BasicTestUser = { + id: '', + email: createRandomEmail(), + name: 'Beatrice' + } + await createTestUser(userA) + await createTestUser(userB) + const project: BasicTestStream = { + id: '', + ownerId: '', + name: 'Test Project', + isPublic: false + } + await createTestStream(project, userA) + + const { users } = await lookupUsers({ query: 'beatrice', projectId: project.id }) + expect(users.some((user) => user.id === userA.id)).to.equal(true) + expect(users.some((user) => user.id === userB.id)).to.equal(false) + }) + }) + + describe('bulkLookupUsers', () => { + it('should find matches for all emails provided', async () => { + const userA: BasicTestUser = { + id: '', + email: createRandomEmail(), + name: 'Harald' + } + const userB: BasicTestUser = { + id: '', + email: createRandomEmail(), + name: 'Beatrice' + } + await createTestUser(userA) + await createTestUser(userB) + + const { users } = await bulkLookupUsers({ emails: [userA.email, userB.email] }) + + expect(users.length).to.equal(2) + expect(users.some((user) => user.id === userA.id)).to.equal(true) + expect(users.some((user) => user.id === userB.id)).to.equal(true) + }) + it('should find matches for all emails provided, case insensitive', async () => { + const testUser: BasicTestUser = { + id: '', + email: 'barBAZ@example.org', + name: 'Bonathon' + } + await createTestUser(testUser) + + const { users } = await bulkLookupUsers({ emails: ['BARbaz@example.org'] }) + + expect(users.length).to.equal(1) + expect(users.some((user) => user.id === testUser.id)).to.equal(true) + }) + }) }) From f4d08947306beb136053c31f848013d34cbe6e5f Mon Sep 17 00:00:00 2001 From: Charles Driesler Date: Wed, 18 Dec 2024 18:31:51 +0000 Subject: [PATCH 3/3] chore(users): fe gqlgen --- .../frontend-2/lib/common/generated/gql/graphql.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/frontend-2/lib/common/generated/gql/graphql.ts b/packages/frontend-2/lib/common/generated/gql/graphql.ts index b44de5b36c..c820989c7c 100644 --- a/packages/frontend-2/lib/common/generated/gql/graphql.ts +++ b/packages/frontend-2/lib/common/generated/gql/graphql.ts @@ -538,6 +538,12 @@ export type BranchUpdateInput = { streamId: Scalars['String']['input']; }; +export type BulkUsersRetrievalInput = { + cursor?: InputMaybe; + emails: Array; + limit?: InputMaybe; +}; + export type CancelCheckoutSessionInput = { sessionId: Scalars['ID']['input']; workspaceId: Scalars['ID']['input']; @@ -2578,6 +2584,8 @@ export type Query = { userSearch: UserSearchResultCollection; /** Look up server users */ users: UserSearchResultCollection; + /** Look up server users with a collection of emails */ + usersByEmail: UserSearchResultCollection; /** Validates the slug, to make sure it contains only valid characters and its not taken. */ validateWorkspaceSlug: Scalars['Boolean']['output']; workspace: Workspace; @@ -2724,6 +2732,11 @@ export type QueryUsersArgs = { }; +export type QueryUsersByEmailArgs = { + input: BulkUsersRetrievalInput; +}; + + export type QueryValidateWorkspaceSlugArgs = { slug: Scalars['String']['input']; }; @@ -7578,6 +7591,7 @@ export type QueryFieldArgs = { userPwdStrength: QueryUserPwdStrengthArgs, userSearch: QueryUserSearchArgs, users: QueryUsersArgs, + usersByEmail: QueryUsersByEmailArgs, validateWorkspaceSlug: QueryValidateWorkspaceSlugArgs, workspace: QueryWorkspaceArgs, workspaceBySlug: QueryWorkspaceBySlugArgs,