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

Clean up other email tokens #1572

Merged
merged 21 commits into from
Sep 28, 2023
3 changes: 2 additions & 1 deletion packages/bsky/tests/indexing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
66 changes: 6 additions & 60 deletions packages/pds/src/api/com/atproto/server/deleteAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand All @@ -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) => {
Expand All @@ -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()
}
13 changes: 3 additions & 10 deletions packages/pds/src/api/com/atproto/server/requestAccountDelete.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand All @@ -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 })
},
})
Expand Down
14 changes: 3 additions & 11 deletions packages/pds/src/api/com/atproto/server/requestPasswordReset.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand All @@ -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 },
Expand Down
68 changes: 7 additions & 61 deletions packages/pds/src/api/com/atproto/server/resetPassword.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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()
}
29 changes: 19 additions & 10 deletions packages/pds/src/api/com/atproto/server/updateEmail.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 0 additions & 2 deletions packages/pds/src/db/database-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -40,7 +39,6 @@ export type DatabaseSchemaType = runtimeFlag.PartialDB &
notification.PartialDB &
blob.PartialDB &
repoBlob.PartialDB &
deleteAccountToken.PartialDB &
emailToken.PartialDB &
moderation.PartialDB &
mute.PartialDB &
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
}
1 change: 1 addition & 0 deletions packages/pds/src/db/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
9 changes: 0 additions & 9 deletions packages/pds/src/db/tables/delete-account-token.ts

This file was deleted.

2 changes: 0 additions & 2 deletions packages/pds/src/db/tables/user-account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading