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

Changed userHandle behavior v10 breaks definitely with existing registrations #563

Closed joostdebruijn closed 7 months ago

joostdebruijn commented 7 months ago

PR #552 breaks with credentials that already exists and I think it's impossible to upgrade those existing applications to v10 due to this change.

I've already credentials with a userHandle (created with the PHP-framework web-auth/webauthn-framework) that is encoded with base64url. The v10 implementation of startRegistration() will decode that with base64url, but it did not do that in the past (v9 and earlier) - therefore the existing authenticators have stored the base64url-encoded value.

Now, when I receive a response from an already existing authenticator the base64-encoded userHandle provided by the authenticator is encoded again by startAuthentication() resulting in a double encoded string. However, in the backend the userHandle is just decoded once. This results in a validation error, because the userHandle is not matching with what I'd expect.

For new registrations this will work, because the authenticator will then store the decoded value, which will be encoded by startAuthentication() again and will be decoded by the application backend.

I've studied the upgrade documentation, but implementing the refactor guidance will only work for new implementations. Or do I overlook something?

MasterKale commented 7 months ago

Hello @joostdebruijn, for the record I only officially support issues with combined use of @simplewebauthn/browser and @simplewebauthn/server - the fact that browser can work with others server libraries is a happy coincidence.

That said, can you solve this with an additional base64url encode of userID, and adding an initial base64url decoding of userHandle after auth? These operations could happen on the back end.

Alternatively, this could be handled on the front end, before calling startRegistration() and startAuthentication(), so that your server receives bytes in the expected order. But I'd say you have less flexibility to do this in the front end because of fewer built-in library tools to handle this.

joostdebruijn commented 7 months ago

Hi @MasterKale! Thanks for your answer, I know you don't support interaction with other frameworks, but the browser library is really easy to implement. đŸ˜„ I really like it!

However, I see that the server package isn't validating the userHandle during an authentication request and therefore this change is indeed not breaking existing implementations in a definite way when using the browser and server combination. An extra decoding step (when needed for the given credential) in the backend seems to be to most logical the solution or I should an implementation for the frontend myself (or find another one elsewhere). For now, I just stick with v9. đŸ˜‰ Thanks again!