Facilitate authing w/ PDS based on DID doc #1727
Conversation
| @@ -36,7 +36,8 @@ | |||
| "@atproto/xrpc": "workspace:^", | |||
| "multiformats": "^9.9.0", | |||
| "tlds": "^1.234.0", | |||
| "typed-emitter": "^2.1.0" | |||
| "typed-emitter": "^2.1.0", | |||
| "zod": "^3.21.4" | |||
There was a problem hiding this comment.
This isn't an addition to the api because its dependencies already depended on it
packages/api/src/did/did-doc.ts
Outdated
| export function getPdsEndpoint(doc: unknown): URL | undefined { | ||
| if (isValidDidDoc(doc)) { | ||
| const pds = doc.service?.find((s) => s.type === 'AtprotoPersonalDataServer') | ||
| if (pds && typeof pds.serviceEndpoint === 'string') { |
There was a problem hiding this comment.
Note: this is currently taking the first PDS service entry and only handling the string value. I don't know what a non-string value would look like.
There was a problem hiding this comment.
yeah that'd be considered a valid json-ld did document, but not supported by atproto rn
this is similar to how we validate services in our identity resolver 👍
|
@devinivy I tested by fudging some responses but I don't have a great way to test this meaningfully or in the test suite, and I'm only somewhat confident that it's correct. We either need to cook up good automated tests or be really aggressive about testing in staging scenarios before deploying. |
packages/api/src/agent.ts
Outdated
| @@ -157,6 +165,7 @@ export class AtpAgent { | |||
| this.session.email = res.data.email | |||
| this.session.handle = res.data.handle | |||
| this.session.emailConfirmed = res.data.emailConfirmed | |||
| this._updateApiEndpoint(res.data.didDoc) | |||
There was a problem hiding this comment.
Oops, missed this in original review. This endpoint getSession doesn't include the did doc—if the credentials are no longer valid due to having migrated PDSes, the client will receive an ExpiredToken error and get set back on track via the refresh flow.
| this._updateApiEndpoint(res.data.didDoc) |
packages/api/src/did/did-doc.ts
Outdated
| @@ -0,0 +1,40 @@ | |||
| import { z } from 'zod' | |||
|
|
|||
| export const verificationMethod = z.object({ | |||
There was a problem hiding this comment.
hmmm we already have something very similar this over in @atproto/identity - we may want to just use the code there instead of duplicating it over to the api package 🤔
There was a problem hiding this comment.
or if the client doesn't agree with the dependencies of that package, porting just these types & verification methods over to @atproto/common-web
packages/api/src/did/did-doc.ts
Outdated
|
|
||
| export function getPdsEndpoint(doc: unknown): URL | undefined { | ||
| if (isValidDidDoc(doc)) { | ||
| const pds = doc.service?.find((s) => s.type === 'AtprotoPersonalDataServer') |
There was a problem hiding this comment.
in @atproto/identity, we find the first service entry with id #atproto_pds & then check the type
|
@dholms took a quick swing at addressing your feedback. |
|
@devinivy if you dont mind just making any changes you think are needed, seems minor |
|
@pfrazee yup I got it from here! |
packages/api/src/did/did-doc.ts
Outdated
| @@ -0,0 +1,24 @@ | |||
| import { DidDocument, didDocument } from '@atproto/common-web' | |||
|
|
|||
| export function isValidDidDoc(doc: unknown): doc is DidDocument { | |||
There was a problem hiding this comment.
not required, but I think we could take this one step further & move this to common-web as well
* lexicon for did doc w/ auth credentials * include did doc w/ session when configured. configure on dev-env. * Add dynamic PDS URL adoption to the client * remove usage of did doc field from getsession in client * dry-up did doc type and validation * remove explicit dep on zod by identity package * move more did doc parsing to common-web * go back to strings * rollback breaking changes to identity package * add changeset --------- Co-authored-by: Paul Frazee <[email protected]> Co-authored-by: dholms <[email protected]>
didDocoutput to lexicons used to create or refresh a session.didDocon those lexicon methods.