From 9c4de72be6ea761c22d3a1c2dd3c40a3d53ce79e Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 13:30:45 +0100 Subject: [PATCH 01/29] LogError class --- backend/src/server/LogError.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 backend/src/server/LogError.ts diff --git a/backend/src/server/LogError.ts b/backend/src/server/LogError.ts new file mode 100644 index 000000000..b753d204e --- /dev/null +++ b/backend/src/server/LogError.ts @@ -0,0 +1,9 @@ +import { backendLogger as logger } from './logger' + +export default class LogError extends Error { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + constructor(msg: string, ...details: any[]) { + super(msg) + logger.error(msg, ...details) + } +} From 25e03c35b2a610946c592707ad3b75ce9b4b48c5 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 13:31:10 +0100 Subject: [PATCH 02/29] refactor UserResolver to use LogError, clean comments --- backend/src/graphql/resolver/UserResolver.ts | 131 ++++++------------- 1 file changed, 38 insertions(+), 93 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index c630c240a..773e1bac1 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -63,6 +63,7 @@ import { isValidPassword } from '@/password/EncryptorUtils' import { FULL_CREATION_AVAILABLE } from './const/const' import { encryptPassword, verifyPassword } from '@/password/PasswordEncryptor' import { PasswordEncryptionType } from '../enum/PasswordEncryptionType' +import LogError from '@/server/LogError' // eslint-disable-next-line @typescript-eslint/no-var-requires const sodium = require('sodium-native') @@ -134,22 +135,18 @@ export class UserResolver { email = email.trim().toLowerCase() const dbUser = await findUserByEmail(email) if (dbUser.deletedAt) { - logger.error('The User was permanently deleted in database.') - throw new Error('This user was permanently deleted. Contact support for questions.') + throw new LogError('This user was permanently deleted. Contact support for questions', dbUser) } if (!dbUser.emailContact.emailChecked) { - logger.error('The Users email is not validate yet.') - throw new Error('User email not validated') + throw new LogError('The Users email is not validate yet', dbUser) } if (dbUser.password === BigInt(0)) { - logger.error('The User has not set a password yet.') // TODO we want to catch this on the frontend and ask the user to check his emails or resend code - throw new Error('User has no password set yet') + throw new LogError('The User has not set a password yet', dbUser) } if (!verifyPassword(dbUser, password)) { - logger.error('The User has no valid credentials.') - throw new Error('No user with this credentials') + throw new LogError('No user with this credentials', dbUser) } if (dbUser.passwordEncryptionType !== PasswordEncryptionType.GRADIDO_ID) { @@ -309,30 +306,19 @@ export class UserResolver { await queryRunner.startTransaction('REPEATABLE READ') try { dbUser = await queryRunner.manager.save(dbUser).catch((error) => { - logger.error('Error while saving dbUser', error) - throw new Error('error saving user') + throw new LogError('Error while saving dbUser', error) }) let emailContact = newEmailContact(email, dbUser.id) emailContact = await queryRunner.manager.save(emailContact).catch((error) => { - logger.error('Error while saving emailContact', error) - throw new Error('error saving email user contact') + throw new LogError('Error while saving user email contact', error) }) dbUser.emailContact = emailContact dbUser.emailId = emailContact.id await queryRunner.manager.save(dbUser).catch((error) => { - logger.error('Error while updating dbUser', error) - throw new Error('error updating user') + throw new LogError('Error while updating dbUser', error) }) - /* - const emailOptIn = newEmailOptIn(dbUser.id) - await queryRunner.manager.save(emailOptIn).catch((error) => { - logger.error('Error while saving emailOptIn', error) - throw new Error('error saving email opt in') - }) - */ - const activationLink = CONFIG.EMAIL_LINK_VERIFICATION.replace( /{optin}/g, emailContact.emailVerificationCode.toString(), @@ -358,9 +344,8 @@ export class UserResolver { await queryRunner.commitTransaction() logger.addContext('user', dbUser.id) } catch (e) { - logger.error(`error during create user with ${e}`) await queryRunner.rollbackTransaction() - throw e + throw new LogError(`Error during create user with error: ${e}`, e) } finally { await queryRunner.release() } @@ -392,13 +377,8 @@ 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`, - ) - throw new Error( - `email already sent less than ${printTimeDuration( + throw new LogError( + `Email already sent less than ${printTimeDuration( CONFIG.EMAIL_CODE_REQUEST_TIME, )} minutes ago`, ) @@ -409,8 +389,7 @@ export class UserResolver { 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.') + throw new LogError('Unable to save email verification code', user.emailContact) }) logger.info(`optInCode for ${email}=${user.emailContact}`) @@ -445,33 +424,22 @@ export class UserResolver { logger.info(`setPassword(${code}, ***)...`) // Validate Password if (!isValidPassword(password)) { - logger.error('Password entered is lexically invalid') - throw new Error( + throw new LogError( 'Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!', ) } - // Load code - /* - const optInCode = await LoginEmailOptIn.findOneOrFail({ verificationCode: code }).catch(() => { - logger.error('Could not login with emailVerificationCode') - throw new Error('Could not login with emailVerificationCode') - }) - */ + // load code const userContact = await DbUserContact.findOneOrFail( { emailVerificationCode: code }, { relations: ['user'] }, ).catch(() => { - logger.error('Could not login with emailVerificationCode') - throw new Error('Could not login with emailVerificationCode') + throw new LogError('Could not login with emailVerificationCode') }) logger.debug('userContact loaded...') // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { - logger.error( - `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, - ) - throw new Error( + throw new LogError( `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) } @@ -498,13 +466,11 @@ export class UserResolver { try { // Save user await queryRunner.manager.save(user).catch((error) => { - logger.error('error saving user: ' + error) - throw new Error('error saving user: ' + error) + throw new LogError(`Error saving user: ${error}`, error) }) // Save userContact await queryRunner.manager.save(userContact).catch((error) => { - logger.error('error saving userContact: ' + error) - throw new Error('error saving userContact: ' + error) + throw new LogError(`error saving userContact: ${error}`, error) }) await queryRunner.commitTransaction() @@ -515,8 +481,7 @@ export class UserResolver { eventProtocol.writeEvent(event.setEventActivateAccount(eventActivateAccount)) } catch (e) { await queryRunner.rollbackTransaction() - logger.error('Error on writing User and UserContact data:' + e) - throw e + throw new LogError(`Error on writing User and UserContact data: ${e}`, e) } finally { await queryRunner.release() } @@ -530,7 +495,7 @@ export class UserResolver { `klicktippSignIn(${userContact.email}, ${user.language}, ${user.firstName}, ${user.lastName})`, ) } catch (e) { - logger.error('Error subscribe to klicktipp:' + e) + logger.error(`Error subscribe to klicktipp: ${e}`, e) // TODO is this a problem? // eslint-disable-next-line no-console /* uncomment this, when you need the activation link on the console @@ -550,10 +515,7 @@ export class UserResolver { logger.debug(`found optInCode=${userContact}`) // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { - logger.error( - `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, - ) - throw new Error( + throw new LogError( `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) } @@ -589,8 +551,7 @@ export class UserResolver { if (language) { if (!isLanguage(language)) { - logger.error(`"${language}" isn't a valid language`) - throw new Error(`"${language}" isn't a valid language`) + throw new LogError(`"${language}" isn't a valid language`, language) } userEntity.language = language i18n.setLocale(language) @@ -599,15 +560,13 @@ export class UserResolver { if (password && passwordNew) { // Validate Password if (!isValidPassword(passwordNew)) { - logger.error('newPassword does not fullfil the rules') - throw new Error( + throw new LogError( 'Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!', ) } if (!verifyPassword(userEntity, password)) { - logger.error(`Old password is invalid`) - throw new Error(`Old password is invalid`) + throw new LogError(`Old password is invalid`) } // Save new password hash and newly encrypted private key @@ -630,16 +589,14 @@ export class UserResolver { try { await queryRunner.manager.save(userEntity).catch((error) => { - logger.error('error saving user: ' + error) - throw new Error('error saving user: ' + error) + throw new LogError(`error saving user: ${error}`, error) }) await queryRunner.commitTransaction() logger.debug('writing User data successful...') } catch (e) { await queryRunner.rollbackTransaction() - logger.error(`error on writing updated user data: ${e}`) - throw e + throw new LogError(`error on writing updated user data: ${e}`, e) } finally { await queryRunner.release() } @@ -766,14 +723,12 @@ export class UserResolver { const user = await DbUser.findOne({ id: userId }) // user exists ? if (!user) { - logger.error(`Could not find user with userId: ${userId}`) - throw new Error(`Could not find user with userId: ${userId}`) + throw new LogError(`Could not find user with userId: ${userId}`, userId) } // administrator user changes own role? const moderatorUser = getUser(context) if (moderatorUser.id === userId) { - logger.error('Administrator can not change his own role!') - throw new Error('Administrator can not change his own role!') + throw new LogError('Administrator can not change his own role') } // change isAdmin switch (user.isAdmin) { @@ -781,16 +736,14 @@ export class UserResolver { if (isAdmin === true) { user.isAdmin = new Date() } else { - logger.error('User is already a usual user!') - throw new Error('User is already a usual user!') + throw new LogError('User is already an usual user') } break default: if (isAdmin === false) { user.isAdmin = null } else { - logger.error('User is already admin!') - throw new Error('User is already admin!') + throw new LogError('User is already admin') } break } @@ -808,14 +761,12 @@ export class UserResolver { const user = await DbUser.findOne({ id: userId }) // user exists ? if (!user) { - logger.error(`Could not find user with userId: ${userId}`) - throw new Error(`Could not find user with userId: ${userId}`) + throw new LogError(`Could not find user with userId: ${userId}`, userId) } // moderator user disabled own account? const moderatorUser = getUser(context) if (moderatorUser.id === userId) { - logger.error('Moderator can not delete his own account!') - throw new Error('Moderator can not delete his own account!') + throw new LogError('Moderator can not delete his own account') } // soft-delete user await user.softRemove() @@ -828,12 +779,10 @@ export class UserResolver { async unDeleteUser(@Arg('userId', () => Int) userId: number): Promise { const user = await DbUser.findOne({ id: userId }, { withDeleted: true }) if (!user) { - logger.error(`Could not find user with userId: ${userId}`) - throw new Error(`Could not find user with userId: ${userId}`) + throw new LogError(`Could not find user with userId: ${userId}`, userId) } if (!user.deletedAt) { - logger.error('User is not deleted') - throw new Error('User is not deleted') + throw new LogError('User is not deleted') } await user.recover() return null @@ -846,17 +795,14 @@ export class UserResolver { // const user = await dbUser.findOne({ id: emailContact.userId }) const user = await findUserByEmail(email) if (!user) { - logger.error(`Could not find User to emailContact: ${email}`) - throw new Error(`Could not find User to emailContact: ${email}`) + throw new LogError(`Could not find User to emailContact: ${email}`, email) } if (user.deletedAt) { - logger.error(`User with emailContact: ${email} is deleted.`) - throw new Error(`User with emailContact: ${email} is deleted.`) + throw new LogError(`User with emailContact: ${email} is deleted`, email) } const emailContact = user.emailContact if (emailContact.deletedAt) { - logger.error(`The emailContact: ${email} of this User is deleted.`) - throw new Error(`The emailContact: ${email} of this User is deleted.`) + throw new LogError(`The emailContact: ${email} of this User is deleted`, email) } emailContact.emailResendCount++ @@ -893,8 +839,7 @@ export async function findUserByEmail(email: string): Promise { { email: email }, { withDeleted: true, relations: ['user'] }, ).catch(() => { - logger.error(`UserContact with email=${email} does not exists`) - throw new Error('No user with this credentials') + throw new LogError('No user with this credentials', email) }) const dbUser = dbUserContact.user dbUser.emailContact = dbUserContact From 412499d5f2d3d3200ce743df44c7c002a739c26f Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 21:04:39 +0100 Subject: [PATCH 03/29] fix unit tests for UserResolver --- .../src/graphql/resolver/UserResolver.test.ts | 60 ++++++++++++------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 073bad04d..3b2ff0a73 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -549,7 +549,9 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Password entered is lexically invalid') + expect(logger.error).toBeCalledWith( + 'Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!', + ) }) }) @@ -606,9 +608,7 @@ describe('UserResolver', () => { }) it('logs the error found', () => { - expect(logger.error).toBeCalledWith( - 'UserContact with email=bibi@bloxberg.de does not exists', - ) + expect(logger.error).toBeCalledWith('No user with this credentials', variables.email) }) }) @@ -668,7 +668,7 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The User has no valid credentials.') + expect(logger.error).toBeCalledWith('No user with this credentials', variables.email) }) }) }) @@ -828,7 +828,7 @@ describe('UserResolver', () => { expect.objectContaining({ errors: [ new GraphQLError( - `email already sent less than ${printTimeDuration( + `Email already sent less than ${printTimeDuration( CONFIG.EMAIL_CODE_REQUEST_TIME, )} minutes ago`, ), @@ -870,13 +870,13 @@ describe('UserResolver', () => { CONFIG.EMAIL_CODE_REQUEST_TIME = emailCodeRequestTime await expect(mutate({ mutation: forgotPassword, variables })).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('email already sent less than 10 minutes minutes ago')], + errors: [new GraphQLError('Email already sent less than 10 minutes minutes ago')], }), ) }) it('logs the error found', () => { - expect(logger.error).toBeCalledWith(`email already sent less than 10 minutes minutes ago`) + expect(logger.error).toBeCalledWith(`Email already sent less than 10 minutes minutes ago`) }) }) }) @@ -1007,7 +1007,7 @@ describe('UserResolver', () => { }) it('logs the error found', () => { - expect(logger.error).toBeCalledWith(`"not-valid" isn't a valid language`) + expect(logger.error).toBeCalledWith(`"not-valid" isn't a valid language`, 'not-valid') }) }) @@ -1058,7 +1058,9 @@ describe('UserResolver', () => { }) it('logs the error found', () => { - expect(logger.error).toBeCalledWith('newPassword does not fullfil the rules') + expect(logger.error).toBeCalledWith( + 'Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!', + ) }) }) @@ -1116,7 +1118,7 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The User has no valid credentials.') + expect(logger.error).toBeCalledWith('Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!') }) }) }) @@ -1328,7 +1330,10 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith(`Could not find user with userId: ${admin.id + 1}`) + expect(logger.error).toBeCalledWith( + `Could not find user with userId: ${admin.id + 1}`, + admin.id + 1, + ) }) }) @@ -1379,12 +1384,12 @@ describe('UserResolver', () => { mutate({ mutation: setUserRole, variables: { userId: admin.id, isAdmin: false } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('Administrator can not change his own role!')], + errors: [new GraphQLError('Administrator can not change his own role')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Administrator can not change his own role!') + expect(logger.error).toBeCalledWith('Administrator can not change his own role') }) }) @@ -1400,13 +1405,13 @@ describe('UserResolver', () => { mutate({ mutation: setUserRole, variables: { userId: user.id, isAdmin: true } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('User is already admin!')], + errors: [new GraphQLError('User is already admin')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('User is already admin!') + expect(logger.error).toBeCalledWith('User is already admin') }) }) @@ -1421,13 +1426,13 @@ describe('UserResolver', () => { mutate({ mutation: setUserRole, variables: { userId: user.id, isAdmin: false } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('User is already a usual user!')], + errors: [new GraphQLError('User is already an usual user')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('User is already a usual user!') + expect(logger.error).toBeCalledWith('User is already an usual user') }) }) }) @@ -1500,7 +1505,10 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith(`Could not find user with userId: ${admin.id + 1}`) + expect(logger.error).toBeCalledWith( + `Could not find user with userId: ${admin.id + 1}`, + admin.id + 1, + ) }) }) @@ -1511,13 +1519,13 @@ describe('UserResolver', () => { mutate({ mutation: deleteUser, variables: { userId: admin.id } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('Moderator can not delete his own account!')], + errors: [new GraphQLError('Moderator can not delete his own account')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Moderator can not delete his own account!') + expect(logger.error).toBeCalledWith('Moderator can not delete his own account') }) }) @@ -1551,7 +1559,10 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith(`Could not find user with userId: ${user.id}`) + expect(logger.error).toBeCalledWith( + `Could not find user with userId: ${user.id}`, + user.id, + ) }) }) }) @@ -1623,7 +1634,10 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith(`Could not find user with userId: ${admin.id + 1}`) + expect(logger.error).toBeCalledWith( + `Could not find user with userId: ${admin.id + 1}`, + admin.id + 1, + ) }) }) From 45a92e637a190753a81d36d3e021df5758528d56 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 21:07:07 +0100 Subject: [PATCH 04/29] deleted old commented code --- backend/src/graphql/resolver/UserResolver.ts | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 773e1bac1..fe33d9287 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -854,31 +854,16 @@ async function checkEmailExists(email: string): Promise { return false } -/* -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 isTimeExpired = (updatedAt: Date, duration: number): boolean => { const timeElapsed = Date.now() - new Date(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 isEmailVerificationCodeValid = (updatedAt: Date): boolean => { return isTimeExpired(updatedAt, CONFIG.EMAIL_CODE_VALID_TIME) } -/* -const canResendOptIn = (optIn: LoginEmailOptIn): boolean => { - return !isTimeExpired(optIn, CONFIG.EMAIL_CODE_REQUEST_TIME) -} -*/ + const canEmailResend = (updatedAt: Date): boolean => { return !isTimeExpired(updatedAt, CONFIG.EMAIL_CODE_REQUEST_TIME) } From 702a39af9ae9ffa179d7760dc677d00997d7b6bf Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 21:34:30 +0100 Subject: [PATCH 05/29] test case deleted User --- .../src/graphql/resolver/UserResolver.test.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 3b2ff0a73..b1b1b015f 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -671,6 +671,40 @@ describe('UserResolver', () => { expect(logger.error).toBeCalledWith('No user with this credentials', variables.email) }) }) + + describe('user is in database but deleted', () => { + beforeAll(async () => { + jest.clearAllMocks() + const user = await userFactory(testEnv, bibiBloxberg) + // Hint: softRemove does not soft-delete the email contact of this user + await user.softRemove() + result = await mutate({ mutation: login, variables }) + }) + + afterAll(async () => { + await cleanDB() + }) + + it('returns an error', () => { + expect(result).toEqual( + expect.objectContaining({ + errors: [ + new GraphQLError('This user was permanently deleted. Contact support for questions'), + ], + }), + ) + }) + + it('logs the error thrown', () => { + expect(logger.error).toBeCalledWith( + 'This user was permanently deleted. Contact support for questions', + expect.objectContaining({ + firstName: bibiBloxberg.firstName, + lastName: bibiBloxberg.lastName, + }), + ) + }) + }) }) describe('logout', () => { From d0d45aaf6a67e91ed8ddaa8cb8f47f05393a4313 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 22:11:58 +0100 Subject: [PATCH 06/29] more tests and a potential bug --- .../src/graphql/resolver/UserResolver.test.ts | 77 ++++++++++++++++++- backend/src/graphql/resolver/UserResolver.ts | 1 + 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index b1b1b015f..8f8c5757e 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -675,9 +675,82 @@ describe('UserResolver', () => { describe('user is in database but deleted', () => { beforeAll(async () => { jest.clearAllMocks() + await userFactory(testEnv, stephenHawking) + const variables = { + email: stephenHawking.email, + password: 'Aa12345_', + publisherId: 1234, + } + result = await mutate({ mutation: login, variables }) + }) + + afterAll(async () => { + await cleanDB() + }) + + it('returns an error', () => { + expect(result).toEqual( + expect.objectContaining({ + errors: [ + new GraphQLError('This user was permanently deleted. Contact support for questions'), + ], + }), + ) + }) + + it('logs the error thrown', () => { + expect(logger.error).toBeCalledWith( + 'This user was permanently deleted. Contact support for questions', + expect.objectContaining({ + firstName: stephenHawking.firstName, + lastName: stephenHawking.lastName, + }), + ) + }) + }) + + describe('user is in database but email not confirmed', () => { + beforeAll(async () => { + jest.clearAllMocks() + await userFactory(testEnv, garrickOllivander) + const variables = { + email: garrickOllivander.email, + password: 'Aa12345_', + publisherId: 1234, + } + result = await mutate({ mutation: login, variables }) + }) + + afterAll(async () => { + await cleanDB() + }) + + it('returns an error', () => { + expect(result).toEqual( + expect.objectContaining({ + errors: [new GraphQLError('The Users email is not validate yet')], + }), + ) + }) + + it('logs the error thrown', () => { + expect(logger.error).toBeCalledWith( + 'The Users email is not validate yet', + expect.objectContaining({ + firstName: garrickOllivander.firstName, + lastName: garrickOllivander.lastName, + }), + ) + }) + }) + + describe.skip('user is in database but password is not set', () => { + beforeAll(async () => { + jest.clearAllMocks() + // TODO: we need an user without password set const user = await userFactory(testEnv, bibiBloxberg) - // Hint: softRemove does not soft-delete the email contact of this user - await user.softRemove() + user.password = BigInt(0) + await user.save() result = await mutate({ mutation: login, variables }) }) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index fe33d9287..b50cf3af5 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -140,6 +140,7 @@ export class UserResolver { if (!dbUser.emailContact.emailChecked) { throw new LogError('The Users email is not validate yet', dbUser) } + // TODO: at least in test this does not work since `dbUser.password = 0` and `BigInto(0) = 0n` if (dbUser.password === BigInt(0)) { // TODO we want to catch this on the frontend and ask the user to check his emails or resend code throw new LogError('The User has not set a password yet', dbUser) From 6e663f609329f8d507a6607a7844069e78955708 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 22:13:13 +0100 Subject: [PATCH 07/29] even tho skipped have the correct error message in test --- backend/src/graphql/resolver/UserResolver.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 8f8c5757e..28f5d816d 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -761,16 +761,14 @@ describe('UserResolver', () => { it('returns an error', () => { expect(result).toEqual( expect.objectContaining({ - errors: [ - new GraphQLError('This user was permanently deleted. Contact support for questions'), - ], + errors: [new GraphQLError('The User has not set a password yet')], }), ) }) it('logs the error thrown', () => { expect(logger.error).toBeCalledWith( - 'This user was permanently deleted. Contact support for questions', + 'The User has not set a password yet', expect.objectContaining({ firstName: bibiBloxberg.firstName, lastName: bibiBloxberg.lastName, From 945295bb410a72379523b342510bd1c6724c1ea2 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 22:18:43 +0100 Subject: [PATCH 08/29] lint fixes --- backend/src/graphql/resolver/UserResolver.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index 28f5d816d..fba468217 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -1223,7 +1223,9 @@ describe('UserResolver', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!') + expect(logger.error).toBeCalledWith( + 'Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character!', + ) }) }) }) From 7d304593f66668d7d65be2e3502518e407219fc7 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 22:44:31 +0100 Subject: [PATCH 09/29] LogError test --- backend/src/server/LogError.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 backend/src/server/LogError.test.ts diff --git a/backend/src/server/LogError.test.ts b/backend/src/server/LogError.test.ts new file mode 100644 index 000000000..ef8d6a23c --- /dev/null +++ b/backend/src/server/LogError.test.ts @@ -0,0 +1,23 @@ +import { logger } from '@test/testSetup' + +import LogError from './LogError' + +describe('LogError', () => { + it('logs an Error when created', () => { + new LogError('new LogError') + expect(logger.error).toBeCalledWith('new LogError') + }) + + it('logs an Error including additional data when created', () => { + new LogError('new LogError', { some: 'data' }) + expect(logger.error).toBeCalledWith('new LogError', { some: 'data' }) + }) + + it('does not contain additional data in Error object when thrown', () => { + try { + throw new LogError('new LogError', { someWeirdValue123: 'arbitraryData456' }) + } catch (e: any) { + expect(e.stack).not.toMatch(/(someWeirdValue123|arbitraryData456)/i) + } + }) +}) From 5de22d46e6451e30c68e114fc092c0227b8b1f96 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 22:58:14 +0100 Subject: [PATCH 10/29] fix another test --- backend/src/graphql/resolver/EmailOptinCodes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/EmailOptinCodes.test.ts b/backend/src/graphql/resolver/EmailOptinCodes.test.ts index d7c0b9bd6..e4e96e6a1 100644 --- a/backend/src/graphql/resolver/EmailOptinCodes.test.ts +++ b/backend/src/graphql/resolver/EmailOptinCodes.test.ts @@ -96,7 +96,7 @@ describe('EmailOptinCodes', () => { mutate({ mutation: forgotPassword, variables: { email: 'peter@lustig.de' } }), ).resolves.toMatchObject({ data: null, - errors: [new GraphQLError('email already sent less than 10 minutes minutes ago')], + errors: [new GraphQLError('Email already sent less than 10 minutes minutes ago')], }) }) From 7845ffd8e45055d2ec0ebec7db3d81c14a4d7a07 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 23:02:17 +0100 Subject: [PATCH 11/29] fix another test --- backend/src/graphql/resolver/TransactionResolver.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/TransactionResolver.test.ts b/backend/src/graphql/resolver/TransactionResolver.test.ts index 6115ef846..50b2b3690 100644 --- a/backend/src/graphql/resolver/TransactionResolver.test.ts +++ b/backend/src/graphql/resolver/TransactionResolver.test.ts @@ -89,7 +89,7 @@ describe('send coins', () => { }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith(`UserContact with email=wrong@email.com does not exists`) + expect(logger.error).toBeCalledWith('No user with this credentials', 'wrong@email.com') }) describe('deleted recipient', () => { From be902ae07e6683a72371740ddf011ab3f9c3fe24 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 31 Jan 2023 23:06:57 +0100 Subject: [PATCH 12/29] lint fixes --- backend/src/server/LogError.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/src/server/LogError.test.ts b/backend/src/server/LogError.test.ts index ef8d6a23c..6654c42f3 100644 --- a/backend/src/server/LogError.test.ts +++ b/backend/src/server/LogError.test.ts @@ -4,11 +4,13 @@ import LogError from './LogError' describe('LogError', () => { it('logs an Error when created', () => { + /* eslint-disable-next-line no-new */ new LogError('new LogError') expect(logger.error).toBeCalledWith('new LogError') }) it('logs an Error including additional data when created', () => { + /* eslint-disable-next-line no-new */ new LogError('new LogError', { some: 'data' }) expect(logger.error).toBeCalledWith('new LogError', { some: 'data' }) }) @@ -16,6 +18,7 @@ describe('LogError', () => { it('does not contain additional data in Error object when thrown', () => { try { throw new LogError('new LogError', { someWeirdValue123: 'arbitraryData456' }) + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ } catch (e: any) { expect(e.stack).not.toMatch(/(someWeirdValue123|arbitraryData456)/i) } From fbec0d44b53939ce509dc3219cc4418e0d74d6df Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:51:17 +0100 Subject: [PATCH 13/29] corrected error message create user --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 9fe4b9320..b907bea20 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -346,7 +346,7 @@ export class UserResolver { logger.addContext('user', dbUser.id) } catch (e) { await queryRunner.rollbackTransaction() - throw new LogError(`Error during create user with error: ${e}`, e) + throw new LogError('Error creating user', e) } finally { await queryRunner.release() } From 0f7f38e2cdb087f77832c8da4dab09a787371aa5 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:52:58 +0100 Subject: [PATCH 14/29] corrected error message to not expose too much info --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index b907bea20..3494db003 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -465,7 +465,7 @@ export class UserResolver { try { // Save user await queryRunner.manager.save(user).catch((error) => { - throw new LogError(`Error saving user: ${error}`, error) + throw new LogError('Error saving user', error) }) // Save userContact await queryRunner.manager.save(userContact).catch((error) => { From 9fa9bb127b06f8852a71dd4b31d5e5eb93409ab0 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:54:35 +0100 Subject: [PATCH 15/29] error message start with upper case consistency --- backend/src/graphql/resolver/EmailOptinCodes.test.ts | 4 ++-- backend/src/graphql/resolver/UserResolver.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/graphql/resolver/EmailOptinCodes.test.ts b/backend/src/graphql/resolver/EmailOptinCodes.test.ts index 92f2ee9d0..988595723 100644 --- a/backend/src/graphql/resolver/EmailOptinCodes.test.ts +++ b/backend/src/graphql/resolver/EmailOptinCodes.test.ts @@ -75,7 +75,7 @@ describe('EmailOptinCodes', () => { query({ query: queryOptIn, variables: { optIn: optinCode } }), ).resolves.toMatchObject({ data: null, - errors: [new GraphQLError('email was sent more than 24 hours ago')], + errors: [new GraphQLError('Email was sent more than 24 hours ago')], }) }) @@ -84,7 +84,7 @@ describe('EmailOptinCodes', () => { mutate({ mutation: setPassword, variables: { code: optinCode, password: 'Aa12345_' } }), ).resolves.toMatchObject({ data: null, - errors: [new GraphQLError('email was sent more than 24 hours ago')], + errors: [new GraphQLError('Email was sent more than 24 hours ago')], }) }) }) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 3494db003..3fa148b88 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -439,7 +439,7 @@ export class UserResolver { // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { throw new LogError( - `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, + `Email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) } logger.debug('EmailVerificationCode is valid...') @@ -515,7 +515,7 @@ export class UserResolver { // Code is only valid for `CONFIG.EMAIL_CODE_VALID_TIME` minutes if (!isEmailVerificationCodeValid(userContact.updatedAt || userContact.createdAt)) { throw new LogError( - `email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, + `Email was sent more than ${printTimeDuration(CONFIG.EMAIL_CODE_VALID_TIME)} ago`, ) } logger.info(`queryOptIn(${optIn}) successful...`) From de4ee4f6147455ddac9c0ade1f733bcfddcd2ab9 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:56:39 +0100 Subject: [PATCH 16/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 3fa148b88..0e09cfac3 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -480,7 +480,7 @@ export class UserResolver { eventProtocol.writeEvent(event.setEventActivateAccount(eventActivateAccount)) } catch (e) { await queryRunner.rollbackTransaction() - throw new LogError(`Error on writing User and UserContact data: ${e}`, e) + throw new LogError('Error on writing User and User Contact data', e) } finally { await queryRunner.release() } From 4300b10df70b97fae6f146b8590f16c06cc6b5a5 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:59:34 +0100 Subject: [PATCH 17/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 0e09cfac3..d63f05108 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -588,7 +588,7 @@ export class UserResolver { try { await queryRunner.manager.save(userEntity).catch((error) => { - throw new LogError(`error saving user: ${error}`, error) + throw new LogError('Error saving user', error) }) await queryRunner.commitTransaction() From 09ac97faba07873bd5ffa8f9cfec21c92f54cb5e Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:56:07 +0100 Subject: [PATCH 18/29] do not expose so much error information --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index d63f05108..4128cee1f 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -469,7 +469,7 @@ export class UserResolver { }) // Save userContact await queryRunner.manager.save(userContact).catch((error) => { - throw new LogError(`error saving userContact: ${error}`, error) + throw new LogError('Error saving userContact', error) }) await queryRunner.commitTransaction() From be53d6bbe0194af0c027b80c3abcd45a1085b470 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 01:58:52 +0100 Subject: [PATCH 19/29] don't expose klicktipp errors --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 4128cee1f..6f7e0177a 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -494,7 +494,7 @@ export class UserResolver { `klicktippSignIn(${userContact.email}, ${user.language}, ${user.firstName}, ${user.lastName})`, ) } catch (e) { - logger.error(`Error subscribe to klicktipp: ${e}`, e) + logger.error('Error subscribing to klicktipp', e) // TODO is this a problem? // eslint-disable-next-line no-console /* uncomment this, when you need the activation link on the console From 46eb45a14e6ada4e5f45b73a72543b3249ab3a8d Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:01:45 +0100 Subject: [PATCH 20/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 6f7e0177a..f51d4d32a 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -595,7 +595,7 @@ export class UserResolver { logger.debug('writing User data successful...') } catch (e) { await queryRunner.rollbackTransaction() - throw new LogError(`error on writing updated user data: ${e}`, e) + throw new LogError(`Error on writing updated user data', e) } finally { await queryRunner.release() } From 064e3221bd2e5e2bdf01323ffec5fd5cecf356fb Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:02:41 +0100 Subject: [PATCH 21/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index f51d4d32a..d5fedbfcf 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -722,7 +722,7 @@ export class UserResolver { const user = await DbUser.findOne({ id: userId }) // user exists ? if (!user) { - throw new LogError(`Could not find user with userId: ${userId}`, userId) + throw new LogError('Could not find user with given ID', userId) } // administrator user changes own role? const moderatorUser = getUser(context) From dce2abf1c36601db6a71e94d7f4dab2ec1ca2b1d Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:03:29 +0100 Subject: [PATCH 22/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index d5fedbfcf..cdf03362c 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -760,7 +760,7 @@ export class UserResolver { const user = await DbUser.findOne({ id: userId }) // user exists ? if (!user) { - throw new LogError(`Could not find user with userId: ${userId}`, userId) + throw new LogError('Could not find user with given ID', userId) } // moderator user disabled own account? const moderatorUser = getUser(context) From 2cbf01b8dfe25f6159a40b32ecd5b68853d75257 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:03:55 +0100 Subject: [PATCH 23/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index cdf03362c..0c0e3623a 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -778,7 +778,7 @@ export class UserResolver { async unDeleteUser(@Arg('userId', () => Int) userId: number): Promise { const user = await DbUser.findOne({ id: userId }, { withDeleted: true }) if (!user) { - throw new LogError(`Could not find user with userId: ${userId}`, userId) + throw new LogError('Could not find user with given ID', userId) } if (!user.deletedAt) { throw new LogError('User is not deleted') From 42e2490d848063e36b2db77c2a67a1a9f9ec463e Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:04:25 +0100 Subject: [PATCH 24/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 0c0e3623a..bfa3d7cdd 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -794,7 +794,7 @@ export class UserResolver { // const user = await dbUser.findOne({ id: emailContact.userId }) const user = await findUserByEmail(email) if (!user) { - throw new LogError(`Could not find User to emailContact: ${email}`, email) + throw new LogError('Could not find user to given email contact', email) } if (user.deletedAt) { throw new LogError(`User with emailContact: ${email} is deleted`, email) From cd2eeee40d82cb03ab5749e48c55b08ac55b9b78 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:04:38 +0100 Subject: [PATCH 25/29] Update backend/src/graphql/resolver/UserResolver.ts Co-authored-by: Moriz Wahl --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index bfa3d7cdd..d8f1ab299 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -797,7 +797,7 @@ export class UserResolver { throw new LogError('Could not find user to given email contact', email) } if (user.deletedAt) { - throw new LogError(`User with emailContact: ${email} is deleted`, email) + throw new LogError('User with given email contact is deleted', email) } const emailContact = user.emailContact if (emailContact.deletedAt) { From 3c3ccdb133c4b4867e38c03692214c464d4c056f Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:06:39 +0100 Subject: [PATCH 26/29] fix suggestion typo --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index d8f1ab299..1710bb7e9 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -595,7 +595,7 @@ export class UserResolver { logger.debug('writing User data successful...') } catch (e) { await queryRunner.rollbackTransaction() - throw new LogError(`Error on writing updated user data', e) + throw new LogError('Error on writing updated user data', e) } finally { await queryRunner.release() } From 80ff425e3966259bc34ea4d1a4f72ced91781e5e Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:07:30 +0100 Subject: [PATCH 27/29] do not expose email in error --- backend/src/graphql/resolver/UserResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index 1710bb7e9..b14a5e50d 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -801,7 +801,7 @@ export class UserResolver { } const emailContact = user.emailContact if (emailContact.deletedAt) { - throw new LogError(`The emailContact: ${email} of this User is deleted`, email) + throw new LogError('The given email contact for this user is deleted', email) } emailContact.emailResendCount++ From 1c7a2267c0495c86763f63aaddb55b34ec8d9452 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:12:12 +0100 Subject: [PATCH 28/29] dont reference the language on error --- backend/src/graphql/resolver/UserResolver.test.ts | 4 ++-- backend/src/graphql/resolver/UserResolver.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index cadd77c92..b26d92d76 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -1106,13 +1106,13 @@ describe('UserResolver', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError(`"not-valid" isn't a valid language`)], + errors: [new GraphQLError('Given language is not a valid language')], }), ) }) it('logs the error found', () => { - expect(logger.error).toBeCalledWith(`"not-valid" isn't a valid language`, 'not-valid') + expect(logger.error).toBeCalledWith('Given language is not a valid language', 'not-valid') }) }) diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index b14a5e50d..d92958009 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -550,7 +550,7 @@ export class UserResolver { if (language) { if (!isLanguage(language)) { - throw new LogError(`"${language}" isn't a valid language`, language) + throw new LogError('Given language is not a valid language', language) } userEntity.language = language i18n.setLocale(language) From b89509149605f8cfc71874d3c35423ba06030c14 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 02:47:46 +0100 Subject: [PATCH 29/29] fix error messages expected in tests --- .../src/graphql/resolver/UserResolver.test.ts | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/backend/src/graphql/resolver/UserResolver.test.ts b/backend/src/graphql/resolver/UserResolver.test.ts index b26d92d76..17eddca94 100644 --- a/backend/src/graphql/resolver/UserResolver.test.ts +++ b/backend/src/graphql/resolver/UserResolver.test.ts @@ -1431,16 +1431,13 @@ describe('UserResolver', () => { mutate({ mutation: setUserRole, variables: { userId: admin.id + 1, isAdmin: true } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError(`Could not find user with userId: ${admin.id + 1}`)], + errors: [new GraphQLError('Could not find user with given ID')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `Could not find user with userId: ${admin.id + 1}`, - admin.id + 1, - ) + expect(logger.error).toBeCalledWith('Could not find user with given ID', admin.id + 1) }) }) @@ -1606,16 +1603,13 @@ describe('UserResolver', () => { mutate({ mutation: deleteUser, variables: { userId: admin.id + 1 } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError(`Could not find user with userId: ${admin.id + 1}`)], + errors: [new GraphQLError('Could not find user with given ID')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `Could not find user with userId: ${admin.id + 1}`, - admin.id + 1, - ) + expect(logger.error).toBeCalledWith('Could not find user with given ID', admin.id + 1) }) }) @@ -1660,16 +1654,13 @@ describe('UserResolver', () => { mutate({ mutation: deleteUser, variables: { userId: user.id } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError(`Could not find user with userId: ${user.id}`)], + errors: [new GraphQLError('Could not find user with given ID')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `Could not find user with userId: ${user.id}`, - user.id, - ) + expect(logger.error).toBeCalledWith('Could not find user with given ID', user.id) }) }) }) @@ -1735,16 +1726,13 @@ describe('UserResolver', () => { mutate({ mutation: unDeleteUser, variables: { userId: admin.id + 1 } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError(`Could not find user with userId: ${admin.id + 1}`)], + errors: [new GraphQLError('Could not find user with given ID')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `Could not find user with userId: ${admin.id + 1}`, - admin.id + 1, - ) + expect(logger.error).toBeCalledWith('Could not find user with given ID', admin.id + 1) }) })