Reset password updates (bluesky-social#422)

* Log emails to stdout if no smtp mailer is set (useful for debugging)

* Fancier reset-password email template

* Update reset password flow to use a human-enterable OTP

* Tidy

* Run the email template through prettier

Co-authored-by: Devin Ivy <[email protected]>

packages/pds/src/api/com/atproto/password-reset.ts

@@ -1,83 +1,74 @@
-import jwt from 'jsonwebtoken'
-import { AuthScopes } from '../../../auth'
-import { ServerConfig } from '../../../config'
-import { User } from '../../../db/tables/user'
+import { randomStr } from '@atproto/crypto'
+import Database from '../../../db'
 import { Server } from '../../../lexicon'
 import * as locals from '../../../locals'
 
 export default function (server: Server) {
   server.com.atproto.account.requestPasswordReset(async ({ input, res }) => {
-    const { db, services, mailer, config } = locals.get(res)
+    const { db, services, mailer } = locals.get(res)
     const email = input.body.email.toLowerCase()
 
     const user = await services.actor(db).getUserByEmail(email)
 
     if (user) {
-      // By signing with the password hash, this jwt becomes invalid once the user changes their password.
-      // This allows us to ensure it's essentially single-use (combined with the jwt's short-livedness).
-      const token = jwt.sign(
-        {
-          sub: user.did,
-          scope: AuthScopes.ResetPassword,
-        },
-        getSigningKey(user, config),
-        { expiresIn: '15mins' },
-      )
-
+      const token = getSixDigitToken()
+      const grantedAt = new Date().toISOString()
+      await db.db
+        .updateTable('user')
+        .where('handle', '=', user.handle)
+        .set({
+          passwordResetToken: token,
+          passwordResetGrantedAt: grantedAt,
+        })
+        .execute()
       await mailer.sendResetPassword({ token }, { to: user.email })
     }
   })
 
   server.com.atproto.account.resetPassword(async ({ input, res }) => {
-    const { db, services, config } = locals.get(res)
+    const { db, services } = locals.get(res)
     const { token, password } = input.body
 
-    const tokenBody = jwt.decode(token)
-    if (tokenBody === null || typeof tokenBody === 'string') {
-      return createInvalidTokenError('Malformed token')
-    }
+    const tokenInfo = await db.db
+      .selectFrom('user')
+      .select(['handle', 'passwordResetGrantedAt'])
+      .where('passwordResetToken', '=', token)
+      .executeTakeFirst()
 
-    const { sub: did, scope } = tokenBody
-    if (typeof did !== 'string' || scope !== AuthScopes.ResetPassword) {
-      return createInvalidTokenError('Malformed token')
+    if (!tokenInfo?.passwordResetGrantedAt) {
+      return createInvalidTokenError()
     }
 
-    const user = await services.actor(db).getUser(did)
-    if (!user) {
-      return createInvalidTokenError('Token could not be verified')
-    }
+    const now = new Date()
+    const grantedAt = new Date(tokenInfo.passwordResetGrantedAt)
+    const expiresAt = new Date(grantedAt.getTime() + 15 * minsToMs)
 
-    try {
-      jwt.verify(token, getSigningKey(user, config))
-    } catch (err) {
-      if (err instanceof jwt.TokenExpiredError) {
-        return createExpiredTokenError()
-      }
-      return createInvalidTokenError('Token could not be verified')
+    if (now > expiresAt) {
+      await unsetResetToken(db, tokenInfo.handle)
+      return createExpiredTokenError()
     }
 
-    // Token had correct scope, was not expired, and referenced
-    // a user whose password has not changed since token issuance.
-
-    await services.actor(db).updateUserPassword(user.handle, password)
+    await db.transaction(async (dbTxn) => {
+      await unsetResetToken(dbTxn, tokenInfo.handle)
+      await services.actor(dbTxn).updateUserPassword(tokenInfo.handle, password)
+    })
   })
 }
 
-const getSigningKey = (user: User, config: ServerConfig) =>
-  `${config.jwtSecret}::${user.password}`
-
 type ErrorResponse = {
   status: number
   error: string
   message: string
 }
 
-const createInvalidTokenError = (
-  message: string,
-): ErrorResponse & { error: 'InvalidToken' } => ({
+const minsToMs = 60 * 1000
+
+const createInvalidTokenError = (): ErrorResponse & {
+  error: 'InvalidToken'
+} => ({
   status: 400,
   error: 'InvalidToken',
-  message,
+  message: 'Token is invalid',
 })
 
 const createExpiredTokenError = (): ErrorResponse & {
@@ -87,3 +78,16 @@ const createExpiredTokenError = (): ErrorResponse & {
   error: 'ExpiredToken',
   message: 'The password reset token has expired',
 })
+
+const getSixDigitToken = () => randomStr(4, 'base10').slice(0, 6)
+
+const unsetResetToken = async (db: Database, handle: string) => {
+  await db.db
+    .updateTable('user')
+    .where('handle', '=', handle)
+    .set({
+      passwordResetToken: null,
+      passwordResetGrantedAt: null,
+    })
+    .execute()
+}

packages/pds/src/auth.ts

@@ -1,10 +1,6 @@
 import * as auth from '@atproto/auth'
 import * as crypto from '@atproto/crypto'
-import {
-  AuthRequiredError,
-  ForbiddenError,
-  InvalidRequestError,
-} from '@atproto/xrpc-server'
+import { AuthRequiredError, InvalidRequestError } from '@atproto/xrpc-server'
 import * as uint8arrays from 'uint8arrays'
 import { DidResolver } from '@atproto/did-resolver'
 import express from 'express'
@@ -24,7 +20,6 @@ export type ServerAuthOpts = {
 export enum AuthScopes {
   Access = 'com.atproto.access',
   Refresh = 'com.atproto.refresh',
-  ResetPassword = 'com.atproto.resetAccountPassword',
 }
 
 export type AuthToken = {

packages/pds/src/db/migrations/20221215T220356370Z-password-reset-otp.ts

@@ -0,0 +1,35 @@
+import { Kysely } from 'kysely'
+
+const userTable = 'user'
+
+export async function up(db: Kysely<unknown>): Promise<void> {
+  await db.schema
+    .alterTable(userTable)
+    .addColumn('passwordResetToken', 'varchar')
+    .execute()
+  await db.schema
+    .alterTable(userTable)
+    .addColumn('passwordResetGrantedAt', 'varchar')
+    .execute()
+  await db.schema
+    .createIndex('user_password_reset_token_idx')
+    .unique()
+    .on('user')
+    .column('passwordResetToken')
+    .execute()
+}
+
+export async function down(db: Kysely<unknown>): Promise<void> {
+  await db.schema
+    .dropIndex('user_password_reset_token_idx')
+    .on('user')
+    .execute()
+  await db.schema
+    .alterTable(userTable)
+    .dropColumn('passwordResetToken')
+    .execute()
+  await db.schema
+    .alterTable(userTable)
+    .dropColumn('passwordResetGrantedAt')
+    .execute()
+}

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

@@ -1,4 +1,4 @@
-// NOTE this file can be edited by hand, but it is also appended to by the migrations:create command.
+// NOTE this file can be edited by hand, but it is also appended to by the migration:create command.
 // It's important that every migration is exported from here with the proper name. We'd simplify
 // this with kysely's FileMigrationProvider, but it doesn't play nicely with the build process.
 
@@ -7,3 +7,4 @@ export * as _20221116T234458063Z from './20221116T234458063Z-duplicate-records'
 export * as _20221202T212459280Z from './20221202T212459280Z-blobs'
 export * as _20221209T210026294Z from './20221209T210026294Z-banners'
 export * as _20221212T195416407Z from './20221212T195416407Z-post-media'
+export * as _20221215T220356370Z from './20221215T220356370Z-password-reset-otp'

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

@@ -4,6 +4,8 @@ export interface User {
   password: string
   lastSeenNotifs: string
   createdAt: string
+  passwordResetToken: string | null
+  passwordResetGrantedAt: string | null
 }
 
 export const tableName = 'user'

packages/pds/src/logger.ts

@@ -5,6 +5,7 @@ import * as jwt from 'jsonwebtoken'
 import { parseBasicAuth } from './auth'
 
 export const dbLogger = subsystemLogger('pds:db')
+export const mailerLogger = subsystemLogger('pds:mailer')
 export const httpLogger = subsystemLogger('pds')
 
 export const loggerMiddleware = pinoHttp({

packages/pds/src/mailer/index.ts

@@ -5,6 +5,7 @@ import { htmlToText } from 'nodemailer-html-to-text'
 import Mail from 'nodemailer/lib/mailer'
 import SMTPTransport from 'nodemailer/lib/smtp-transport'
 import { ServerConfig } from '../config'
+import { mailerLogger } from '../logger'
 
 export class ServerMailer {
   private config: ServerConfig
@@ -44,11 +45,18 @@ export class ServerMailer {
       ...params,
       config: ServerMailer.getEmailConfig(this.config),
     })
-    return await this.transporter.sendMail({
+    const res = await this.transporter.sendMail({
       ...mailOpts,
       from: mailOpts.from ?? this.config.emailNoReplyAddress,
       html,
     })
+    if (!this.config.emailSmtpUrl) {
+      mailerLogger.debug(
+        'No SMTP URL has been configured. Intended to send email:\n' +
+          JSON.stringify(res, null, 2),
+      )
+    }
+    return res
   }
 
   private compile(name) {