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

Developer should be able to specify pubKeyCredParams to support. #64

Closed
agektmr opened this issue Oct 19, 2020 · 3 comments
Closed

Developer should be able to specify pubKeyCredParams to support. #64

agektmr opened this issue Oct 19, 2020 · 3 comments
Labels
package:server @simplewebauthn/server

Comments

@agektmr
Copy link
Contributor

agektmr commented Oct 19, 2020

Currently the library is passing all potential algorithms to pubKeyCredParams in the PublicKeyCredentialCreationOptions but it should be selectable by the RP because some authenticators reject when unsupported algorithm is passed.

@MasterKale
Copy link
Owner

generateAttestationOptions()'s optional supportedAlgorithmIDs argument is the current mechanism for specifying a custom array of supported algorithm identifiers for pubKeyCredParams. It's true it defaults to all current algorithms, but if you specify an array of algorithm ID's for supportedAlgorithmIDs those'll get used instead.

Does this work for your needs?

@MasterKale MasterKale added the package:server @simplewebauthn/server label Oct 19, 2020
@agektmr
Copy link
Contributor Author

agektmr commented Oct 20, 2020

Ah, I overlooked it. Yes, I think this works fine.

This is an API design decision you'd make, but if I'd suggest my idea arrogantly, I would make the API as much as the same as PublicKeyCredentialCreationOptions and PublicKeyCredentialRequestOptions. In this issue's case, I'd make the API the same way as pubKeyCredParams so developers will have one less new API to learn to adopt this library. Same for other properties. I'd accept even if it will be a breaking change.

Anyway, kudos to your great work!

@agektmr agektmr closed this as completed Oct 20, 2020
@MasterKale
Copy link
Owner

@agektmr Thank you for the feedback, I sincerely appreciate it. The goal of the library has always been to make implementing WebAuthn as simple as possible, but I'll be the first to admit there'll always be room for improvement.

Based on your comment above, I understand how complex I made things by trying to simplify how you specify algorithm IDs for pubKeyCredParams. I'm going to review the APIs of both generate...Options() methods and try to draw a more distinct line between what I simplify and what I expose more directly. I'm thinking there'll be a clear line between "typical use" and "advanced use" that I'll use to not simplify WebAuthn options that are for more advanced use cases.

Please stay tuned 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:server @simplewebauthn/server
Projects
None yet
Development

No branches or pull requests

2 participants