Skip to content

Commit

Permalink
User table fixes (bluesky-social#536)
Browse files Browse the repository at this point in the history
* change pkey on user table to did & rename to user_account

* migration

* tidy

* fixes suggested by bryn

* missed merge thing
  • Loading branch information
dholms authored Feb 15, 2023
1 parent 807e2a7 commit ac48330
Show file tree
Hide file tree
Showing 24 changed files with 449 additions and 174 deletions.
6 changes: 3 additions & 3 deletions packages/pds/src/api/app/bsky/actor/getSuggestions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export default function (server: Server, ctx: AppContext) {
const { ref } = db.dynamic

const suggestionsQb = db
.selectFrom('user')
.innerJoin('did_handle', 'user.handle', 'did_handle.handle')
.selectFrom('user_account')
.innerJoin('did_handle', 'user_account.did', 'did_handle.did')
.innerJoin('repo_root', 'repo_root.did', 'did_handle.did')
.leftJoin('profile', 'profile.creator', 'did_handle.did')
.where(notSoftDeletedClause(ref('repo_root')))
Expand All @@ -41,7 +41,7 @@ export default function (server: Server, ctx: AppContext) {
'profile.description as description',
'profile.avatarCid as avatarCid',
'profile.indexedAt as indexedAt',
'user.createdAt as createdAt',
'user_account.createdAt as createdAt',
db
.selectFrom('post')
.whereRef('creator', '=', ref('did_handle.did'))
Expand Down
6 changes: 3 additions & 3 deletions packages/pds/src/api/app/bsky/notification/getCount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export default function (server: Server, ctx: AppContext) {
const result = await ctx.db.db
.selectFrom('user_notification as notif')
.select(countAll.as('count'))
.innerJoin('did_handle', 'did_handle.did', 'notif.userDid')
.innerJoin('user', 'user.handle', 'did_handle.handle')
.innerJoin('user_account', 'user_account.did', 'notif.userDid')
.innerJoin('user_state', 'user_state.did', 'user_account.did')
.innerJoin(
'repo_root as author_repo',
'author_repo.did',
Expand All @@ -31,7 +31,7 @@ export default function (server: Server, ctx: AppContext) {
.whereRef('did', '=', ref('notif.author'))
.where('mutedByDid', '=', requester),
)
.whereRef('notif.indexedAt', '>', 'user.lastSeenNotifs')
.whereRef('notif.indexedAt', '>', 'user_state.lastSeenNotifs')
.executeTakeFirst()

const count = result?.count ?? 0
Expand Down
14 changes: 10 additions & 4 deletions packages/pds/src/api/app/bsky/notification/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,18 @@ export default function (server: Server, ctx: AppContext) {
keyset,
})

const [user, notifs] = await Promise.all([
ctx.services.actor(ctx.db).getUser(requester),
const userStateQuery = ctx.db.db
.selectFrom('user_state')
.selectAll()
.where('did', '=', requester)
.executeTakeFirst()

const [userState, notifs] = await Promise.all([
userStateQuery,
notifBuilder.execute(),
])

if (!user) {
if (!userState) {
throw new InvalidRequestError(`Could not find user: ${requester}`)
}

Expand All @@ -96,7 +102,7 @@ export default function (server: Server, ctx: AppContext) {
reason: notif.reason,
reasonSubject: notif.reasonSubject || undefined,
record: common.cborBytesToRecord(notif.recordBytes),
isRead: notif.indexedAt <= user.lastSeenNotifs,
isRead: notif.indexedAt <= userState.lastSeenNotifs,
indexedAt: notif.indexedAt,
}))

Expand Down
4 changes: 2 additions & 2 deletions packages/pds/src/api/app/bsky/notification/updateSeen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ export default function (server: Server, ctx: AppContext) {
}

await ctx.db.db
.updateTable('user')
.updateTable('user_state')
.set({ lastSeenNotifs: parsed })
.where('handle', '=', user.handle)
.where('did', '=', user.did)
.executeTakeFirst()
},
})
Expand Down
57 changes: 28 additions & 29 deletions packages/pds/src/api/com/atproto/account/create.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { InvalidRequestError } from '@atproto/xrpc-server'
import * as crypto from '@atproto/crypto'
import * as handleLib from '@atproto/handle'
import { cidForCbor } from '@atproto/common'
import { Server, APP_BSKY_SYSTEM } from '../../../../lexicon'
Expand Down Expand Up @@ -28,10 +27,6 @@ export default function (server: Server, ctx: AppContext) {
throw err
}

// In order to perform the significant db updates ahead of
// registering the did, we will use a temp invalid did. Once everything
// goes well and a fresh did is registered, we'll replace the temp values.
const tempDid = crypto.randomStr(16, 'base32')
const now = new Date().toISOString()

const result = await ctx.db.transaction(async (dbTxn) => {
Expand Down Expand Up @@ -68,33 +63,44 @@ export default function (server: Server, ctx: AppContext) {
}
}

// Pre-register user before going out to PLC to get a real did
try {
await actorTxn.preregisterDid(handle, tempDid)
} catch (err) {
if (err instanceof UserAlreadyExistsError) {
throw new InvalidRequestError(`Handle already taken: ${handle}`)
}
throw err
// format create op, but don't send until we ensure the username & email are available
const plcCreate = await ctx.plcClient.formatCreateOp(
ctx.keypair,
recoveryKey || ctx.cfg.recoveryKey,
handle,
ctx.cfg.publicUrl,
)
const did = plcCreate.did

const declaration = {
$type: lex.ids.AppBskySystemDeclaration,
actorType: APP_BSKY_SYSTEM.ActorUser,
}

// Register user before going out to PLC to get a real did
try {
await actorTxn.registerUser(email, handle, password)
await actorTxn.registerUser(
email,
handle,
plcCreate.did,
password,
declaration,
)
} catch (err) {
if (err instanceof UserAlreadyExistsError) {
throw new InvalidRequestError(`Email already taken: ${email}`)
const got = await actorTxn.getUser(handle, true)
if (got) {
throw new InvalidRequestError(`Handle already taken: ${handle}`)
} else {
throw new InvalidRequestError(`Email already taken: ${email}`)
}
}
throw err
}

// Generate a real did with PLC
let did: string
try {
did = await ctx.plcClient.createDid(
ctx.keypair,
recoveryKey || ctx.cfg.recoveryKey,
handle,
ctx.cfg.publicUrl,
)
await ctx.plcClient.sendOperation(did, plcCreate.op)
} catch (err) {
req.log.error(
{ didKey: ctx.keypair.did(), handle },
Expand All @@ -103,13 +109,6 @@ export default function (server: Server, ctx: AppContext) {
throw err
}

// Now that we have a real did, we create the declaration & replace the tempDid
// and setup the repo root. This _should_ succeed under typical conditions.
const declaration = {
$type: lex.ids.AppBskySystemDeclaration,
actorType: APP_BSKY_SYSTEM.ActorUser,
}
await actorTxn.finalizeDid(handle, did, tempDid, declaration)
if (ctx.cfg.inviteRequired && inviteCode) {
await dbTxn.db
.insertInto('invite_code_use')
Expand Down
2 changes: 1 addition & 1 deletion packages/pds/src/api/com/atproto/account/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default function (server: Server, ctx: AppContext) {
const { did, password, token } = input.body
const validPass = await ctx.services
.actor(ctx.db)
.verifyUserDidPassword(did, password)
.verifyUserPassword(did, password)
if (!validPass) {
throw new AuthRequiredError('Invalid did or password')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export default function (server: Server, ctx: AppContext) {
const token = getSixDigitToken()
const grantedAt = new Date().toISOString()
await ctx.db.db
.updateTable('user')
.where('handle', '=', user.handle)
.updateTable('user_account')
.where('did', '=', user.did)
.set({
passwordResetToken: token,
passwordResetGrantedAt: grantedAt,
Expand Down
16 changes: 8 additions & 8 deletions packages/pds/src/api/com/atproto/account/resetPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export default function (server: Server, ctx: AppContext) {
const { token, password } = input.body

const tokenInfo = await ctx.db.db
.selectFrom('user')
.select(['handle', 'passwordResetGrantedAt'])
.selectFrom('user_account')
.select(['did', 'passwordResetGrantedAt'])
.where('passwordResetToken', '=', token)
.executeTakeFirst()

Expand All @@ -21,15 +21,15 @@ export default function (server: Server, ctx: AppContext) {
const expiresAt = new Date(grantedAt.getTime() + 15 * minsToMs)

if (now > expiresAt) {
await unsetResetToken(ctx.db, tokenInfo.handle)
await unsetResetToken(ctx.db, tokenInfo.did)
return createExpiredTokenError()
}

await ctx.db.transaction(async (dbTxn) => {
await unsetResetToken(dbTxn, tokenInfo.handle)
await unsetResetToken(dbTxn, tokenInfo.did)
await ctx.services
.actor(dbTxn)
.updateUserPassword(tokenInfo.handle, password)
.updateUserPassword(tokenInfo.did, password)
})
})
}
Expand Down Expand Up @@ -58,10 +58,10 @@ const createExpiredTokenError = (): ErrorResponse & {
message: 'The password reset token has expired',
})

const unsetResetToken = async (db: Database, handle: string) => {
const unsetResetToken = async (db: Database, did: string) => {
await db.db
.updateTable('user')
.where('handle', '=', handle)
.updateTable('user_account')
.where('did', '=', did)
.set({
passwordResetToken: null,
passwordResetGrantedAt: null,
Expand Down
5 changes: 1 addition & 4 deletions packages/pds/src/api/com/atproto/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ export default function (server: Server, ctx: AppContext) {
throw new AuthRequiredError('Invalid identifier or password')
}

const validPass = await actorService.verifyUserPassword(
user.handle,
password,
)
const validPass = await actorService.verifyUserPassword(user.did, password)

if (!validPass) {
throw new AuthRequiredError('Invalid identifier or password')
Expand Down
6 changes: 4 additions & 2 deletions packages/pds/src/db/database-schema.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Kysely } from 'kysely'
import * as user from './tables/user'
import * as userAccount from './tables/user-account'
import * as userState from './tables/user-state'
import * as didHandle from './tables/did-handle'
import * as repoRoot from './tables/repo-root'
import * as refreshToken from './tables/refresh-token'
Expand Down Expand Up @@ -28,7 +29,8 @@ import * as moderation from './tables/moderation'
import * as mute from './tables/mute'
import * as repoSeq from './tables/repo-seq'

export type DatabaseSchemaType = user.PartialDB &
export type DatabaseSchemaType = userAccount.PartialDB &
userState.PartialDB &
didHandle.PartialDB &
refreshToken.PartialDB &
repoRoot.PartialDB &
Expand Down
Loading

0 comments on commit ac48330

Please sign in to comment.