Skip to content

Commit

Permalink
Lengthen pass reset and invite codes (bluesky-social#912)
Browse files Browse the repository at this point in the history
* Lengthen pass reset and invite codes, invalidation on reset

* Tidy

* Adjust token format for reset and deletion

* Update invite codes to use same token gen as pass reset and acct deletion
  • Loading branch information
devinivy authored Apr 27, 2023
1 parent 7d1243f commit 5249e52
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 20 deletions.
2 changes: 1 addition & 1 deletion packages/pds/src/api/com/atproto/server/deleteAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default function (server: Server, ctx: AppContext) {
.selectFrom('did_handle')
.innerJoin('delete_account_token as token', 'token.did', 'did_handle.did')
.where('did_handle.did', '=', did)
.where('token.token', '=', token)
.where('token.token', '=', token.toUpperCase())
.select([
'token.token as token',
'token.requestedAt as requestedAt',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { InvalidRequestError } from '@atproto/xrpc-server'
import { Server } from '../../../../lexicon'
import AppContext from '../../../../context'
import { randomStr } from '@atproto/crypto'
import { getRandomToken } from './util'

export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.requestAccountDelete({
auth: ctx.accessVerifierCheckTakedown,
handler: async ({ auth }) => {
const did = auth.credentials.did
const token = getSixDigitToken()
const token = getRandomToken().toUpperCase()
const requestedAt = new Date().toISOString()
const user = await ctx.services.account(ctx.db).getAccount(did)
if (!user) {
Expand All @@ -25,5 +25,3 @@ export default function (server: Server, ctx: AppContext) {
},
})
}

const getSixDigitToken = () => randomStr(4, 'base10').slice(0, 6)
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { randomStr } from '@atproto/crypto'
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,7 +9,7 @@ export default function (server: Server, ctx: AppContext) {
const user = await ctx.services.account(ctx.db).getAccountByEmail(email)

if (user) {
const token = getSixDigitToken()
const token = getRandomToken().toUpperCase()
const grantedAt = new Date().toISOString()
await ctx.db.db
.updateTable('user_account')
Expand All @@ -26,5 +26,3 @@ export default function (server: Server, ctx: AppContext) {
}
})
}

const getSixDigitToken = () => randomStr(4, 'base10').slice(0, 6)
5 changes: 4 additions & 1 deletion packages/pds/src/api/com/atproto/server/resetPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default function (server: Server, ctx: AppContext) {
const tokenInfo = await ctx.db.db
.selectFrom('user_account')
.select(['did', 'passwordResetGrantedAt'])
.where('passwordResetToken', '=', token)
.where('passwordResetToken', '=', token.toUpperCase())
.executeTakeFirst()

if (!tokenInfo?.passwordResetGrantedAt) {
Expand All @@ -30,6 +30,9 @@ export default function (server: Server, ctx: AppContext) {
await ctx.services
.account(dbTxn)
.updateUserPassword(tokenInfo.did, password)
await await ctx.services
.auth(dbTxn)
.revokeRefreshTokensByDid(tokenInfo.did)
})
})
}
Expand Down
15 changes: 10 additions & 5 deletions packages/pds/src/api/com/atproto/server/util.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import * as crypto from '@atproto/crypto'
import { ServerConfig } from '../../../../config'

// generate a 7 char b32 invite code - preceded by the hostname
// generate an invite code preceded by the hostname
// with '.'s replaced by '-'s so it is not mistakable for a link
// ex: bsky-app-abc2345
// regex: bsky-app-[a-z2-7]{7}
// ex: bsky-app-abc234-567xy
// regex: bsky-app-[a-z2-7]{5}-[a-z2-7]{5}
export const genInvCode = (cfg: ServerConfig): string => {
const code = crypto.randomStr(7, 'base32').slice(0, 7)
return cfg.publicHostname.replaceAll('.', '-') + '-' + code
return cfg.publicHostname.replaceAll('.', '-') + '-' + getRandomToken()
}

export const genInvCodes = (cfg: ServerConfig, count: number): string[] => {
Expand All @@ -17,3 +16,9 @@ export const genInvCodes = (cfg: ServerConfig, count: number): string[] => {
}
return codes
}

// Formatted xxxxx-xxxxx where digits are in base32
export const getRandomToken = () => {
const token = crypto.randomStr(8, 'base32').slice(0, 10)
return token.slice(0, 5) + '-' + token.slice(5, 10)
}
2 changes: 1 addition & 1 deletion packages/pds/tests/account-deletion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('account deletion', () => {
}

const getTokenFromMail = (mail: Mail.Options) =>
mail.html?.toString().match(/>(\d{6})</)?.[1]
mail.html?.toString().match(/>([a-z0-9]{5}-[a-z0-9]{5})</i)?.[1]

let token

Expand Down
40 changes: 36 additions & 4 deletions packages/pds/tests/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ describe('account', () => {
it('creates an invite code', async () => {
inviteCode = await createInviteCode(agent, 1)
const split = inviteCode.split('-')
const host = split.slice(0, -1).join('.')
const code = split.at(-1)
const host = split.slice(0, -2).join('.')
const code = split.slice(-2).join('-')
expect(host).toBe('pds.public.url') // Hostname of public url
expect(code?.length).toBe(7)
expect(code.length).toBe(11)
})

it('serves the accounts system config', async () => {
Expand Down Expand Up @@ -394,7 +394,7 @@ describe('account', () => {
}

const getTokenFromMail = (mail: Mail.Options) =>
mail.html?.toString().match(/>(\d{6})</)?.[1]
mail.html?.toString().match(/>([a-z0-9]{5}-[a-z0-9]{5})</i)?.[1]

it('can reset account password', async () => {
const mail = await getMailFrom(
Expand Down Expand Up @@ -467,6 +467,38 @@ describe('account', () => {
).resolves.toBeDefined()
})

it('changing password invalidates past refresh tokens', async () => {
const mail = await getMailFrom(
agent.api.com.atproto.server.requestPasswordReset({ email }),
)

expect(mail.to).toEqual(email)
expect(mail.html).toContain('Reset your password')
expect(mail.html).toContain('alice.test')

const token = getTokenFromMail(mail)

if (token === undefined) {
return expect(token).toBeDefined()
}

const session = await agent.api.com.atproto.server.createSession({
identifier: handle,
password,
})

await agent.api.com.atproto.server.resetPassword({
token: token.toLowerCase(), // Reset should work case-insensitively
password,
})

await expect(
agent.api.com.atproto.server.refreshSession(undefined, {
headers: { authorization: `Bearer ${session.data.refreshJwt}` },
}),
).rejects.toThrow('Token has been revoked')
})

it('allows only unexpired password reset tokens', async () => {
await agent.api.com.atproto.server.requestPasswordReset({ email })

Expand Down

0 comments on commit 5249e52

Please sign in to comment.