From ea62a7f7100d62309b10a1a2a564671b301d7e7d Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Tue, 22 Nov 2022 18:01:36 +0100 Subject: [PATCH 1/4] fix(backend): email verification code never expired --- .../graphql/resolver/EmailOptinCodes.test.ts | 125 ++++++++++++++++++ .../src/graphql/resolver/UserResolver.test.ts | 24 ++-- backend/src/graphql/resolver/UserResolver.ts | 106 +++------------ backend/test/helpers.ts | 5 +- 4 files changed, 157 insertions(+), 103 deletions(-) create mode 100644 backend/src/graphql/resolver/EmailOptinCodes.test.ts diff --git a/backend/src/graphql/resolver/EmailOptinCodes.test.ts b/backend/src/graphql/resolver/EmailOptinCodes.test.ts new file mode 100644 index 000000000..1cf22850d --- /dev/null +++ b/backend/src/graphql/resolver/EmailOptinCodes.test.ts @@ -0,0 +1,125 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ + +import { testEnvironment, cleanDB } from '@test/helpers' +import { User as DbUser } from '@entity/User' +import { createUser, setPassword, forgotPassword } from '@/seeds/graphql/mutations' +import { queryOptIn } from '@/seeds/graphql/queries' +import CONFIG from '@/config' +import { GraphQLError } from 'graphql' + +let mutate: any, query: any, con: any +let testEnv: any + +CONFIG.EMAIL_CODE_VALID_TIME = 1440 +CONFIG.EMAIL_CODE_REQUEST_TIME = 10 + +beforeAll(async () => { + testEnv = await testEnvironment() + mutate = testEnv.mutate + query = testEnv.query + con = testEnv.con + await cleanDB() +}) + +afterAll(async () => { + await cleanDB() + await con.close() +}) + +describe('EmailOptinCodes', () => { + let optinCode: string + beforeAll(async () => { + const variables = { + email: 'peter@lustig.de', + firstName: 'Peter', + lastName: 'Lustig', + language: 'de', + } + const { + data: { createUser: user }, + } = await mutate({ mutation: createUser, variables }) + const dbObject = await DbUser.findOneOrFail({ + where: { id: user.id }, + relations: ['emailContact'], + }) + optinCode = dbObject.emailContact.emailVerificationCode.toString() + }) + + describe('queryOptIn', () => { + it('has a valid optin code', async () => { + await expect( + query({ query: queryOptIn, variables: { optIn: optinCode } }), + ).resolves.toMatchObject({ + data: { + queryOptIn: true, + }, + errors: undefined, + }) + }) + + describe('run time forward until code must be expired', () => { + beforeAll(() => { + jest.useFakeTimers() + setTimeout(jest.fn(), CONFIG.EMAIL_CODE_VALID_TIME * 60 * 1000) + jest.runAllTimers() + }) + + afterAll(() => { + jest.useRealTimers() + }) + + it('throws an error', async () => { + await expect( + query({ query: queryOptIn, variables: { optIn: optinCode } }), + ).resolves.toMatchObject({ + data: null, + errors: [new GraphQLError('email was sent more than 24 hours ago')], + }) + }) + + it('does not allow to set password', async () => { + await expect( + mutate({ mutation: setPassword, variables: { code: optinCode, password: 'Aa12345_' } }), + ).resolves.toMatchObject({ + data: null, + errors: [new GraphQLError('email was sent more than 24 hours ago')], + }) + }) + }) + }) + + describe('forgotPassword', () => { + it('throws an error', async () => { + await expect( + mutate({ mutation: forgotPassword, variables: { email: 'peter@lustig.de' } }), + ).resolves.toMatchObject({ + data: null, + errors: [new GraphQLError('email already sent less than 10 minutes minutes ago')], + }) + }) + + describe('run time forward until code can be resent', () => { + beforeAll(() => { + jest.useFakeTimers() + setTimeout(jest.fn(), CONFIG.EMAIL_CODE_REQUEST_TIME * 60 * 1000) + jest.runAllTimers() + }) + + afterAll(() => { + jest.useRealTimers() + }) + + it('cann send email again', async () => { + await expect( + mutate({ mutation: forgotPassword, variables: { email: 'peter@lustig.de' } }), + ).resolves.toMatchObject({ + data: { + forgotPassword: true, + }, + errors: undefined, + }) + }) + }) + }) +}) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 6323abfde..c382d8bc2 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -21,7 +21,7 @@ import CONFIG from '@/config' import { sendAccountActivationEmail } from '@/mailer/sendAccountActivationEmail' import { sendAccountMultiRegistrationEmail } from '@/emails/sendEmailVariants' import { sendResetPasswordEmail } from '@/mailer/sendResetPasswordEmail' -import { printTimeDuration, activationLink } from './UserResolver' +import { printTimeDuration } from './UserResolver' import { contributionLinkFactory } from '@/seeds/factory/contributionLink' import { transactionLinkFactory } from '@/seeds/factory/transactionLink' import { ContributionLink } from '@model/ContributionLink' @@ -804,12 +804,8 @@ describe('UserResolver', () => { }) describe('user exists in DB', () => { - let emailContact: UserContact - beforeAll(async () => { await userFactory(testEnv, bibiBloxberg) - // await resetEntity(LoginEmailOptIn) - emailContact = await UserContact.findOneOrFail(variables) }) afterAll(async () => { @@ -818,7 +814,7 @@ describe('UserResolver', () => { }) describe('duration not expired', () => { - it('returns true', async () => { + it('throws an error', async () => { await expect(mutate({ mutation: forgotPassword, variables })).resolves.toEqual( expect.objectContaining({ errors: [ @@ -844,15 +840,15 @@ describe('UserResolver', () => { }), ) }) - }) - it('sends reset password email', () => { - expect(sendResetPasswordEmail).toBeCalledWith({ - link: activationLink(emailContact.emailVerificationCode), - firstName: 'Bibi', - lastName: 'Bloxberg', - email: 'bibi@bloxberg.de', - duration: expect.any(String), + it('sends reset password email', () => { + expect(sendResetPasswordEmail).toBeCalledWith({ + link: expect.any(String), + firstName: 'Bibi', + lastName: 'Bloxberg', + email: 'bibi@bloxberg.de', + duration: expect.any(String), + }) }) }) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 707b7ac49..e6a86bba5 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -149,16 +149,6 @@ const SecretKeyCryptographyCreateKey = (salt: string, password: string): Buffer[ return [encryptionKeyHash, encryptionKey] } -/* -const getEmailHash = (email: string): Buffer => { - logger.trace('getEmailHash...') - const emailHash = Buffer.alloc(sodium.crypto_generichash_BYTES) - sodium.crypto_generichash(emailHash, Buffer.from(email)) - logger.debug(`getEmailHash...successful: ${emailHash}`) - return emailHash -} -*/ - const SecretKeyCryptographyEncrypt = (message: Buffer, encryptionKey: Buffer): Buffer => { logger.trace('SecretKeyCryptographyEncrypt...') const encrypted = Buffer.alloc(message.length + sodium.crypto_secretbox_MACBYTES) @@ -194,89 +184,33 @@ const newEmailContact = (email: string, userId: number): DbUserContact => { logger.debug(`newEmailContact...successful: ${emailContact}`) return emailContact } -/* -const newEmailOptIn = (userId: number): LoginEmailOptIn => { - logger.trace('newEmailOptIn...') - const emailOptIn = new LoginEmailOptIn() - emailOptIn.verificationCode = random(64) - emailOptIn.userId = userId - emailOptIn.emailOptInTypeId = OptInType.EMAIL_OPT_IN_REGISTER - logger.debug(`newEmailOptIn...successful: ${emailOptIn}`) - return emailOptIn -} -*/ -/* -// needed by AdminResolver -// checks if given code exists and can be resent -// if optIn does not exits, it is created -export const checkOptInCode = async ( - optInCode: LoginEmailOptIn | undefined, - user: DbUser, - optInType: OptInType = OptInType.EMAIL_OPT_IN_REGISTER, -): Promise => { - logger.info(`checkOptInCode... ${optInCode}`) - if (optInCode) { - if (!canResendOptIn(optInCode)) { - logger.error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - throw new Error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - } - optInCode.updatedAt = new Date() - optInCode.resendCount++ - } else { - logger.trace('create new OptIn for userId=' + user.id) - optInCode = newEmailOptIn(user.id) - } - if (user.emailChecked) { - optInCode.emailOptInTypeId = optInType - } - await LoginEmailOptIn.save(optInCode).catch(() => { - logger.error('Unable to save optin code= ' + optInCode) - throw new Error('Unable to save optin code.') - }) - logger.debug(`checkOptInCode...successful: ${optInCode} for userid=${user.id}`) - return optInCode -} -*/ export const checkEmailVerificationCode = async ( emailContact: DbUserContact, optInType: OptInType = OptInType.EMAIL_OPT_IN_REGISTER, ): Promise => { logger.info(`checkEmailVerificationCode... ${emailContact}`) - if (emailContact.updatedAt) { - if (!canEmailResend(emailContact.updatedAt)) { - logger.error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - throw new Error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - } - emailContact.updatedAt = new Date() - emailContact.emailResendCount++ - } else { - logger.trace('create new EmailVerificationCode for userId=' + emailContact.userId) - emailContact.emailChecked = false - emailContact.emailVerificationCode = random(64) + if (!canEmailResend(emailContact.updatedAt || emailContact.createdAt)) { + logger.error( + `email already sent less than ${printTimeDuration( + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, + ) + throw new Error( + `email already sent less than ${printTimeDuration( + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, + ) } + emailContact.updatedAt = new Date() + emailContact.emailResendCount++ + emailContact.emailVerificationCode = random(64) emailContact.emailOptInTypeId = optInType await DbUserContact.save(emailContact).catch(() => { logger.error('Unable to save email verification code= ' + emailContact) throw new Error('Unable to save email verification code.') }) - logger.debug(`checkEmailVerificationCode...successful: ${emailContact}`) + logger.info(`checkEmailVerificationCode...successful: ${emailContact}`) return emailContact } @@ -384,6 +318,7 @@ export class UserResolver { @Authorized([RIGHTS.LOGOUT]) @Mutation(() => String) async logout(): Promise { + // TODO: Event still missing here!! // TODO: We dont need this anymore, but might need this in the future in oder to invalidate a valid JWT-Token. // Furthermore this hook can be useful for tracking user behaviour (did he logout or not? Warn him if he didn't on next login) // The functionality is fully client side - the client just needs to delete his token with the current implementation. @@ -657,7 +592,7 @@ export class UserResolver { }) logger.debug('userContact loaded...') // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes - if (!isEmailVerificationCodeValid(userContact.updatedAt)) { + if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { logger.error( `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) @@ -760,7 +695,7 @@ export class UserResolver { const userContact = await DbUserContact.findOneOrFail({ emailVerificationCode: optIn }) logger.debug(`found optInCode=${userContact}`) // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes - if (!isEmailVerificationCodeValid(userContact.updatedAt)) { + if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { logger.error( `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) @@ -935,10 +870,7 @@ const isOptInValid = (optIn: LoginEmailOptIn): boolean => { return isTimeExpired(optIn, CONFIG.EMAIL_CODE_VALID_TIME) } */ -const isEmailVerificationCodeValid = (updatedAt: Date | null): boolean => { - if (updatedAt == null) { - return true - } +const isEmailVerificationCodeValid = (updatedAt: Date): boolean => { return isTimeExpired(updatedAt, CONFIG.EMAIL_CODE_VALID_TIME) } /* diff --git a/backend/test/helpers.ts b/backend/test/helpers.ts index 7ee8e6052..1935b01a0 100644 --- a/backend/test/helpers.ts +++ b/backend/test/helpers.ts @@ -5,6 +5,7 @@ import { createTestClient } from 'apollo-server-testing' import createServer from '../src/server/createServer' import { initialize } from '@dbTools/helpers' import { entities } from '@entity/index' +import { i18n, logger } from './testSetup' export const headerPushMock = jest.fn((t) => { context.token = t.value @@ -26,8 +27,8 @@ export const cleanDB = async () => { } } -export const testEnvironment = async (logger?: any, localization?: any) => { - const server = await createServer(context, logger, localization) +export const testEnvironment = async (testLogger: any = logger, testI18n: any = i18n) => { + const server = await createServer(context, testLogger, testI18n) const con = server.con const testClient = createTestClient(server.apollo) const mutate = testClient.mutate From bbd163f1e05a589384e63d66c2adf5f6c62a448c Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Wed, 30 Nov 2022 14:34:09 +0100 Subject: [PATCH 2/4] integrate export const checkEmailVerificationCode = async ( --- backend/src/graphql/resolver/UserResolver.ts | 66 +++++++------------- 1 file changed, 24 insertions(+), 42 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 087654269..2e294196a 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -141,35 +141,6 @@ const newEmailContact = (email: string, userId: number): DbUserContact => { return emailContact } -export const checkEmailVerificationCode = async ( - emailContact: DbUserContact, - optInType: OptInType = OptInType.EMAIL_OPT_IN_REGISTER, -): Promise => { - logger.info(`checkEmailVerificationCode... ${emailContact}`) - if (!canEmailResend(emailContact.updatedAt || emailContact.createdAt)) { - logger.error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - throw new Error( - `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, - ) - } - emailContact.updatedAt = new Date() - emailContact.emailResendCount++ - emailContact.emailVerificationCode = random(64) - emailContact.emailOptInTypeId = optInType - await DbUserContact.save(emailContact).catch(() => { - logger.error('Unable to save email verification code= ' + emailContact) - throw new Error('Unable to save email verification code.') - }) - logger.info(`checkEmailVerificationCode...successful: ${emailContact}`) - return emailContact -} - export const activationLink = (verificationCode: BigInt): string => { logger.debug(`activationLink(${verificationCode})...`) return CONFIG.EMAIL_LINK_SETPASSWORD.replace(/{optin}/g, verificationCode.toString()) @@ -492,21 +463,32 @@ export class UserResolver { return true } - // can be both types: REGISTER and RESET_PASSWORD - // let optInCode = await LoginEmailOptIn.findOne({ - // userId: user.id, - // }) - // let optInCode = user.emailContact.emailVerificationCode - const dbUserContact = await checkEmailVerificationCode( - user.emailContact, - OptInType.EMAIL_OPT_IN_RESET_PASSWORD, - ) + if (!canEmailResend(user.emailContact.updatedAt || user.emailContact.createdAt)) { + logger.error( + `email already sent less than ${printTimeDuration( + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, + ) + throw new Error( + `email already sent less than ${printTimeDuration( + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, + ) + } + + user.emailContact.updatedAt = new Date() + user.emailContact.emailResendCount++ + user.emailContact.emailVerificationCode = random(64) + user.emailContact.emailOptInTypeId = OptInType.EMAIL_OPT_IN_RESET_PASSWORD + await user.emailContact.save().catch(() => { + logger.error('Unable to save email verification code= ' + user.emailContact) + throw new Error('Unable to save email verification code.') + }) - // optInCode = await checkOptInCode(optInCode, user, OptInType.EMAIL_OPT_IN_RESET_PASSWORD) - logger.info(`optInCode for ${email}=${dbUserContact}`) + logger.info(`optInCode for ${email}=${user.emailContact}`) // eslint-disable-next-line @typescript-eslint/no-unused-vars const emailSent = await sendResetPasswordEmailMailer({ - link: activationLink(dbUserContact.emailVerificationCode), + link: activationLink(user.emailContact.emailVerificationCode), firstName: user.firstName, lastName: user.lastName, email, @@ -516,7 +498,7 @@ export class UserResolver { /* uncomment this, when you need the activation link on the console */ // In case EMails are disabled log the activation link for the user if (!emailSent) { - logger.debug(`Reset password link: ${activationLink(dbUserContact.emailVerificationCode)}`) + logger.debug(`Reset password link: ${activationLink(user.emailContact.emailVerificationCode)}`) } logger.info(`forgotPassword(${email}) successful...`) From 796f5af2c6d76d6cdabe8de85f4074d601ce387f Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Wed, 30 Nov 2022 14:45:02 +0100 Subject: [PATCH 3/4] remove unused function, count resent by admin --- backend/src/graphql/resolver/AdminResolver.ts | 3 +++ backend/src/graphql/resolver/UserResolver.ts | 14 ++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/backend/src/graphql/resolver/AdminResolver.ts b/backend/src/graphql/resolver/AdminResolver.ts index 80c69a864..40b7ae176 100644 --- a/backend/src/graphql/resolver/AdminResolver.ts +++ b/backend/src/graphql/resolver/AdminResolver.ts @@ -654,6 +654,9 @@ export class AdminResolver { throw new Error(`The emailContact: ${email} of htis User is deleted.`) } + emailContact.emailResendCount++ + await emailContact.save() + // eslint-disable-next-line @typescript-eslint/no-unused-vars const emailSent = await sendAccountActivationEmail({ link: activationLink(emailContact.emailVerificationCode), diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 2e294196a..1d7cf49da 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -466,16 +466,16 @@ export class UserResolver { if (!canEmailResend(user.emailContact.updatedAt || user.emailContact.createdAt)) { logger.error( `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, ) throw new Error( `email already sent less than ${printTimeDuration( - CONFIG.EMAIL_CODE_REQUEST_TIME, - )} minutes ago`, + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, ) } - + user.emailContact.updatedAt = new Date() user.emailContact.emailResendCount++ user.emailContact.emailVerificationCode = random(64) @@ -498,7 +498,9 @@ export class UserResolver { /* uncomment this, when you need the activation link on the console */ // In case EMails are disabled log the activation link for the user if (!emailSent) { - logger.debug(`Reset password link: ${activationLink(user.emailContact.emailVerificationCode)}`) + logger.debug( + `Reset password link: ${activationLink(user.emailContact.emailVerificationCode)}`, + ) } logger.info(`forgotPassword(${email}) successful...`) From 02656ee117be29f2486bb4f2337f2d23a6154d10 Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Tue, 13 Dec 2022 15:49:41 +0100 Subject: [PATCH 4/4] fix tests after merge --- .../graphql/resolver/EmailOptinCodes.test.ts | 1 + .../src/graphql/resolver/UserResolver.test.ts | 24 +++++++++---------- backend/src/graphql/resolver/UserResolver.ts | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/backend/src/graphql/resolver/EmailOptinCodes.test.ts b/backend/src/graphql/resolver/EmailOptinCodes.test.ts index 1cf22850d..d7c0b9bd6 100644 --- a/backend/src/graphql/resolver/EmailOptinCodes.test.ts +++ b/backend/src/graphql/resolver/EmailOptinCodes.test.ts @@ -13,6 +13,7 @@ let testEnv: any CONFIG.EMAIL_CODE_VALID_TIME = 1440 CONFIG.EMAIL_CODE_REQUEST_TIME = 10 +CONFIG.EMAIL = false beforeAll(async () => { testEnv = await testEnvironment() diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index c6535ba2b..053905012 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -25,7 +25,6 @@ import { sendAccountMultiRegistrationEmail, sendResetPasswordEmail, } from '@/emails/sendEmailVariants' -import { activationLink } from './UserResolver' import { contributionLinkFactory } from '@/seeds/factory/contributionLink' import { transactionLinkFactory } from '@/seeds/factory/transactionLink' import { ContributionLink } from '@model/ContributionLink' @@ -844,17 +843,18 @@ describe('UserResolver', () => { ) }) - it('sends reset password email', () => { - expect(sendResetPasswordEmail).toBeCalledWith({ - firstName: 'Bibi', - lastName: 'Bloxberg', - email: 'bibi@bloxberg.de', - language: 'de', - resetLink: activationLink(emailContact.emailVerificationCode), - timeDurationObject: expect.objectContaining({ - hours: expect.any(Number), - minutes: expect.any(Number), - }), + it('sends reset password email', () => { + expect(sendResetPasswordEmail).toBeCalledWith({ + firstName: 'Bibi', + lastName: 'Bloxberg', + email: 'bibi@bloxberg.de', + language: 'de', + resetLink: expect.any(String), + timeDurationObject: expect.objectContaining({ + hours: expect.any(Number), + minutes: expect.any(Number), + }), + }) }) }) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index eebff8344..ed10bb803 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -496,7 +496,7 @@ export class UserResolver { lastName: user.lastName, email, language: user.language, - resetLink: activationLink(dbUserContact.emailVerificationCode), + resetLink: activationLink(user.emailContact.emailVerificationCode), timeDurationObject: getTimeDurationObject(CONFIG.EMAIL_CODE_VALID_TIME), })