From a97c3286bffbdb200507f40fa344e75245c90958 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 21:25:58 +0800 Subject: [PATCH 01/41] add status to user table --- packages/db/migrations/0088.do.add_status_to_user.sql | 11 +++++++++++ .../db/migrations/0088.undo.add_status_to_user.sql | 11 +++++++++++ 2 files changed, 22 insertions(+) create mode 100755 packages/db/migrations/0088.do.add_status_to_user.sql create mode 100755 packages/db/migrations/0088.undo.add_status_to_user.sql diff --git a/packages/db/migrations/0088.do.add_status_to_user.sql b/packages/db/migrations/0088.do.add_status_to_user.sql new file mode 100755 index 000000000..ba0416fb8 --- /dev/null +++ b/packages/db/migrations/0088.do.add_status_to_user.sql @@ -0,0 +1,11 @@ +-- Type: DO +-- Name: add_status_to_user +-- Description: Add status to user table + +BEGIN; + +CREATE TYPE user_status_type AS ENUM ('ACTIVE', 'PENDING'); + +ALTER TABLE omnivore.user ADD COLUMN status user_status_type NOT NULL DEFAULT 'ACTIVE'; + +COMMIT; diff --git a/packages/db/migrations/0088.undo.add_status_to_user.sql b/packages/db/migrations/0088.undo.add_status_to_user.sql new file mode 100755 index 000000000..182e1aef0 --- /dev/null +++ b/packages/db/migrations/0088.undo.add_status_to_user.sql @@ -0,0 +1,11 @@ +-- Type: UNDO +-- Name: add_status_to_user +-- Description: Add status to user table + +BEGIN; + +DROP TYPE IF EXISTS user_status_type CASCADE; + +ALTER TABLE omnivore.user DROP COLUMN IF EXISTS status; + +COMMIT; From 35e060890e69fba15c9472131aa51eebe11832a4 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 21:29:26 +0800 Subject: [PATCH 02/41] add status to user entity --- packages/api/src/datalayer/user/model.ts | 5 +++++ packages/api/src/entity/user.ts | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/api/src/datalayer/user/model.ts b/packages/api/src/datalayer/user/model.ts index a538f0dcf..5e10038e9 100644 --- a/packages/api/src/datalayer/user/model.ts +++ b/packages/api/src/datalayer/user/model.ts @@ -57,6 +57,11 @@ export enum RegistrationType { Email = 'EMAIL', } +export enum StatusType { + Active = 'ACTIVE', + Pending = 'PENDING', +} + export const keys = [ 'id', 'name', diff --git a/packages/api/src/entity/user.ts b/packages/api/src/entity/user.ts index 320d26885..50ad7e7a9 100644 --- a/packages/api/src/entity/user.ts +++ b/packages/api/src/entity/user.ts @@ -7,7 +7,11 @@ import { PrimaryGeneratedColumn, UpdateDateColumn, } from 'typeorm' -import { MembershipTier, RegistrationType } from '../datalayer/user/model' +import { + MembershipTier, + RegistrationType, + StatusType, +} from '../datalayer/user/model' import { NewsletterEmail } from './newsletter_email' import { Profile } from './profile' import { Label } from './label' @@ -53,4 +57,7 @@ export class User { @OneToMany(() => Subscription, (subscription) => subscription.user) subscriptions?: Subscription[] + + @Column({ type: 'enum', enum: StatusType }) + status!: string } From ba73eec9e5990fd646cdad7cbf79d45c98e0d4bf Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 21:35:53 +0800 Subject: [PATCH 03/41] create pending user first --- packages/api/src/resolvers/user/index.ts | 1 + packages/api/src/services/create_user.ts | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 7a41f0878..fbd747124 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -350,6 +350,7 @@ export const signupResolver: ResolverFn< pictureUrl: pictureUrl || undefined, bio: bio || undefined, password: hashedPassword, + pendingConfirmation: true, }) return { diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index bd94ead22..6ccc7c3b6 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -1,5 +1,5 @@ import { AuthProvider } from '../routers/auth/auth_types' -import { MembershipTier } from '../datalayer/user/model' +import { MembershipTier, StatusType } from '../datalayer/user/model' import { EntityManager } from 'typeorm' import { User } from '../entity/user' import { Profile } from '../entity/profile' @@ -22,6 +22,7 @@ export const createUser = async (input: { membershipTier?: MembershipTier inviteCode?: string password?: string + pendingConfirmation?: boolean }): Promise<[User, Profile]> => { const existingUser = await getUser(input.email) if (existingUser) { @@ -68,6 +69,9 @@ export const createUser = async (input: { email: input.email, sourceUserId: input.sourceUserId, password: input.password, + status: input.pendingConfirmation + ? StatusType.Pending + : StatusType.Active, }) const profile = await t.getRepository(Profile).save({ username: input.username, From 1346140a2031f3e66d21b270f0cc71873b17fbcf Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 21:58:43 +0800 Subject: [PATCH 04/41] add sender email address in env --- packages/api/.env.example | 5 ++++- packages/api/src/util.ts | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/api/.env.example b/packages/api/.env.example index 6a9950594..6e90843b6 100644 --- a/packages/api/.env.example +++ b/packages/api/.env.example @@ -24,4 +24,7 @@ GCS_UPLOAD_SA_KEY_FILE_PATH= TWITTER_BEARER_TOKEN= PREVIEW_IMAGE_WRAPPER_ID='selected_highlight_wrapper' REMINDER_TASK_HANDLER_URL= -ELASTIC_URL=http://localhost:9200 \ No newline at end of file +ELASTIC_URL=http://localhost:9200 +SENDER_MESSAGE=msgs@sender.domain +SENDER_FEEDBACK=feedback@sender.domain +SENDER_GENERAL=no-reply@sender.domain diff --git a/packages/api/src/util.ts b/packages/api/src/util.ts index bda3ae1ae..71b1f2c07 100755 --- a/packages/api/src/util.ts +++ b/packages/api/src/util.ts @@ -73,6 +73,11 @@ interface BackendEnv { username: string password: string } + sender: { + message: string + feedback: string + general: string + } } /*** @@ -214,6 +219,11 @@ export function getEnv(): BackendEnv { username: parse('ELASTIC_USERNAME'), password: parse('ELASTIC_PASSWORD'), } + const sender = { + message: parse('SENDER_MESSAGE'), + feedback: parse('SENDER_FEEDBACK'), + general: parse('SENDER_GENERAL'), + } return { pg, @@ -230,6 +240,7 @@ export function getEnv(): BackendEnv { fileUpload, queue, elastic, + sender, } } From bd5b289168f60359beec4d1e3ae6f0391f59afb3 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 21:59:06 +0800 Subject: [PATCH 05/41] add verification token generation --- packages/api/src/utils/auth.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 12b34cce3..38ce9e345 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -79,3 +79,12 @@ export const getClaimsByToken = async ( return claims } + +export const generateVerificationToken = (userId: string) => { + const iat = Math.floor(Date.now() / 1000) + const exp = Math.floor( + new Date(Date.now() + 1000 * 60 * 60 * 24).getTime() / 1000 + ) + + return jwt.sign({ uid: userId, iat, exp }, env.server.jwtSecret) +} From 7293469468fc79686502e16a29f94d6320aee5c2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 22:00:38 +0800 Subject: [PATCH 06/41] use sender email address in env --- .../api/src/events/reports/content_display_report_created.ts | 2 +- packages/api/src/resolvers/send_install_instructions/index.ts | 3 ++- packages/api/src/routers/svc/emails.ts | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/api/src/events/reports/content_display_report_created.ts b/packages/api/src/events/reports/content_display_report_created.ts index e1cc365a7..7a8dbb5b1 100644 --- a/packages/api/src/events/reports/content_display_report_created.ts +++ b/packages/api/src/events/reports/content_display_report_created.ts @@ -28,7 +28,7 @@ export class ContentDisplayReportSubscriber if (!env.dev.isLocal) { // If we are in the local environment, just log a message, otherwise email the report await sendEmail({ - to: 'feedback@omnivore.app', + to: env.sender.feedback, subject: 'New content display report', text: message, from: 'msgs@omnivore.app', diff --git a/packages/api/src/resolvers/send_install_instructions/index.ts b/packages/api/src/resolvers/send_install_instructions/index.ts index 4fffe5de6..4b9b103b8 100644 --- a/packages/api/src/resolvers/send_install_instructions/index.ts +++ b/packages/api/src/resolvers/send_install_instructions/index.ts @@ -7,6 +7,7 @@ import { authorized } from '../../utils/helpers' import { sendEmail } from '../../utils/sendEmail' import { AppDataSource } from '../../server' import { User } from '../../entity/user' +import { env } from '../../env' const INSTALL_INSTRUCTIONS_EMAIL_TEMPLATE_ID = 'd-c576bdc3b9a849dab250655ba14c7794' @@ -25,7 +26,7 @@ export const sendInstallInstructionsResolver = authorized< } const sendInstallInstructions = await sendEmail({ - from: 'msgs@omnivore.app', + from: env.sender.message, templateId: INSTALL_INSTRUCTIONS_EMAIL_TEMPLATE_ID, to: user?.email, }) diff --git a/packages/api/src/routers/svc/emails.ts b/packages/api/src/routers/svc/emails.ts index 03f5f8fba..01498f83a 100644 --- a/packages/api/src/routers/svc/emails.ts +++ b/packages/api/src/routers/svc/emails.ts @@ -89,7 +89,7 @@ export function emailsServiceRouter() { // forward non-newsletter emails to the registered email address const result = await sendEmail({ - from: 'msgs@omnivore.app', + from: env.sender.message, to: newsletterEmail.user.email, subject: `Fwd: ${data.subject}`, html: data.html, From 1ea34bde6296f110e55cb7d2af8a3552e1932b7d Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 22:03:58 +0800 Subject: [PATCH 07/41] send confirmation email to pending user --- packages/api/src/services/create_user.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 6ccc7c3b6..ccd44dbbb 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -9,6 +9,9 @@ import { Invite } from '../entity/groups/invite' import { GroupMembership } from '../entity/groups/group_membership' import { AppDataSource } from '../server' import { getRepository } from '../entity/utils' +import { generateVerificationToken } from '../utils/auth' +import { env } from '../env' +import { sendEmail } from '../utils/sendEmail' export const createUser = async (input: { provider: AuthProvider @@ -90,6 +93,20 @@ export const createUser = async (input: { } ) + // send confirmation email + if (input.pendingConfirmation) { + // generate confirmation link + const confirmationToken = generateVerificationToken(user.id) + const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` + // send email + await sendEmail({ + from: env.sender.message, + to: user.email, + subject: 'Confirm your email', + text: `Please confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, + }) + } + return [user, profile] } From b98f7c6ba26a6561ea914cc5915d9b0a4dc4f6d5 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 22:14:04 +0800 Subject: [PATCH 08/41] reject with unknow reason if failed to send confirmation email --- packages/api/src/services/create_user.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index ccd44dbbb..b112a6a9f 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -99,12 +99,16 @@ export const createUser = async (input: { const confirmationToken = generateVerificationToken(user.id) const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` // send email - await sendEmail({ + const sent = await sendEmail({ from: env.sender.message, to: user.email, subject: 'Confirm your email', text: `Please confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, }) + + if (!sent) { + return Promise.reject({ errorCode: SignupErrorCode.Unknown }) + } } return [user, profile] From b52043bd952da7714086f2feccf7112a8f1b8b72 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 19 Jul 2022 22:24:10 +0800 Subject: [PATCH 09/41] add a test to check if use has a pending status after creation --- packages/api/test/db.ts | 4 +++- packages/api/test/services/create_user.test.ts | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts index 4d9928c82..4af92259b 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -71,7 +71,8 @@ export const deleteTestUser = async (name: string) => { export const createTestUser = async ( name: string, invite?: string | undefined, - password?: string + password?: string, + pendingConfirmation?: boolean ): Promise => { const [newUser] = await createUser({ provider: 'GOOGLE', @@ -82,6 +83,7 @@ export const createTestUser = async ( name: name, inviteCode: invite, password: password, + pendingConfirmation, }) return newUser diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index b5081679f..973cccd1a 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -12,6 +12,7 @@ import { getUserFollowers, getUserFollowing, } from '../../src/services/followers' +import { StatusType } from '../../src/datalayer/user/model' describe('create a user with an invite', () => { it('follows the other user in the group', async () => { @@ -51,3 +52,16 @@ describe('create a user with an invite', () => { expect(profile).to.exist }) }) + +describe('create a pending user', () => { + it('creates a pending user', async () => { + after(async () => { + await deleteTestUser(name) + }) + + const name = 'pendingUser' + const user = await createTestUser(name, undefined, undefined, true) + + expect(user.status).to.equal(StatusType.Pending) + }) +}) From d10880623418fe9069bf33351c4c6390d58eb7ec Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 20 Jul 2022 22:53:13 +0800 Subject: [PATCH 10/41] make sender address nullable env var --- packages/api/src/util.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/api/src/util.ts b/packages/api/src/util.ts index 71b1f2c07..ae9fe5051 100755 --- a/packages/api/src/util.ts +++ b/packages/api/src/util.ts @@ -119,6 +119,9 @@ const nullableEnvVars = [ 'ELASTIC_USERNAME', 'ELASTIC_PASSWORD', 'GCS_UPLOAD_PRIVATE_BUCKET', + 'SENDER_MESSAGE', + 'SENDER_FEEDBACK', + 'SENDER_GENERAL', ] // Allow some vars to be null/empty /* If not in GAE and Prod/QA/Demo env (f.e. on localhost/dev env), allow following env vars to be null */ From d187c6c99315e2899efde0472e56c8d3fc428d14 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 20 Jul 2022 22:54:22 +0800 Subject: [PATCH 11/41] use general sender for confirmation email --- 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 b112a6a9f..cc7c06a15 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -100,7 +100,7 @@ export const createUser = async (input: { const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` // send email const sent = await sendEmail({ - from: env.sender.message, + from: env.sender.general, to: user.email, subject: 'Confirm your email', text: `Please confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, From e4e985cfce71b358d1213badc9e1b54c6c7104e1 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 20 Jul 2022 22:54:31 +0800 Subject: [PATCH 12/41] add more test --- packages/api/test/resolvers/user.test.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index 7115ca7ed..ebcef7128 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -360,20 +360,30 @@ describe('User API', () => { }) context('when inputs are valid and user not exists', () => { - before(() => { + beforeEach(() => { password = correctPassword username = 'Some_username' email = `${username}@fake.com` }) - after(async () => { + afterEach(async () => { await deleteTestUser(username) }) it('responds with 200', async () => { - const res = await graphqlRequest(query).expect(200) + return graphqlRequest(query).expect(200) + }) + + it('returns the user', async () => { + const res = await graphqlRequest(query).send() + expect(res.body.data.signup.me.profile.username).to.eql(username) + }) + + it('creates user with correct username', async () => { + const res = await graphqlRequest(query).send() + const user = await getUser(res.body.data.signup.me.id) - expect(user).to.exist + expect(user?.profile?.username).to.eql(username) }) }) From 7339a0daf37c91bc4b3779dc4a7cb9acbad844de Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:14:43 +0800 Subject: [PATCH 13/41] add sinon and sinon-chai for faking sendMail --- packages/api/package.json | 4 +++ yarn.lock | 75 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/packages/api/package.json b/packages/api/package.json index d6445d3a8..ff1b46c8a 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -107,6 +107,8 @@ "@types/oauth": "^0.9.1", "@types/private-ip": "^1.0.0", "@types/sanitize-html": "^1.27.1", + "@types/sinon": "^10.0.13", + "@types/sinon-chai": "^3.2.8", "@types/supertest": "^2.0.11", "@types/urlsafe-base64": "^1.0.28", "@types/uuid": "^8.3.0", @@ -120,6 +122,8 @@ "nock": "^13.2.4", "nyc": "^15.1.0", "postgrator": "^4.2.0", + "sinon": "^14.0.0", + "sinon-chai": "^3.7.0", "ts-node-dev": "^1.1.8" }, "engines": { diff --git a/yarn.lock b/yarn.lock index 57ff3bf6a..09856e25c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5415,13 +5415,20 @@ resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.14.0.tgz#9fb3a3cf3132328151f353de4632e01e52102bea" integrity sha512-9NET910DNaIPngYnLLPeg+Ogzqsi9uM4mSboU5y6p8S5DzMTVEsJZrawi+BoDNUVBa2DhJqQYUFvMDfgU062LQ== -"@sinonjs/commons@^1", "@sinonjs/commons@^1.3.0", "@sinonjs/commons@^1.4.0", "@sinonjs/commons@^1.7.0": +"@sinonjs/commons@^1", "@sinonjs/commons@^1.3.0", "@sinonjs/commons@^1.4.0", "@sinonjs/commons@^1.6.0", "@sinonjs/commons@^1.7.0", "@sinonjs/commons@^1.8.3": version "1.8.3" resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.8.3.tgz#3802ddd21a50a949b6721ddd72da36e67e7f1b2d" integrity sha512-xkNcLAn/wZaX14RPlwizcKicDk9G3F8m2nU3L7Ukm5zBgTwiT0wsoFAHx9Jq56fJA1z/7uKGtCRu16sOUCLIHQ== dependencies: type-detect "4.0.8" +"@sinonjs/fake-timers@>=5", "@sinonjs/fake-timers@^9.1.2": + version "9.1.2" + resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-9.1.2.tgz#4eaab737fab77332ab132d396a3c0d364bd0ea8c" + integrity sha512-BPS4ynJW/o92PUR4wgriz2Ud5gpST5vz6GQfMixEDK0Z8ZCUv2M7SkBLykH56T++Xs+8ln9zTGbOvNGIe02/jw== + dependencies: + "@sinonjs/commons" "^1.7.0" + "@sinonjs/fake-timers@^8.0.1": version "8.0.1" resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-8.0.1.tgz#1c1c9a91419f804e59ae8df316a07dd1c3a76b94" @@ -5446,6 +5453,15 @@ array-from "^2.1.1" lodash "^4.17.15" +"@sinonjs/samsam@^6.1.1": + version "6.1.1" + resolved "https://registry.yarnpkg.com/@sinonjs/samsam/-/samsam-6.1.1.tgz#627f7f4cbdb56e6419fa2c1a3e4751ce4f6a00b1" + integrity sha512-cZ7rKJTLiE7u7Wi/v9Hc2fs3Ucc3jrWeMgPHbbTCeVAB2S0wOBbYlkJVeNSL04i7fdhT8wIbDq1zhC/PXTD2SA== + dependencies: + "@sinonjs/commons" "^1.6.0" + lodash.get "^4.4.2" + type-detect "^4.0.8" + "@sinonjs/text-encoding@^0.7.1": version "0.7.1" resolved "https://registry.yarnpkg.com/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz#8da5c6530915653f3a1f38fd5f101d8c3f8079c5" @@ -7994,6 +8010,26 @@ "@types/mime" "^1" "@types/node" "*" +"@types/sinon-chai@^3.2.8": + version "3.2.8" + resolved "https://registry.yarnpkg.com/@types/sinon-chai/-/sinon-chai-3.2.8.tgz#5871d09ab50d671d8e6dd72e9073f8e738ac61dc" + integrity sha512-d4ImIQbT/rKMG8+AXpmcan5T2/PNeSjrYhvkwet6z0p8kzYtfgA32xzOBlbU0yqJfq+/0Ml805iFoODO0LP5/g== + dependencies: + "@types/chai" "*" + "@types/sinon" "*" + +"@types/sinon@*", "@types/sinon@^10.0.13": + version "10.0.13" + resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-10.0.13.tgz#60a7a87a70d9372d0b7b38cc03e825f46981fb83" + integrity sha512-UVjDqJblVNQYvVNUsj0PuYYw0ELRmgt1Nt5Vk0pT5f16ROGfcKJY8o1HVuMOJOpD727RrGB9EGvoaTQE5tgxZQ== + dependencies: + "@types/sinonjs__fake-timers" "*" + +"@types/sinonjs__fake-timers@*": + version "8.1.2" + resolved "https://registry.yarnpkg.com/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-8.1.2.tgz#bf2e02a3dbd4aecaf95942ecd99b7402e03fad5e" + integrity sha512-9GcLXF0/v3t80caGs5p2rRfkB+a8VBGLJZVih6CNFkx8IZ994wiKKLSRs9nuFwk1HevWs/1mnUmkApGrSGsShA== + "@types/sinonjs__fake-timers@8.1.1": version "8.1.1" resolved "https://registry.yarnpkg.com/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-8.1.1.tgz#b49c2c70150141a15e0fa7e79cf1f92a72934ce3" @@ -12105,6 +12141,11 @@ diff@^4.0.1: resolved "https://registry.yarnpkg.com/diff/-/diff-4.0.2.tgz#60f3aecb89d5fae520c11aa19efc2bb982aade7d" integrity sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A== +diff@^5.0.0: + version "5.1.0" + resolved "https://registry.yarnpkg.com/diff/-/diff-5.1.0.tgz#bc52d298c5ea8df9194800224445ed43ffc87e40" + integrity sha512-D+mk+qE8VC/PAUrlAU34N+VfXev0ghe5ywmpqrawphmVZc1bEfn56uo9qpyGp1p4xpzOHkSW4ztBd6L7Xx4ACw== + diff@~1.0.7: version "1.0.8" resolved "https://registry.yarnpkg.com/diff/-/diff-1.0.8.tgz#343276308ec991b7bc82267ed55bc1411f971666" @@ -18738,6 +18779,17 @@ nise@^1.5.2: lolex "^5.0.1" path-to-regexp "^1.7.0" +nise@^5.1.1: + version "5.1.1" + resolved "https://registry.yarnpkg.com/nise/-/nise-5.1.1.tgz#ac4237e0d785ecfcb83e20f389185975da5c31f3" + integrity sha512-yr5kW2THW1AkxVmCnKEh4nbYkJdB3I7LUkiUgOvEkOp414mc2UMaHMA7pjq1nYowhdoJZGwEKGaQVbxfpWj10A== + dependencies: + "@sinonjs/commons" "^1.8.3" + "@sinonjs/fake-timers" ">=5" + "@sinonjs/text-encoding" "^0.7.1" + just-extend "^4.0.2" + path-to-regexp "^1.7.0" + no-case@^2.2.0, no-case@^2.3.2: version "2.3.2" resolved "https://registry.yarnpkg.com/no-case/-/no-case-2.3.2.tgz#60b813396be39b3f1288a4c1ed5d1e7d28b464ac" @@ -22318,6 +22370,23 @@ simple-swizzle@^0.2.2: dependencies: is-arrayish "^0.3.1" +sinon-chai@^3.7.0: + version "3.7.0" + resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-3.7.0.tgz#cfb7dec1c50990ed18c153f1840721cf13139783" + integrity sha512-mf5NURdUaSdnatJx3uhoBOrY9dtL19fiOtAdT1Azxg3+lNJFiuN0uzaU3xX1LeAfL17kHQhTAJgpsfhbMJMY2g== + +sinon@^14.0.0: + version "14.0.0" + resolved "https://registry.yarnpkg.com/sinon/-/sinon-14.0.0.tgz#203731c116d3a2d58dc4e3cbe1f443ba9382a031" + integrity sha512-ugA6BFmE+WrJdh0owRZHToLd32Uw3Lxq6E6LtNRU+xTVBefx632h03Q7apXWRsRdZAJ41LB8aUfn2+O4jsDNMw== + dependencies: + "@sinonjs/commons" "^1.8.3" + "@sinonjs/fake-timers" "^9.1.2" + "@sinonjs/samsam" "^6.1.1" + diff "^5.0.0" + nise "^5.1.1" + supports-color "^7.2.0" + sinon@^7.3.2: version "7.5.0" resolved "https://registry.yarnpkg.com/sinon/-/sinon-7.5.0.tgz#e9488ea466070ea908fd44a3d6478fd4923c67ec" @@ -23144,7 +23213,7 @@ supports-color@^5.3.0, supports-color@^5.5.0: dependencies: has-flag "^3.0.0" -supports-color@^7.0.0, supports-color@^7.1.0: +supports-color@^7.0.0, supports-color@^7.1.0, supports-color@^7.2.0: version "7.2.0" resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-7.2.0.tgz#1b7dcdcb32b8138801b3e478ba6a51caa89648da" integrity sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw== @@ -23874,7 +23943,7 @@ type-detect@0.1.1: resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-0.1.1.tgz#0ba5ec2a885640e470ea4e8505971900dac58822" integrity sha1-C6XsKohWQORw6k6FBZcZANrFiCI= -type-detect@4.0.8, type-detect@^4.0.0, type-detect@^4.0.5: +type-detect@4.0.8, type-detect@^4.0.0, type-detect@^4.0.5, type-detect@^4.0.8: version "4.0.8" resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.8.tgz#7646fb5f18871cfbb7749e69bd39a6388eb7450c" integrity sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g== From 7a11d90d2c8ac4586725dfddb043a4ad44708bab Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:15:12 +0800 Subject: [PATCH 14/41] use mocha in the test --- packages/api/test/resolvers/article_saving_request.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/test/resolvers/article_saving_request.test.ts b/packages/api/test/resolvers/article_saving_request.test.ts index cc8aabc1e..aa398831d 100644 --- a/packages/api/test/resolvers/article_saving_request.test.ts +++ b/packages/api/test/resolvers/article_saving_request.test.ts @@ -7,12 +7,12 @@ import { createTestUser, deleteTestUser } from '../db' import { graphqlRequest, request } from '../util' import { createPubSubClient } from '../../src/datalayer/pubsub' import { expect } from 'chai' -import { describe } from 'mocha' import { getPageById } from '../../src/elastic/pages' import { ArticleSavingRequestErrorCode, CreateArticleSavingRequestErrorCode, } from '../../src/generated/graphql' +import 'mocha' const articleSavingRequestQuery = (id: string) => ` query { From 4e11000ac0b995f0b6ec4bec5f691ce785bd5f89 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:17:09 +0800 Subject: [PATCH 15/41] add return type --- packages/api/src/utils/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 38ce9e345..44eac2f4e 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -80,7 +80,7 @@ export const getClaimsByToken = async ( return claims } -export const generateVerificationToken = (userId: string) => { +export const generateVerificationToken = (userId: string): string => { const iat = Math.floor(Date.now() / 1000) const exp = Math.floor( new Date(Date.now() + 1000 * 60 * 60 * 24).getTime() / 1000 From b0a0020fc7eeb687f66672004875ffc1213561a5 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:17:33 +0800 Subject: [PATCH 16/41] fix return false when sending failed --- packages/api/src/utils/sendEmail.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/api/src/utils/sendEmail.ts b/packages/api/src/utils/sendEmail.ts index 9ef0d1b4d..cb75de464 100644 --- a/packages/api/src/utils/sendEmail.ts +++ b/packages/api/src/utils/sendEmail.ts @@ -30,14 +30,18 @@ export const sendEmail = async (msg: MailDataRequired): Promise => { console.log('sending email', msg) - await client.send(msg).catch((error) => { + try { + const response = await client.send(msg) + console.log('email sent', response) + + return true + } catch (error) { console.log('error sending email', error) const err = asSendGridError(error) if (err) { console.log('sendgrid error:', JSON.stringify(err.response?.body)) } - return false - }) - return true + return false + } } From 01c4bbad005d6d6e43f483e002b04aa244a72fd9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:18:05 +0800 Subject: [PATCH 17/41] delete pending user if failed to send confirmation email --- packages/api/src/services/create_user.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index cc7c06a15..0dbe854cc 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -8,7 +8,7 @@ import { validateUsername } from '../utils/usernamePolicy' import { Invite } from '../entity/groups/invite' import { GroupMembership } from '../entity/groups/group_membership' import { AppDataSource } from '../server' -import { getRepository } from '../entity/utils' +import { getRepository, setClaims } from '../entity/utils' import { generateVerificationToken } from '../utils/auth' import { env } from '../env' import { sendEmail } from '../utils/sendEmail' @@ -107,6 +107,11 @@ export const createUser = async (input: { }) if (!sent) { + // delete user if email failed to send + await AppDataSource.transaction(async (e) => { + await setClaims(e, user.id) + return e.getRepository(User).delete(user.id) + }) return Promise.reject({ errorCode: SignupErrorCode.Unknown }) } } From 638294e17bc7097e1d3f8f89b1ced7fda06c146f Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:18:24 +0800 Subject: [PATCH 18/41] fake sendMail in testing --- .../api/test/services/create_user.test.ts | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 973cccd1a..808aba882 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -1,5 +1,5 @@ import 'mocha' -import { expect } from 'chai' +import chai, { expect } from 'chai' import 'chai/register-should' import { createTestUser, @@ -13,6 +13,12 @@ import { getUserFollowing, } from '../../src/services/followers' import { StatusType } from '../../src/datalayer/user/model' +import sinonChai from 'sinon-chai' +import sinon from 'sinon' +import * as util from '../../src/utils/sendEmail' +import { MailDataRequired } from '@sendgrid/helpers/classes/mail' + +chai.use(sinonChai) describe('create a user with an invite', () => { it('follows the other user in the group', async () => { @@ -53,15 +59,28 @@ describe('create a user with an invite', () => { }) }) -describe('create a pending user', () => { - it('creates a pending user', async () => { - after(async () => { - await deleteTestUser(name) - }) +describe('create a user with pending confirmation', () => { + const name = 'pendingUser' + let fake: (msg: MailDataRequired) => Promise - const name = 'pendingUser' + beforeEach(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(async () => { + sinon.restore() + await deleteTestUser(name) + }) + + it('creates a user with pending status', async () => { const user = await createTestUser(name, undefined, undefined, true) expect(user.status).to.equal(StatusType.Pending) }) + + it('sends an email to the user', async () => { + await createTestUser(name, undefined, undefined, true) + + expect(fake).to.have.been.calledOnce + }) }) From d284a3d30283a43ffd776ba3b92e6b705c15ed6e Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 13:33:17 +0800 Subject: [PATCH 19/41] fake sendMail in sign up test --- packages/api/src/services/create_user.ts | 2 +- packages/api/test/resolvers/user.test.ts | 22 ++++++++++--------- .../api/test/services/create_user.test.ts | 5 +++-- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 0dbe854cc..7b9f62986 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -100,7 +100,7 @@ export const createUser = async (input: { const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` // send email const sent = await sendEmail({ - from: env.sender.general, + from: env.sender.message, to: user.email, subject: 'Confirm your email', text: `Please confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index ebcef7128..e905264dd 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -10,6 +10,9 @@ import { import { User } from '../../src/entity/user' import { hashPassword } from '../../src/utils/auth' import 'mocha' +import { MailDataRequired } from '@sendgrid/helpers/classes/mail' +import sinon from 'sinon' +import * as util from '../../src/utils/sendEmail' describe('User API', () => { const username = 'fake_user' @@ -360,30 +363,29 @@ describe('User API', () => { }) context('when inputs are valid and user not exists', () => { + let fake: (msg: MailDataRequired) => Promise + beforeEach(() => { password = correctPassword username = 'Some_username' email = `${username}@fake.com` + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) }) afterEach(async () => { await deleteTestUser(username) + sinon.restore() }) it('responds with 200', async () => { return graphqlRequest(query).expect(200) }) - it('returns the user', async () => { - const res = await graphqlRequest(query).send() - expect(res.body.data.signup.me.profile.username).to.eql(username) - }) - - it('creates user with correct username', async () => { - const res = await graphqlRequest(query).send() - - const user = await getUser(res.body.data.signup.me.id) - expect(user?.profile?.username).to.eql(username) + it('returns the user with the lowercase username', async () => { + const res = await graphqlRequest(query).expect(200) + expect(res.body.data.signup.me.profile.username).to.eql( + username.toLowerCase() + ) }) }) diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 808aba882..eb825c9a0 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -72,10 +72,11 @@ describe('create a user with pending confirmation', () => { await deleteTestUser(name) }) - it('creates a user with pending status', async () => { + it('creates the user with pending status and correct name', async () => { const user = await createTestUser(name, undefined, undefined, true) - expect(user.status).to.equal(StatusType.Pending) + expect(user.status).to.eql(StatusType.Pending) + expect(user.name).to.eql(name) }) it('sends an email to the user', async () => { From 068684d16b207c66097f7ce24d31d046416a4ea9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 14:55:11 +0800 Subject: [PATCH 20/41] add test for failing to send confirmation email when signup --- packages/api/src/generated/graphql.ts | 1 + packages/api/src/generated/schema.graphql | 1 + packages/api/src/schema.ts | 1 + packages/api/src/services/create_user.ts | 4 +- packages/api/test/resolvers/user.test.ts | 46 ++++-- .../api/test/services/create_user.test.ts | 140 ++++++++++-------- 6 files changed, 122 insertions(+), 71 deletions(-) diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 2beab998c..142f114b9 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -1834,6 +1834,7 @@ export enum SignupErrorCode { AccessDenied = 'ACCESS_DENIED', ExpiredToken = 'EXPIRED_TOKEN', GoogleAuthError = 'GOOGLE_AUTH_ERROR', + InvalidEmail = 'INVALID_EMAIL', InvalidPassword = 'INVALID_PASSWORD', InvalidUsername = 'INVALID_USERNAME', Unknown = 'UNKNOWN', diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 187b8d072..610450dfd 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -1355,6 +1355,7 @@ enum SignupErrorCode { ACCESS_DENIED EXPIRED_TOKEN GOOGLE_AUTH_ERROR + INVALID_EMAIL INVALID_PASSWORD INVALID_USERNAME UNKNOWN diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 9dea9b1ec..259e66c9d 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -168,6 +168,7 @@ const schema = gql` USER_EXISTS EXPIRED_TOKEN INVALID_PASSWORD + INVALID_EMAIL } type GoogleSignupError { diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 7b9f62986..ae944fcd8 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -110,9 +110,11 @@ export const createUser = async (input: { // delete user if email failed to send await AppDataSource.transaction(async (e) => { await setClaims(e, user.id) + return e.getRepository(User).delete(user.id) }) - return Promise.reject({ errorCode: SignupErrorCode.Unknown }) + + return Promise.reject({ errorCode: SignupErrorCode.InvalidEmail }) } } diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index e905264dd..78abb33df 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -333,6 +333,7 @@ describe('User API', () => { let email: string let password: string let username: string + let fake: (msg: MailDataRequired) => Promise beforeEach(() => { query = ` @@ -363,29 +364,52 @@ describe('User API', () => { }) context('when inputs are valid and user not exists', () => { - let fake: (msg: MailDataRequired) => Promise - beforeEach(() => { password = correctPassword username = 'Some_username' email = `${username}@fake.com` - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) }) afterEach(async () => { await deleteTestUser(username) - sinon.restore() }) - it('responds with 200', async () => { - return graphqlRequest(query).expect(200) + context('when confirmation email sent', () => { + beforeEach(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(() => { + sinon.restore() + }) + + it('responds with 200', async () => { + return graphqlRequest(query).expect(200) + }) + + it('returns the user with the lowercase username', async () => { + const res = await graphqlRequest(query).expect(200) + expect(res.body.data.signup.me.profile.username).to.eql( + username.toLowerCase() + ) + }) }) - it('returns the user with the lowercase username', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.signup.me.profile.username).to.eql( - username.toLowerCase() - ) + context('when confirmation email not sent', () => { + before(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) + }) + + after(() => { + sinon.restore() + }) + + it('responds with error code INVALID_EMAIL', async () => { + const res = await graphqlRequest(query).expect(200) + expect(res.body.data.signup.errorCodes).to.eql([ + SignupErrorCode.InvalidEmail, + ]) + }) }) }) diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index eb825c9a0..9615250db 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -17,71 +17,93 @@ import sinonChai from 'sinon-chai' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import { MailDataRequired } from '@sendgrid/helpers/classes/mail' +import { getRepository } from '../../src/entity/utils' +import { User } from '../../src/entity/user' chai.use(sinonChai) -describe('create a user with an invite', () => { - it('follows the other user in the group', async () => { - after(async () => { - await deleteTestUser(testOwner) - await deleteTestUser(testUser) +describe('create user', () => { + context('create a user with an invite', () => { + it('follows the other user in the group', async () => { + after(async () => { + await deleteTestUser(testOwner) + await deleteTestUser(testUser) + }) + + const testOwner = 'testowner' + const testUser = 'testuser' + + const adminUser = await createTestUser(testOwner) + const [, invite] = await createGroup({ + admin: adminUser, + name: 'testgroup', + }) + const user = await createTestUser(testUser, invite.code) + + expect(await getUserFollowers(user)).to.eql([adminUser]) + expect(await getUserFollowing(user)).to.eql([adminUser]) + expect(await getUserFollowers(adminUser)).to.eql([user]) + expect(await getUserFollowing(adminUser)).to.eql([user]) + }).timeout(10000) + + it('creates profile when user exists but profile not', async () => { + after(async () => { + await deleteTestUser(name) + }) + + const name = 'userWithoutProfile' + const user = await createUserWithoutProfile(name) + + await createTestUser(user.name) + + const profile = await getProfile(user) + + expect(profile).to.exist + }) + }) + + context('create a user with pending confirmation', () => { + const name = 'pendingUser' + let fake: (msg: MailDataRequired) => Promise + + context('when email sends successfully', () => { + beforeEach(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(async () => { + sinon.restore() + await deleteTestUser(name) + }) + + it('creates the user with pending status and correct name', async () => { + const user = await createTestUser(name, undefined, undefined, true) + + expect(user.status).to.eql(StatusType.Pending) + expect(user.name).to.eql(name) + }) + + it('sends an email to the user', async () => { + await createTestUser(name, undefined, undefined, true) + + expect(fake).to.have.been.calledOnce + }) }) - const testOwner = 'testowner' - const testUser = 'testuser' + context('when failed to send email', () => { + before(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) + }) - const adminUser = await createTestUser(testOwner) - const [, invite] = await createGroup({ - admin: adminUser, - name: 'testgroup', + after(() => { + sinon.restore() + }) + + it('does not create the user', async () => { + await expect(createTestUser(name, undefined, undefined, true)).to.be + .rejected + expect(await getRepository(User).findOneBy({ name })).to.be.null + }) }) - const user = await createTestUser(testUser, invite.code) - - expect(await getUserFollowers(user)).to.eql([adminUser]) - expect(await getUserFollowing(user)).to.eql([adminUser]) - expect(await getUserFollowers(adminUser)).to.eql([user]) - expect(await getUserFollowing(adminUser)).to.eql([user]) - }).timeout(10000) - - it('creates profile when user exists but profile not', async () => { - after(async () => { - await deleteTestUser(name) - }) - - const name = 'userWithoutProfile' - const user = await createUserWithoutProfile(name) - - await createTestUser(user.name) - - const profile = await getProfile(user) - - expect(profile).to.exist - }) -}) - -describe('create a user with pending confirmation', () => { - const name = 'pendingUser' - let fake: (msg: MailDataRequired) => Promise - - beforeEach(() => { - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) - }) - - afterEach(async () => { - sinon.restore() - await deleteTestUser(name) - }) - - it('creates the user with pending status and correct name', async () => { - const user = await createTestUser(name, undefined, undefined, true) - - expect(user.status).to.eql(StatusType.Pending) - expect(user.name).to.eql(name) - }) - - it('sends an email to the user', async () => { - await createTestUser(name, undefined, undefined, true) - - expect(fake).to.have.been.calledOnce }) }) From 80639311d7d3c363573bc4e9979cc683c4cd98f8 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 15:04:49 +0800 Subject: [PATCH 21/41] improve confirmation email format --- packages/api/src/services/create_user.ts | 27 ++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index ae944fcd8..aaef52cb1 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -93,20 +93,8 @@ export const createUser = async (input: { } ) - // send confirmation email if (input.pendingConfirmation) { - // generate confirmation link - const confirmationToken = generateVerificationToken(user.id) - const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` - // send email - const sent = await sendEmail({ - from: env.sender.message, - to: user.email, - subject: 'Confirm your email', - text: `Please confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, - }) - - if (!sent) { + if (!(await sendConfirmationEmail(user))) { // delete user if email failed to send await AppDataSource.transaction(async (e) => { await setClaims(e, user.id) @@ -149,3 +137,16 @@ const getUser = async (email: string): Promise => { relations: ['profile'], }) } + +export const sendConfirmationEmail = async (user: User): Promise => { + // generate confirmation link + const confirmationToken = generateVerificationToken(user.id) + const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` + // send email + return sendEmail({ + from: `Omnivore <${env.sender.message}>`, + to: user.email, + subject: 'Confirm your email', + text: `Hey ${user.name},\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, + }) +} From f7053622a58b0dc5d8a7164a0237983dfa6d2be0 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 19:19:38 +0800 Subject: [PATCH 22/41] remove signup gql resolver --- .../api/src/resolvers/function_resolvers.ts | 3 - packages/api/src/resolvers/user/index.ts | 41 +---- packages/api/src/routers/auth/auth_router.ts | 65 +++----- packages/api/src/schema.ts | 20 --- packages/api/test/resolvers/user.test.ts | 132 ---------------- ...article_router.test.ts => article.test.ts} | 0 packages/api/test/routers/auth.test.ts | 142 ++++++++++++++++++ ...nders_router.test.ts => reminders.test.ts} | 0 8 files changed, 167 insertions(+), 236 deletions(-) rename packages/api/test/routers/{article_router.test.ts => article.test.ts} (100%) create mode 100644 packages/api/test/routers/auth.test.ts rename packages/api/test/routers/{reminders_router.test.ts => reminders.test.ts} (100%) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 0ba6ac410..6150cd145 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -73,7 +73,6 @@ import { setShareHighlightResolver, setUserPersonalizationResolver, setWebhookResolver, - signupResolver, subscribeResolver, subscriptionsResolver, typeaheadSearchResolver, @@ -155,7 +154,6 @@ export const functionResolvers = { updateLabel: updateLabelResolver, deleteLabel: deleteLabelResolver, login: loginResolver, - signup: signupResolver, setLabels: setLabelsResolver, generateApiKey: generateApiKeyResolver, unsubscribe: unsubscribeResolver, @@ -573,7 +571,6 @@ export const functionResolvers = { ...resultResolveTypeResolver('CreateLabel'), ...resultResolveTypeResolver('DeleteLabel'), ...resultResolveTypeResolver('Login'), - ...resultResolveTypeResolver('Signup'), ...resultResolveTypeResolver('SetLabels'), ...resultResolveTypeResolver('GenerateApiKey'), ...resultResolveTypeResolver('Search'), diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index fbd747124..df35f5e2a 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -11,14 +11,12 @@ import { MutationGoogleLoginArgs, MutationGoogleSignupArgs, MutationLoginArgs, - MutationSignupArgs, MutationUpdateUserArgs, MutationUpdateUserProfileArgs, QueryUserArgs, QueryValidateUsernameArgs, ResolverFn, SignupErrorCode, - SignupResult, UpdateUserError, UpdateUserErrorCode, UpdateUserProfileError, @@ -37,7 +35,7 @@ import { env } from '../../env' import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' -import { comparePassword, hashPassword } from '../../utils/auth' +import { comparePassword } from '../../utils/auth' import { deletePagesByParam } from '../../elastic/pages' import { setClaims } from '../../entity/utils' import { User as UserEntity } from '../../entity/user' @@ -328,43 +326,6 @@ export const loginResolver: ResolverFn< return { me: userDataToUser(user) } } -export const signupResolver: ResolverFn< - SignupResult, - Record, - WithDataSourcesContext, - MutationSignupArgs -> = async (_obj, { input }) => { - const { email, username, name, bio, password, pictureUrl } = input - const lowerCasedUsername = username.toLowerCase() - - try { - // hash password - const hashedPassword = await hashPassword(password) - - const [user, profile] = await createUser({ - email, - provider: 'EMAIL', - sourceUserId: email, - name, - username: lowerCasedUsername, - pictureUrl: pictureUrl || undefined, - bio: bio || undefined, - password: hashedPassword, - pendingConfirmation: true, - }) - - return { - me: userDataToUser({ ...user, profile: { ...profile, private: false } }), - } - } catch (err) { - console.log('error', err) - if (isErrorWithCode(err)) { - return { errorCodes: [err.errorCode as SignupErrorCode] } - } - return { errorCodes: [SignupErrorCode.Unknown] } - } -} - export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index d9289111e..661bf701f 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -37,6 +37,9 @@ import { RegistrationType, UserData, } from '../../datalayer/user/model' +import { hashPassword } from '../../utils/auth' +import { createUser } from '../../services/create_user' +import { isErrorWithCode } from '../../resolvers' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -422,53 +425,33 @@ export function authRouter() { '/email-signup', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const { email, password, name, username, bio } = req.body - if (!email || !password || !name || !username) { - res.redirect(`${env.client.url}/email-signup?errorCodes=BAD_DATA`) - return - } - - const query = ` - mutation signup { - signup(input: { - email: "${email}", - password: "${password}", - name: "${name}", - username: "${username}", - bio: "${bio ?? ''}" - }) { - __typename - ... on SignupSuccess { - me { - id - name - profile { - username - } - } - } - ... on SignupError { - errorCodes - } - } - }` + const { email, password, name, username, bio, pictureUrl } = req.body + const lowerCasedUsername = username.toLowerCase() try { - const result = await axios.post(env.server.gateway_url + '/graphql', { - query, - }) - const { data } = result.data + // hash password + const hashedPassword = await hashPassword(password) - if (data.signup.__typename === 'SignupError') { - const errorCodes = data.signup.errorCodes.join(',') - return res.redirect( - `${env.client.url}/email-signup?errorCodes=${errorCodes}` - ) - } + await createUser({ + email, + provider: 'EMAIL', + sourceUserId: email, + name, + username: lowerCasedUsername, + pictureUrl, + bio, + password: hashedPassword, + pendingConfirmation: true, + }) res.redirect(`${env.client.url}/email-login?message=SIGNUP_SUCCESS`) } catch (e) { - logger.info('email-signup exception:', e) + logger.error('email-signup exception:', e) + if (isErrorWithCode(e)) { + return res.redirect( + `${env.client.url}/email-signup?errorCodes=${e.errorCode}` + ) + } res.redirect(`${env.client.url}/email-signup?errorCodes=UNKNOWN`) } } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 259e66c9d..3ab828e1a 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -1415,25 +1415,6 @@ const schema = gql` email: String! } - input SignupInput { - email: String! - password: String! @sanitize(maxLength: 40) - username: String! - name: String! - pictureUrl: String - bio: String - } - - type SignupSuccess { - me: User! - } - - type SignupError { - errorCodes: [SignupErrorCode]! - } - - union SignupResult = SignupSuccess | SignupError - input SetLabelsInput { pageId: ID! labelIds: [ID!]! @@ -1845,7 +1826,6 @@ const schema = gql` updateLabel(input: UpdateLabelInput!): UpdateLabelResult! deleteLabel(id: ID!): DeleteLabelResult! login(input: LoginInput!): LoginResult! - signup(input: SignupInput!): SignupResult! setLabels(input: SetLabelsInput!): SetLabelsResult! generateApiKey(input: GenerateApiKeyInput!): GenerateApiKeyResult! unsubscribe(name: String!): UnsubscribeResult! diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index 78abb33df..afe6e7f53 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -3,16 +3,12 @@ import { graphqlRequest, request } from '../util' import { expect } from 'chai' import { LoginErrorCode, - SignupErrorCode, UpdateUserErrorCode, UpdateUserProfileErrorCode, } from '../../src/generated/graphql' import { User } from '../../src/entity/user' import { hashPassword } from '../../src/utils/auth' import 'mocha' -import { MailDataRequired } from '@sendgrid/helpers/classes/mail' -import sinon from 'sinon' -import * as util from '../../src/utils/sendEmail' describe('User API', () => { const username = 'fake_user' @@ -327,132 +323,4 @@ describe('User API', () => { }) }) }) - - describe('signup', () => { - let query: string - let email: string - let password: string - let username: string - let fake: (msg: MailDataRequired) => Promise - - beforeEach(() => { - query = ` - mutation { - signup( - input: { - email: "${email}" - password: "${password}" - name: "Some name" - username: "${username}" - } - ) { - ... on SignupSuccess { - me { - id - name - profile { - username - } - } - } - ... on SignupError { - errorCodes - } - } - } - ` - }) - - context('when inputs are valid and user not exists', () => { - beforeEach(() => { - password = correctPassword - username = 'Some_username' - email = `${username}@fake.com` - }) - - afterEach(async () => { - await deleteTestUser(username) - }) - - context('when confirmation email sent', () => { - beforeEach(() => { - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) - }) - - afterEach(() => { - sinon.restore() - }) - - it('responds with 200', async () => { - return graphqlRequest(query).expect(200) - }) - - it('returns the user with the lowercase username', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.signup.me.profile.username).to.eql( - username.toLowerCase() - ) - }) - }) - - context('when confirmation email not sent', () => { - before(() => { - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) - }) - - after(() => { - sinon.restore() - }) - - it('responds with error code INVALID_EMAIL', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.InvalidEmail, - ]) - }) - }) - }) - - context('when password is too long', () => { - before(() => { - email = 'Some_email' - password = 'Some_password_that_is_too_long_for_database' - username = 'Some_username' - }) - - it('responds with status code 400', async () => { - return graphqlRequest(query).expect(400) - }) - }) - - context('when user exists', () => { - before(() => { - email = user.email - password = 'Some password' - username = 'Some username' - }) - - it('responds with error code UserExists', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.UserExists, - ]) - }) - }) - - context('when username is invalid', () => { - before(() => { - email = 'Some_email' - password = correctPassword - username = 'omnivore_admin' - }) - - it('responds with error code InvalidUsername', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.InvalidUsername, - ]) - }) - }) - }) }) diff --git a/packages/api/test/routers/article_router.test.ts b/packages/api/test/routers/article.test.ts similarity index 100% rename from packages/api/test/routers/article_router.test.ts rename to packages/api/test/routers/article.test.ts diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts new file mode 100644 index 000000000..1086acb56 --- /dev/null +++ b/packages/api/test/routers/auth.test.ts @@ -0,0 +1,142 @@ +import { createTestUser, deleteTestUser } from '../db' +import { request } from '../util' +import { expect } from 'chai' +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 sinon from 'sinon' +import * as util from '../../src/utils/sendEmail' +import supertest from 'supertest' + +describe('auth router', () => { + const route = '/api/auth' + const signupRequest = ( + email: string, + password: string, + name: string, + username: string + ): supertest.Test => { + return request.post(`${route}/email-signup`).send({ + email, + password, + name, + username, + }) + } + + describe('email signup', () => { + const validPassword = 'validPassword' + + let email: string + let password: string + let username: string + let name: string + + context('when inputs are valid and user not exists', () => { + let fake: (msg: MailDataRequired) => Promise + + before(() => { + password = validPassword + username = 'Some_username' + email = `${username}@fake.com` + name = 'Some name' + }) + + afterEach(async () => { + await deleteTestUser(username) + }) + + context('when confirmation email sent', () => { + beforeEach(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(() => { + sinon.restore() + }) + + it('redirects to login page', async () => { + const res = await signupRequest( + email, + password, + name, + username + ).expect(302) + expect(res.header.location).to.endWith( + '/email-login?message=SIGNUP_SUCCESS' + ) + }) + + it('creates the user with pending status and correct name', async () => { + await signupRequest(email, password, name, username).expect(302) + const user = await getRepository(User).findOneBy({ name }) + + expect(user?.status).to.eql(StatusType.Pending) + expect(user?.name).to.eql(name) + }) + }) + + context('when confirmation email not sent', () => { + before(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) + }) + + after(() => { + sinon.restore() + }) + + it('redirects to sign up page with error code INVALID_EMAIL', async () => { + const res = await signupRequest( + email, + password, + name, + username + ).expect(302) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=INVALID_EMAIL' + ) + }) + }) + }) + + context('when user exists', () => { + before(async () => { + username = 'Some_username' + const user = await createTestUser(username) + email = user.email + password = 'Some password' + }) + + after(async () => { + await deleteTestUser(username) + }) + + it('redirects to sign up page with error code USER_EXISTS', async () => { + const res = await signupRequest(email, password, name, username).expect( + 302 + ) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=USER_EXISTS' + ) + }) + }) + + context('when username is invalid', () => { + before(() => { + email = 'Some_email' + password = validPassword + username = 'omnivore_admin' + }) + + it('redirects to sign up page with error code INVALID_USERNAME', async () => { + const res = await signupRequest(email, password, name, username).expect( + 302 + ) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=INVALID_USERNAME' + ) + }) + }) + }) +}) diff --git a/packages/api/test/routers/reminders_router.test.ts b/packages/api/test/routers/reminders.test.ts similarity index 100% rename from packages/api/test/routers/reminders_router.test.ts rename to packages/api/test/routers/reminders.test.ts From 86b950183c8fcd488559d9a5ef3b4ed5ca9324f9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 19:22:33 +0800 Subject: [PATCH 23/41] fix test --- packages/api/test/routers/auth.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 1086acb56..c35e49e94 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -94,7 +94,7 @@ describe('auth router', () => { username ).expect(302) expect(res.header.location).to.endWith( - '/email-registration?errorCodes=INVALID_EMAIL' + '/email-signup?errorCodes=INVALID_EMAIL' ) }) }) @@ -117,7 +117,7 @@ describe('auth router', () => { 302 ) expect(res.header.location).to.endWith( - '/email-registration?errorCodes=USER_EXISTS' + '/email-signup?errorCodes=USER_EXISTS' ) }) }) @@ -134,7 +134,7 @@ describe('auth router', () => { 302 ) expect(res.header.location).to.endWith( - '/email-registration?errorCodes=INVALID_USERNAME' + '/email-signup?errorCodes=INVALID_USERNAME' ) }) }) From abb1a414c1790eda2ed42b0692b57b2361079a4c Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 20:17:14 +0800 Subject: [PATCH 24/41] remove login gql resolver --- .../api/src/resolvers/function_resolvers.ts | 3 - packages/api/src/resolvers/user/index.ts | 33 ------- packages/api/src/routers/auth/auth_router.ts | 65 ++++++-------- packages/api/src/schema.ts | 6 -- packages/api/src/services/create_user.ts | 2 +- packages/api/test/resolvers/user.test.ts | 86 ------------------- packages/api/test/routers/auth.test.ts | 82 ++++++++++++++++++ 7 files changed, 109 insertions(+), 168 deletions(-) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 6150cd145..8ed87340f 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -50,7 +50,6 @@ import { googleLoginResolver, googleSignupResolver, labelsResolver, - loginResolver, logOutResolver, mergeHighlightResolver, newsletterEmailsResolver, @@ -153,7 +152,6 @@ export const functionResolvers = { createLabel: createLabelResolver, updateLabel: updateLabelResolver, deleteLabel: deleteLabelResolver, - login: loginResolver, setLabels: setLabelsResolver, generateApiKey: generateApiKeyResolver, unsubscribe: unsubscribeResolver, @@ -570,7 +568,6 @@ export const functionResolvers = { ...resultResolveTypeResolver('Labels'), ...resultResolveTypeResolver('CreateLabel'), ...resultResolveTypeResolver('DeleteLabel'), - ...resultResolveTypeResolver('Login'), ...resultResolveTypeResolver('SetLabels'), ...resultResolveTypeResolver('GenerateApiKey'), ...resultResolveTypeResolver('Search'), diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index df35f5e2a..d9a6f3828 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -10,7 +10,6 @@ import { MutationDeleteAccountArgs, MutationGoogleLoginArgs, MutationGoogleSignupArgs, - MutationLoginArgs, MutationUpdateUserArgs, MutationUpdateUserProfileArgs, QueryUserArgs, @@ -35,7 +34,6 @@ import { env } from '../../env' import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' -import { comparePassword } from '../../utils/auth' import { deletePagesByParam } from '../../elastic/pages' import { setClaims } from '../../entity/utils' import { User as UserEntity } from '../../entity/user' @@ -295,37 +293,6 @@ export function isErrorWithCode(error: unknown): error is ErrorWithCode { ) } -export const loginResolver: ResolverFn< - LoginResult, - unknown, - WithDataSourcesContext, - MutationLoginArgs -> = async (_obj, { input }, { models, setAuth }) => { - const { email, password } = input - - const user = await models.user.getWhere({ - email, - }) - if (!user?.id) { - return { errorCodes: [LoginErrorCode.UserNotFound] } - } - - if (!user?.password) { - // user has no password, so they need to set one - return { errorCodes: [LoginErrorCode.WrongSource] } - } - - // check if password is correct - const validPassword = await comparePassword(password, user.password) - if (!validPassword) { - return { errorCodes: [LoginErrorCode.InvalidCredentials] } - } - - // set auth cookie in response header - await setAuth({ uid: user.id }) - return { me: userDataToUser(user) } -} - export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 661bf701f..6e5b26cbd 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -37,9 +37,10 @@ import { RegistrationType, UserData, } from '../../datalayer/user/model' -import { hashPassword } from '../../utils/auth' +import { comparePassword, hashPassword } from '../../utils/auth' import { createUser } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' +import { initModels } from '../../server' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -359,56 +360,42 @@ export function authRouter() { cors(corsConfig), async (req: express.Request, res: express.Response) => { const { email, password } = req.body - if (!email || !password) { - res.redirect(`${env.client.url}/email-login?errorCodes=AUTH_FAILED`) - return - } - - const query = ` - mutation login{ - login(input: { - email: "${email}", - password: "${password}" - }) { - __typename - ... on LoginError { errorCodes } - ... on LoginSuccess { - me { - id - name - profile { - username - } - } - } - } - }` try { - const result = await axios.post(env.server.gateway_url + '/graphql', { - query, + const models = initModels(kx, false) + const user = await models.user.getWhere({ + email, }) - const { data } = result.data - - if (data.login.__typename === 'LoginError') { - const errorCodes = data.login.errorCodes.join(',') + if (!user?.id) { return res.redirect( - `${env.client.url}/email-login?errorCodes=${errorCodes}` + `${env.client.url}/email-login?errorCodes=${LoginErrorCode.UserNotFound}` ) } - if (!result.headers['set-cookie']) { + if (!user?.password) { + // user has no password, so they need to set one return res.redirect( - `${env.client.url}/${ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (req.params as any)?.action - }?errorCodes=unknown` + `${env.client.url}/email-login?errorCodes=${LoginErrorCode.WrongSource}` ) } - res.setHeader('set-cookie', result.headers['set-cookie']) + // check if password is correct + const validPassword = await comparePassword(password, user.password) + if (!validPassword) { + return res.redirect( + `${env.client.url}/email-login?errorCodes=${LoginErrorCode.InvalidCredentials}` + ) + } - await handleSuccessfulLogin(req, res, data.login.me, false) + // set auth cookie in response header + const token = await signToken({ uid: user.id }, env.server.jwtSecret) + + res.cookie('auth', token, { + httpOnly: true, + expires: new Date(new Date().getTime() + 365 * 24 * 60 * 60 * 1000), + }) + + await handleSuccessfulLogin(req, res, user, false) } catch (e) { logger.info('email-login exception:', e) res.redirect(`${env.client.url}/email-login?errorCodes=AUTH_FAILED`) diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 3ab828e1a..336056564 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -1410,11 +1410,6 @@ const schema = gql` union UpdateLabelResult = UpdateLabelSuccess | UpdateLabelError - input LoginInput { - password: String! - email: String! - } - input SetLabelsInput { pageId: ID! labelIds: [ID!]! @@ -1825,7 +1820,6 @@ const schema = gql` createLabel(input: CreateLabelInput!): CreateLabelResult! updateLabel(input: UpdateLabelInput!): UpdateLabelResult! deleteLabel(id: ID!): DeleteLabelResult! - login(input: LoginInput!): LoginResult! setLabels(input: SetLabelsInput!): SetLabelsResult! generateApiKey(input: GenerateApiKeyInput!): GenerateApiKeyResult! unsubscribe(name: String!): UnsubscribeResult! diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index aaef52cb1..b97075b48 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -147,6 +147,6 @@ export const sendConfirmationEmail = async (user: User): Promise => { from: `Omnivore <${env.sender.message}>`, to: user.email, subject: 'Confirm your email', - text: `Hey ${user.name},\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, + text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, }) } diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index afe6e7f53..58ebd8cb6 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -2,7 +2,6 @@ import { createTestUser, deleteTestUser, getProfile, getUser } from '../db' import { graphqlRequest, request } from '../util' import { expect } from 'chai' import { - LoginErrorCode, UpdateUserErrorCode, UpdateUserProfileErrorCode, } from '../../src/generated/graphql' @@ -238,89 +237,4 @@ describe('User API', () => { return graphqlRequest(query, invalidAuthToken).expect(500) }) }) - - describe('login', () => { - let query: string - let email: string - let password: string - - beforeEach(() => { - query = ` - mutation { - login( - input: { - email: "${email}" - password: "${password}" - } - ) { - ... on LoginSuccess { - me { - id - name - profile { - username - } - } - } - ... on LoginError { - errorCodes - } - } - } - ` - }) - - context('when email and password are valid', () => { - before(() => { - email = user.email - password = correctPassword - }) - - it('responds with 200', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.login.me.id).to.eql(user.id) - }) - }) - - context('when user not exists', () => { - before(() => { - email = 'Some email' - }) - - it('responds with error code UserNotFound', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.login.errorCodes).to.eql([ - LoginErrorCode.UserNotFound, - ]) - }) - }) - - context('when user has no password stored in db', () => { - before(() => { - email = anotherUser.email - password = 'Some password' - }) - - it('responds with error code WrongSource', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.login.errorCodes).to.eql([ - LoginErrorCode.WrongSource, - ]) - }) - }) - - context('when password is wrong', () => { - before(() => { - email = user.email - password = 'Some password' - }) - - it('responds with error code UserNotFound', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.login.errorCodes).to.eql([ - LoginErrorCode.InvalidCredentials, - ]) - }) - }) - }) }) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index c35e49e94..bd82ed49d 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -8,6 +8,7 @@ import { MailDataRequired } from '@sendgrid/helpers/classes/mail' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import supertest from 'supertest' +import { hashPassword } from '../../src/utils/auth' describe('auth router', () => { const route = '/api/auth' @@ -139,4 +140,85 @@ describe('auth router', () => { }) }) }) + + describe('login', () => { + const loginRequest = (email: string, password: string): supertest.Test => { + return request.post(`${route}/email-login`).send({ + email, + password, + }) + } + const correctPassword = 'correctPassword' + + let user: User + let email: string + let password: string + + before(async () => { + const hashedPassword = await hashPassword(correctPassword) + user = await createTestUser('login_test_user', undefined, hashedPassword) + }) + + after(async () => { + await deleteTestUser(user.name) + }) + + context('when email and password are valid', () => { + before(() => { + email = user.email + password = correctPassword + }) + + it('redirects to waitlist page', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header.location).to.endWith('/waitlist') + }) + }) + + context('when user not exists', () => { + before(() => { + email = 'Some email' + }) + + it('redirects with error code UserNotFound', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header.location).to.endWith( + '/email-login?errorCodes=USER_NOT_FOUND' + ) + }) + }) + + context('when user has no password stored in db', () => { + before(async () => { + const anotherUser = await createTestUser('another_user') + email = anotherUser.email + password = 'Some password' + }) + + after(async () => { + await deleteTestUser('another_user') + }) + + it('redirects with error code WrongSource', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header.location).to.endWith( + '/email-login?errorCodes=WRONG_SOURCE' + ) + }) + }) + + context('when password is wrong', () => { + before(() => { + email = user.email + password = 'Wrong password' + }) + + it('redirects with error code InvalidCredentials', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header.location).to.endWith( + '/email-login?errorCodes=INVALID_CREDENTIALS' + ) + }) + }) + }) }) From bd2327a3ae22f51b0b60f03a29c102a34123e670 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 21:01:00 +0800 Subject: [PATCH 25/41] test set auth token after login --- packages/api/test/routers/auth.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index bd82ed49d..9ddede738 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -173,6 +173,12 @@ describe('auth router', () => { const res = await loginRequest(email, password).expect(302) expect(res.header.location).to.endWith('/waitlist') }) + + it('set auth token in cookie', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header['set-cookie']).to.be.an('array') + expect(res.header['set-cookie'][0]).to.contain('auth') + }) }) context('when user not exists', () => { From 9ea9bd9ea1304154b7bb7230fc03fbb570257cf2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 22:13:46 +0800 Subject: [PATCH 26/41] send confirmation email to pending user when login --- packages/api/src/datalayer/user/model.ts | 2 + packages/api/src/entity/user.ts | 2 +- packages/api/src/generated/graphql.ts | 67 ------------------- packages/api/src/generated/schema.graphql | 26 ------- packages/api/src/routers/auth/auth_router.ts | 14 +++- packages/api/src/services/create_user.ts | 15 ++--- packages/api/test/routers/auth.test.ts | 43 ++++++++++-- .../api/test/services/create_user.test.ts | 7 +- 8 files changed, 62 insertions(+), 114 deletions(-) diff --git a/packages/api/src/datalayer/user/model.ts b/packages/api/src/datalayer/user/model.ts index 5e10038e9..945d07753 100644 --- a/packages/api/src/datalayer/user/model.ts +++ b/packages/api/src/datalayer/user/model.ts @@ -44,6 +44,7 @@ export interface UserData { private: boolean } password?: string | null + status?: StatusType } export enum MembershipTier { @@ -72,6 +73,7 @@ export const keys = [ 'sourceUserId', 'createdAt', 'password', + 'status', ] as const export const defaultedKeys = ['id', 'createdAt'] as const diff --git a/packages/api/src/entity/user.ts b/packages/api/src/entity/user.ts index 50ad7e7a9..99a6e1afd 100644 --- a/packages/api/src/entity/user.ts +++ b/packages/api/src/entity/user.ts @@ -59,5 +59,5 @@ export class User { subscriptions?: Subscription[] @Column({ type: 'enum', enum: StatusType }) - status!: string + status!: StatusType } diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 142f114b9..2310ca181 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -825,11 +825,6 @@ export enum LoginErrorCode { WrongSource = 'WRONG_SOURCE' } -export type LoginInput = { - email: Scalars['String']; - password: Scalars['String']; -}; - export type LoginResult = LoginError | LoginSuccess; export type LoginSuccess = { @@ -893,7 +888,6 @@ export type Mutation = { googleLogin: LoginResult; googleSignup: GoogleSignupResult; logOut: LogOutResult; - login: LoginResult; mergeHighlight: MergeHighlightResult; reportItem: ReportItemResult; revokeApiKey: RevokeApiKeyResult; @@ -911,7 +905,6 @@ export type Mutation = { setShareHighlight: SetShareHighlightResult; setUserPersonalization: SetUserPersonalizationResult; setWebhook: SetWebhookResult; - signup: SignupResult; subscribe: SubscribeResult; unsubscribe: UnsubscribeResult; updateHighlight: UpdateHighlightResult; @@ -1022,11 +1015,6 @@ export type MutationGoogleSignupArgs = { }; -export type MutationLoginArgs = { - input: LoginInput; -}; - - export type MutationMergeHighlightArgs = { input: MergeHighlightInput; }; @@ -1112,11 +1100,6 @@ export type MutationSetWebhookArgs = { }; -export type MutationSignupArgs = { - input: SignupInput; -}; - - export type MutationSubscribeArgs = { name: Scalars['String']; }; @@ -1825,11 +1808,6 @@ export type SharedArticleSuccess = { article: Article; }; -export type SignupError = { - __typename?: 'SignupError'; - errorCodes: Array>; -}; - export enum SignupErrorCode { AccessDenied = 'ACCESS_DENIED', ExpiredToken = 'EXPIRED_TOKEN', @@ -1841,22 +1819,6 @@ export enum SignupErrorCode { UserExists = 'USER_EXISTS' } -export type SignupInput = { - bio?: InputMaybe; - email: Scalars['String']; - name: Scalars['String']; - password: Scalars['String']; - pictureUrl?: InputMaybe; - username: Scalars['String']; -}; - -export type SignupResult = SignupError | SignupSuccess; - -export type SignupSuccess = { - __typename?: 'SignupSuccess'; - me: User; -}; - export enum SortBy { PublishedAt = 'PUBLISHED_AT', SavedAt = 'SAVED_AT', @@ -2573,7 +2535,6 @@ export type ResolversTypes = { LogOutSuccess: ResolverTypeWrapper; LoginError: ResolverTypeWrapper; LoginErrorCode: LoginErrorCode; - LoginInput: LoginInput; LoginResult: ResolversTypes['LoginError'] | ResolversTypes['LoginSuccess']; LoginSuccess: ResolverTypeWrapper; MergeHighlightError: ResolverTypeWrapper; @@ -2677,11 +2638,7 @@ export type ResolversTypes = { SharedArticleErrorCode: SharedArticleErrorCode; SharedArticleResult: ResolversTypes['SharedArticleError'] | ResolversTypes['SharedArticleSuccess']; SharedArticleSuccess: ResolverTypeWrapper; - SignupError: ResolverTypeWrapper; SignupErrorCode: SignupErrorCode; - SignupInput: SignupInput; - SignupResult: ResolversTypes['SignupError'] | ResolversTypes['SignupSuccess']; - SignupSuccess: ResolverTypeWrapper; SortBy: SortBy; SortOrder: SortOrder; SortParams: SortParams; @@ -2901,7 +2858,6 @@ export type ResolversParentTypes = { LogOutResult: ResolversParentTypes['LogOutError'] | ResolversParentTypes['LogOutSuccess']; LogOutSuccess: LogOutSuccess; LoginError: LoginError; - LoginInput: LoginInput; LoginResult: ResolversParentTypes['LoginError'] | ResolversParentTypes['LoginSuccess']; LoginSuccess: LoginSuccess; MergeHighlightError: MergeHighlightError; @@ -2985,10 +2941,6 @@ export type ResolversParentTypes = { SharedArticleError: SharedArticleError; SharedArticleResult: ResolversParentTypes['SharedArticleError'] | ResolversParentTypes['SharedArticleSuccess']; SharedArticleSuccess: SharedArticleSuccess; - SignupError: SignupError; - SignupInput: SignupInput; - SignupResult: ResolversParentTypes['SignupError'] | ResolversParentTypes['SignupSuccess']; - SignupSuccess: SignupSuccess; SortParams: SortParams; String: Scalars['String']; SubscribeError: SubscribeError; @@ -3715,7 +3667,6 @@ export type MutationResolvers>; googleSignup?: Resolver>; logOut?: Resolver; - login?: Resolver>; mergeHighlight?: Resolver>; reportItem?: Resolver>; revokeApiKey?: Resolver>; @@ -3733,7 +3684,6 @@ export type MutationResolvers>; setUserPersonalization?: Resolver>; setWebhook?: Resolver>; - signup?: Resolver>; subscribe?: Resolver>; unsubscribe?: Resolver>; updateHighlight?: Resolver>; @@ -4125,20 +4075,6 @@ export type SharedArticleSuccessResolvers; }; -export type SignupErrorResolvers = { - errorCodes?: Resolver>, ParentType, ContextType>; - __isTypeOf?: IsTypeOfResolverFn; -}; - -export type SignupResultResolvers = { - __resolveType: TypeResolveFn<'SignupError' | 'SignupSuccess', ParentType, ContextType>; -}; - -export type SignupSuccessResolvers = { - me?: Resolver; - __isTypeOf?: IsTypeOfResolverFn; -}; - export type SubscribeErrorResolvers = { errorCodes?: Resolver, ParentType, ContextType>; __isTypeOf?: IsTypeOfResolverFn; @@ -4628,9 +4564,6 @@ export type Resolvers = { SharedArticleError?: SharedArticleErrorResolvers; SharedArticleResult?: SharedArticleResultResolvers; SharedArticleSuccess?: SharedArticleSuccessResolvers; - SignupError?: SignupErrorResolvers; - SignupResult?: SignupResultResolvers; - SignupSuccess?: SignupSuccessResolvers; SubscribeError?: SubscribeErrorResolvers; SubscribeResult?: SubscribeResultResolvers; SubscribeSuccess?: SubscribeSuccessResolvers; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 610450dfd..d3170ed12 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -730,11 +730,6 @@ enum LoginErrorCode { WRONG_SOURCE } -input LoginInput { - email: String! - password: String! -} - union LoginResult = LoginError | LoginSuccess type LoginSuccess { @@ -794,7 +789,6 @@ type Mutation { googleLogin(input: GoogleLoginInput!): LoginResult! googleSignup(input: GoogleSignupInput!): GoogleSignupResult! logOut: LogOutResult! - login(input: LoginInput!): LoginResult! mergeHighlight(input: MergeHighlightInput!): MergeHighlightResult! reportItem(input: ReportItemInput!): ReportItemResult! revokeApiKey(id: ID!): RevokeApiKeyResult! @@ -812,7 +806,6 @@ type Mutation { setShareHighlight(input: SetShareHighlightInput!): SetShareHighlightResult! setUserPersonalization(input: SetUserPersonalizationInput!): SetUserPersonalizationResult! setWebhook(input: SetWebhookInput!): SetWebhookResult! - signup(input: SignupInput!): SignupResult! subscribe(name: String!): SubscribeResult! unsubscribe(name: String!): UnsubscribeResult! updateHighlight(input: UpdateHighlightInput!): UpdateHighlightResult! @@ -1347,10 +1340,6 @@ type SharedArticleSuccess { article: Article! } -type SignupError { - errorCodes: [SignupErrorCode]! -} - enum SignupErrorCode { ACCESS_DENIED EXPIRED_TOKEN @@ -1362,21 +1351,6 @@ enum SignupErrorCode { USER_EXISTS } -input SignupInput { - bio: String - email: String! - name: String! - password: String! - pictureUrl: String - username: String! -} - -union SignupResult = SignupError | SignupSuccess - -type SignupSuccess { - me: User! -} - enum SortBy { PUBLISHED_AT SAVED_AT diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 6e5b26cbd..4c29d3b3c 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -35,10 +35,11 @@ import cors from 'cors' import { MembershipTier, RegistrationType, + StatusType, UserData, } from '../../datalayer/user/model' import { comparePassword, hashPassword } from '../../utils/auth' -import { createUser } from '../../services/create_user' +import { createUser, sendConfirmationEmail } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' import { initModels } from '../../server' @@ -372,6 +373,17 @@ export function authRouter() { ) } + if (user.status === StatusType.Pending && user.email) { + await sendConfirmationEmail({ + id: user.id, + email: user.email, + name: user.name, + }) + return res.redirect( + `${env.client.url}/email-login?errorCodes=PENDING_VERIFICATION` + ) + } + if (!user?.password) { // user has no password, so they need to set one return res.redirect( diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index b97075b48..fc340c870 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -8,7 +8,7 @@ import { validateUsername } from '../utils/usernamePolicy' import { Invite } from '../entity/groups/invite' import { GroupMembership } from '../entity/groups/group_membership' import { AppDataSource } from '../server' -import { getRepository, setClaims } from '../entity/utils' +import { getRepository } from '../entity/utils' import { generateVerificationToken } from '../utils/auth' import { env } from '../env' import { sendEmail } from '../utils/sendEmail' @@ -95,13 +95,6 @@ export const createUser = async (input: { if (input.pendingConfirmation) { if (!(await sendConfirmationEmail(user))) { - // delete user if email failed to send - await AppDataSource.transaction(async (e) => { - await setClaims(e, user.id) - - return e.getRepository(User).delete(user.id) - }) - return Promise.reject({ errorCode: SignupErrorCode.InvalidEmail }) } } @@ -138,7 +131,11 @@ const getUser = async (email: string): Promise => { }) } -export const sendConfirmationEmail = async (user: User): Promise => { +export const sendConfirmationEmail = async (user: { + id: string + name: string + email: string +}): Promise => { // generate confirmation link const confirmationToken = generateVerificationToken(user.id) const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 9ddede738..a3ea6fa59 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -181,6 +181,38 @@ describe('auth router', () => { }) }) + context('when user is not confirmed', async () => { + const pendingUser = await createTestUser( + 'pending_user', + undefined, + correctPassword, + true + ) + + before(async () => { + email = pendingUser.email + password = correctPassword + }) + + after(async () => { + await deleteTestUser(pendingUser.name) + }) + + it('redirects with error code PendingVerification', async () => { + const res = await loginRequest(email, password).expect(302) + expect(res.header.location).to.endWith( + '/email-login?errorCodes=PENDING_VERIFICATION' + ) + }) + + it('sends a verification email', async () => { + const fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + await loginRequest(email, password).expect(302) + sinon.restore() + expect(fake).to.have.been.calledOnce + }) + }) + context('when user not exists', () => { before(() => { email = 'Some email' @@ -194,15 +226,16 @@ describe('auth router', () => { }) }) - context('when user has no password stored in db', () => { - before(async () => { - const anotherUser = await createTestUser('another_user') - email = anotherUser.email + context('when user has no password stored in db', async () => { + const socialAccountUser = await createTestUser('social_account_user') + + before(() => { + email = socialAccountUser.email password = 'Some password' }) after(async () => { - await deleteTestUser('another_user') + await deleteTestUser(socialAccountUser.name) }) it('redirects with error code WrongSource', async () => { diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 9615250db..1901e41b6 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -17,8 +17,6 @@ import sinonChai from 'sinon-chai' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import { MailDataRequired } from '@sendgrid/helpers/classes/mail' -import { getRepository } from '../../src/entity/utils' -import { User } from '../../src/entity/user' chai.use(sinonChai) @@ -99,10 +97,9 @@ describe('create user', () => { sinon.restore() }) - it('does not create the user', async () => { - await expect(createTestUser(name, undefined, undefined, true)).to.be + it('rejects with error', async () => { + return expect(createTestUser(name, undefined, undefined, true)).to.be .rejected - expect(await getRepository(User).findOneBy({ name })).to.be.null }) }) }) From e4ea49c05f9403c036db6b68246ed61cf65bcef7 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 22:32:09 +0800 Subject: [PATCH 27/41] add confirm-email router --- packages/api/src/routers/auth/auth_router.ts | 58 +++++++++++++++++++- packages/api/src/services/create_user.ts | 2 +- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 4c29d3b3c..b439b2bf2 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -38,10 +38,16 @@ import { StatusType, UserData, } from '../../datalayer/user/model' -import { comparePassword, hashPassword } from '../../utils/auth' +import { + comparePassword, + getClaimsByToken, + hashPassword, +} from '../../utils/auth' import { createUser, sendConfirmationEmail } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' import { initModels } from '../../server' +import { getRepository } from '../../entity/utils' +import { User } from '../../entity/user' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -445,7 +451,7 @@ export function authRouter() { res.redirect(`${env.client.url}/email-login?message=SIGNUP_SUCCESS`) } catch (e) { - logger.error('email-signup exception:', e) + logger.info('email-signup exception:', e) if (isErrorWithCode(e)) { return res.redirect( `${env.client.url}/email-signup?errorCodes=${e.errorCode}` @@ -456,5 +462,53 @@ export function authRouter() { } ) + router.options( + '/confirm-email', + cors({ ...corsConfig, maxAge: 600 }) + ) + + router.get( + '/confirm-email/:token', + cors(corsConfig), + async (req: express.Request, res: express.Response) => { + const token = req.params.token + + try { + // verify token + const claims = await getClaimsByToken(token) + if (!claims) { + return res.redirect( + `${env.client.url}/confirm-email?errorCodes=INVALID_TOKEN` + ) + } + + const user = await getRepository(User).findOneBy({ id: claims.uid }) + if (!user) { + return res.redirect( + `${env.client.url}/confirm-email?errorCodes=USER_NOT_FOUND` + ) + } + + if (user.status === StatusType.Pending) { + await getRepository(User).update( + { id: user.id }, + { status: StatusType.Active } + ) + } + + res.redirect(`${env.client.url}/email-login?message=EMAIL_VERIFIED`) + } catch (e) { + logger.info('confirm-email exception:', e) + if (e instanceof jwt.TokenExpiredError) { + return res.redirect( + `${env.client.url}/confirm-email?errorCodes=TOKEN_EXPIRED` + ) + } + + res.redirect(`${env.client.url}/confirm-email?errorCodes=INVALID_TOKEN`) + } + } + ) + return router } diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index fc340c870..4e1a93578 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -138,7 +138,7 @@ export const sendConfirmationEmail = async (user: { }): Promise => { // generate confirmation link const confirmationToken = generateVerificationToken(user.id) - const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` + const confirmationLink = `${env.client.url}/api/auth/confirm-email/${confirmationToken}` // send email return sendEmail({ from: `Omnivore <${env.sender.message}>`, From a87f75f319dcc1d6b9b902aca44bbc329678f585 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 23:01:35 +0800 Subject: [PATCH 28/41] add test for confirm-email router --- packages/api/src/utils/auth.ts | 28 +++++---- packages/api/test/routers/auth.test.ts | 78 +++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 44eac2f4e..5dfb0e9b1 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -68,22 +68,28 @@ export const getClaimsByToken = async ( try { jwt.verify(token, env.server.jwtSecret) && (claims = jwt.decode(token) as Claims) - } catch (e) { - if (e instanceof jwt.JsonWebTokenError) { - console.log(`not a jwt token, checking api key`, { token }) - claims = await claimsFromApiKey(token) - } else { - throw e - } - } - return claims + return claims + } catch (e) { + if ( + e instanceof jwt.JsonWebTokenError && + !(e instanceof jwt.TokenExpiredError) + ) { + console.log(`not a jwt token, checking api key`, { token }) + return claimsFromApiKey(token) + } + + throw e + } } -export const generateVerificationToken = (userId: string): string => { +export const generateVerificationToken = ( + userId: string, + expireInDays = 1 +): string => { const iat = Math.floor(Date.now() / 1000) const exp = Math.floor( - new Date(Date.now() + 1000 * 60 * 60 * 24).getTime() / 1000 + new Date(Date.now() + 1000 * 60 * 60 * 24 * expireInDays).getTime() / 1000 ) return jwt.sign({ uid: userId, iat, exp }, env.server.jwtSecret) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index a3ea6fa59..925e9c587 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -1,5 +1,5 @@ import { createTestUser, deleteTestUser } from '../db' -import { request } from '../util' +import { generateFakeUuid, request } from '../util' import { expect } from 'chai' import { StatusType } from '../../src/datalayer/user/model' import { getRepository } from '../../src/entity/utils' @@ -8,7 +8,7 @@ import { MailDataRequired } from '@sendgrid/helpers/classes/mail' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import supertest from 'supertest' -import { hashPassword } from '../../src/utils/auth' +import { generateVerificationToken, hashPassword } from '../../src/utils/auth' describe('auth router', () => { const route = '/api/auth' @@ -260,4 +260,78 @@ describe('auth router', () => { }) }) }) + + describe('confirm-email', () => { + const confirmEmailRequest = (token: string): supertest.Test => { + return request.get(`${route}/confirm-email/${token}`).send() + } + + let user: User + let token: string + + before(async () => { + user = await createTestUser('pendingUser', undefined, 'password', true) + }) + + after(async () => { + await deleteTestUser(user.name) + }) + + context('when token is valid', () => { + before(() => { + token = generateVerificationToken(user.id) + }) + + it('redirects to email-login page', async () => { + const res = await confirmEmailRequest(token).expect(302) + expect(res.header.location).to.endWith( + '/email-login?message=EMAIL_VERIFIED' + ) + }) + + it('sets user as active', async () => { + await confirmEmailRequest(token).expect(302) + const updatedUser = await getRepository(User).findOneBy({ + name: user.name, + }) + expect(updatedUser?.status).to.eql(StatusType.Active) + }) + }) + + context('when token is invalid', () => { + it('redirects to confirm-email with error code InvalidToken', async () => { + const res = await confirmEmailRequest('invalid_token').expect(302) + expect(res.header.location).to.endWith( + '/confirm-email?errorCodes=INVALID_TOKEN' + ) + }) + }) + + context('when token is expired', () => { + before(() => { + token = generateVerificationToken(user.id, -1) + }) + + it('redirects to confirm-email page with error code TokenExpired', async () => { + const res = await confirmEmailRequest(token).expect(302) + expect(res.header.location).to.endWith( + '/confirm-email?errorCodes=TOKEN_EXPIRED' + ) + }) + }) + + context('when user is not found', () => { + before(() => { + const nonExistsUserId = generateFakeUuid() + token = generateVerificationToken(nonExistsUserId) + }) + + it('redirects to confirm-email page with error code UserNotFound', async () => { + const res = await confirmEmailRequest(token).expect(302) + expect(res.header.location).to.endWith( + '/confirm-email?errorCodes=USER_NOT_FOUND' + ) + }) + }) + }) }) From 4c75260229ed0ea9dbbc490efff2f62ef9db1f40 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 21 Jul 2022 23:05:10 +0800 Subject: [PATCH 29/41] replace hard-coded email address with env var for the remaining files --- .../api/src/events/reports/content_display_report_created.ts | 2 +- packages/api/src/routers/svc/reminders.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/events/reports/content_display_report_created.ts b/packages/api/src/events/reports/content_display_report_created.ts index 7a8dbb5b1..d8d1ab44e 100644 --- a/packages/api/src/events/reports/content_display_report_created.ts +++ b/packages/api/src/events/reports/content_display_report_created.ts @@ -31,7 +31,7 @@ export class ContentDisplayReportSubscriber to: env.sender.feedback, subject: 'New content display report', text: message, - from: 'msgs@omnivore.app', + from: env.sender.message, }) } } diff --git a/packages/api/src/routers/svc/reminders.ts b/packages/api/src/routers/svc/reminders.ts index 76825d175..445f6ea92 100644 --- a/packages/api/src/routers/svc/reminders.ts +++ b/packages/api/src/routers/svc/reminders.ts @@ -102,7 +102,7 @@ export function remindersServiceRouter() { console.log('dynamic template data:', dynamicTemplateData) await sendEmail({ - from: 'msgs@omnivore.app', + from: env.sender.message, dynamicTemplateData: dynamicTemplateData, templateId: process.env.SENDGRID_REMINDER_TEMPLATE_ID, to: user.email, From ea9d98aa951f5862671460feadb1ed4325401f9a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 00:00:02 +0800 Subject: [PATCH 30/41] make confirm-email a post api --- packages/api/src/routers/auth/auth_router.ts | 6 +++--- packages/api/src/services/create_user.ts | 4 ++-- packages/api/test/routers/auth.test.ts | 4 +++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index b439b2bf2..109f202dd 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -467,11 +467,11 @@ export function authRouter() { cors({ ...corsConfig, maxAge: 600 }) ) - router.get( - '/confirm-email/:token', + router.post( + '/confirm-email', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const token = req.params.token + const token = req.body.token try { // verify token diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 4e1a93578..5248aee2a 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -138,10 +138,10 @@ export const sendConfirmationEmail = async (user: { }): Promise => { // generate confirmation link const confirmationToken = generateVerificationToken(user.id) - const confirmationLink = `${env.client.url}/api/auth/confirm-email/${confirmationToken}` + const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` // send email return sendEmail({ - from: `Omnivore <${env.sender.message}>`, + from: env.sender.message, to: user.email, subject: 'Confirm your email', text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 925e9c587..159a8cd00 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -263,17 +263,19 @@ describe('auth router', () => { describe('confirm-email', () => { const confirmEmailRequest = (token: string): supertest.Test => { - return request.get(`${route}/confirm-email/${token}`).send() + return request.post(`${route}/confirm-email`).send({ token }) } let user: User let token: string before(async () => { + sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) user = await createTestUser('pendingUser', undefined, 'password', true) }) after(async () => { + sinon.restore() await deleteTestUser(user.name) }) From bab96aaa1eb6dae0eeb4ea38e7d3c8be2c5c9648 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 11:01:06 +0800 Subject: [PATCH 31/41] remove membership from user --- packages/api/src/datalayer/user/model.ts | 8 -------- packages/api/src/entity/user.ts | 9 +-------- packages/api/src/events/user/user_created.ts | 1 - packages/api/src/routers/auth/auth_router.ts | 7 +------ .../src/routers/auth/mobile/account_creation.ts | 4 +--- packages/api/src/services/create_user.ts | 6 +----- packages/api/src/utils/helpers.ts | 14 ++------------ packages/api/test/routers/auth.test.ts | 4 ++-- packages/api/test/services/create_user.test.ts | 2 +- .../0089.do.drop_membership_from_user.sql | 12 ++++++++++++ .../0089.undo.drop_membership_from_user.sql | 14 ++++++++++++++ 11 files changed, 35 insertions(+), 46 deletions(-) create mode 100755 packages/db/migrations/0089.do.drop_membership_from_user.sql create mode 100755 packages/db/migrations/0089.undo.drop_membership_from_user.sql diff --git a/packages/api/src/datalayer/user/model.ts b/packages/api/src/datalayer/user/model.ts index 945d07753..f8b182cf6 100644 --- a/packages/api/src/datalayer/user/model.ts +++ b/packages/api/src/datalayer/user/model.ts @@ -12,7 +12,6 @@ import { exclude, Partialize, PickTuple } from '../../util' // source_user_id | text | | not null | // created_at | timestamp with time zone | | not null | CURRENT_TIMESTAMP // updated_at | timestamp with time zone | | not null | CURRENT_TIMESTAMP -// membership | omnivore.membership_tier | | not null | 'WAIT_LIST'::omnivore.membership_tier // Table "omnivore.user_profile" // Column | Type | Collation | Nullable | Default @@ -30,7 +29,6 @@ export interface UserData { id: string name: string source: string - membership: string email?: string | null phone?: string | null sourceUserId: string @@ -47,11 +45,6 @@ export interface UserData { status?: StatusType } -export enum MembershipTier { - WaitList = 'WAIT_LIST', - Beta = 'BETA', -} - export enum RegistrationType { Google = 'GOOGLE', Apple = 'APPLE', @@ -67,7 +60,6 @@ export const keys = [ 'id', 'name', 'source', - 'membership', 'email', 'phone', 'sourceUserId', diff --git a/packages/api/src/entity/user.ts b/packages/api/src/entity/user.ts index 99a6e1afd..58c4a6975 100644 --- a/packages/api/src/entity/user.ts +++ b/packages/api/src/entity/user.ts @@ -7,11 +7,7 @@ import { PrimaryGeneratedColumn, UpdateDateColumn, } from 'typeorm' -import { - MembershipTier, - RegistrationType, - StatusType, -} from '../datalayer/user/model' +import { RegistrationType, StatusType } from '../datalayer/user/model' import { NewsletterEmail } from './newsletter_email' import { Profile } from './profile' import { Label } from './label' @@ -34,9 +30,6 @@ export class User { @Column('text') sourceUserId!: string - @Column({ type: 'enum', enum: MembershipTier }) - membership!: string - @CreateDateColumn() createdAt!: Date diff --git a/packages/api/src/events/user/user_created.ts b/packages/api/src/events/user/user_created.ts index 71d49db17..2f1460770 100644 --- a/packages/api/src/events/user/user_created.ts +++ b/packages/api/src/events/user/user_created.ts @@ -86,7 +86,6 @@ export class IdentifySegmentUser implements EntitySubscriberInterface { traits: { name: profile.user.name, email: profile.user.email, - plan: profile.user.membership, source: profile.user.source, env: env.server.apiEnv, }, diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 109f202dd..17311a4a5 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -33,7 +33,6 @@ import { corsConfig } from '../../utils/corsConfig' import cors from 'cors' import { - MembershipTier, RegistrationType, StatusType, UserData, @@ -297,7 +296,7 @@ export function authRouter() { res.setHeader('set-cookie', result.headers['set-cookie']) - handleSuccessfulLogin(req, res, user, data.googleLogin.newUser) + await handleSuccessfulLogin(req, res, user, data.googleLogin.newUser) }) async function handleSuccessfulLogin( @@ -333,10 +332,6 @@ export function authRouter() { ) } - if (user.membership === MembershipTier.WaitList) { - return res.redirect(`${env.client.url}/waitlist`) - } - return res.redirect( url.resolve(env.client.url, decodeURIComponent(redirectUri || 'home')) ) diff --git a/packages/api/src/routers/auth/mobile/account_creation.ts b/packages/api/src/routers/auth/mobile/account_creation.ts index 5957dfebf..b19607e1d 100644 --- a/packages/api/src/routers/auth/mobile/account_creation.ts +++ b/packages/api/src/routers/auth/mobile/account_creation.ts @@ -1,11 +1,10 @@ import { JsonResponsePayload, UserProfile } from '../auth_types' import { - decodePendingUserToken, createMobileAuthPayload, + decodePendingUserToken, } from './../jwt_helpers' import { createUser } from '../../../services/create_user' import { SignupErrorCode } from '../../../generated/graphql' -import { MembershipTier } from '../../../datalayer/user/model' export async function createMobileAccountCreationResponse( pendingUserToken?: string, @@ -34,7 +33,6 @@ export async function createMobileAccountCreationResponse( username: userProfile.username, pictureUrl: undefined, bio: userProfile.bio || undefined, - membershipTier: MembershipTier.Beta, }) const mobileAuthPayload = await createMobileAuthPayload(user.id) diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 5248aee2a..2b742e4ac 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -1,5 +1,5 @@ import { AuthProvider } from '../routers/auth/auth_types' -import { MembershipTier, StatusType } from '../datalayer/user/model' +import { StatusType } from '../datalayer/user/model' import { EntityManager } from 'typeorm' import { User } from '../entity/user' import { Profile } from '../entity/profile' @@ -22,7 +22,6 @@ export const createUser = async (input: { pictureUrl?: string bio?: string groups?: [string] - membershipTier?: MembershipTier inviteCode?: string password?: string pendingConfirmation?: boolean @@ -65,9 +64,6 @@ export const createUser = async (input: { } const user = await t.getRepository(User).save({ source: input.provider, - membership: - input.membershipTier || - (hasInvite ? MembershipTier.Beta : MembershipTier.WaitList), name: input.name, email: input.email, sourceUserId: input.sourceUserId, diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts index c4db77d44..70cdb1154 100644 --- a/packages/api/src/utils/helpers.ts +++ b/packages/api/src/utils/helpers.ts @@ -7,11 +7,7 @@ import { ResolverFn, } from '../generated/graphql' import { Claims, WithDataSourcesContext } from '../resolvers/types' -import { - MembershipTier, - RegistrationType, - UserData, -} from '../datalayer/user/model' +import { RegistrationType, UserData } from '../datalayer/user/model' import crypto from 'crypto' import slugify from 'voca/slugify' import { Merge } from '../util' @@ -128,7 +124,6 @@ export const userDataToUser = ( id: string name: string source: RegistrationType - membership: MembershipTier email?: string | null phone?: string | null picture?: string | null @@ -149,11 +144,10 @@ export const userDataToUser = ( ...user, name: user.name, source: user.source as RegistrationType, - membership: user.membership as MembershipTier, createdAt: user.createdAt || new Date(), friendsCount: user.friendsCount || 0, followersCount: user.followersCount || 0, - isFullUser: isFullUser(user.membership as MembershipTier), + isFullUser: true, viewerIsFollowing: user.viewerIsFollowing || user.isFriend || false, picture: user.profile.picture_url, sharedArticles: [], @@ -166,10 +160,6 @@ export const userDataToUser = ( }, }) -export const isFullUser = (membership: MembershipTier): boolean => { - return membership != MembershipTier.WaitList -} - export const generateSlug = (title: string): string => { return slugify(title).substring(0, 64) + '-' + Date.now().toString(16) } diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 159a8cd00..877fdf521 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -169,9 +169,9 @@ describe('auth router', () => { password = correctPassword }) - it('redirects to waitlist page', async () => { + it('redirects to home page', async () => { const res = await loginRequest(email, password).expect(302) - expect(res.header.location).to.endWith('/waitlist') + expect(res.header.location).to.endWith('/home') }) it('set auth token in cookie', async () => { diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 1901e41b6..60abdadd8 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -42,7 +42,7 @@ describe('create user', () => { expect(await getUserFollowing(user)).to.eql([adminUser]) expect(await getUserFollowers(adminUser)).to.eql([user]) expect(await getUserFollowing(adminUser)).to.eql([user]) - }).timeout(10000) + }) it('creates profile when user exists but profile not', async () => { after(async () => { diff --git a/packages/db/migrations/0089.do.drop_membership_from_user.sql b/packages/db/migrations/0089.do.drop_membership_from_user.sql new file mode 100755 index 000000000..4ffa64dfd --- /dev/null +++ b/packages/db/migrations/0089.do.drop_membership_from_user.sql @@ -0,0 +1,12 @@ +-- Type: DO +-- Name: drop_membership_from_user +-- Description: drop membership column from user table + +BEGIN; + +ALTER TABLE omnivore.user + DROP column membership; + +DROP TYPE omnivore.membership_tier; + +COMMIT; diff --git a/packages/db/migrations/0089.undo.drop_membership_from_user.sql b/packages/db/migrations/0089.undo.drop_membership_from_user.sql new file mode 100755 index 000000000..db8cc2186 --- /dev/null +++ b/packages/db/migrations/0089.undo.drop_membership_from_user.sql @@ -0,0 +1,14 @@ +-- Type: UNDO +-- Name: drop_membership_from_user +-- Description: drop membership column from user table + +BEGIN; + +CREATE TYPE omnivore.membership_tier AS ENUM ('WAIT_LIST', 'BETA'); + +ALTER TABLE omnivore.user + ADD column membership omnivore.membership_tier NOT NULL DEFAULT 'WAIT_LIST'; + +UPDATE omnivore.user SET membership = 'BETA'; + +COMMIT; From 53fd7f52df31f30aa923247b32cc1b13c999ab76 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 11:10:45 +0800 Subject: [PATCH 32/41] delete user after testing --- packages/api/test/services/create_user.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 60abdadd8..029d6027e 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -93,8 +93,9 @@ describe('create user', () => { fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) }) - after(() => { + after(async () => { sinon.restore() + await deleteTestUser(name) }) it('rejects with error', async () => { From 286d167769176d5f423cac1c91ae65f6725fd400 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 15:37:42 +0800 Subject: [PATCH 33/41] add router handler for email reset password --- packages/api/src/routers/auth/auth_router.ts | 71 ++++++++- packages/api/src/services/create_user.ts | 17 +++ packages/api/test/routers/auth.test.ts | 150 ++++++++++++++++--- 3 files changed, 218 insertions(+), 20 deletions(-) diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 17311a4a5..03ebddc54 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -42,7 +42,11 @@ import { getClaimsByToken, hashPassword, } from '../../utils/auth' -import { createUser, sendConfirmationEmail } from '../../services/create_user' +import { + createUser, + sendConfirmationEmail, + sendPasswordResetEmail, +} from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' import { initModels } from '../../server' import { getRepository } from '../../entity/utils' @@ -302,7 +306,7 @@ export function authRouter() { async function handleSuccessfulLogin( req: express.Request, res: express.Response, - user: UserData, + user: UserData | User, newUser: boolean ): Promise { try { @@ -363,6 +367,12 @@ export function authRouter() { async (req: express.Request, res: express.Response) => { const { email, password } = req.body + if (!email || !password) { + return res.redirect( + `${env.client.url}/email-login?errorCodes=${LoginErrorCode.InvalidCredentials}` + ) + } + try { const models = initModels(kx, false) const user = await models.user.getWhere({ @@ -426,6 +436,12 @@ export function authRouter() { cors(corsConfig), async (req: express.Request, res: express.Response) => { const { email, password, name, username, bio, pictureUrl } = req.body + + if (!email || !password || !name || !username) { + return res.redirect( + `${env.client.url}/email-signup?errorCodes=INVALID_CREDENTIALS` + ) + } const lowerCasedUsername = username.toLowerCase() try { @@ -491,7 +507,7 @@ export function authRouter() { ) } - res.redirect(`${env.client.url}/email-login?message=EMAIL_VERIFIED`) + await handleSuccessfulLogin(req, res, user, false) } catch (e) { logger.info('confirm-email exception:', e) if (e instanceof jwt.TokenExpiredError) { @@ -505,5 +521,54 @@ export function authRouter() { } ) + router.options( + '/email-reset-password', + cors({ ...corsConfig, maxAge: 600 }) + ) + + router.post( + '/email-reset-password', + cors(corsConfig), + async (req: express.Request, res: express.Response) => { + const email = req.body.email + if (!email) { + return res.redirect( + `${env.client.url}/email-reset-password?errorCodes=INVALID_EMAIL` + ) + } + + try { + const user = await getRepository(User).findOneBy({ + email, + }) + if (!user) { + return res.redirect( + `${env.client.url}/email-reset-password?errorCodes=USER_NOT_FOUND` + ) + } + + if (user.status === StatusType.Pending) { + return res.redirect( + `${env.client.url}/email-login?errorCodes=PENDING_VERIFICATION` + ) + } + + if (!(await sendPasswordResetEmail(user))) { + return res.redirect( + `${env.client.url}/email-reset-password?errorCodes=INVALID_EMAIL` + ) + } + + res.redirect(`${env.client.url}/email-reset-password?message=SUCCESS`) + } catch (e) { + logger.info('email-reset-password exception:', e) + + res.redirect( + `${env.client.url}/email-reset-password?errorCodes=UNKNOWN` + ) + } + } + ) + return router } diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 2b742e4ac..39cea746a 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -143,3 +143,20 @@ export const sendConfirmationEmail = async (user: { text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, }) } + +export const sendPasswordResetEmail = async (user: { + id: string + name: string + email: string +}): Promise => { + // generate link + const token = generateVerificationToken(user.id) + const link = `${env.client.url}/reset-password/${token}` + // send email + return sendEmail({ + from: env.sender.message, + to: user.email, + subject: 'Reset your password', + text: `Hey ${user.name},\n\nPlease reset your password by clicking the link below:\n\n${link}\n\n`, + }) +} diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 877fdf521..e3457271c 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -12,21 +12,21 @@ import { generateVerificationToken, hashPassword } from '../../src/utils/auth' describe('auth router', () => { const route = '/api/auth' - const signupRequest = ( - email: string, - password: string, - name: string, - username: string - ): supertest.Test => { - return request.post(`${route}/email-signup`).send({ - email, - password, - name, - username, - }) - } describe('email signup', () => { + const signupRequest = ( + email: string, + password: string, + name: string, + username: string + ): supertest.Test => { + return request.post(`${route}/email-signup`).send({ + email, + password, + name, + username, + }) + } const validPassword = 'validPassword' let email: string @@ -284,11 +284,9 @@ describe('auth router', () => { token = generateVerificationToken(user.id) }) - it('redirects to email-login page', async () => { + it('logs in and redirects to home page', async () => { const res = await confirmEmailRequest(token).expect(302) - expect(res.header.location).to.endWith( - '/email-login?message=EMAIL_VERIFIED' - ) + expect(res.header.location).to.endWith('/home') }) it('sets user as active', async () => { @@ -336,4 +334,122 @@ describe('auth router', () => { }) }) }) + + describe('email-reset-password', () => { + const emailResetPasswordReq = (email: string): supertest.Test => { + return request.post(`${route}/email-reset-password`).send({ + email, + }) + } + + let email: string + + context('when email is not empty', () => { + before(() => { + email = `some_email@domain.app` + }) + + context('when user exists', () => { + let user: User + + before(async () => { + user = await createTestUser('test_user') + email = user.email + }) + + after(async () => { + await deleteTestUser(user.name) + }) + + context('when email is verified', () => { + let fake: (msg: MailDataRequired) => Promise + + before(async () => { + await getRepository(User).update(user.id, { + status: StatusType.Active, + }) + }) + + context('when reset password email sent', () => { + before(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + after(() => { + sinon.restore() + }) + + it('redirects to email-reset-password page with success message', async () => { + const res = await emailResetPasswordReq(email).expect(302) + expect(res.header.location).to.endWith( + '/email-reset-password?message=SUCCESS' + ) + }) + }) + + context('when reset password email not sent', () => { + before(() => { + fake = sinon.replace( + util, + 'sendEmail', + sinon.fake.resolves(false) + ) + }) + + after(() => { + sinon.restore() + }) + + it('redirects to sign up page with error code INVALID_EMAIL', async () => { + const res = await emailResetPasswordReq(email).expect(302) + expect(res.header.location).to.endWith( + '/email-reset-password?errorCodes=INVALID_EMAIL' + ) + }) + }) + }) + + context('when email is not verified', () => { + before(async () => { + await getRepository(User).update(user.id, { + status: StatusType.Pending, + }) + }) + + it('redirects to email-login page with error code PENDING_VERIFICATION', async () => { + const res = await emailResetPasswordReq(email).expect(302) + expect(res.header.location).to.endWith( + '/email-login?errorCodes=PENDING_VERIFICATION' + ) + }) + }) + }) + + context('when user does not exist', () => { + before(() => { + email = 'non_exists_email@domain.app' + }) + + it('redirects to email-reset-password page with error code USER_NOT_FOUND', async () => { + const res = await emailResetPasswordReq(email).expect(302) + expect(res.header.location).to.endWith( + '/email-reset-password?errorCodes=USER_NOT_FOUND' + ) + }) + }) + }) + + context('when email is empty', () => { + before(() => { + email = '' + }) + + it('redirects to email-reset-password page with error code INVALID_EMAIL', async () => { + const res = await emailResetPasswordReq(email).expect(302) + expect(res.header.location).to.endWith( + '/email-reset-password?errorCodes=INVALID_EMAIL' + ) + }) + }) + }) }) From 6699ec834d8f29757c002cce631f1204c190f459 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 16:38:21 +0800 Subject: [PATCH 34/41] add router handler for reset password --- packages/api/src/apollo.ts | 11 +- packages/api/src/routers/auth/auth_router.ts | 104 +++++++++++++++---- packages/api/src/services/create_user.ts | 38 +------ packages/api/src/services/send_emails.ts | 37 +++++++ packages/api/src/utils/auth.ts | 20 +++- packages/api/test/routers/auth.test.ts | 18 ++-- 6 files changed, 152 insertions(+), 76 deletions(-) create mode 100644 packages/api/src/services/send_emails.ts diff --git a/packages/api/src/apollo.ts b/packages/api/src/apollo.ts index c55ee5ecf..075245092 100644 --- a/packages/api/src/apollo.ts +++ b/packages/api/src/apollo.ts @@ -24,7 +24,7 @@ import ScalarResolvers from './scalars' import * as Sentry from '@sentry/node' import { createPubSubClient } from './datalayer/pubsub' import { initModels } from './server' -import { getClaimsByToken } from './utils/auth' +import { getClaimsByToken, setAuthInCookie } from './utils/auth' const signToken = promisify(jwt.sign) const logger = buildLogger('app.dispatch') @@ -76,14 +76,7 @@ const contextFunc: ContextFunction = async ({ setAuth: async ( claims: ClaimsToSet, secret: string = env.server.jwtSecret - ) => { - const token = await signToken(claims, secret) - - res.cookie('auth', token, { - httpOnly: true, - expires: new Date(new Date().getTime() + 365 * 24 * 60 * 60 * 1000), - }) - }, + ) => await setAuthInCookie(claims, res, secret), setClaims, authTrx: ( cb: (tx: Knex.Transaction) => TResult, diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 03ebddc54..32fe75122 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -41,16 +41,17 @@ import { comparePassword, getClaimsByToken, hashPassword, + setAuthInCookie, } from '../../utils/auth' -import { - createUser, - sendConfirmationEmail, - sendPasswordResetEmail, -} from '../../services/create_user' +import { createUser } from '../../services/create_user' import { isErrorWithCode } from '../../resolvers' import { initModels } from '../../server' import { getRepository } from '../../entity/utils' import { User } from '../../entity/user' +import { + sendConfirmationEmail, + sendPasswordResetEmail, +} from '../../services/send_emails' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -325,6 +326,13 @@ export function authRouter() { } } + const message = res.get('Message') + if (message) { + return res.redirect( + `${env.client.url}/home?message=${encodeURIComponent(message)}` + ) + } + if (newUser) { if (redirectUri && redirectUri !== '/') { return res.redirect( @@ -411,13 +419,7 @@ export function authRouter() { } // set auth cookie in response header - const token = await signToken({ uid: user.id }, env.server.jwtSecret) - - res.cookie('auth', token, { - httpOnly: true, - expires: new Date(new Date().getTime() + 365 * 24 * 60 * 60 * 1000), - }) - + await setAuthInCookie({ uid: user.id }, res) await handleSuccessfulLogin(req, res, user, false) } catch (e) { logger.info('email-login exception:', e) @@ -507,6 +509,8 @@ export function authRouter() { ) } + res.set('Message', 'CONFIRMATION_SUCCESS') + await setAuthInCookie({ uid: user.id }, res) await handleSuccessfulLogin(req, res, user, false) } catch (e) { logger.info('confirm-email exception:', e) @@ -522,18 +526,18 @@ export function authRouter() { ) router.options( - '/email-reset-password', + '/forgot-password', cors({ ...corsConfig, maxAge: 600 }) ) router.post( - '/email-reset-password', + '/forgot-password', cors(corsConfig), async (req: express.Request, res: express.Response) => { const email = req.body.email if (!email) { return res.redirect( - `${env.client.url}/email-reset-password?errorCodes=INVALID_EMAIL` + `${env.client.url}/forgot-password?errorCodes=INVALID_EMAIL` ) } @@ -543,7 +547,7 @@ export function authRouter() { }) if (!user) { return res.redirect( - `${env.client.url}/email-reset-password?errorCodes=USER_NOT_FOUND` + `${env.client.url}/forgot-password?errorCodes=USER_NOT_FOUND` ) } @@ -555,16 +559,76 @@ export function authRouter() { if (!(await sendPasswordResetEmail(user))) { return res.redirect( - `${env.client.url}/email-reset-password?errorCodes=INVALID_EMAIL` + `${env.client.url}/forgot-password?errorCodes=INVALID_EMAIL` ) } - res.redirect(`${env.client.url}/email-reset-password?message=SUCCESS`) + res.redirect(`${env.client.url}/forgot-password?message=SUCCESS`) } catch (e) { - logger.info('email-reset-password exception:', e) + logger.info('forgot-password exception:', e) + + res.redirect(`${env.client.url}/forgot-password?errorCodes=UNKNOWN`) + } + } + ) + + router.options( + '/reset-password', + cors({ ...corsConfig, maxAge: 600 }) + ) + + router.post( + '/reset-password', + cors(corsConfig), + async (req: express.Request, res: express.Response) => { + const { token, password } = req.body + if (!token || !password) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=INVALID_CREDENTIALS` + ) + } + + try { + // verify token + const claims = await getClaimsByToken(token) + if (!claims) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=INVALID_TOKEN` + ) + } + + const user = await getRepository(User).findOneBy({ id: claims.uid }) + if (!user) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=USER_NOT_FOUND` + ) + } + + if (user.status === StatusType.Pending) { + return res.redirect( + `${env.client.url}/email-login?errorCodes=PENDING_VERIFICATION` + ) + } + + const hashedPassword = await hashPassword(password) + await getRepository(User).update( + { id: user.id }, + { password: hashedPassword } + ) + + res.set('Message', 'PASSWORD_RESET_SUCCESS') + await setAuthInCookie({ uid: user.id }, res) + await handleSuccessfulLogin(req, res, user, false) + } catch (e) { + logger.info('reset-password exception:', e) + if (e instanceof jwt.TokenExpiredError) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=TOKEN_EXPIRED` + ) + } res.redirect( - `${env.client.url}/email-reset-password?errorCodes=UNKNOWN` + `${env.client.url}/reset-password?errorCodes=INVALID_TOKEN` ) } } diff --git a/packages/api/src/services/create_user.ts b/packages/api/src/services/create_user.ts index 39cea746a..2d6267b6c 100644 --- a/packages/api/src/services/create_user.ts +++ b/packages/api/src/services/create_user.ts @@ -9,9 +9,7 @@ import { Invite } from '../entity/groups/invite' import { GroupMembership } from '../entity/groups/group_membership' import { AppDataSource } from '../server' import { getRepository } from '../entity/utils' -import { generateVerificationToken } from '../utils/auth' -import { env } from '../env' -import { sendEmail } from '../utils/sendEmail' +import { sendConfirmationEmail } from './send_emails' export const createUser = async (input: { provider: AuthProvider @@ -126,37 +124,3 @@ const getUser = async (email: string): Promise => { relations: ['profile'], }) } - -export const sendConfirmationEmail = async (user: { - id: string - name: string - email: string -}): Promise => { - // generate confirmation link - const confirmationToken = generateVerificationToken(user.id) - const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` - // send email - return sendEmail({ - from: env.sender.message, - to: user.email, - subject: 'Confirm your email', - text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, - }) -} - -export const sendPasswordResetEmail = async (user: { - id: string - name: string - email: string -}): Promise => { - // generate link - const token = generateVerificationToken(user.id) - const link = `${env.client.url}/reset-password/${token}` - // send email - return sendEmail({ - from: env.sender.message, - to: user.email, - subject: 'Reset your password', - text: `Hey ${user.name},\n\nPlease reset your password by clicking the link below:\n\n${link}\n\n`, - }) -} diff --git a/packages/api/src/services/send_emails.ts b/packages/api/src/services/send_emails.ts new file mode 100644 index 000000000..e578d4cd7 --- /dev/null +++ b/packages/api/src/services/send_emails.ts @@ -0,0 +1,37 @@ +import { generateVerificationToken } from '../utils/auth' +import { env } from '../env' +import { sendEmail } from '../utils/sendEmail' + +export const sendConfirmationEmail = async (user: { + id: string + name: string + email: string +}): Promise => { + // generate confirmation link + const confirmationToken = generateVerificationToken(user.id) + const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` + // send email + return sendEmail({ + from: env.sender.message, + to: user.email, + subject: 'Confirm your email', + text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, + }) +} + +export const sendPasswordResetEmail = async (user: { + id: string + name: string + email: string +}): Promise => { + // generate link + const token = generateVerificationToken(user.id) + const link = `${env.client.url}/reset-password/${token}` + // send email + return sendEmail({ + from: env.sender.message, + to: user.email, + subject: 'Reset your password', + text: `Hey ${user.name},\n\nPlease reset your password by clicking the link below:\n\n${link}\n\n`, + }) +} diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 5dfb0e9b1..8885d279e 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -1,11 +1,15 @@ import * as bcrypt from 'bcryptjs' import { v4 as uuidv4 } from 'uuid' -import { Claims } from '../resolvers/types' +import { Claims, ClaimsToSet } from '../resolvers/types' import { getRepository } from '../entity/utils' import { ApiKey } from '../entity/api_key' import crypto from 'crypto' import * as jwt from 'jsonwebtoken' import { env } from '../env' +import express from 'express' +import { promisify } from 'util' + +const signToken = promisify(jwt.sign) export const hashPassword = async (password: string, salt = 10) => { return bcrypt.hash(password, salt) @@ -94,3 +98,17 @@ export const generateVerificationToken = ( return jwt.sign({ uid: userId, iat, exp }, env.server.jwtSecret) } + +export const setAuthInCookie = async ( + claims: ClaimsToSet, + res: express.Response, + secret: string = env.server.jwtSecret +) => { + // set auth cookie in response header + const token = await signToken(claims, secret) + + res.cookie('auth', token, { + httpOnly: true, + expires: new Date(new Date().getTime() + 365 * 24 * 60 * 60 * 1000), + }) +} diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index e3457271c..54fd2a59f 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -335,9 +335,9 @@ describe('auth router', () => { }) }) - describe('email-reset-password', () => { + describe('forgot-password', () => { const emailResetPasswordReq = (email: string): supertest.Test => { - return request.post(`${route}/email-reset-password`).send({ + return request.post(`${route}/forgot-password`).send({ email, }) } @@ -379,10 +379,10 @@ describe('auth router', () => { sinon.restore() }) - it('redirects to email-reset-password page with success message', async () => { + it('redirects to forgot-password page with success message', async () => { const res = await emailResetPasswordReq(email).expect(302) expect(res.header.location).to.endWith( - '/email-reset-password?message=SUCCESS' + '/forgot-password?message=SUCCESS' ) }) }) @@ -403,7 +403,7 @@ describe('auth router', () => { it('redirects to sign up page with error code INVALID_EMAIL', async () => { const res = await emailResetPasswordReq(email).expect(302) expect(res.header.location).to.endWith( - '/email-reset-password?errorCodes=INVALID_EMAIL' + '/forgot-password?errorCodes=INVALID_EMAIL' ) }) }) @@ -430,10 +430,10 @@ describe('auth router', () => { email = 'non_exists_email@domain.app' }) - it('redirects to email-reset-password page with error code USER_NOT_FOUND', async () => { + it('redirects to forgot-password page with error code USER_NOT_FOUND', async () => { const res = await emailResetPasswordReq(email).expect(302) expect(res.header.location).to.endWith( - '/email-reset-password?errorCodes=USER_NOT_FOUND' + '/forgot-password?errorCodes=USER_NOT_FOUND' ) }) }) @@ -444,10 +444,10 @@ describe('auth router', () => { email = '' }) - it('redirects to email-reset-password page with error code INVALID_EMAIL', async () => { + it('redirects to forgot-password page with error code INVALID_EMAIL', async () => { const res = await emailResetPasswordReq(email).expect(302) expect(res.header.location).to.endWith( - '/email-reset-password?errorCodes=INVALID_EMAIL' + '/forgot-password?errorCodes=INVALID_EMAIL' ) }) }) From bd77a7f8eee5d13b2b49e0088969c19c6f2bcc3e Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 17:41:43 +0800 Subject: [PATCH 35/41] add test for reset password --- packages/api/.nycrc | 2 +- packages/api/src/routers/auth/auth_router.ts | 24 +++-- packages/api/test/routers/auth.test.ts | 102 ++++++++++++++++++- 3 files changed, 114 insertions(+), 14 deletions(-) diff --git a/packages/api/.nycrc b/packages/api/.nycrc index da90d3922..4a0a0dfb4 100644 --- a/packages/api/.nycrc +++ b/packages/api/.nycrc @@ -8,7 +8,7 @@ "reporter": [ "text-summary" ], - "branches": 0, + "branches": 40, "lines": 0, "functions": 0, "statements": 60 diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 32fe75122..c6e7ce3d3 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -509,7 +509,7 @@ export function authRouter() { ) } - res.set('Message', 'CONFIRMATION_SUCCESS') + res.set('Message', 'EMAIL_CONFIRMED') await setAuthInCookie({ uid: user.id }, res) await handleSuccessfulLogin(req, res, user, false) } catch (e) { @@ -582,11 +582,6 @@ export function authRouter() { cors(corsConfig), async (req: express.Request, res: express.Response) => { const { token, password } = req.body - if (!token || !password) { - return res.redirect( - `${env.client.url}/reset-password?errorCodes=INVALID_CREDENTIALS` - ) - } try { // verify token @@ -597,6 +592,12 @@ export function authRouter() { ) } + if (!password) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=INVALID_PASSWORD` + ) + } + const user = await getRepository(User).findOneBy({ id: claims.uid }) if (!user) { return res.redirect( @@ -611,14 +612,17 @@ export function authRouter() { } const hashedPassword = await hashPassword(password) - await getRepository(User).update( + const updated = await getRepository(User).update( { id: user.id }, { password: hashedPassword } ) + if (!updated.affected) { + return res.redirect( + `${env.client.url}/reset-password?errorCodes=UNKNOWN` + ) + } - res.set('Message', 'PASSWORD_RESET_SUCCESS') - await setAuthInCookie({ uid: user.id }, res) - await handleSuccessfulLogin(req, res, user, false) + res.redirect(`${env.client.url}/reset-password?message=SUCCESS`) } catch (e) { logger.info('reset-password exception:', e) if (e instanceof jwt.TokenExpiredError) { diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 54fd2a59f..106eb2f68 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -8,7 +8,11 @@ import { MailDataRequired } from '@sendgrid/helpers/classes/mail' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import supertest from 'supertest' -import { generateVerificationToken, hashPassword } from '../../src/utils/auth' +import { + comparePassword, + generateVerificationToken, + hashPassword, +} from '../../src/utils/auth' describe('auth router', () => { const route = '/api/auth' @@ -284,9 +288,15 @@ describe('auth router', () => { token = generateVerificationToken(user.id) }) - it('logs in and redirects to home page', async () => { + it('set auth token in cookie', async () => { const res = await confirmEmailRequest(token).expect(302) - expect(res.header.location).to.endWith('/home') + expect(res.header['set-cookie']).to.be.an('array') + expect(res.header['set-cookie'][0]).to.contain('auth') + }) + + it('redirects to home page', async () => { + const res = await confirmEmailRequest(token).expect(302) + expect(res.header.location).to.endWith('/home?message=EMAIL_CONFIRMED') }) it('sets user as active', async () => { @@ -452,4 +462,90 @@ describe('auth router', () => { }) }) }) + + describe('reset-password', () => { + const resetPasswordRequest = ( + token: string, + password: string + ): supertest.Test => { + return request.post(`${route}/reset-password`).send({ + token, + password, + }) + } + + let user: User + let token: string + + before(async () => { + user = await createTestUser('test_user', undefined, 'test_password') + }) + + after(async () => { + await deleteTestUser(user.name) + }) + + context('when token is valid', () => { + before(async () => { + token = generateVerificationToken(user.id) + }) + + context('when password is not empty', () => { + it('redirects to reset-password page with success message', async () => { + const res = await resetPasswordRequest(token, 'new_password').expect( + 302 + ) + expect(res.header.location).to.endWith( + '/reset-password?message=SUCCESS' + ) + }) + + it('resets password', async () => { + const password = 'test_reset_password' + await resetPasswordRequest(token, password).expect(302) + const updatedUser = await getRepository(User).findOneBy({ + id: user?.id, + }) + expect(await comparePassword(password, updatedUser?.password!)).to.be + .true + }) + }) + + context('when password is empty', () => { + it('redirects to reset-password page with error code INVALID_PASSWORD', async () => { + const res = await resetPasswordRequest(token, '').expect(302) + expect(res.header.location).to.endWith( + '/reset-password?errorCodes=INVALID_PASSWORD' + ) + }) + }) + }) + + context('when token is invalid', () => { + it('redirects to reset-password page with error code InvalidToken', async () => { + const res = await resetPasswordRequest( + 'invalid_token', + 'new_password' + ).expect(302) + expect(res.header.location).to.endWith( + '/reset-password?errorCodes=INVALID_TOKEN' + ) + }) + + context('when token is expired', () => { + before(() => { + token = generateVerificationToken(user.id, -1) + }) + + it('redirects to reset-password page with error code ExpiredToken', async () => { + const res = await resetPasswordRequest(token, 'new_password').expect( + 302 + ) + expect(res.header.location).to.endWith( + '/reset-password?errorCodes=TOKEN_EXPIRED' + ) + }) + }) + }) + }) }) From 024d4bd25cc59c6f0ac76116132f33f477685220 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 18:32:13 +0800 Subject: [PATCH 36/41] stop referring to user entity before migration --- packages/api/test/routers/auth.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index 106eb2f68..ad12a3a35 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -186,14 +186,15 @@ describe('auth router', () => { }) context('when user is not confirmed', async () => { - const pendingUser = await createTestUser( - 'pending_user', - undefined, - correctPassword, - true - ) + let pendingUser: User before(async () => { + pendingUser = await createTestUser( + 'pending_user', + undefined, + correctPassword, + true + ) email = pendingUser.email password = correctPassword }) @@ -231,9 +232,10 @@ describe('auth router', () => { }) context('when user has no password stored in db', async () => { - const socialAccountUser = await createTestUser('social_account_user') + let socialAccountUser: User - before(() => { + before(async () => { + socialAccountUser = await createTestUser('social_account_user') email = socialAccountUser.email password = 'Some password' }) From 43d221698356675f32bf11d5ee539342aff15480 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 19:20:14 +0800 Subject: [PATCH 37/41] fix test --- packages/api/test/routers/auth.test.ts | 31 +++++++++++++------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index ad12a3a35..f77100852 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -186,21 +186,18 @@ describe('auth router', () => { }) context('when user is not confirmed', async () => { - let pendingUser: User - before(async () => { - pendingUser = await createTestUser( - 'pending_user', - undefined, - correctPassword, - true - ) - email = pendingUser.email + await getRepository(User).update(user.id, { + status: StatusType.Pending, + }) + email = user.email password = correctPassword }) after(async () => { - await deleteTestUser(pendingUser.name) + await getRepository(User).update(user.id, { + status: StatusType.Active, + }) }) it('redirects with error code PendingVerification', async () => { @@ -232,16 +229,18 @@ describe('auth router', () => { }) context('when user has no password stored in db', async () => { - let socialAccountUser: User - before(async () => { - socialAccountUser = await createTestUser('social_account_user') - email = socialAccountUser.email - password = 'Some password' + await getRepository(User).update(user.id, { + password: '', + }) + email = user.email + password = user.password! }) after(async () => { - await deleteTestUser(socialAccountUser.name) + await getRepository(User).update(user.id, { + password, + }) }) it('redirects with error code WrongSource', async () => { From 560f95b57ec8404f0201af7dd4360a185035a088 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 22 Jul 2022 19:44:00 +0800 Subject: [PATCH 38/41] fix test --- packages/api/test/routers/auth.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index f77100852..526ccce02 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -186,7 +186,10 @@ describe('auth router', () => { }) context('when user is not confirmed', async () => { - before(async () => { + let fake: (msg: MailDataRequired) => Promise + + beforeEach(async () => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) await getRepository(User).update(user.id, { status: StatusType.Pending, }) @@ -194,10 +197,11 @@ describe('auth router', () => { password = correctPassword }) - after(async () => { + afterEach(async () => { await getRepository(User).update(user.id, { status: StatusType.Active, }) + sinon.restore() }) it('redirects with error code PendingVerification', async () => { @@ -208,9 +212,7 @@ describe('auth router', () => { }) it('sends a verification email', async () => { - const fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) await loginRequest(email, password).expect(302) - sinon.restore() expect(fake).to.have.been.calledOnce }) }) From fe180c5b50c8d543b05bf7062a526db99c320524 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 25 Jul 2022 18:43:13 +0800 Subject: [PATCH 39/41] add confirm-email and password-reset email template id in env --- packages/api/src/util.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/api/src/util.ts b/packages/api/src/util.ts index ae9fe5051..d1c6d9fd4 100755 --- a/packages/api/src/util.ts +++ b/packages/api/src/util.ts @@ -78,6 +78,12 @@ interface BackendEnv { feedback: string general: string } + sendgrid: { + confirmationTemplateId: string + reminderTemplateId: string + resetPasswordTemplateId: string + installationTemplateId: string + } } /*** @@ -122,6 +128,10 @@ const nullableEnvVars = [ 'SENDER_MESSAGE', 'SENDER_FEEDBACK', 'SENDER_GENERAL', + 'SENDGRID_CONFIRMATION_TEMPLATE_ID', + 'SENDGRID_REMINDER_TEMPLATE_ID', + 'SENDGRID_RESET_PASSWORD_TEMPLATE_ID', + 'SENDGRID_INSTALLATION_TEMPLATE_ID', ] // Allow some vars to be null/empty /* If not in GAE and Prod/QA/Demo env (f.e. on localhost/dev env), allow following env vars to be null */ @@ -228,6 +238,13 @@ export function getEnv(): BackendEnv { general: parse('SENDER_GENERAL'), } + const sendgrid = { + confirmationTemplateId: parse('SENDGRID_CONFIRMATION_TEMPLATE_ID'), + reminderTemplateId: parse('SENDGRID_REMINDER_TEMPLATE_ID'), + resetPasswordTemplateId: parse('SENDGRID_RESET_PASSWORD_TEMPLATE_ID'), + installationTemplateId: parse('SENDGRID_INSTALLATION_TEMPLATE_ID'), + } + return { pg, client, @@ -244,6 +261,7 @@ export function getEnv(): BackendEnv { queue, elastic, sender, + sendgrid, } } From 8d5da15064d27dabac6702b37efe1a1fd2c0b37f Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 25 Jul 2022 18:43:52 +0800 Subject: [PATCH 40/41] use template id stored in env --- packages/api/src/resolvers/send_install_instructions/index.ts | 4 +++- packages/api/src/routers/svc/reminders.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/api/src/resolvers/send_install_instructions/index.ts b/packages/api/src/resolvers/send_install_instructions/index.ts index 4b9b103b8..2f050d4f8 100644 --- a/packages/api/src/resolvers/send_install_instructions/index.ts +++ b/packages/api/src/resolvers/send_install_instructions/index.ts @@ -27,7 +27,9 @@ export const sendInstallInstructionsResolver = authorized< const sendInstallInstructions = await sendEmail({ from: env.sender.message, - templateId: INSTALL_INSTRUCTIONS_EMAIL_TEMPLATE_ID, + templateId: + env.sendgrid.installationTemplateId || + INSTALL_INSTRUCTIONS_EMAIL_TEMPLATE_ID, to: user?.email, }) diff --git a/packages/api/src/routers/svc/reminders.ts b/packages/api/src/routers/svc/reminders.ts index 445f6ea92..9536f28ff 100644 --- a/packages/api/src/routers/svc/reminders.ts +++ b/packages/api/src/routers/svc/reminders.ts @@ -104,7 +104,7 @@ export function remindersServiceRouter() { await sendEmail({ from: env.sender.message, dynamicTemplateData: dynamicTemplateData, - templateId: process.env.SENDGRID_REMINDER_TEMPLATE_ID, + templateId: env.sendgrid.reminderTemplateId, to: user.email, }) From 82da9e6edef6bbf9cda092187643776213943d0a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 25 Jul 2022 18:44:29 +0800 Subject: [PATCH 41/41] use sendgrid template for confirmation and password-reset email --- packages/api/src/services/send_emails.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/api/src/services/send_emails.ts b/packages/api/src/services/send_emails.ts index e578d4cd7..a6fbf6d42 100644 --- a/packages/api/src/services/send_emails.ts +++ b/packages/api/src/services/send_emails.ts @@ -8,14 +8,19 @@ export const sendConfirmationEmail = async (user: { email: string }): Promise => { // generate confirmation link - const confirmationToken = generateVerificationToken(user.id) - const confirmationLink = `${env.client.url}/confirm-email/${confirmationToken}` + const token = generateVerificationToken(user.id) + const link = `${env.client.url}/confirm-email/${token}` // send email + const dynamicTemplateData = { + name: user.name, + link, + } + return sendEmail({ from: env.sender.message, to: user.email, - subject: 'Confirm your email', - text: `Hey ${user.name},\n\nPlease confirm your email by clicking the link below:\n\n${confirmationLink}\n\n`, + templateId: env.sendgrid.confirmationTemplateId, + dynamicTemplateData, }) } @@ -28,10 +33,15 @@ export const sendPasswordResetEmail = async (user: { const token = generateVerificationToken(user.id) const link = `${env.client.url}/reset-password/${token}` // send email + const dynamicTemplateData = { + name: user.name, + link, + } + return sendEmail({ from: env.sender.message, to: user.email, - subject: 'Reset your password', - text: `Hey ${user.name},\n\nPlease reset your password by clicking the link below:\n\n${link}\n\n`, + templateId: env.sendgrid.resetPasswordTemplateId, + dynamicTemplateData, }) }