diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 18d1143db..ee602a343 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -528,7 +528,7 @@ jobs: report_name: Coverage Backend type: lcov result_path: ./backend/coverage/lcov.info - min_coverage: 54 + min_coverage: 55 token: ${{ github.token }} ########################################################################## diff --git a/backend/.env.dist b/backend/.env.dist index b8a3d5dbe..5e3a89f82 100644 --- a/backend/.env.dist +++ b/backend/.env.dist @@ -1,4 +1,4 @@ -CONFIG_VERSION=v1.2022-03-18 +CONFIG_VERSION=v2.2022-03-24 # Server PORT=4000 @@ -41,8 +41,9 @@ EMAIL_PASSWORD=xxx EMAIL_SMTP_URL=gmail.com EMAIL_SMTP_PORT=587 EMAIL_LINK_VERIFICATION=http://localhost/checkEmail/{optin}{code} -EMAIL_LINK_SETPASSWORD=http://localhost/reset/{optin} -EMAIL_CODE_VALID_TIME=10 +EMAIL_LINK_SETPASSWORD=http://localhost/reset/{code} +EMAIL_CODE_VALID_TIME=1440 +EMAIL_CODE_REQUEST_TIME=10 # Webhook WEBHOOK_ELOPAGE_SECRET=secret \ No newline at end of file diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index dcfea2bdb..caedac08e 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -14,7 +14,7 @@ const constants = { DECAY_START_TIME: new Date('2021-05-13 17:46:31'), // GMT+0 CONFIG_VERSION: { DEFAULT: 'DEFAULT', - EXPECTED: 'v1.2022-03-18', + EXPECTED: 'v2.2022-03-24', CURRENT: '', }, } @@ -70,8 +70,13 @@ const email = { process.env.EMAIL_LINK_VERIFICATION || 'http://localhost/checkEmail/{optin}{code}', EMAIL_LINK_SETPASSWORD: process.env.EMAIL_LINK_SETPASSWORD || 'http://localhost/reset-password/{optin}', + // time in minutes a optin code is valid EMAIL_CODE_VALID_TIME: process.env.EMAIL_CODE_VALID_TIME - ? parseInt(process.env.EMAIL_CODE_VALID_TIME) || 10 + ? parseInt(process.env.EMAIL_CODE_VALID_TIME) || 1440 + : 1440, + // time in minutes that must pass to request a new optin code + EMAIL_CODE_REQUEST_TIME: process.env.EMAIL_CODE_REQUEST_TIME + ? parseInt(process.env.EMAIL_CODE_REQUEST_TIME) || 10 : 10, } diff --git a/backend/src/graphql/enum/OptInType.ts b/backend/src/graphql/enum/OptInType.ts new file mode 100644 index 000000000..2dd2d07b0 --- /dev/null +++ b/backend/src/graphql/enum/OptInType.ts @@ -0,0 +1,11 @@ +import { registerEnumType } from 'type-graphql' + +export enum OptInType { + EMAIL_OPT_IN_REGISTER = 1, + EMAIL_OPT_IN_RESET_PASSWORD = 2, +} + +registerEnumType(OptInType, { + name: 'OptInType', // this one is mandatory + description: 'Type of the email optin', // this one is optional +}) diff --git a/backend/src/graphql/resolver/AdminResolver.ts b/backend/src/graphql/resolver/AdminResolver.ts index 283a4ed5a..9013a91f4 100644 --- a/backend/src/graphql/resolver/AdminResolver.ts +++ b/backend/src/graphql/resolver/AdminResolver.ts @@ -39,6 +39,8 @@ import Paginated from '@arg/Paginated' import TransactionLinkFilters from '@arg/TransactionLinkFilters' import { Order } from '@enum/Order' import { communityUser } from '@/util/communityUser' +import { checkOptInCode, activationLink } from './UserResolver' +import { sendAccountActivationEmail } from '@/mailer/sendAccountActivationEmail' // const EMAIL_OPT_IN_REGISTER = 1 // const EMAIL_OPT_UNKNOWN = 3 // elopage? @@ -375,6 +377,39 @@ export class AdminResolver { return userTransactions.map((t) => new Transaction(t, new User(user), communityUser)) } + @Authorized([RIGHTS.SEND_ACTIVATION_EMAIL]) + @Mutation(() => Boolean) + async sendActivationEmail(@Arg('email') email: string): Promise { + email = email.trim().toLowerCase() + const user = await dbUser.findOneOrFail({ email: email }) + + // can be both types: REGISTER and RESET_PASSWORD + let optInCode = await LoginEmailOptIn.findOne({ + where: { userId: user.id }, + order: { updatedAt: 'DESC' }, + }) + + optInCode = await checkOptInCode(optInCode, user.id) + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const emailSent = await sendAccountActivationEmail({ + link: activationLink(optInCode), + firstName: user.firstName, + lastName: user.lastName, + email, + }) + + /* 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) { + // eslint-disable-next-line no-console + console.log(`Account confirmation link: ${activationLink}`) + } + */ + + return true + } + @Authorized([RIGHTS.LIST_TRANSACTION_LINKS_ADMIN]) @Query(() => TransactionLinkResult) async listTransactionLinksAdmin( diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 53f39668e..c01cf2de9 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -11,6 +11,7 @@ import { LoginEmailOptIn } from '@entity/LoginEmailOptIn' import { User } from '@entity/User' import CONFIG from '@/config' import { sendAccountActivationEmail } from '@/mailer/sendAccountActivationEmail' +import { printTimeDuration } from './UserResolver' // import { klicktippSignIn } from '@/apis/KlicktippController' @@ -220,10 +221,6 @@ describe('UserResolver', () => { expect(newUser[0].password).toEqual('3917921995996627700') }) - it('removes the optin', async () => { - await expect(LoginEmailOptIn.find()).resolves.toHaveLength(0) - }) - /* it('calls the klicktipp API', () => { expect(klicktippSignIn).toBeCalledWith( @@ -415,3 +412,17 @@ describe('UserResolver', () => { }) }) }) + +describe('printTimeDuration', () => { + it('works with 10 minutes', () => { + expect(printTimeDuration(10)).toBe('10 minutes') + }) + + it('works with 1440 minutes', () => { + expect(printTimeDuration(1440)).toBe('24 hours') + }) + + it('works with 1410 minutes', () => { + expect(printTimeDuration(1410)).toBe('23 hours and 30 minutes') + }) +}) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index b24aa1b58..07cd90a23 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -3,7 +3,7 @@ import fs from 'fs' import { Resolver, Query, Args, Arg, Authorized, Ctx, UseMiddleware, Mutation } from 'type-graphql' -import { getConnection, getCustomRepository, QueryRunner } from '@dbTools/typeorm' +import { getConnection, getCustomRepository } from '@dbTools/typeorm' import CONFIG from '@/config' import { User } from '@model/User' import { User as DbUser } from '@entity/User' @@ -15,6 +15,7 @@ import UpdateUserInfosArgs from '@arg/UpdateUserInfosArgs' import { klicktippNewsletterStateMiddleware } from '@/middleware/klicktippMiddleware' import { UserSettingRepository } from '@repository/UserSettingRepository' import { Setting } from '@enum/Setting' +import { OptInType } from '@enum/OptInType' import { LoginEmailOptIn } from '@entity/LoginEmailOptIn' import { sendResetPasswordEmail } from '@/mailer/sendResetPasswordEmail' import { sendAccountActivationEmail } from '@/mailer/sendAccountActivationEmail' @@ -24,9 +25,6 @@ import { ROLE_ADMIN } from '@/auth/ROLES' import { hasElopageBuys } from '@/util/hasElopageBuys' import { ServerUser } from '@entity/ServerUser' -const EMAIL_OPT_IN_RESET_PASSWORD = 2 -const EMAIL_OPT_IN_REGISTER = 1 - // eslint-disable-next-line @typescript-eslint/no-var-requires const sodium = require('sodium-native') // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -148,57 +146,47 @@ const SecretKeyCryptographyDecrypt = (encryptedMessage: Buffer, encryptionKey: B return message } -const createEmailOptIn = async ( - loginUserId: number, - queryRunner: QueryRunner, -): Promise => { - let emailOptIn = await LoginEmailOptIn.findOne({ - userId: loginUserId, - emailOptInTypeId: EMAIL_OPT_IN_REGISTER, - }) - if (emailOptIn) { - if (isOptInCodeValid(emailOptIn)) { - throw new Error(`email already sent less than $(CONFIG.EMAIL_CODE_VALID_TIME} minutes ago`) - } - emailOptIn.updatedAt = new Date() - emailOptIn.resendCount++ - } else { - emailOptIn = new LoginEmailOptIn() - emailOptIn.verificationCode = random(64) - emailOptIn.userId = loginUserId - emailOptIn.emailOptInTypeId = EMAIL_OPT_IN_REGISTER - } - await queryRunner.manager.save(emailOptIn).catch((error) => { - // eslint-disable-next-line no-console - console.log('Error while saving emailOptIn', error) - throw new Error('error saving email opt in') - }) + +const newEmailOptIn = (userId: number): LoginEmailOptIn => { + const emailOptIn = new LoginEmailOptIn() + emailOptIn.verificationCode = random(64) + emailOptIn.userId = userId + emailOptIn.emailOptInTypeId = OptInType.EMAIL_OPT_IN_REGISTER return emailOptIn } -const getOptInCode = async (loginUserId: number): Promise => { - let optInCode = await LoginEmailOptIn.findOne({ - userId: loginUserId, - emailOptInTypeId: EMAIL_OPT_IN_RESET_PASSWORD, - }) - - // Check for `CONFIG.EMAIL_CODE_VALID_TIME` minute delay +// 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, + userId: number, + optInType: OptInType = OptInType.EMAIL_OPT_IN_REGISTER, +): Promise => { if (optInCode) { - if (isOptInCodeValid(optInCode)) { - throw new Error(`email already sent less than $(CONFIG.EMAIL_CODE_VALID_TIME} minutes ago`) + if (!canResendOptIn(optInCode)) { + throw new Error( + `email already sent less than ${printTimeDuration( + CONFIG.EMAIL_CODE_REQUEST_TIME, + )} minutes ago`, + ) } optInCode.updatedAt = new Date() optInCode.resendCount++ } else { - optInCode = new LoginEmailOptIn() - optInCode.verificationCode = random(64) - optInCode.userId = loginUserId - optInCode.emailOptInTypeId = EMAIL_OPT_IN_RESET_PASSWORD + optInCode = newEmailOptIn(userId) } - await LoginEmailOptIn.save(optInCode) + optInCode.emailOptInTypeId = optInType + await LoginEmailOptIn.save(optInCode).catch(() => { + throw new Error('Unable to save optin code.') + }) return optInCode } +export const activationLink = (optInCode: LoginEmailOptIn): string => { + return CONFIG.EMAIL_LINK_SETPASSWORD.replace(/{optin}/g, optInCode.verificationCode.toString()) +} + @Resolver() export class UserResolver { @Authorized([RIGHTS.VERIFY_LOGIN]) @@ -363,9 +351,12 @@ export class UserResolver { throw new Error('error saving user') }) - // Store EmailOptIn in DB - // TODO: this has duplicate code with sendResetPasswordEmail - const emailOptIn = await createEmailOptIn(dbUser.id, queryRunner) + const emailOptIn = newEmailOptIn(dbUser.id) + await queryRunner.manager.save(emailOptIn).catch((error) => { + // eslint-disable-next-line no-console + console.log('Error while saving emailOptIn', error) + throw new Error('error saving email opt in') + }) const activationLink = CONFIG.EMAIL_LINK_VERIFICATION.replace( /{optin}/g, @@ -398,67 +389,22 @@ export class UserResolver { return new User(dbUser) } - // THis is used by the admin only - should we move it to the admin resolver? - @Authorized([RIGHTS.SEND_ACTIVATION_EMAIL]) - @Mutation(() => Boolean) - async sendActivationEmail(@Arg('email') email: string): Promise { - email = email.trim().toLowerCase() - const user = await DbUser.findOneOrFail({ email: email }) - - const queryRunner = getConnection().createQueryRunner() - await queryRunner.connect() - await queryRunner.startTransaction('READ UNCOMMITTED') - - try { - const emailOptIn = await createEmailOptIn(user.id, queryRunner) - - const activationLink = CONFIG.EMAIL_LINK_VERIFICATION.replace( - /{optin}/g, - emailOptIn.verificationCode.toString(), - ) - - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const emailSent = await sendAccountActivationEmail({ - link: activationLink, - firstName: user.firstName, - lastName: user.lastName, - email, - }) - - /* 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) { - // eslint-disable-next-line no-console - console.log(`Account confirmation link: ${activationLink}`) - } - */ - await queryRunner.commitTransaction() - } catch (e) { - await queryRunner.rollbackTransaction() - throw e - } finally { - await queryRunner.release() - } - return true - } - @Authorized([RIGHTS.SEND_RESET_PASSWORD_EMAIL]) @Query(() => Boolean) async sendResetPasswordEmail(@Arg('email') email: string): Promise { - // TODO: this has duplicate code with createUser email = email.trim().toLowerCase() const user = await DbUser.findOneOrFail({ email }) - const optInCode = await getOptInCode(user.id) + // can be both types: REGISTER and RESET_PASSWORD + let optInCode = await LoginEmailOptIn.findOne({ + userId: user.id, + }) - const link = CONFIG.EMAIL_LINK_SETPASSWORD.replace( - /{optin}/g, - optInCode.verificationCode.toString(), - ) + optInCode = await checkOptInCode(optInCode, user.id, OptInType.EMAIL_OPT_IN_RESET_PASSWORD) // eslint-disable-next-line @typescript-eslint/no-unused-vars const emailSent = await sendResetPasswordEmail({ - link, + link: activationLink(optInCode), firstName: user.firstName, lastName: user.lastName, email, @@ -494,8 +440,10 @@ export class UserResolver { }) // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes - if (!isOptInCodeValid(optInCode)) { - throw new Error(`email already more than $(CONFIG.EMAIL_CODE_VALID_TIME} minutes ago`) + if (!isOptInValid(optInCode)) { + throw new Error( + `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, + ) } // load user @@ -538,11 +486,6 @@ export class UserResolver { throw new Error('error saving user: ' + error) }) - // Delete Code - await queryRunner.manager.remove(optInCode).catch((error) => { - throw new Error('error deleting code: ' + error) - }) - await queryRunner.commitTransaction() } catch (e) { await queryRunner.rollbackTransaction() @@ -553,7 +496,7 @@ export class UserResolver { // Sign into Klicktipp // TODO do we always signUp the user? How to handle things with old users? - if (optInCode.emailOptInTypeId === EMAIL_OPT_IN_REGISTER) { + if (optInCode.emailOptInTypeId === OptInType.EMAIL_OPT_IN_REGISTER) { try { await klicktippSignIn(user.email, user.language, user.firstName, user.lastName) } catch { @@ -573,8 +516,10 @@ export class UserResolver { async queryOptIn(@Arg('optIn') optIn: string): Promise { const optInCode = await LoginEmailOptIn.findOneOrFail({ verificationCode: optIn }) // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes - if (!isOptInCodeValid(optInCode)) { - throw new Error(`email was sent more than $(CONFIG.EMAIL_CODE_VALID_TIME} minutes ago`) + if (!isOptInValid(optInCode)) { + throw new Error( + `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, + ) } return true } @@ -680,7 +625,34 @@ export class UserResolver { return hasElopageBuys(userEntity.email) } } -function isOptInCodeValid(optInCode: LoginEmailOptIn) { - const timeElapsed = Date.now() - new Date(optInCode.updatedAt).getTime() - return timeElapsed <= CONFIG.EMAIL_CODE_VALID_TIME * 60 * 1000 + +const isTimeExpired = (optIn: LoginEmailOptIn, duration: number): boolean => { + const timeElapsed = Date.now() - new Date(optIn.updatedAt).getTime() + // time is given in minutes + return timeElapsed <= duration * 60 * 1000 +} + +const isOptInValid = (optIn: LoginEmailOptIn): boolean => { + return isTimeExpired(optIn, CONFIG.EMAIL_CODE_VALID_TIME) +} + +const canResendOptIn = (optIn: LoginEmailOptIn): boolean => { + return !isTimeExpired(optIn, CONFIG.EMAIL_CODE_REQUEST_TIME) +} + +const getTimeDurationObject = (time: number): { hours?: number; minutes: number } => { + if (time > 60) { + return { + hours: Math.floor(time / 60), + minutes: time % 60, + } + } + return { minutes: time } +} + +export const printTimeDuration = (duration: number): string => { + const time = getTimeDurationObject(duration) + const result = time.minutes > 0 ? `${time.minutes} minutes` : '' + if (time.hours) return `${time.hours} hours` + (result !== '' ? ` and ${result}` : '') + return result } diff --git a/deployment/bare_metal/.env.dist b/deployment/bare_metal/.env.dist index 88dfff6f5..1cb78299a 100644 --- a/deployment/bare_metal/.env.dist +++ b/deployment/bare_metal/.env.dist @@ -18,7 +18,7 @@ WEBHOOK_GITHUB_SECRET=secret WEBHOOK_GITHUB_BRANCH=master # backend -BACKEND_CONFIG_VERSION=v1.2022-03-18 +BACKEND_CONFIG_VERSION=v2.2022-03-24 EMAIL=true EMAIL_USERNAME=peter@lustig.de diff --git a/frontend/src/pages/ResetPassword.spec.js b/frontend/src/pages/ResetPassword.spec.js index c43f71932..36efa0a11 100644 --- a/frontend/src/pages/ResetPassword.spec.js +++ b/frontend/src/pages/ResetPassword.spec.js @@ -150,13 +150,18 @@ describe('ResetPassword', () => { describe('server response with error code > 10min', () => { beforeEach(async () => { - apolloMutationMock.mockRejectedValue({ message: '...Code is older than 10 minutes' }) + jest.clearAllMocks() + apolloMutationMock.mockRejectedValue({ + message: '...email was sent more than 23 hours and 10 minutes ago', + }) await wrapper.find('form').trigger('submit') await flushPromises() }) it('toasts an error message', () => { - expect(toastErrorSpy).toHaveBeenCalledWith('...Code is older than 10 minutes') + expect(toastErrorSpy).toHaveBeenCalledWith( + '...email was sent more than 23 hours and 10 minutes ago', + ) }) it('router pushes to /forgot-password/resetPassword', () => { diff --git a/frontend/src/pages/ResetPassword.vue b/frontend/src/pages/ResetPassword.vue index 7771be5f6..a737246bc 100644 --- a/frontend/src/pages/ResetPassword.vue +++ b/frontend/src/pages/ResetPassword.vue @@ -108,7 +108,11 @@ export default { }) .catch((error) => { this.toastError(error.message) - if (error.message.includes('Code is older than 10 minutes')) + if ( + error.message.match( + /email was sent more than ([0-9]+ hours)?( and )?([0-9]+ minutes)? ago/, + ) + ) this.$router.push('/forgot-password/resetPassword') }) },