-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add webauthn-pcd
package
#134
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would also be great if you could pipe this all the way through consumer-client
to show it working end-to-end!
45ea552
to
d0fccda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting very close, I'm excited for this one!
README.md
Outdated
@@ -123,6 +123,7 @@ Some of these packages are used to share development configuration between the d | |||
- [`@pcd/semaphore-group-pcd`](packages/semaphore-group-pcd): a pcd which wraps the [Semaphore](https://semaphore.appliedzkp.org/docs/introduction) protocol, which allows PCD-consuming applications to consume and generate Semaphore proofs. | |||
- [`@pcd/semaphore-identity-pcd`](packages/semaphore-identity-pcd): a 'self-evident' PCD, representing the public and private components of a Semaphore identity | |||
- [`@pcd/semaphore-signature-pcd`](packages/semaphore-signature-pcd): like `@pcd/semaphore-group-pcd`, but with a more specific purpose of using the semaphore protocol to 'sign' a particular string message on behalf of a particular revealed commitment id. | |||
- [`@pcd/webauthn-pcd`](packages/webauthn-pcd): a pcd that can be used to attest to a valid attestatinos from [WebAuthn](https://webauthn.guide/) hardware devices, such as facial scanners, fingerprints, Yubikeys, and more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a typo here: attestatinos
also maybe a better description could be something like 'a pcd that can be used to make claims about WebAuthn attestations ...'
apps/consumer-client/package.json
Outdated
@@ -16,8 +16,11 @@ | |||
"@pcd/pcd-types": "0.4.2", | |||
"@pcd/semaphore-group-pcd": "0.4.2", | |||
"@pcd/semaphore-identity-pcd": "0.4.2", | |||
"@pcd/webauthn-pcd": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to use the actual package version
@@ -149,3 +160,55 @@ async function addIdentityPCD() { | |||
|
|||
sendPassportRequest(url); | |||
} | |||
|
|||
async function addWebAuthnPCD() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
verbosely commenting this function and each step may be nice
@@ -0,0 +1 @@ | |||
export default function Page() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this file
apps/passport-client/package.json
Outdated
@@ -19,6 +19,7 @@ | |||
"@pcd/pcd-types": "0.4.2", | |||
"@pcd/semaphore-group-pcd": "0.4.2", | |||
"@pcd/semaphore-identity-pcd": "0.4.2", | |||
"@pcd/webauthn-pcd": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the correct package version
packages/webauthn-pcd/package.json
Outdated
"@types/expect": "^24.3.0", | ||
"@types/json-bigint": "^1.0.1", | ||
"@types/mocha": "^10.0.1", | ||
"@types/sinon": "^10.0.14", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a consequence of the problem described in the above comment, @types/
packages and typescript
itself should be a dependency rather than a dev dependency.
<p> | ||
This PCD represents a signature proof in the context of a WebAuthn | ||
credential. In other words, this is a ZK proof that a particular | ||
credential keypair signed a particular challenge. | ||
</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
/** | ||
* The WebAuthn credential information associated with this PCD. | ||
* Storing bytes as Base64 encoded string to ensure serializability. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to note is that in order for a PCD to be serializable, the claim
an proof
don't actually have to be trivially serializable - since serializing and deserializing them is taken care of the correspondingly named functions.
name: WebAuthnPCDTypeName, | ||
prove, | ||
verify, | ||
serialize, | ||
deserialize, | ||
renderCardBody: WebAuthnCardBody, | ||
getDisplayOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 👍
packages/webauthn-pcd/tsconfig.json
Outdated
@@ -0,0 +1,5 @@ | |||
{ | |||
"extends": "@pcd/tsconfig/server.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should extend @pcd/tsconfig/ts-library.json
instead
Also, you should rebase to fix merge conflicts. |
059ab37
to
a3f4db4
Compare
I'm getting a test failure on this PR. Please make sure to run all three of
Before submitting a PR
|
ef09b6e
to
51ece36
Compare
Sorry should be fixed now! Ended up using jest over mocha because mocha had issues with using / mocking ESM libraries (e.g. @simplewebauthn/browser) -- hope that's all good. Build and lint also pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good now!!! Finally :D
Closes #114
Demo:
Screen.Recording.2023-04-23.at.11.51.38.PM.mov