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

Bring your own DID #1011

Merged
merged 5 commits into from
May 12, 2023
Merged

Bring your own DID #1011

merged 5 commits into from
May 12, 2023

Conversation

dholms
Copy link
Collaborator

@dholms dholms commented May 9, 2023

This allows you to bring your own DID document on account creation.

When you bring your own DID document, we attempt to resolve it & ensure that the data within matches up with the intended values. Namely:

  • handle matches requested handle
  • pds service endpoint matches the service endpoint you're attempting account creation on
  • atproto key matches the server's repoSigningKey
  • if it is a PLC DID, then we ensure the server's plcRotationKey is included in the documents rotation keys

Buyer beware: we do not do that last check for did:web. If you are using a did:web DID, it is your obligation to keep your DID document up to date with the service you're using - otherwise your data may no longer be indexed.

In service of: #1001

@dholms dholms marked this pull request as ready for review May 9, 2023 17:48
'DID document pds endpoint does not match service endpoint',
'PoorlyFormattedDidDoc',
)
} else if (atpData.signingKey !== ctx.repoSigningKey.did(0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can go, may actually be responsible for CI complaining.

Suggested change
} else if (atpData.signingKey !== ctx.repoSigningKey.did(0)) {
} else if (atpData.signingKey !== ctx.repoSigningKey.did()) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hah yup i actually had that change locally but not saved 😅

{"name": "UnsupportedDomain"}
{"name": "UnsupportedDomain"},
{"name": "CouldNotResolveDid"},
{"name": "PoorlyFormattedDidDoc"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the total nit, but I'm not feeling 100% sure about this error name. My understanding is that this is used when there's a valid did doc but it's not compatible with the PDS that the account is being created on. What if it were something like IncompatibleDidDoc? Promise I'm not going to get hung-up on this if you prefer it as-is 😆

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup very in support. i didn't like the name even as i was writing it 😅

Copy link
Collaborator

@devinivy devinivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sweet!

@dholms dholms merged commit 5804716 into main May 12, 2023
@dholms dholms deleted the byo-did branch May 12, 2023 21:18

if (input.did.startsWith('did:plc')) {
const data = await ctx.plcClient.getDocumentData(input.did)
if (!data.rotationKeys.includes(ctx.plcRotationKey.did())) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be a good idea.
If the reason a user is bringing their own did is that they don't want the PDS to be able to update their did:plc this will prevent them from using this PDS.

There is a real use case of I want to manage my own DID and just use the PDS to manage the repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we will loosen this in the future, but I think it's important for now

If the PDS does not have the ability to rotate documents, then

  • it cannot help out with handle changes
  • if it rotates it's repo signing key, it can no longer publish records on a repo's behalf

This should be allowed at some point, but for now, for usability's sake, we constrain the provided DID

mloar pushed a commit to mloar/atproto that referenced this pull request Sep 26, 2023
* allow bringing your own did

* tests + tidy

* one more check/test

* fix typo

* better err names
mloar pushed a commit to mloar/atproto that referenced this pull request Nov 15, 2023
* allow bringing your own did

* tests + tidy

* one more check/test

* fix typo

* better err names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants