diff --git a/packages/server/modules/auth/index.ts b/packages/server/modules/auth/index.ts index 5171d9268..138068db9 100644 --- a/packages/server/modules/auth/index.ts +++ b/packages/server/modules/auth/index.ts @@ -14,7 +14,7 @@ import { import setupStrategiesFactory from '@/modules/auth/strategies' import githubStrategyBuilderFactory from '@/modules/auth/strategies/github' import { getServerInfo } from '@/modules/core/services/generic' -import { getUserByEmail, validatePasssword } from '@/modules/core/services/users' +import { validatePasssword } from '@/modules/core/services/users' import { validateServerInviteFactory, finalizeInvitedServerRegistrationFactory, @@ -37,6 +37,7 @@ import { passportAuthenticateHandlerBuilderFactory } from '@/modules/auth/servic import { countAdminUsersFactory, getUserFactory, + legacyGetUserByEmailFactory, legacyGetUserFactory, storeUserAclFactory, storeUserFactory @@ -109,7 +110,7 @@ const resolveAuthRedirectPath = resolveAuthRedirectPathFactory() const commonBuilderDeps = { getServerInfo, - getUserByEmail, + getUserByEmail: legacyGetUserByEmailFactory({ db }), findOrCreateUser, validateServerInvite, finalizeInvitedServerRegistration, diff --git a/packages/server/modules/auth/strategies/azureAd.ts b/packages/server/modules/auth/strategies/azureAd.ts index 0514b5432..f2093b3ce 100644 --- a/packages/server/modules/auth/strategies/azureAd.ts +++ b/packages/server/modules/auth/strategies/azureAd.ts @@ -1,7 +1,6 @@ /* istanbul ignore file */ import passport from 'passport' import { OIDCStrategy, IProfile, VerifyCallback } from 'passport-azure-ad' -import { getUserByEmail } from '@/modules/core/services/users' import { getServerInfo } from '@/modules/core/services/generic' import { @@ -28,12 +27,15 @@ import { ValidateServerInvite } from '@/modules/serverinvites/services/operations' import { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations' -import { FindOrCreateValidatedUser } from '@/modules/core/domain/users/operations' +import { + FindOrCreateValidatedUser, + LegacyGetUserByEmail +} from '@/modules/core/domain/users/operations' const azureAdStrategyBuilderFactory = (deps: { getServerInfo: typeof getServerInfo - getUserByEmail: typeof getUserByEmail + getUserByEmail: LegacyGetUserByEmail findOrCreateUser: FindOrCreateValidatedUser validateServerInvite: ValidateServerInvite finalizeInvitedServerRegistration: FinalizeInvitedServerRegistration diff --git a/packages/server/modules/auth/strategies/github.ts b/packages/server/modules/auth/strategies/github.ts index 22184b138..9a98399dd 100644 --- a/packages/server/modules/auth/strategies/github.ts +++ b/packages/server/modules/auth/strategies/github.ts @@ -3,7 +3,6 @@ import passport from 'passport' import type { VerifyCallback } from 'passport-oauth2' import { Strategy as GithubStrategy, type Profile } from 'passport-github2' -import { getUserByEmail } from '@/modules/core/services/users' import { getServerInfo } from '@/modules/core/services/generic' import { UserInputError, @@ -28,13 +27,16 @@ import { ValidateServerInvite } from '@/modules/serverinvites/services/operations' import { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations' -import { FindOrCreateValidatedUser } from '@/modules/core/domain/users/operations' +import { + FindOrCreateValidatedUser, + LegacyGetUserByEmail +} from '@/modules/core/domain/users/operations' import crs from 'crypto-random-string' const githubStrategyBuilderFactory = (deps: { getServerInfo: typeof getServerInfo - getUserByEmail: typeof getUserByEmail + getUserByEmail: LegacyGetUserByEmail findOrCreateUser: FindOrCreateValidatedUser validateServerInvite: ValidateServerInvite finalizeInvitedServerRegistration: FinalizeInvitedServerRegistration diff --git a/packages/server/modules/auth/strategies/google.ts b/packages/server/modules/auth/strategies/google.ts index a863cf35b..094e9a09e 100644 --- a/packages/server/modules/auth/strategies/google.ts +++ b/packages/server/modules/auth/strategies/google.ts @@ -1,7 +1,6 @@ /* istanbul ignore file */ import passport from 'passport' import { Strategy as GoogleStrategy } from 'passport-google-oauth20' -import { getUserByEmail } from '@/modules/core/services/users' import { getServerInfo } from '@/modules/core/services/generic' import { @@ -24,12 +23,15 @@ import { ValidateServerInvite } from '@/modules/serverinvites/services/operations' import { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations' -import { FindOrCreateValidatedUser } from '@/modules/core/domain/users/operations' +import { + FindOrCreateValidatedUser, + LegacyGetUserByEmail +} from '@/modules/core/domain/users/operations' const googleStrategyBuilderFactory = (deps: { getServerInfo: typeof getServerInfo - getUserByEmail: typeof getUserByEmail + getUserByEmail: LegacyGetUserByEmail findOrCreateUser: FindOrCreateValidatedUser validateServerInvite: ValidateServerInvite finalizeInvitedServerRegistration: FinalizeInvitedServerRegistration diff --git a/packages/server/modules/auth/strategies/local.ts b/packages/server/modules/auth/strategies/local.ts index aec345d8c..a38c7a361 100644 --- a/packages/server/modules/auth/strategies/local.ts +++ b/packages/server/modules/auth/strategies/local.ts @@ -1,4 +1,4 @@ -import { validatePasssword, getUserByEmail } from '@/modules/core/services/users' +import { validatePasssword } from '@/modules/core/services/users' import { getServerInfo } from '@/modules/core/services/generic' import { sendRateLimitResponse, @@ -19,12 +19,15 @@ import { ResolveAuthRedirectPath, ValidateServerInvite } from '@/modules/serverinvites/services/operations' -import { CreateValidatedUser } from '@/modules/core/domain/users/operations' +import { + CreateValidatedUser, + LegacyGetUserByEmail +} from '@/modules/core/domain/users/operations' const localStrategyBuilderFactory = (deps: { validatePassword: typeof validatePasssword - getUserByEmail: typeof getUserByEmail + getUserByEmail: LegacyGetUserByEmail getServerInfo: typeof getServerInfo getRateLimitResult: typeof getRateLimitResult validateServerInvite: ValidateServerInvite diff --git a/packages/server/modules/auth/strategies/oidc.ts b/packages/server/modules/auth/strategies/oidc.ts index b648044f7..3b881c013 100644 --- a/packages/server/modules/auth/strategies/oidc.ts +++ b/packages/server/modules/auth/strategies/oidc.ts @@ -1,7 +1,6 @@ /* istanbul ignore file */ import passport from 'passport' import { Issuer, Strategy } from 'openid-client' -import { getUserByEmail } from '@/modules/core/services/users' import { getServerInfo } from '@/modules/core/services/generic' import { getOidcDiscoveryUrl, @@ -24,12 +23,15 @@ import { ValidateServerInvite } from '@/modules/serverinvites/services/operations' import { PassportAuthenticateHandlerBuilder } from '@/modules/auth/domain/operations' -import { FindOrCreateValidatedUser } from '@/modules/core/domain/users/operations' +import { + FindOrCreateValidatedUser, + LegacyGetUserByEmail +} from '@/modules/core/domain/users/operations' const oidcStrategyBuilderFactory = (deps: { getServerInfo: typeof getServerInfo - getUserByEmail: typeof getUserByEmail + getUserByEmail: LegacyGetUserByEmail findOrCreateUser: FindOrCreateValidatedUser validateServerInvite: ValidateServerInvite finalizeInvitedServerRegistration: FinalizeInvitedServerRegistration diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js index 7d93f366b..618a4c22e 100644 --- a/packages/server/modules/auth/tests/auth.spec.js +++ b/packages/server/modules/auth/tests/auth.spec.js @@ -3,7 +3,6 @@ const chai = require('chai') const request = require('supertest') const { updateServerInfo, getServerInfo } = require('@/modules/core/services/generic') -const { getUserByEmail } = require('@/modules/core/services/users') const { TIME } = require('@speckle/shared') const { RATE_LIMITERS, createConsumer } = require('@/modules/core/services/ratelimiter') const { beforeEachContext, initializeTestServer } = require('@/test/hooks') @@ -50,7 +49,8 @@ const { getUserFactory, storeUserFactory, countAdminUsersFactory, - storeUserAclFactory + storeUserAclFactory, + legacyGetUserByEmailFactory } = require('@/modules/core/repositories/users') const { findEmailFactory, @@ -138,6 +138,7 @@ const createUser = createUserFactory({ }), usersEventsEmitter: UsersEmitter.emit }) +const getUserByEmail = legacyGetUserByEmailFactory({ db }) const expect = chai.expect diff --git a/packages/server/modules/core/domain/users/operations.ts b/packages/server/modules/core/domain/users/operations.ts index 0830ad0a2..d6954146e 100644 --- a/packages/server/modules/core/domain/users/operations.ts +++ b/packages/server/modules/core/domain/users/operations.ts @@ -42,6 +42,10 @@ export type StoreUserAcl = (params: { acl: ServerAclRecord }) => Promise +export type LegacyGetUserByEmail = (params: { + email: string +}) => Promise>> + export type LegacyGetUser = (id: string) => Promise export type LegacyGetPaginatedUsers = ( diff --git a/packages/server/modules/core/graph/resolvers/users.js b/packages/server/modules/core/graph/resolvers/users.js index 065d44fce..94f184ba4 100644 --- a/packages/server/modules/core/graph/resolvers/users.js +++ b/packages/server/modules/core/graph/resolvers/users.js @@ -1,5 +1,4 @@ const { - getUserByEmail, getUserRole, deleteUser, searchUsers, @@ -16,7 +15,8 @@ const { const { Roles, Scopes } = require('@speckle/shared') const { markOnboardingComplete, - legacyGetUserFactory + legacyGetUserFactory, + legacyGetUserByEmailFactory } = require('@/modules/core/repositories/users') const { UsersMeta } = require('@/modules/core/dbSchema') const { getServerInfo } = require('@/modules/core/services/generic') @@ -31,6 +31,7 @@ const { BadRequestError } = require('@/modules/shared/errors') const { saveActivityFactory } = require('@/modules/activitystream/repositories') const getUser = legacyGetUserFactory({ db }) +const getUserByEmail = legacyGetUserByEmailFactory({ db }) /** @type {import('@/modules/core/graph/generated/graphql').Resolvers} */ module.exports = { diff --git a/packages/server/modules/core/repositories/users.ts b/packages/server/modules/core/repositories/users.ts index 3e010d653..670ddaadb 100644 --- a/packages/server/modules/core/repositories/users.ts +++ b/packages/server/modules/core/repositories/users.ts @@ -19,6 +19,7 @@ import { LegacyGetPaginatedUsers, LegacyGetPaginatedUsersCount, LegacyGetUser, + LegacyGetUserByEmail, StoreUser, StoreUserAcl } from '@/modules/core/domain/users/operations' @@ -333,6 +334,29 @@ export const legacyGetPaginatedUsersCount = return parseInt(userCount.count) } +/** + * @deprecated Use getUserByEmail instead + */ +export const legacyGetUserByEmailFactory = + (deps: { db: Knex }): LegacyGetUserByEmail => + async ({ email }) => { + const user = await tables + .users(deps.db) + .leftJoin(UserEmails.name, UserEmails.col.userId, Users.col.id) + .where({ [UserEmails.col.primary]: true }) + .whereRaw('lower("user_emails"."email") = lower(?)', [email]) + .columns([ + ...Object.values(omit(Users.col, ['email', 'verified'])), + knex.raw(`(array_agg("user_emails"."email"))[1] as email`), + knex.raw(`(array_agg("user_emails"."verified"))[1] as verified`) + ]) + .groupBy(Users.col.id) + .first() + if (!user) return null + delete user.passwordDigest + return user + } + export const storeUserFactory = (deps: { db: Knex }): StoreUser => async (params) => { diff --git a/packages/server/modules/core/services/users.js b/packages/server/modules/core/services/users.js index 3187fb9d4..45f4e501e 100644 --- a/packages/server/modules/core/services/users.js +++ b/packages/server/modules/core/services/users.js @@ -45,24 +45,6 @@ const _ensureAtleastOneAdminRemains = async (userId) => { const getUserByEmail = getUserByEmailFactory({ db }) module.exports = { - // TODO: this should be moved to repository - async getUserByEmail({ email }) { - const user = await Users() - .leftJoin(UserEmails.name, UserEmails.col.userId, UsersSchema.col.id) - .where({ [UserEmails.col.primary]: true }) - .whereRaw('lower("user_emails"."email") = lower(?)', [email]) - .columns([ - ...Object.values(omit(UsersSchema.col, ['email', 'verified'])), - knex.raw(`(array_agg("user_emails"."email"))[1] as email`), - knex.raw(`(array_agg("user_emails"."verified"))[1] as verified`) - ]) - .groupBy(UsersSchema.col.id) - .first() - if (!user) return null - delete user.passwordDigest - return user - }, - async getUserRole(id) { const { role } = (await Acl().where({ userId: id }).select('role').first()) || { role: null diff --git a/packages/server/modules/core/tests/integration/userEmails.spec.ts b/packages/server/modules/core/tests/integration/userEmails.spec.ts index d201bfc36..e9873f9fe 100644 --- a/packages/server/modules/core/tests/integration/userEmails.spec.ts +++ b/packages/server/modules/core/tests/integration/userEmails.spec.ts @@ -7,12 +7,12 @@ import { getUserFactory, legacyGetPaginatedUsersCount, legacyGetPaginatedUsersFactory, + legacyGetUserByEmailFactory, listUsers, markUserAsVerified, storeUserAclFactory, storeUserFactory } from '@/modules/core/repositories/users' -import * as UsersService from '@/modules/core/services/users' import { db } from '@/db/knex' import { createRandomEmail, @@ -82,6 +82,7 @@ const createUser = createUserFactory({ usersEventsEmitter: UsersEmitter.emit }) const getUserByEmail = getUserByEmailFactory({ db }) +const legacyGetUserByEmail = legacyGetUserByEmailFactory({ db }) describe('Core @user-emails', () => { before(async () => { @@ -489,22 +490,22 @@ describe('Core @user-emails', () => { expect(user?.verified).to.be }) - it('with UsersService.getUserByEmail()', async () => { - const user = await UsersService.getUserByEmail({ + it('with legacyGetUserByEmail()', async () => { + const user = await legacyGetUserByEmail({ email: randomizeCase(randomCaseGuy.email) }) expect(user).to.be.ok assertLowercaseEquality(user?.email, randomCaseGuy.email) }) - it('with UsersService.getUsers()', async () => { + it('with legacyGetPaginatedUsers()', async () => { const users = await getUsers(10, 0, randomizeCase(randomCaseGuy.email)) expect(users).to.be.ok expect(users).to.have.length(1) assertLowercaseEquality(users[0].email, randomCaseGuy.email) }) - it('with UsersService.countUsers()', async () => { + it('with legacyGetPaginatedUsersCount()', async () => { const count = await countUsers(randomizeCase(randomCaseGuy.email)) expect(count).to.eq(1) }) diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index ddc4140cb..35ae80a36 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/packages/server/modules/core/tests/users.spec.js @@ -4,7 +4,6 @@ const assert = require('assert') const { changeUserRole, - getUserByEmail, searchUsers, updateUser, deleteUser, @@ -89,7 +88,8 @@ const { legacyGetUserFactory, storeUserFactory, countAdminUsersFactory, - storeUserAclFactory + storeUserAclFactory, + legacyGetUserByEmailFactory } = require('@/modules/core/repositories/users') const { findEmailFactory, @@ -211,6 +211,7 @@ const findOrCreateUser = findOrCreateUserFactory({ createUser, findPrimaryEmailForUser: findPrimaryEmailForUserFactory({ db }) }) +const getUserByEmail = legacyGetUserByEmailFactory({ db }) describe('Actors & Tokens @user-services', () => { const myTestActor = { diff --git a/packages/server/scripts/streamObjects.js b/packages/server/scripts/streamObjects.js index 62dd7316a..572ddc55c 100644 --- a/packages/server/scripts/streamObjects.js +++ b/packages/server/scripts/streamObjects.js @@ -1,5 +1,4 @@ require('../bootstrap') -const { getUserByEmail } = require('@/modules/core/services/users') const { createPersonalAccessToken } = require('@/modules/core/services/tokens') const { createManyObjects } = require('@/test/helpers') @@ -42,7 +41,11 @@ const { } = require('@/modules/activitystream/services/streamActivity') const { saveActivityFactory } = require('@/modules/activitystream/repositories') const { publish } = require('@/modules/shared/utils/subscriptions') -const { getUsersFactory, getUserFactory } = require('@/modules/core/repositories/users') +const { + getUsersFactory, + getUserFactory, + legacyGetUserByEmailFactory +} = require('@/modules/core/repositories/users') const getUsers = getUsersFactory({ db }) const getUser = getUserFactory({ db }) @@ -78,6 +81,7 @@ const createStream = legacyCreateStreamFactory({ projectsEventsEmitter: ProjectsEmitter.emit }) }) +const getUserByEmail = legacyGetUserByEmailFactory({ db }) const main = async () => { const testStream = {