Clean up other email tokens (#1572)

* lexicons

* codegen

* email templates

* request routes

* impl

* migration

* tidy

* tests

* tidy & bugfixes

* format

* fix api test

* fix auth test

* impl

* update constraint name

* temporarily disable unconfirmed updates

* tidy

* fix some tests

* validate email syntax

packages/bsky/tests/indexing.test.ts

@@ -648,8 +648,9 @@ describe('indexing', () => {
         headers: sc.getHeaders(alice),
       })
       const { token } = await network.pds.ctx.db.db
-        .selectFrom('delete_account_token')
+        .selectFrom('email_token')
         .selectAll()
+        .where('purpose', '=', 'delete_account')
         .where('did', '=', alice)
         .executeTakeFirstOrThrow()
       await pdsAgent.api.com.atproto.server.deleteAccount({

packages/pds/src/api/com/atproto/server/deleteAccount.ts

@@ -2,7 +2,6 @@ import { AuthRequiredError } from '@atproto/xrpc-server'
 import { Server } from '../../../../lexicon'
 import { TAKEDOWN } from '../../../../lexicon/types/com/atproto/admin/defs'
 import AppContext from '../../../../context'
-import Database from '../../../../db'
 import { MINUTE } from '@atproto/common'
 
 const REASON_ACCT_DELETION = 'ACCOUNT DELETION'
@@ -22,40 +21,18 @@ export default function (server: Server, ctx: AppContext) {
         throw new AuthRequiredError('Invalid did or password')
       }
 
-      const tokenInfo = await ctx.db.db
-        .selectFrom('did_handle')
-        .innerJoin(
-          'delete_account_token as token',
-          'token.did',
-          'did_handle.did',
-        )
-        .where('did_handle.did', '=', did)
-        .where('token.token', '=', token.toUpperCase())
-        .select([
-          'token.token as token',
-          'token.requestedAt as requestedAt',
-          'token.did as did',
-        ])
-        .executeTakeFirst()
-
-      if (!tokenInfo) {
-        return createInvalidTokenError()
-      }
+      await ctx.services
+        .account(ctx.db)
+        .assertValidToken(did, 'delete_account', token)
 
       const now = new Date()
-      const requestedAt = new Date(tokenInfo.requestedAt)
-      const expiresAt = new Date(requestedAt.getTime() + 15 * minsToMs)
-      if (now > expiresAt) {
-        await removeDeleteToken(ctx.db, tokenInfo.did)
-        return createExpiredTokenError()
-      }
-
       await ctx.db.transaction(async (dbTxn) => {
+        const accountService = ctx.services.account(dbTxn)
         const moderationTxn = ctx.services.moderation(dbTxn)
         const [currentAction] = await moderationTxn.getCurrentActions({ did })
         if (currentAction?.action === TAKEDOWN) {
           // Do not disturb an existing takedown, continue with account deletion
-          return await removeDeleteToken(dbTxn, did)
+          return await accountService.deleteEmailToken(did, 'delete_account')
         }
         if (currentAction) {
           // Reverse existing action to replace it with a self-takedown
@@ -74,7 +51,7 @@ export default function (server: Server, ctx: AppContext) {
           createdAt: now,
         })
         await moderationTxn.takedownRepo({ did, takedownId: takedown.id })
-        await removeDeleteToken(dbTxn, did)
+        await accountService.deleteEmailToken(did, 'delete_account')
       })
 
       ctx.backgroundQueue.add(async (db) => {
@@ -90,34 +67,3 @@ export default function (server: Server, ctx: AppContext) {
     },
   })
 }
-
-type ErrorResponse = {
-  status: number
-  error: string
-  message: string
-}
-
-const minsToMs = 60 * 1000
-
-const createInvalidTokenError = (): ErrorResponse & {
-  error: 'InvalidToken'
-} => ({
-  status: 400,
-  error: 'InvalidToken',
-  message: 'Token is invalid',
-})
-
-const createExpiredTokenError = (): ErrorResponse & {
-  error: 'ExpiredToken'
-} => ({
-  status: 400,
-  error: 'ExpiredToken',
-  message: 'The password reset token has expired',
-})
-
-const removeDeleteToken = async (db: Database, did: string) => {
-  await db.db
-    .deleteFrom('delete_account_token')
-    .where('delete_account_token.did', '=', did)
-    .execute()
-}

packages/pds/src/api/com/atproto/server/requestAccountDelete.ts

@@ -1,7 +1,6 @@
 import { InvalidRequestError } from '@atproto/xrpc-server'
 import { Server } from '../../../../lexicon'
 import AppContext from '../../../../context'
-import { getRandomToken } from './util'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.server.requestAccountDelete({
@@ -12,15 +11,9 @@ export default function (server: Server, ctx: AppContext) {
       if (!user) {
         throw new InvalidRequestError('user not found')
       }
-      const token = getRandomToken().toUpperCase()
-      const requestedAt = new Date().toISOString()
-      await ctx.db.db
-        .insertInto('delete_account_token')
-        .values({ did, token, requestedAt })
-        .onConflict((oc) =>
-          oc.column('did').doUpdateSet({ token, requestedAt }),
-        )
-        .execute()
+      const token = await ctx.services
+        .account(ctx.db)
+        .createEmailToken(did, 'delete_account')
       await ctx.mailer.sendAccountDelete({ token }, { to: user.email })
     },
   })

packages/pds/src/api/com/atproto/server/requestPasswordReset.ts

@@ -1,6 +1,5 @@
 import AppContext from '../../../../context'
 import { Server } from '../../../../lexicon'
-import { getRandomToken } from './util'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.server.requestPasswordReset(async ({ input }) => {
@@ -9,16 +8,9 @@ export default function (server: Server, ctx: AppContext) {
     const user = await ctx.services.account(ctx.db).getAccountByEmail(email)
 
     if (user) {
-      const token = getRandomToken().toUpperCase()
-      const grantedAt = new Date().toISOString()
-      await ctx.db.db
-        .updateTable('user_account')
-        .where('did', '=', user.did)
-        .set({
-          passwordResetToken: token,
-          passwordResetGrantedAt: grantedAt,
-        })
-        .execute()
+      const token = await ctx.services
+        .account(ctx.db)
+        .createEmailToken(user.did, 'reset_password')
       await ctx.mailer.sendResetPassword(
         { handle: user.handle, token },
         { to: user.email },

packages/pds/src/api/com/atproto/server/resetPassword.ts

@@ -1,6 +1,5 @@
 import AppContext from '../../../../context'
 import { Server } from '../../../../lexicon'
-import Database from '../../../../db'
 import { MINUTE } from '@atproto/common'
 
 export default function (server: Server, ctx: AppContext) {
@@ -14,69 +13,16 @@ export default function (server: Server, ctx: AppContext) {
     handler: async ({ input }) => {
       const { token, password } = input.body
 
-      const tokenInfo = await ctx.db.db
-        .selectFrom('user_account')
-        .select(['did', 'passwordResetGrantedAt'])
-        .where('passwordResetToken', '=', token.toUpperCase())
-        .executeTakeFirst()
-
-      if (!tokenInfo?.passwordResetGrantedAt) {
-        return createInvalidTokenError()
-      }
-
-      const now = new Date()
-      const grantedAt = new Date(tokenInfo.passwordResetGrantedAt)
-      const expiresAt = new Date(grantedAt.getTime() + 15 * minsToMs)
-
-      if (now > expiresAt) {
-        await unsetResetToken(ctx.db, tokenInfo.did)
-        return createExpiredTokenError()
-      }
+      const did = await ctx.services
+        .account(ctx.db)
+        .assertValidTokenAndFindDid('reset_password', token)
 
       await ctx.db.transaction(async (dbTxn) => {
-        await unsetResetToken(dbTxn, tokenInfo.did)
-        await ctx.services
-          .account(dbTxn)
-          .updateUserPassword(tokenInfo.did, password)
-        await await ctx.services
-          .auth(dbTxn)
-          .revokeRefreshTokensByDid(tokenInfo.did)
+        const accountService = ctx.services.account(ctx.db)
+        await accountService.updateUserPassword(did, password)
+        await accountService.deleteEmailToken(did, 'reset_password')
+        await ctx.services.auth(dbTxn).revokeRefreshTokensByDid(did)
       })
     },
   })
 }
-
-type ErrorResponse = {
-  status: number
-  error: string
-  message: string
-}
-
-const minsToMs = 60 * 1000
-
-const createInvalidTokenError = (): ErrorResponse & {
-  error: 'InvalidToken'
-} => ({
-  status: 400,
-  error: 'InvalidToken',
-  message: 'Token is invalid',
-})
-
-const createExpiredTokenError = (): ErrorResponse & {
-  error: 'ExpiredToken'
-} => ({
-  status: 400,
-  error: 'ExpiredToken',
-  message: 'The password reset token has expired',
-})
-
-const unsetResetToken = async (db: Database, did: string) => {
-  await db.db
-    .updateTable('user_account')
-    .where('did', '=', did)
-    .set({
-      passwordResetToken: null,
-      passwordResetGrantedAt: null,
-    })
-    .execute()
-}

packages/pds/src/api/com/atproto/server/updateEmail.ts

@@ -1,29 +1,38 @@
 import { Server } from '../../../../lexicon'
 import AppContext from '../../../../context'
 import { InvalidRequestError } from '@atproto/xrpc-server'
+import disposable from 'disposable-email'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.server.updateEmail({
     auth: ctx.accessVerifierCheckTakedown,
     handler: async ({ auth, input }) => {
       const did = auth.credentials.did
       const { token, email } = input.body
+      if (!disposable.validate(email)) {
+        throw new InvalidRequestError(
+          'This email address is not supported, please use a different email.',
+        )
+      }
       const user = await ctx.services.account(ctx.db).getAccount(did)
       if (!user) {
         throw new InvalidRequestError('user not found')
       }
+      if (!user.emailConfirmedAt) {
+        throw new InvalidRequestError('email must be confirmed (temporary)')
+      }
       // require valid token
-      // @TODO re-enable updating non-verified emails
-      // if (user.emailConfirmedAt) {
-      if (!token) {
-        throw new InvalidRequestError(
-          'confirmation token required',
-          'TokenRequired',
-        )
+      if (user.emailConfirmedAt) {
+        if (!token) {
+          throw new InvalidRequestError(
+            'confirmation token required',
+            'TokenRequired',
+          )
+        }
+        await ctx.services
+          .account(ctx.db)
+          .assertValidToken(did, 'update_email', token)
       }
-      await ctx.services
-        .account(ctx.db)
-        .assertValidToken(did, 'update_email', token)
 
       await ctx.db.transaction(async (dbTxn) => {
         const accntSrvce = ctx.services.account(dbTxn)

packages/pds/src/db/database-schema.ts

@@ -14,7 +14,6 @@ import * as inviteCode from './tables/invite-code'
 import * as notification from './tables/user-notification'
 import * as blob from './tables/blob'
 import * as repoBlob from './tables/repo-blob'
-import * as deleteAccountToken from './tables/delete-account-token'
 import * as emailToken from './tables/email-token'
 import * as moderation from './tables/moderation'
 import * as mute from './tables/mute'
@@ -40,7 +39,6 @@ export type DatabaseSchemaType = runtimeFlag.PartialDB &
   notification.PartialDB &
   blob.PartialDB &
   repoBlob.PartialDB &
-  deleteAccountToken.PartialDB &
   emailToken.PartialDB &
   moderation.PartialDB &
   mute.PartialDB &

packages/pds/src/db/migrations/20230926T195532354Z-email-tokens.ts

@@ -10,7 +10,10 @@ export async function up(db: Kysely<unknown>, dialect: Dialect): Promise<void> {
     .addColumn('token', 'varchar', (col) => col.notNull())
     .addColumn('requestedAt', timestamp, (col) => col.notNull())
     .addPrimaryKeyConstraint('email_token_pkey', ['purpose', 'did'])
-    .addUniqueConstraint('email_token_token_unique', ['purpose', 'token'])
+    .addUniqueConstraint('email_token_purpose_token_unique', [
+      'purpose',
+      'token',
+    ])
     .execute()
 
   await db.schema

packages/pds/src/db/migrations/20230927T212334019Z-simplify-email-tokens.ts

@@ -0,0 +1,34 @@
+import { Kysely } from 'kysely'
+
+export async function up(db: Kysely<unknown>): Promise<void> {
+  await db.schema
+    .alterTable('user_account')
+    .dropColumn('passwordResetToken')
+    .execute()
+
+  await db.schema
+    .alterTable('user_account')
+    .dropColumn('passwordResetGrantedAt')
+    .execute()
+
+  await db.schema.dropTable('delete_account_token').execute()
+}
+
+export async function down(db: Kysely<unknown>): Promise<void> {
+  await db.schema
+    .alterTable('user_account')
+    .addColumn('passwordResetToken', 'varchar')
+    .execute()
+
+  await db.schema
+    .alterTable('user_account')
+    .addColumn('passwordResetGrantedAt', 'varchar')
+    .execute()
+
+  await db.schema
+    .createTable('delete_account_token')
+    .addColumn('did', 'varchar', (col) => col.primaryKey())
+    .addColumn('token', 'varchar', (col) => col.notNull())
+    .addColumn('requestedAt', 'varchar', (col) => col.notNull())
+    .execute()
+}

packages/pds/src/db/migrations/index.ts

@@ -67,3 +67,4 @@ export * as _20230825T142507884Z from './20230825T142507884Z-blob-tempkey-idx'
 export * as _20230828T153013575Z from './20230828T153013575Z-repo-history-rewrite'
 export * as _20230922T033938477Z from './20230922T033938477Z-remove-appview'
 export * as _20230926T195532354Z from './20230926T195532354Z-email-tokens'
+export * as _20230927T212334019Z from './20230927T212334019Z-simplify-email-tokens'

packages/pds/src/db/tables/user-account.ts

@@ -6,8 +6,6 @@ export interface UserAccount {
   passwordScrypt: string
   createdAt: string
   emailConfirmedAt: string | null
-  passwordResetToken: string | null
-  passwordResetGrantedAt: string | null
   invitesDisabled: Generated<0 | 1>
   inviteNote: string | null
 }