MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.62k stars 137 forks source link

Add `expectedType` for verifyAuthenticationResponse and verifyRegistrationResponse #436

Closed fabiancook closed 1 year ago

fabiancook commented 1 year ago

This enables the functionality for authentication types like payment.get

This would be used in this code right here: https://github.com/opennetwork/logistics/blob/7a305ea1a51d2276e2b77ebbc8be42e80c361893/src/listen/auth/webauthn.ts#L593C9-L604C12

const {verified, authenticationInfo} = await verifyAuthenticationResponse({
  response: payment.details,
  expectedChallenge,
  expectedOrigin: WEBAUTHN_RP_ORIGIN || origin,
  expectedRPID: WEBAUTHN_RP_ID || hostname,
  expectedType: "payment.get",
  authenticator: {
    credentialID: new Uint8Array(toArrayBuffer(found.credentialId, true)),
    credentialPublicKey: new Uint8Array(toArrayBuffer(found.credentialPublicKey, true)),
    transports: found.authenticatorTransports,
    counter: found.credentialCounter ?? 0
  }
});

Related to https://github.com/MasterKale/SimpleWebAuthn/discussions/402

This isn't complete support for secure payment confirmation, but its the only runtime level change needed. Some types need to be widened to allow additional extensions.

MasterKale commented 1 year ago

Hey, regarding the issues you're having getting tests working, I discovered a bootstrapping issue with the monorepo related to how workspaces are set up. I needed to explicitly perform a special sequence of commands to get root dependencies installed which make it possible to actually build the various packages after a fresh clone that will ultimately get pnpm to understand the dnt-built workspace folders are in place for symlinking.

tl;dr: I just pushed up a new pnpm command to the master branch that you can run to hopefully get tests working locally:

pnpm run bootstrap-monorepo

Give it a try and let me know what you think

fabiancook commented 1 year ago

Upgrading deno and running that command worked :)

Tests are now running locally, but I do get a couple of failures, and it seems I'm running into some changes from https://github.com/MasterKale/SimpleWebAuthn/blob/master/packages/server/deno.lock that my local deno must be looking to change.

fabiancook commented 1 year ago

Added my tests:

should throw an error if type not the expected type ... ok (3ms)
should throw an error if type not in list of expected types ... ok (3ms)

should throw when response type is not expected value ... ok (3ms)
should throw when response type is not in list of expected types ... ok (3ms)

should validate when attestation type is not webauthn.create and expected type provided ... ok (3ms)

Locally I get these couple errors:

should verify Android KeyStore response ... FAILED (11ms)
should validate Android-Key response ... FAILED (6ms)

error: Error: Cannot get schema for 'n' target
    at U.get (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:5714)
    at Function.fromASN (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:8533)
    at Function.parse (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:8441)
    at verifyAttestationAndroidKey (file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts:77:39)
    at verifyRegistrationResponse (file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifyRegistrationResponse.ts:255:22)
    at eventLoopTick (ext:core/01_core.js:183:11)
    at async file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifications/verifyAttestationAndroidKey.test.ts:16:24
should validate Android-Key response => ./src/registration/verifyRegistrationResponse.test.ts:596:6
error: Error: Cannot get schema for 'n' target
    at U.get (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:5714)
    at Function.fromASN (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:8533)
    at Function.parse (https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/deno/asn1-schema.mjs:2:8441)
    at verifyAttestationAndroidKey (file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts:77:39)
    at verifyRegistrationResponse (file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifyRegistrationResponse.ts:255:22)
    at eventLoopTick (ext:core/01_core.js:183:11)
    at async file:///Users/fabiancook/src/SimpleWebAuthn/packages/server/src/registration/verifyRegistrationResponse.test.ts:598:24
MasterKale commented 1 year ago

@fabiancook I regenerated server's Deno lock file which should resolve those issues you're seeing when running tests. Try rebasing off of the latest master branch and see if it fixes the problem on your end too.

fabiancook commented 1 year ago

I can see the tests run successfully for me as part of pnpm run bootstrap-monorepo but not when I run pnpm run test directly

> @simplewebauthn/server@8.1.1 test
> node test_runner.js

See:

image

But then for pnpm run test:

image

deno --version
deno 1.36.4 (release, aarch64-apple-darwin)
v8 11.6.189.12
typescript 5.1.6
node --version
v20.6.1

Can see deno v1.37.0 popped up 2 days ago

running it with the updated version of:

deno --version
deno 1.37.0 (release, aarch64-apple-darwin)
v8 11.8.172.3
typescript 5.2.2

pnpm run test

> simplewebauthn-monorepo@0.0.0 test /Users/fabiancook/src/virtualstate/SimpleWebAuthn
> pnpm run test:browser; pnpm run test:server

> simplewebauthn-monorepo@0.0.0 test:browser /Users/fabiancook/src/virtualstate/SimpleWebAuthn
> lerna run test --scope=@simplewebauthn/browser

lerna notice cli v7.1.5
lerna notice filter including "@simplewebauthn/browser"
lerna info filter [ '@simplewebauthn/browser' ]

> @simplewebauthn/browser:test  [existing outputs match the cache, left as is]

> @simplewebauthn/browser@8.0.2 test /Users/fabiancook/src/virtualstate/SimpleWebAuthn/packages/browser
> jest

 PASS  src/index.test.ts
 PASS  src/helpers/browserSupportsWebAuthn.test.ts
 PASS  src/methods/startRegistration.test.ts
 PASS  src/helpers/webAuthnAbortService.test.ts
 PASS  src/helpers/platformAuthenticatorIsAvailable.test.ts
 PASS  src/methods/startAuthentication.test.ts

Test Suites: 6 passed, 6 total
Tests:       2 skipped, 59 passed, 61 total
Snapshots:   0 total
Time:        2.465 s, estimated 3 s
Ran all test suites.

 ————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  Lerna (powered by Nx)   Successfully ran target test for project @simplewebauthn/browser (23ms)

   Nx read the output from the cache instead of running the command for 1 out of 1 tasks.

> simplewebauthn-monorepo@0.0.0 test:server /Users/fabiancook/src/virtualstate/SimpleWebAuthn
> lerna run test --scope=@simplewebauthn/server

lerna notice cli v7.1.5
lerna notice filter including "@simplewebauthn/server"
lerna info filter [ '@simplewebauthn/server' ]

> @simplewebauthn/server:test

> @simplewebauthn/server@8.1.1 test /Users/fabiancook/src/virtualstate/SimpleWebAuthn/packages/server
> deno test -A src/
error: The source code is invalid, as it does not match the expected hash in the lock file.
  Specifier: https://esm.sh/@peculiar/asn1-android@2.3.6
  Lock file: /Users/fabiancook/src/virtualstate/SimpleWebAuthn/packages/server/deno.lock
 ELIFECYCLE  Test failed. See above for more details.

 ————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  Lerna (powered by Nx)   Ran target test for project @simplewebauthn/server (685ms)

    ✖    1/1 failed
    ✔    0/1 succeeded [0 read from cache]

 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Test failed. See above for more details.
 ```
 
 Still fails. Is something off with how I am installing it? I can see CI passes... 
MasterKale commented 1 year ago

Hey @fabiancook, I pulled down your branch and rebased on the latest master, and the tests passed just fine:

should throw when response type is not expected value ... ok (5ms)
should throw when response type is not in list of expected types ... ok (4ms)
...
should throw an error if type not the expected type ... ok (4ms)
should throw an error if type not in list of expected types ... ok (5ms)

CI is happy too so I think this is good to go.

MasterKale commented 1 year ago

This is now available in the newly published @simplewebauthn/server@8.2.0 ✌️