From 7dd32c3e87c8213cae3cc22e9261a1ad5aa326ea Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 14 Mar 2023 12:57:23 +0800 Subject: [PATCH 1/5] Trim whitespace text from user email address --- packages/api/src/routers/auth/auth_router.ts | 69 ++++++++++++++------ packages/api/src/services/create_user.ts | 4 +- 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 85321680f..dd7a33bdf 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -44,7 +44,7 @@ import { } from '../../utils/auth' import { createUser } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' -import { AppDataSource, initModels } from '../../server' +import { AppDataSource } from '../../server' import { getRepository, setClaims } from '../../entity/utils' import { User } from '../../entity/user' import { @@ -53,6 +53,7 @@ import { } from '../../services/send_emails' import { createWebAuthToken } from './jwt_helpers' import { createSsoToken, ssoRedirectURL } from '../../utils/sso' +import { ILike } from 'typeorm' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -373,18 +374,27 @@ export function authRouter() { '/email-login', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const { email, password } = req.body - - if (!email || !password) { + interface LoginRequest { + email: string + password: string + } + function isValidLoginRequest(obj: any): obj is LoginRequest { + return ( + 'email' in obj && + obj.email.trim().length > 0 && // email must not be empty + 'password' in obj && + obj.password.length >= 8 // password must be at least 8 characters + ) + } + if (!isValidLoginRequest(req.body)) { return res.redirect( `${env.client.url}/auth/email-login?errorCodes=${LoginErrorCode.InvalidCredentials}` ) } - + const { email, password } = req.body try { - const models = initModels(kx, false) - const user = await models.user.getWhere({ - email, + const user = await getRepository(User).findOneBy({ + email: ILike(email.trim()), // case insensitive }) if (!user?.id) { return res.redirect( @@ -409,7 +419,6 @@ export function authRouter() { `${env.client.url}/auth/email-login?errorCodes=${LoginErrorCode.WrongSource}` ) } - // check if password is correct const validPassword = await comparePassword(password, user.password) if (!validPassword) { @@ -437,25 +446,43 @@ export function authRouter() { '/email-signup', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const { email, password, name, username, bio, pictureUrl } = req.body - - if (!email || !password || !name || !username) { + interface SignupRequest { + email: string + password: string + name: string + username: string + bio?: string + pictureUrl?: string + } + function isValidSignupRequest(obj: any): obj is SignupRequest { + return ( + 'email' in obj && + obj.email.trim().length > 0 && // email must not be empty + 'password' in obj && + obj.password.length >= 8 && // password must be at least 8 characters + 'name' in obj && + obj.name.trim().length > 0 && // name must not be empty + 'username' in obj && + obj.username.trim().length > 0 // username must not be empty + ) + } + if (!isValidSignupRequest(req.body)) { return res.redirect( `${env.client.url}/auth/email-signup?errorCodes=INVALID_CREDENTIALS` ) } - const lowerCasedUsername = username.toLowerCase() - + const { email, password, name, username, bio, pictureUrl } = req.body + // trim whitespace in email address + const trimmedEmail = email.trim() try { // hash password const hashedPassword = await hashPassword(password) - await createUser({ - email, + email: trimmedEmail, provider: 'EMAIL', - sourceUserId: email, - name, - username: lowerCasedUsername, + sourceUserId: trimmedEmail, + name: name.trim(), + username: username.trim().toLowerCase(), // lowercase username pictureUrl, bio, password: hashedPassword, @@ -547,7 +574,7 @@ export function authRouter() { '/forgot-password', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const email = req.body.email + const email = req.body.email?.trim() as string // trim whitespace if (!email) { return res.redirect( `${env.client.url}/auth/forgot-password?errorCodes=INVALID_EMAIL` @@ -556,7 +583,7 @@ export function authRouter() { try { const user = await getRepository(User).findOneBy({ - email, + email: ILike(email), // case insensitive }) if (!user) { return res.redirect(`${env.client.url}/auth/reset-sent`) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index e23467e25..44591966b 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -1,6 +1,6 @@ import { AuthProvider } from '../routers/auth/auth_types' import { StatusType } from '../datalayer/user/model' -import { EntityManager } from 'typeorm' +import { EntityManager, ILike } from 'typeorm' import { User } from '../entity/user' import { Profile } from '../entity/profile' import { SignupErrorCode } from '../generated/graphql' @@ -118,7 +118,7 @@ const getUser = async (email: string): Promise => { const userRepo = getRepository(User) return userRepo.findOne({ - where: { email: email }, + where: { email: ILike(email) }, relations: ['profile'], }) } From 461b348dc0208ec9f78a267c6308d97502f841ca Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 14 Mar 2023 14:42:00 +0800 Subject: [PATCH 2/5] Use lower function in the query --- packages/api/src/routers/auth/auth_router.ts | 11 +++-------- packages/api/src/services/create_user.ts | 15 +++++++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index dd7a33bdf..8721fb9ca 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -42,7 +42,7 @@ import { hashPassword, setAuthInCookie, } from '../../utils/auth' -import { createUser } from '../../services/create_user' +import { createUser, getUserByEmail } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' import { AppDataSource } from '../../server' import { getRepository, setClaims } from '../../entity/utils' @@ -53,7 +53,6 @@ import { } from '../../services/send_emails' import { createWebAuthToken } from './jwt_helpers' import { createSsoToken, ssoRedirectURL } from '../../utils/sso' -import { ILike } from 'typeorm' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -393,9 +392,7 @@ export function authRouter() { } const { email, password } = req.body try { - const user = await getRepository(User).findOneBy({ - email: ILike(email.trim()), // case insensitive - }) + const user = await getUserByEmail(email.trim()) if (!user?.id) { return res.redirect( `${env.client.url}/auth/email-login?errorCodes=${LoginErrorCode.UserNotFound}` @@ -582,9 +579,7 @@ export function authRouter() { } try { - const user = await getRepository(User).findOneBy({ - email: ILike(email), // case insensitive - }) + const user = await getUserByEmail(email) if (!user) { return res.redirect(`${env.client.url}/auth/reset-sent`) } diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 44591966b..4efdd1205 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -24,7 +24,7 @@ export const createUser = async (input: { password?: string pendingConfirmation?: boolean }): Promise<[User, Profile]> => { - const existingUser = await getUser(input.email) + const existingUser = await getUserByEmail(input.email) if (existingUser) { if (existingUser.profile) { return Promise.reject({ errorCode: SignupErrorCode.UserExists }) @@ -114,11 +114,10 @@ const validateInvite = async ( return true } -const getUser = async (email: string): Promise => { - const userRepo = getRepository(User) - - return userRepo.findOne({ - where: { email: ILike(email) }, - relations: ['profile'], - }) +export const getUserByEmail = async (email: string): Promise => { + return getRepository(User) + .createQueryBuilder('user') + .leftJoinAndSelect('user.profile', 'profile') + .where('LOWER(email) = LOWER(:email)', { email }) + .getOne() } From ee2487ff01f68b18f72dcd33c6537ca1c84b4bd0 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 14 Mar 2023 14:42:56 +0800 Subject: [PATCH 3/5] Case insensitive --- packages/api/src/services/create_user.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 4efdd1205..cff7aab22 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -118,6 +118,6 @@ export const getUserByEmail = async (email: string): Promise => { return getRepository(User) .createQueryBuilder('user') .leftJoinAndSelect('user.profile', 'profile') - .where('LOWER(email) = LOWER(:email)', { email }) + .where('LOWER(email) = LOWER(:email)', { email }) // case insensitive .getOne() } From 490ae58499876d74e4a7e20095b75289f9ae14e4 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 14 Mar 2023 14:46:00 +0800 Subject: [PATCH 4/5] Update test --- packages/api/test/routers/auth.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 5659c4288..716849774 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -50,7 +50,7 @@ describe('auth router', () => { before(() => { password = validPassword username = 'Some_username' - email = `${username}@omnivore.app` + email = `${username}@omnivore.app ` // space at the end is intentional name = 'Some name' }) From c3f2262b6b52418246a79bf97f725fa0500c822b Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 14 Mar 2023 14:52:42 +0800 Subject: [PATCH 5/5] Update test --- packages/api/src/services/create_user.ts | 16 ++++++++-------- packages/api/test/routers/auth.test.ts | 24 ++++++++++++------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index cff7aab22..f8e2fbea2 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -1,14 +1,14 @@ -import { AuthProvider } from '../routers/auth/auth_types' +import { EntityManager } from 'typeorm' import { StatusType } from '../datalayer/user/model' -import { EntityManager, ILike } from 'typeorm' -import { User } from '../entity/user' -import { Profile } from '../entity/profile' -import { SignupErrorCode } from '../generated/graphql' -import { validateUsername } from '../utils/usernamePolicy' -import { Invite } from '../entity/groups/invite' import { GroupMembership } from '../entity/groups/group_membership' -import { AppDataSource } from '../server' +import { Invite } from '../entity/groups/invite' +import { Profile } from '../entity/profile' +import { User } from '../entity/user' import { getRepository } from '../entity/utils' +import { SignupErrorCode } from '../generated/graphql' +import { AuthProvider } from '../routers/auth/auth_types' +import { AppDataSource } from '../server' +import { validateUsername } from '../utils/usernamePolicy' import { sendConfirmationEmail } from './send_emails' export const createUser = async (input: { diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 716849774..4e4044a17 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -1,22 +1,22 @@ -import { createTestUser, deleteTestUser, updateTestUser } from '../db' -import { generateFakeUuid, request } from '../util' -import { StatusType } from '../../src/datalayer/user/model' -import { getRepository } from '../../src/entity/utils' -import { User } from '../../src/entity/user' import { MailDataRequired } from '@sendgrid/helpers/classes/mail' +import chai, { expect } from 'chai' import sinon from 'sinon' -import * as util from '../../src/utils/sendEmail' +import sinonChai from 'sinon-chai' import supertest from 'supertest' +import { StatusType } from '../../src/datalayer/user/model' +import { searchPages } from '../../src/elastic/pages' +import { User } from '../../src/entity/user' +import { getRepository } from '../../src/entity/utils' +import { AuthProvider } from '../../src/routers/auth/auth_types' +import { createPendingUserToken } from '../../src/routers/auth/jwt_helpers' import { comparePassword, generateVerificationToken, hashPassword, } from '../../src/utils/auth' -import sinonChai from 'sinon-chai' -import chai, { expect } from 'chai' -import { searchPages } from '../../src/elastic/pages' -import { createPendingUserToken } from '../../src/routers/auth/jwt_helpers' -import { AuthProvider } from '../../src/routers/auth/auth_types' +import * as util from '../../src/utils/sendEmail' +import { createTestUser, deleteTestUser, updateTestUser } from '../db' +import { generateFakeUuid, request } from '../util' chai.use(sinonChai) @@ -178,7 +178,7 @@ describe('auth router', () => { context('when email and password are valid', () => { before(() => { - email = user.email + email = user.email + ' ' // space at the end is intentional password = correctPassword })