Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: migrate EmailVerification to pg #9492

Merged
merged 4 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions packages/server/__tests__/autoJoin.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import faker from 'faker'
import getRethink from '../database/rethinkDriver'
import createEmailVerification from '../email/createEmailVerification'
import getKysely from '../postgres/getKysely'
import {getUserTeams, sendIntranet, sendPublic} from './common'

const signUpVerified = async (email: string) => {
Expand All @@ -23,12 +23,14 @@ const signUpVerified = async (email: string) => {
// manually generate verification token so also the founder can be verified
await createEmailVerification({email, password})

const r = await getRethink()
const verificationToken = await r
.table('EmailVerification')
.getAll(email, {index: 'email'})
.nth(0)('token')
.run()
const pg = getKysely()
const verificationToken = (
await pg
.selectFrom('EmailVerification')
.select('token')
.where('email', '=', email)
.executeTakeFirstOrThrow(() => new Error(`No verification token found for ${email}`))
).token

const verifyEmail = await sendPublic({
query: `
Expand All @@ -55,9 +57,9 @@ const signUpVerified = async (email: string) => {
expect(verifyEmail).toMatchObject({
data: {
verifyEmail: {
authToken: expect.toBeString(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting TS failures here

authToken: expect.any(String),
user: {
id: expect.toBeString()
id: expect.any(String)
}
}
}
Expand Down Expand Up @@ -153,7 +155,7 @@ test.skip('autoJoin on multiple teams does not create duplicate `OrganizationUse
const newEmail = `${faker.internet.userName()}@${domain}`.toLowerCase()
const {user: newUser} = await signUpVerified(newEmail)

expect(newUser.tms).toIncludeSameMembers(teamIds)
expect(newUser.tms).toEqual(expect.arrayContaining(teamIds))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here...

expect(newUser.organizations).toMatchObject([
{
id: orgId
Expand Down
32 changes: 0 additions & 32 deletions packages/server/database/types/EmailVerification.ts

This file was deleted.

25 changes: 14 additions & 11 deletions packages/server/email/createEmailVerficationForExistingUser.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import base64url from 'base64url'
import crypto from 'crypto'
import getRethink from '../database/rethinkDriver'
import {Threshold} from 'parabol-client/types/constEnums'
import AuthIdentityLocal from '../database/types/AuthIdentityLocal'
import EmailVerification from '../database/types/EmailVerification'
import {DataLoaderWorker} from '../graphql/graphql'
import getKysely from '../postgres/getKysely'
import emailVerificationEmailCreator from './emailVerificationEmailCreator'
import getMailManager from './getMailManager'

Expand Down Expand Up @@ -34,15 +34,18 @@ const createEmailVerficationForExistingUser = async (
if (!success) {
return new Error('Unable to send verification email')
}
const r = await getRethink()
const emailVerification = new EmailVerification({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken
})
await r.table('EmailVerification').insert(emailVerification).run()
const pg = getKysely()
await pg
.insertInto('EmailVerification')
.values({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken,
expiration: new Date(Date.now() + Threshold.EMAIL_VERIFICATION_LIFESPAN)
})
.execute()
return undefined
}

Expand Down
26 changes: 14 additions & 12 deletions packages/server/email/createEmailVerification.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import base64url from 'base64url'
import bcrypt from 'bcryptjs'
import crypto from 'crypto'
import {Security} from 'parabol-client/types/constEnums'
import getRethink from '../database/rethinkDriver'
import EmailVerification from '../database/types/EmailVerification'
import {Security, Threshold} from 'parabol-client/types/constEnums'
import getKysely from '../postgres/getKysely'
import emailVerificationEmailCreator from './emailVerificationEmailCreator'
import getMailManager from './getMailManager'

Expand Down Expand Up @@ -35,16 +34,19 @@ const createEmailVerification = async (props: SignUpWithPasswordMutationVariable
if (!success) {
return {error: {message: 'Unable to send verification email'}}
}
const r = await getRethink()
const hashedPassword = await bcrypt.hash(password, Security.SALT_ROUNDS)
const emailVerification = new EmailVerification({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken
})
await r.table('EmailVerification').insert(emailVerification).run()
const pg = getKysely()
await pg
.insertInto('EmailVerification')
.values({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken,
expiration: new Date(Date.now() + Threshold.EMAIL_VERIFICATION_LIFESPAN)
})
.execute()
Comment on lines +38 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for database operations.

The insertion operation should handle potential errors to avoid unhandled promise rejections.

-  await pg
+  try {
+    await pg
     .insertInto('EmailVerification')
     .values({
       email,
       token: verifiedEmailToken,
       hashedPassword,
       pseudoId,
       invitationToken,
       expiration: new Date(Date.now() + Threshold.EMAIL_VERIFICATION_LIFESPAN)
     })
     .execute()
+  } catch (error) {
+    return {error: {message: 'Database error: Unable to save verification data'}}
+  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const pg = getKysely()
await pg
.insertInto('EmailVerification')
.values({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken,
expiration: new Date(Date.now() + Threshold.EMAIL_VERIFICATION_LIFESPAN)
})
.execute()
const pg = getKysely()
try {
await pg
.insertInto('EmailVerification')
.values({
email,
token: verifiedEmailToken,
hashedPassword,
pseudoId,
invitationToken,
expiration: new Date(Date.now() + Threshold.EMAIL_VERIFICATION_LIFESPAN)
})
.execute()
} catch (error) {
return {error: {message: 'Database error: Unable to save verification data'}}
}

return {error: {message: 'Verification required. Check your inbox.'}}
}

Expand Down
19 changes: 8 additions & 11 deletions packages/server/graphql/public/mutations/signUpWithPassword.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import bcrypt from 'bcryptjs'
import {AuthenticationError, Security} from 'parabol-client/types/constEnums'
import {URLSearchParams} from 'url'
import getRethink from '../../../database/rethinkDriver'
import {RValue} from '../../../database/stricterR'
import createEmailVerification from '../../../email/createEmailVerification'
import {USER_PREFERRED_NAME_LIMIT} from '../../../postgres/constants'
import getKysely from '../../../postgres/getKysely'
import createNewLocalUser from '../../../utils/createNewLocalUser'
import encodeAuthToken from '../../../utils/encodeAuthToken'
import isEmailVerificationRequired from '../../../utils/isEmailVerificationRequired'
Expand All @@ -21,7 +19,7 @@ const signUpWithPassword: MutationResolvers['signUpWithPassword'] = async (
if (email.length > USER_PREFERRED_NAME_LIMIT) {
return {error: {message: 'Email is too long'}}
}
const r = await getRethink()
const pg = getKysely()
const isOrganic = !invitationToken
const {ip, dataLoader} = context
const loginAttempt = await attemptLogin(email, password, ip)
Expand Down Expand Up @@ -49,13 +47,12 @@ const signUpWithPassword: MutationResolvers['signUpWithPassword'] = async (
}
const verificationRequired = await isEmailVerificationRequired(email, dataLoader)
if (verificationRequired) {
const existingVerification = await r
.table('EmailVerification')
.getAll(email, {index: 'email'})
.filter((row: RValue) => row('expiration').gt(new Date()))
.nth(0)
.default(null)
.run()
const existingVerification = await pg
.selectFrom('EmailVerification')
.selectAll()
.where('email', '=', email)
.where('expiration', '>', new Date())
.executeTakeFirst()
if (existingVerification) {
return {error: {message: 'Verification email already sent'}}
}
Expand Down
16 changes: 7 additions & 9 deletions packages/server/graphql/public/mutations/verifyEmail.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {AuthIdentityTypeEnum} from '../../../../client/types/constEnums'
import getRethink from '../../../database/rethinkDriver'
import AuthIdentityLocal from '../../../database/types/AuthIdentityLocal'
import AuthToken from '../../../database/types/AuthToken'
import EmailVerification from '../../../database/types/EmailVerification'
import getKysely from '../../../postgres/getKysely'
import {getUserByEmail} from '../../../postgres/queries/getUsersByEmails'
import updateUser from '../../../postgres/queries/updateUser'
import createNewLocalUser from '../../../utils/createNewLocalUser'
Expand All @@ -16,14 +15,13 @@ const verifyEmail: MutationResolvers['verifyEmail'] = async (
context
) => {
const {dataLoader} = context
const r = await getRethink()
const pg = getKysely()
const now = new Date()
const emailVerification = (await r
.table('EmailVerification')
.getAll(verificationToken, {index: 'token'})
.nth(0)
.default(null)
.run()) as EmailVerification
const emailVerification = await pg
.selectFrom('EmailVerification')
.selectAll()
.where('token', '=', verificationToken)
.executeTakeFirst()
Comment on lines +18 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for database operations.

The database query operation should handle potential errors to avoid unhandled promise rejections.

-  const emailVerification = await pg
-    .selectFrom('EmailVerification')
-    .selectAll()
-    .where('token', '=', verificationToken)
-    .executeTakeFirst()
+  let emailVerification
+  try {
+    emailVerification = await pg
+      .selectFrom('EmailVerification')
+      .selectAll()
+      .where('token', '=', verificationToken)
+      .executeTakeFirst()
+  } catch (error) {
+    return {error: {message: 'Database error: Unable to verify email'}}
+  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const pg = getKysely()
const now = new Date()
const emailVerification = (await r
.table('EmailVerification')
.getAll(verificationToken, {index: 'token'})
.nth(0)
.default(null)
.run()) as EmailVerification
const emailVerification = await pg
.selectFrom('EmailVerification')
.selectAll()
.where('token', '=', verificationToken)
.executeTakeFirst()
const pg = getKysely()
const now = new Date()
let emailVerification
try {
emailVerification = await pg
.selectFrom('EmailVerification')
.selectAll()
.where('token', '=', verificationToken)
.executeTakeFirst()
} catch (error) {
return {error: {message: 'Database error: Unable to verify email'}}
}


if (!emailVerification) {
return {error: {message: 'Invalid verification token'}}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import {Kysely, PostgresDialect, sql} from 'kysely'
import {Client} from 'pg'
import {r} from 'rethinkdb-ts'
import connectRethinkDB from '../../database/connectRethinkDB'
import getPg from '../getPg'
import getPgConfig from '../getPgConfig'

export async function up() {
await connectRethinkDB()
const pg = new Kysely<any>({
dialect: new PostgresDialect({
pool: getPg()
})
})
await sql`
CREATE TABLE "EmailVerification" (
"id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
"email" "citext" NOT NULL,
"expiration" TIMESTAMP WITH TIME ZONE NOT NULL,
"token" VARCHAR(100) NOT NULL,
"hashedPassword" VARCHAR(100),
"invitationToken" VARCHAR(100),
"pseudoId" VARCHAR(100)
);

CREATE INDEX IF NOT EXISTS "idx_EmailVerification_email" ON "EmailVerification"("email");
CREATE INDEX IF NOT EXISTS "idx_EmailVerification_token" ON "EmailVerification"("token");
`.execute(pg)
Comment on lines +8 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for database operations.

The table creation and data migration operations should handle potential errors to avoid unhandled promise rejections.

-  await connectRethinkDB()
-  const pg = new Kysely<any>({
-    dialect: new PostgresDialect({
-      pool: getPg()
-    })
-  })
-  await sql`
-    CREATE TABLE "EmailVerification" (
-      "id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
-      "email" "citext" NOT NULL,
-      "expiration" TIMESTAMP WITH TIME ZONE NOT NULL,
-      "token" VARCHAR(100) NOT NULL,
-      "hashedPassword" VARCHAR(100),
-      "invitationToken" VARCHAR(100),
-      "pseudoId" VARCHAR(100)
-    );
-    CREATE INDEX IF NOT EXISTS "idx_EmailVerification_email" ON "EmailVerification"("email");
-    CREATE INDEX IF NOT EXISTS "idx_EmailVerification_token" ON "EmailVerification"("token");
-  `.execute(pg)
-  const rData = await r.table('EmailVerification').coerceTo('array').run()
-  const insertData = rData.map((row) => {
-    const {email, expiration, hashedPassword, token, invitationToken, pseudoId} = row
-    return {
-      email,
-      expiration,
-      hashedPassword,
-      token,
-      invitationToken,
-      pseudoId
-    }
-  })
-  await pg.insertInto('EmailVerification').values(insertData).execute()
+  try {
+    await connectRethinkDB()
+    const pg = new Kysely<any>({
+      dialect: new PostgresDialect({
+        pool: getPg()
+      })
+    })
+    await sql`
+      CREATE TABLE "EmailVerification" (
+        "id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
+        "email" "citext" NOT NULL,
+        "expiration" TIMESTAMP WITH TIME ZONE NOT NULL,
+        "token" VARCHAR(100) NOT NULL,
+        "hashedPassword" VARCHAR(100),
+        "invitationToken" VARCHAR(100),
+        "pseudoId" VARCHAR(100)
+      );
+      CREATE INDEX IF NOT EXISTS "idx_EmailVerification_email" ON "EmailVerification"("email");
+      CREATE INDEX IF NOT EXISTS "idx_EmailVerification_token" ON "EmailVerification"("token");
+    `.execute(pg)
+    const rData = await r.table('EmailVerification').coerceTo('array').run()
+    const insertData = rData.map((row) => {
+      const {email, expiration, hashedPassword, token, invitationToken, pseudoId} = row
+      return {
+        email,
+        expiration,
+        hashedPassword,
+        token,
+        invitationToken,
+        pseudoId
+      }
+    })
+    await pg.insertInto('EmailVerification').values(insertData).execute()
+  } catch (error) {
+    console.error('Database error:', error)
+    throw error
+  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function up() {
await connectRethinkDB()
const pg = new Kysely<any>({
dialect: new PostgresDialect({
pool: getPg()
})
})
await sql`
CREATE TABLE "EmailVerification" (
"id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
"email" "citext" NOT NULL,
"expiration" TIMESTAMP WITH TIME ZONE NOT NULL,
"token" VARCHAR(100) NOT NULL,
"hashedPassword" VARCHAR(100),
"invitationToken" VARCHAR(100),
"pseudoId" VARCHAR(100)
);
CREATE INDEX IF NOT EXISTS "idx_EmailVerification_email" ON "EmailVerification"("email");
CREATE INDEX IF NOT EXISTS "idx_EmailVerification_token" ON "EmailVerification"("token");
`.execute(pg)
export async function up() {
try {
await connectRethinkDB()
const pg = new Kysely<any>({
dialect: new PostgresDialect({
pool: getPg()
})
})
await sql`
CREATE TABLE "EmailVerification" (
"id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
"email" "citext" NOT NULL,
"expiration" TIMESTAMP WITH TIME ZONE NOT NULL,
"token" VARCHAR(100) NOT NULL,
"hashedPassword" VARCHAR(100),
"invitationToken" VARCHAR(100),
"pseudoId" VARCHAR(100)
);
CREATE INDEX IF NOT EXISTS "idx_EmailVerification_email" ON "EmailVerification"("email");
CREATE INDEX IF NOT EXISTS "idx_EmailVerification_token" ON "EmailVerification"("token");
`.execute(pg)
const rData = await r.table('EmailVerification').coerceTo('array').run()
const insertData = rData.map((row) => {
const {email, expiration, hashedPassword, token, invitationToken, pseudoId} = row
return {
email,
expiration,
hashedPassword,
token,
invitationToken,
pseudoId
}
})
await pg.insertInto('EmailVerification').values(insertData).execute()
} catch (error) {
console.error('Database error:', error)
throw error
}
}


const rData = await r.table('EmailVerification').coerceTo('array').run()
const insertData = rData.map((row) => {
const {email, expiration, hashedPassword, token, invitationToken, pseudoId} = row
return {
email,
expiration,
hashedPassword,
token,
invitationToken,
pseudoId
}
})
if (insertData.length === 0) return
await pg.insertInto('EmailVerification').values(insertData).execute()
}

export async function down() {
const client = new Client(getPgConfig())
await client.connect()
await client.query(`
DROP TABLE IF EXISTS "EmailVerification";
`)
await client.end()
Comment on lines +46 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for database operations.

The table drop operation should handle potential errors to avoid unhandled promise rejections.

-  const client = new Client(getPgConfig())
-  await client.connect()
-  await client.query(`
-    DROP TABLE IF EXISTS "EmailVerification";
-  `)
-  await client.end()
+  const client = new Client(getPgConfig())
+  try {
+    await client.connect()
+    await client.query(`
+      DROP TABLE IF EXISTS "EmailVerification";
+    `)
+  } catch (error) {
+    console.error('Database error:', error)
+    throw error
+  } finally {
+    await client.end()
+  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function down() {
const client = new Client(getPgConfig())
await client.connect()
await client.query(`
DROP TABLE IF EXISTS "EmailVerification";
`)
await client.end()
export async function down() {
const client = new Client(getPgConfig())
try {
await client.connect()
await client.query(`
DROP TABLE IF EXISTS "EmailVerification";
`)
} catch (error) {
console.error('Database error:', error)
throw error
} finally {
await client.end()
}
}

}
Loading