diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 85321680f..8721fb9ca 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -42,9 +42,9 @@ 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, initModels } from '../../server' +import { AppDataSource } from '../../server' import { getRepository, setClaims } from '../../entity/utils' import { User } from '../../entity/user' import { @@ -373,19 +373,26 @@ 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 getUserByEmail(email.trim()) if (!user?.id) { return res.redirect( `${env.client.url}/auth/email-login?errorCodes=${LoginErrorCode.UserNotFound}` @@ -409,7 +416,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 +443,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 +571,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` @@ -555,9 +579,7 @@ export function authRouter() { } try { - const user = await getRepository(User).findOneBy({ - email, - }) + 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 e23467e25..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 { StatusType } from '../datalayer/user/model' import { EntityManager } 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 { StatusType } from '../datalayer/user/model' 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: { @@ -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: 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 }) // case insensitive + .getOne() } diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 5659c4288..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) @@ -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' }) @@ -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 })