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

Improve ergonomics of excludeCredentials and allowCredentials #528

Closed
MasterKale opened this issue Feb 23, 2024 · 3 comments · Fixed by #529
Closed

Improve ergonomics of excludeCredentials and allowCredentials #528

MasterKale opened this issue Feb 23, 2024 · 3 comments · Fixed by #529
Milestone

Comments

@MasterKale
Copy link
Owner

Describe the issue

A case has been made in #524 to allow excludeCredentials in generateRegistrationOptions() and allowCredentials in generateAuthenticationOptions() to accept base64url-encoded strings in addition to Uint8Array's:

As discussed here and here, even though we are storing credentialID as a base64Url, the generateAuthenticationOptions function requires it to be a buffer, and then converts it back to a base64Url.

...This allows users who are storing credentialID as a base64Url (which is recommended), to pass it in directly to generateAuthenticationOptions without having to convert it to a buffer first.

After thinking about this a bit more I've come around to the idea. I agree that it would make these methods easier to use if credential IDs, otherwise represented as base64url-encoded values in WebAuthn, could remain strings from start to finish (at least with respect to calling the various methods in this project to leverage WebAuthn.)

It may even make sense to break the API here and only support use of base64url-encoded strings, for sake of a simpler library API 🤔

h/t @H3RSKO for the idea

@MasterKale
Copy link
Owner Author

This allows users who are storing credentialID as a base64Url (which is recommended)

Digging into this a bit, I can see how I might have been causing confusion within the docs:

Screenshot 2024-02-23 at 8 05 09 AM

The docs have suggested that RP devs base64url-encode credentialID all this time, so why not save them a step? In retrospect credentialID should have always been a string 🤔

@MasterKale
Copy link
Owner Author

I went digging a bit and this idea potentially involves a reverting of some changes made in #97 way back in February 2021 to emphasize more use of Buffer over base64url-encoded strings 🤯

@MasterKale MasterKale added this to the v10.0.0 milestone Apr 12, 2024
@MasterKale
Copy link
Owner Author

This change is now available in the recently-published @simplewebauthn/server@10.0.0 ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant