doy / rbw

unofficial bitwarden cli
https://git.tozt.net/rbw
Other
611 stars 84 forks source link

Add YubiKey Auth Support #76

Closed dave-tucker closed 2 years ago

dave-tucker commented 2 years ago

This commit adds support for YubiKey auth. A pinentry prompt asks the user to touch the Yubikey, which causes it to produce input (like a keyboard). That can then be sent to the Bitwarden API where it can ID the YubiKey and validate the request.

Disclaimer: I've tested this using my Bitwarden account with my Yubikey and it works fine.

Fixes: #7

soenkeliebau commented 2 years ago

Thank you very much for this PR!!

I tried it out yesterday and it works fine for me. I've left two comments that came to mind when looking at the code.

I've played around a bit with your code here out of curiosity: https://github.com/soenkeliebau/rbw/commit/2b1d58b35da368683afaf18de43d14f9907dff6d

Full disclosure, that has not been tested - compiles but thats about it :)

JeDaYoshi commented 2 years ago

Any progress on this? @dave-tucker's, alongside @soenkeliebau's changes are pretty nice. What'd be missing is adding a handler in case you input an incorrect Yubico OTP:

diff --git a/src/api.rs b/src/api.rs
index 75ab80e..e8b398c 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -1122,14 +1122,16 @@ fn classify_login_error(error_res: &ConnectErrorRes, code: u16) -> Error {
             return Error::IncorrectApiKey;
         }
         "" => {
-            // bitwarden_rs returns an empty error and error_description for
+            // vaultwarden returns an empty error and error_description for
             // this case, for some reason
             if error_desc.is_none() || error_desc == Some("") {
                 if let Some(error_model) = error_res.error_model.as_ref() {
                     let message = error_model.message.as_str().to_string();
                     match message.as_str() {
                         "Username or password is incorrect. Try again"
-                        | "TOTP code is not a number" => {
+                        | "TOTP code is not a number"
+                        | "Failed to verify Yubikey against OTP server"
+                        | "Invalid Yubikey OTP length" => {
                             return Error::IncorrectPassword { message };
                         }
                         s => {
soenkeliebau commented 2 years ago

I have not looked at this in a long time tbh. I'm running the changed version locally at the moment. Happy to help if we want to make a combined effort to get this merged of course.

ambroisie commented 2 years ago

I'd love to see this merged, is there still interest from the author (@dave-tucker) and @soenkeliebau?

soenkeliebau commented 2 years ago

I'd be happy to see this merged, yes. I didn't want to hijack Dave's PR though.

JeDaYoshi commented 2 years ago

@dave-tucker seems to have moved on from this PR, though. Well, or isn't paying enough attention to this. So we should probably look into getting it merged ourselves, I'd guess.

soenkeliebau commented 2 years ago

Works for me. I have to admit that I don't have the faintest idea of the code I have written any more. I'll try to give it a quick look over Easter and try to get a pull request up for review.

dave-tucker commented 2 years ago

I'm closing the PR since there has been no response from a maintainer, and therefore, comments addressed or not, it seems unlikely to get merged. @soenkeliebau feel free to carry on with this if you want.

jonas-w commented 1 year ago

@soenkeliebau's yubikey2 branch works great!

Even though it displays the error "rbw login: all supported 2fa methods failed: []", I'm logged in and can use rbw normally.

Thanks @soenkeliebau

archer-65 commented 1 year ago

It seems @doy has been active lately. Should we open this again?