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

Fix did:web encoding/decoding #3454

Merged
merged 3 commits into from
Jan 27, 2025
Merged

Fix did:web encoding/decoding #3454

merged 3 commits into from
Jan 27, 2025

Conversation

matthieusieben
Copy link
Contributor

Current implementation does not properly encode/decode did webs as per spec:

https://w3c-ccg.github.io/did-method-web/#read-resolve

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.

A change like this seems like a good opportunity to fill out some unit tests. Could we add some as part of this?

@matthieusieben
Copy link
Contributor Author

Good point. I added some tests in.

@matthieusieben matthieusieben merged commit cc2a122 into main Jan 27, 2025
10 checks passed
@matthieusieben matthieusieben deleted the msi/did-fixes branch January 27, 2025 00:06
@github-actions github-actions bot mentioned this pull request Jan 26, 2025
@bnewbold
Copy link
Collaborator

bnewbold commented Feb 2, 2025

didn't see this when it first went by, but note that we have a bunch of atproto-specific restrictions on did:web in the specs:

In the context of atproto, only hostname-level did:web DIDs are supported: path-based DIDs are not supported. The same restrictions on top-level domains that apply to handles (eg, no .arpa) also apply to did:web domains. The special localhost hostname is allowed, but only in testing and development environments. Port numbers (with separating colon hex-encoded) are only allowed for localhost, and only in testing and development.

https://atproto.com/specs/did

I'm pretty confident about those restrictions, and that we don't want to support the full did:web functionality. I don't have a direct citation, but some W3C DID folks have also expressed that path-based did:web isn't particularly safe or recommended, and that did:web is still just a draft, so there isn't very strong pressure to implement it exactly as described in the draft doc.

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