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

webauthn-rs command line #414

Merged
merged 20 commits into from
Apr 8, 2024
Merged

webauthn-rs command line #414

merged 20 commits into from
Apr 8, 2024

Conversation

arthurgleckler
Copy link
Contributor

Fixes # This pull request implements a new feature, not a fix.

  • cargo test has been run and passes
  • documentation has been updated with relevant examples (if relevant)

This pull request implements the basic webauthn-rs command line that I mentioned in the discussion of issue #397. The idea is that this wrapper can be used for debugging and experimentation. It can also be used to implement webauthn in programming languages other than Rust. This is simplified by passing JSON to and from the command-line process rather than using a foreign function interface.

If you run make under tutorial/server/cli/, you'll see the result of running the command line for the two steps of passkey registration and the two steps of authentication. It's modeled after the tutorial/server/tide/ example.

Using this wrapper, I've implemented, in some code that is still private, a webauthn registration and login system in the programming language Scheme, with a Javascript front end. I've tested it with both a Yubikey 5C key and a Pixel phone passkey.

I've included minimal documentation in the comments, but I'm happy to add more if you think this pull request is a good idea.

Thank you very much for making webauthn-rs available. Without it, implementing webauthn for a Scheme front end would have been vastly more work.

Copy link
Member

@Firstyear Firstyear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micolous what do you think?

Cargo.toml Outdated Show resolved Hide resolved
Firstyear
Firstyear previously approved these changes Jan 17, 2024
@Firstyear
Copy link
Member

@micolous If you have some item to review as well, that'd be great.

@arthurgleckler
Copy link
Contributor Author

Okay, I've run rust fmt. Sorry, I'm a novice Rustacean.

@Firstyear
Copy link
Member

Its okay, that's what the checks are for. You've done pretty well for a first time :)

@arthurgleckler
Copy link
Contributor Author

Please let me know if there's anything else I should do. Thanks.

@Firstyear
Copy link
Member

Please let me know if there's anything else I should do. Thanks.

@micolous has just been really busy is all :) I'll ask him again to have a look.

@arthurgleckler
Copy link
Contributor Author

Just pinging again. I'd love to write about this on my blog. I'm sure that there are other people who could make use of this. Perhaps I'm flattering myself, but I think a JSON-based API like this with your excellent underlying Rust implementation could allow other programming languages to implement Webauthn more quickly. Webauthn adoption isn't moving as quickly as it should.

Thanks, and sorry if I'm being a pest. I know that everyone's busy.

@Firstyear
Copy link
Member

Oh I'm so sorry this fell by the wayside!

Copy link
Collaborator

@micolous micolous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delays! I think what you've got is a good start – the suggestions here would make things more robust and follow rust-isms better. Thanks!

webauthn-rp-proxy/src/main.rs Outdated Show resolved Hide resolved
webauthn-rp-proxy/src/main.rs Outdated Show resolved Hide resolved
webauthn-rp-proxy/Makefile Outdated Show resolved Hide resolved
webauthn-rp-proxy/src/main.rs Outdated Show resolved Hide resolved
webauthn-rp-proxy/src/main.rs Outdated Show resolved Hide resolved
@arthurgleckler
Copy link
Contributor Author

I believe that I've incorporated all of your suggestions. Would you please take a look when you have a chance?

Thanks.

@arthurgleckler arthurgleckler force-pushed the master branch 2 times, most recently from 04f0d54 to 3e558bc Compare April 7, 2024 04:31
@arthurgleckler
Copy link
Contributor Author

Now that a506d89 is committed, I've rebased and force-pushed onto that. I don't know whether that will fix the PR run failure I received email about.

@Firstyear
Copy link
Member

I restarted the CI to check :)

@arthurgleckler
Copy link
Contributor Author

It failed again, but the underlying error appears to be "error: package bumpalo v3.15.4 cannot be built because it requires rustc 1.73.0 or newer, while the currently active rustc version is 1.70.0". I'm not using bumpalo, at least not directly. Can you think of any reason this would fail?

Oh, I see that it's failing on master, too:

https://github.com/kanidm/webauthn-rs/commits/master/

Copy link
Collaborator

@micolous micolous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, bumpalo breakage is not your fault. #428 should fix that.

Otherwise things look good now, thanks for sticking with this. 😄

@Firstyear Firstyear merged commit 23cf17c into kanidm:master Apr 8, 2024
31 of 33 checks passed
@arthurgleckler
Copy link
Contributor Author

Otherwise things look good now, thanks for sticking with this. 😄

I'm thrilled! Thanks.

@Firstyear
Copy link
Member

@arthurgleckler This has now been released on crates.io. Thank you!

kikuomax pushed a commit to codemonger-io/webauthn-rs that referenced this pull request Nov 24, 2024
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 this pull request may close these issues.

3 participants