kanidm / webauthn-rs

An implementation of webauthn components for Rustlang servers
Mozilla Public License 2.0
483 stars 80 forks source link

Unable to get 1Password to save passkeys for axum example #351

Closed alexhumphreys closed 11 months ago

alexhumphreys commented 11 months ago

I did this

cd webauthn-rs/tutorial/server/axum
cargo run --no-default-features --features javascript

Then navigated to http://localhost:8080/ on firefox (version 118.0 (64-bit)) with the 1password extension installed. Entered foobar in the field and clicked register. 1password popup appears, I select a vault and click "save".

I also did the same thing on safari with the 1password extension installed there.

I expected the following

1password to save the passkey, as it does for example on https://webauthn.io/

What actually happened

In both safari and firefox I got the following error:

Screenshot 2023-09-27 at 01 52 02

There's the error in the Firefox inspector console:

passkey-save-prompt received error: create-passkey-failed [notification.js:2:3560034](moz-extension://4fb6bc19-00d5-4be3-8d2d-8bcde1eb2f6c/inline/notification/notification.js)

I tailed the various logs in ~/Library/Group\ Containers/2BUA8C4S2C.com.1password/Library/Application\ Support/1Password/Data/logs but did not see any interesting messages.

Version (and git commit)

https://github.com/kanidm/webauthn-rs/commit/840a6f5cc7f65bd3c585ab00293addf76f70d973

Operating System / Version

MacOS Ventura 13.5.1 (22G90) Firefox 118.0 (64-bit) 1Password for Mac 8.10.16 (81016047) 1Password in the browser 2.15.1 1Password for safari 2.15.1

Any other comments

I was able to successfully log in on Safari (using presumably the apple keychain) with the same server running, so it seems to be a 1Password issue.

Which of course could mean it's out of your hands and I need to open a ticket with them, but the fact that https://webauthn.io/ works suggests something may be up here.

Firstyear commented 11 months ago

On webauthn.io, can you open "advanced" settings, and on discoverable credential select "discouraged" and try again?

webauthn-rs does the right thing and sets this to discouraged trying to make rk/discoverable as opportunistic, but this might be tripping up 1password.

If that causes webauthn.io to "fail" in the same way, then it's a bug in 1password's handling of discoverable credentials (they CAN and are allowed to make rk/discoverable even under discouraged).

If that doesn't trigger the error, then it will be something else, likely that we are asking for credProtect. Either way I think you're right, it's a 1password error.

alexhumphreys commented 11 months ago

Thanks for the fast reply! I tried again with "discoverable credential" set to "discouraged" and it was still successful. Let me know if I can help test the credProtect theory!

Screenshot 2023-09-27 at 08 53 12
Firstyear commented 11 months ago

Can you try this: https://webauthn.me/debugger ?

You need to check "attestation: direct", "extensions" and "credentialProtectionPolicy: userVerificationRequired". do NOT check "enforce credential protection policy".

Also check "authenticatorSelection" and "userVerification: required".

alexhumphreys commented 11 months ago

Didn't know about https://webauthn.me/debugger, thanks for the heads up!

Unfortunately I can't get 1password to even prompt me to save on https://webauthn.me/debugger, I tried with the settings you recommended and also just the default settings it starts with. On both safari and firefox, only the browser popup appears, not the 1password one.

Even stranger still, I was using "1Password in the browser", but noticed there's also a "1Password in the browser _beta", and the release notes for a recent version of that include:

We’ve fixed an issue with saving and using passkeys on webauthn.me/debugger and CVS.

So I've now tested with the latest "1Password in the browser beta" and the popup also doesn't appear there! This does increase the probability this is a 1password issue...

Firstyear commented 11 months ago

Honestly, I'm a bit stumped now too tbh. But I think you are correct, it's tending to a 1password problem. I'm just not sure what to look at next to help proceed this. I might need to setup 1password myself and try it locally. I won't get to it for a few days though sorry :(

alexhumphreys commented 11 months ago

If you could test with 1Password that'd be great, but there's also no hurry, this is already a lot of quick feedback on an issue!

In the meantime I've contacted 1password support and described both this issue and the issues with https://webauthn.me/debugger, so hopefully they have some input on what's happening 👍

Firstyear commented 11 months ago

Let me know their response from that :)

alexhumphreys commented 11 months ago

So 1Password seem to have pretty good support!

They found a regression with allowing webauthn requests from localhost and have fixed that, and also pushed a fix for https://webauthn.me/debugger on the nightly Chrome browser extension, so should arrive on Firefox too.

They also mentioned a bug in this library, and gave the following details:

However, there still is an issue with webauthn-rs in their excludeCredentials mapping where they forget to change the cred ID from Base64Url to ArrayBuffer.

I tried eyeballing the code here and didn't get very far since my Rust skills are pretty weak, but maybe that means something to you?

Progdrasil commented 11 months ago

Hi @alexhumphreys and @Firstyear. As you mentionned the fixes should be hitting the next beta, but you should be able to test using our nightly extension on Chrome.

I pushed #361 to fix the issue with the id's not being converted in the Javascript code.

alexhumphreys commented 11 months ago

@Progdrasil thanks for getting back to us here!

I just tried your PR with chrome and the nightly 1Password (and running the server with cargo run --no-default-features --features javascript), and I get the following error:

auth.js:15 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'forEach')
at auth.js:15:64

Looking at the line that was added in the inspector, credentialCreationOptions.publicKey doesn't have a excludeCredentials defined. I'm guessing that since this is of type Option here it's not always defined.

Progdrasil commented 11 months ago

Doh 🤦. You're absolutely right. I just pushed an update to the PR using optional chaining. I also added it to the allowCredentials list since it's technically optional even though the demo requires a username to be present to look up the key ids.

alexhumphreys commented 11 months ago

Nice one! With those changes on your PR I was able to successfully register/login with Chrome and nightly 1Password. Thank you!

Firstyear commented 11 months ago

We should just fix the improperly converted id's on our end though. :)

Firstyear commented 11 months ago

I think we the PR that @Progdrasil made, we can close this. Thanks everyone!

wez commented 9 months ago

@alexhumphreys have you gotten this to work with firefox yet? I'm experiencing the same issue. It looks like the firefox extension is lagging over ~a month~ 3 months behind the chrome version? Do you have link(s) to support cases I can comment on?

(edit: it's actually 3 months behind!)

Firstyear commented 9 months ago

@wez We haven't released an updated version of webauthn with the fix, might be your issue here. We plan to do so soon for 0.5.0.

wez commented 9 months ago

I thought the only fixes here were in the javascript; I'm running with the js from the current main branch. It works in chrome with no issues.

Firstyear commented 9 months ago

We ship wasm bindings as well which are part of this library and needed updates IIRC.

wez commented 9 months ago

The 1P folks are working on pushing an update; meanwhile I've confirmed that the beta version of the 1P extension works in FF. https://twitter.com/1Password/status/1732361249130783195

Firstyear commented 9 months ago

Cool, I think beside us finalising the next release this is done then.