-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Print QR code for webauthn verification link #7272
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
base: master
Are you sure you want to change the base?
Print QR code for webauthn verification link #7272
Conversation
|
This is brilliant. |
5bbcce7 to
a5ecbd6
Compare
62f3ca7 to
18e973e
Compare
|
@segiddins I'm not sure I have the knowledge to review this, but I can try to get up to speed if needed. The vendoring looks fine to me, though! One thing that immediately comes to mind though is, should the information before the QR code be updated to give a bit of context about the QR code? Maybe:
|
|
That's a good idea! |
|
@segiddins any plan to update this PR? |
|
Yes, I will circle back here |
18e973e to
ae78666
Compare
Signed-off-by: Samuel Giddins <[email protected]>
ae78666 to
8e81034
Compare
|
@deivid-rodriguez this is now passing with a rebase and the updated message -- let's get it in for 3.6? |
|
@simi Did you plan to review this? I'm not sure I can review the implementation but I'm happy to give the feature a try. |
|
Also happy to ship it for 3.6, yeah! |
|
I tried this and it works fine 🎉. I can open the URL on my phone by scanning the code. My main concern is that it makes the output very noisy when I believe most people is not going to use it. Maybe render the QR when given a |
|
This is only printed when the account has webauthn enabled, and is spinning up the web server, so I personally would err on the side of showing it, otherwise i worry the improved UX won't be discoverable. |
|
I mean, most
|
|
I think that would fix the discoverability issue? |
|
I'm using webauthn now too, but I still occasionally get prompted for an OTP for some reason. |
|
I haven't seen that since I started using webauthn, it sounds like a bug, right? Does it happen when running |
|
In any case, that seems unrelated to this PR, since this won't change anything regarding how to choose whether to ask for an explicit OTP code, or use webauthn. |
|
On the same machines, about 95% of the time, I get a webauthn link, and about 5% of the time I get an OTP code request. |
|
Is that when running |
|
@segiddins Thoughts on my suggestion? I think we may want to evolve it to some configuration since users who use the QR code are likely to want to use it always, and users who don't use it are also likely to never want it, but I feel it's a good start without invasive output that stills give visibility to the new feature. |
|
yeah i guess that makes sense, my personal preference is to have it shown but that's just me :) |
|
Just to make sure I understand, you have your passkeys in your phone and because of this you want the QR always shown because it's handy, or even though you don't use the QR, you still like it always printed? I can see how always printing it may send a "RubyGems is cool" message, but I do find it a bit overwhelming since I won't be using it. |
|
There were some times when it seemed to reliably ask for an OTP for like an hour or two, on all my computers, and then went back to auth link. To be honest, I'm not sure if there was a clear pattern. Maybe the auth link system was broken or disabled for a few hours? |
I guess if the |
|
I just realized that if we add a |

What was the end-user or developer problem that led to this PR?
Accessing the webauthn verification link is difficult if you use your phone for your passkeys
What is your fix for the problem, implemented in this PR?
Use vendored rqrcode_code lib to generate a QR code for the link, which is printed out when the output is a tty
Make sure the following tasks are checked