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

feat: adds type support for JWK formatted public keys #80

Merged
merged 2 commits into from
Feb 26, 2021
Merged

feat: adds type support for JWK formatted public keys #80

merged 2 commits into from
Feb 26, 2021

Conversation

elmariachi111
Copy link
Contributor

e.g. used in did:elem
my linter has "fixed" the comment indentation ;)
JsonWebKey type has been adapted by lib.dom (used in browsers)

Copy link
Member

@mirceanis mirceanis left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

I have some reservations about listing the JWK members that are generally used to encode private or symmetric key material.
Also, I suggest making the data type extensible, with a [x: string]: any catch-all


On a style note, since this interface may be used by IDEs to provide auto-complete, it may be nice to see some known string patterns for use and key_ops. For example:
use?: "enc" | "sig" | string
On the other hand, this looks like overkill :)
What do you think?

src/resolver.ts Outdated Show resolved Hide resolved
src/resolver.ts Outdated Show resolved Hide resolved
src/resolver.ts Outdated Show resolved Hide resolved
src/resolver.ts Show resolved Hide resolved
src/resolver.ts Outdated Show resolved Hide resolved
@elmariachi111
Copy link
Contributor Author

Hey @mirceanis I totally agree to your suggestions ;) Actually I'm myself not really familiar with the JWK specs (so, thanks a lot for that input) but rather just copied them from what my Visual Studio Code suggested - there's a generic typed JsonWebKey in lib.dom.d.ts that I just pasted here:

image

But if you'd like, I'll happily add your suggestions to it ;) I particularly would go with the generic [x: string]: any addition because it allows maximum flexibility...

removes private key related props
adds a generic JsonWebKey prop

Co-authored-by: Mircea Nistor <[email protected]>
Copy link
Member

@mirceanis mirceanis left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@mirceanis mirceanis merged commit f9b9c8d into decentralized-identity:master Feb 26, 2021
uport-automation-bot pushed a commit that referenced this pull request Feb 26, 2021
# [2.2.0](2.1.2...2.2.0) (2021-02-26)

### Features

* add type definition for JWK formatted public keys ([#80](#80)) ([f9b9c8d](f9b9c8d))
@uport-automation-bot
Copy link
Collaborator

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@uport-automation-bot uport-automation-bot added the spec more parties need to agree label Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec more parties need to agree
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants