Implement "login with device"

[quexten]

This is a very WIP pull-request for login-with-device. To run this, you need a new client version, for the web client this is 2023.06 (or latest master as of today) since login-with-device was disabled for self-hosted installations previously.

Basic login-with-device on WebSocket clients (desktop/web/webextension) works, but there is still quite some work to be done.
Screencast from 2023-06-17 23-53-39.webm

This PR implements a few components to make the login-with-device work, each still partially incomplete.

• A few new rocket endpoints for adding / updating requests (mostly done, some request validation / responses missing)
• A new database table for keeping the auth_requests (only done for sqlite)
• A new websocket endpoint for anonymous connections (needed for the login-with-device feature, this duplicates some code but I didn't know how to structure it better in this case)
• Some changes to the password_login. To finish the login-with-device, a regular password-login request to '/identity/connect/token' is made, with an additional field in the body, "authRequest", this contains the UUID of the authrequest, the password is the authCode from the passwordless login-request instead of the actual masterpasswordhash.

Aside from the points missing mentioned above, mobile push is not implemented yet, and a lot of code clean-up needs to be done.

Feel free to comment general comments, keep in mind the PR is very WIP so a lot of parts are missing and there is a lot of clean-up to be done.

I will squash the PR when it is done.

[quexten]

Logging in on mobile, and approving on mobile also works fine now over mobile push notifications.

Somehow it only works when my timezone of the approving device (all platforms) is on UTC. I need to check which timestamp format is expected.

[quexten]

Ok, most things are working now, still missing:

• Testing other databases (only tested sqlite, others are implemented but untested)
• Deleting old requests periodically

@BlackDex, as I understand, start_notification_server starts the old (port 3012) Websocket server and makes rocket manage the state. Additionally, rocket itself handles the same endpoint, but the old port 3012 server is kept for compatibility.

Since no user has the new anonymous hub endpoint in their proxy, there does not need to be a similar separate server for the anonymous hub right? Just using rocket to handle it should be enough. (Currently this PR only handles the anonymous hub connections over rocket).

[tessus]

The current rewrite rule looks like this according to https://github.com/dani-garcia/vaultwarden/wiki/Proxy-examples:

RewriteRule /notifications/hub(.*) ws://<SERVER>:3012/$1 [P,L]

I believe this rule is incorrect, because

• there is no separate rule for /notifications/hub/negotiate (shouldn't this go to http instead of ws)
• the websocket connection does not get the path info passed on, but only everything that is sent after /notifications/hub, which means only the data is sent to ws://<SERVER>:3012/

So how would the ws server differentiate between this connection and yours?

If we were to analogously use a rewrite rule

RewriteRule /notifications/anonymous-hub(.*) ws://<SERVER>:3012/$1 [P,L]

the web socket server would either retrieve <data> or <token> but without path.

P.S.: It seems that all proxy examples only send data to the root of the websocket connection. So I have no idea how #[get("/hub?<data..>")] is even triggered, if only ?<data..> is sent to ws://<SERVER>:3012/
I have to look into this, because I apparently have no clue what is going on.

[BlackDex]

The current rewrite rule looks like this according to https://github.com/dani-garcia/vaultwarden/wiki/Proxy-examples:

RewriteRule /notifications/hub(.*) ws://<SERVER>:3012/$1 [P,L]

I believe this rule is incorrect, because

• there is no separate rule for /notifications/hub/negotiate (shouldn't this go to http instead of ws)
• the websocket connection does not get the path info passed on, but only everything that is sent after /notifications/hub, which means only the data is sent to ws://<SERVER>:3012/

So how would the ws server differentiate between this connection and yours?

The current main/testing version doesn't need a separate port anymore. So all can go via the main port now.

Also, the negotiate endpoint doesn't exist anymore for a long time already. We kept it for a while because of backwards compatibility, but it can be removed now i think.

If we were to analogously use a rewrite rule

RewriteRule /notifications/anonymous-hub(.*) ws://<SERVER>:3012/$1 [P,L]

the web socket server would either retrieve <data> or <token> but without path.

P.S.: It seems that all proxy examples only send data to the root of the websocket connection. So I have no idea how #[get("/hub?<data..>")] is even triggered, if only ?<data..> is sent to ws://<SERVER>:3012/ I have to look into this, because I apparently have no clue what is going on.

<data...> is all that comes after the ? which is just a query string which comes after the URL.

In general, we tend to fully remove the old port. But not yet next release, at least not for the web-socket notifications just yet.

[tessus]

Thanks for the reply.

The current main/testing version doesn't need a separate port anymore. So all can go via the main port now.

Right, but I didn't see a description how to reverse proxy this, nor how this is activated in the config. Or maybe I missed it.

Also, the negotiate endpoint doesn't exist anymore for a long time already.

Good to know. Thanks.

<data...> is all that comes after the ? which is just a query string which comes after the URL.

Right, I understand that. So only the query string is "forwarded" to the ws://.... But this PR introduces another endpoint, which sends a token. So how does the websocket server know which path to follow, if the endpoint is the same (since the data (what was sent until now) and the token (from this PR)) is just forwarded to /.

[quexten]

You do not need any proxy rewrite rules. Not for the regular WebSocket hub and not for the new, anonymous hub. The rewrite rule was needed for the WebSocket endpoint on port 3012 (and still works for compatibility) but you do not need it anymore.

At least using Traefik, it just works out of the box for this PR. I'm not sure whether you have to enable WebSocket support on other proxies.

So how does the websocket server know which path to follow

To establish the WebSocket connection, first a HTTP GET request is made to wss://my.bitwarden.server.com/notifications/anonymous-hub?Token=UUID
The HTTP request contains the info about which "path" the connection is made to, and the query. The response is then 101 Switching Protocols, and the server/client switch to WebSocket.

The Vaultwarden internal routing is done by rocket, thus #[get("/anonymous-hub?<token..>")] is enough.

[tessus]

To establish the WebSocket connection, first a HTTP GET request is made to wss://my.bitwarden.server.com/notifications/anonymous-hub?Token=UUID

This is exactly my point.

I am using :3012 and the Apache rewrite rule I am using is the one that is in the wiki page for proxies.

RewriteRule /notifications/hub(.*) ws://<SERVER>:3012/$1 [P,L]

So what happens is, if a request is sent to the proxy

https://my.bitwarden.server.com/notifications/hub?data=whatever

The proxy makes this request to the backend:

ws:/127.0.0.1:3012/?data=whatever

P.S.: It's the same with the nginx rules on the wiki page. Same behavior.

[quexten]

I am using :3012 and the Apache rewrite rule I am using is the one that is in the wiki page for proxies.

RewriteRule /notifications/hub(.*) ws://:3012/$1 [P,L]

Yes, this was necessary as rocket did not have built in websocket support before. The port 3012 endpoint is dedicated to only the websocket hub, that's why the path is not necessary. Vaultwarden internally has 2 ways to process the websocket notifications, once via the old 3012 endpoint (ws://server:3012/notifications/hub, and once via the rocket endpoint (ws://server:80/notifications/hub).

So what happens is, if a request is sent to the proxy
https://my.bitwarden.server.com/notifications/hub?data=whatever
The proxy makes this request to the backend:
ws:/127.0.0.1:3012/?data=whatever

Exactly, since 127.0.0.1:3012 in this case is vaultwarden's dedicated TCP port, only for websockets, no path is needed. Now that Rocket websocket support is merged, this rewrite is not required.
Technically, something like:

RewriteRule /notifications/hub(.*) ws://:80/notifications/hub$1 [P,L] should work (not tested), and would route over rocket instead, but you probably don't need a rewrite rule at all anymore.

For the anonymous hub of this PR, only rocket notifications are supported. So if you want to rewrite, you would have to use something like my example, but as I said, I don't think any rewrite rules are necessary beyond what's needed to get the HTTP requests to vaultwarden.

[tessus]

I am still not sure what the correct way is. So let's say I want to use the "new" way of things. How do I set:

ROCKET_ADDRESS=0.0.0.0
ROCKET_PORT=8000

WEBSOCKET_ENABLED=false
WEBSOCKET_ADDRESS=0.0.0.0
WEBSOCKET_PORT=8000

and then no more rewrite stuff in Apache?

Maybe we should add a wiki page for that, because these 2 ways of doing notifications is a bit strange when we use both of them at the same time.
e.g. I am running the main branch but haven't changed the vw config nor the Apache reverse proxy config. So this seems to me that I am running in a hybrid mode of sorts.
That is if rocket now automatically creates a ws connection on the same port as defined in ROCKET_PORT.

[quexten]

I think what you describe should work, although I have not tested with Apache yet. I have removed my WebSocket proxy rule from traefik, and set WEBSOCKET_ENABLED to false and the notifications still work.

Maybe we should add a wiki page for that, because these 2 ways of doing notifications is a bit strange when we use both of them at the same time.

Well, there is https://github.com/dani-garcia/vaultwarden/wiki/Enabling-WebSocket-notifications, but I think the page can be removed entirely in the future since WebSockets do not require special proxy configuration anymore.

[BlackDex]

That page can indeed be removed in the future.
All proxy examples need a review so that they will work properly.

[tessus]

All proxy examples need a review so that they will work properly.

I think that is a good idea.

I also think we might still need rewrite rules: ProxyPass / http://<SERVER>:80/ would pass traffic to http://server:80/, but web sockets use a different protocol. So what would happen, if a websoccket connection was routed to http://server:80/? Shouldn't it be proxied to ws://server:80/?

Anyway, I'll stop spamming this PR with my questions. I guess I should start a discussion instead...

[GeekCornerGH]

You'll probably want to remove the "copy" part from the migration folder, right?

[GeekCornerGH]

@BlackDex just asking, would this be a feature that will be in next release? Would be nice, as we have the other push notification part ready!

[quexten]

From my side this is mostly done. I didn't have time yet to test on postgresql or mysql yet though.

Also beware that on the current vaultwarden web-build it will not show the option (all other clients do still immediately benefit from it though, desktop, browser-extension, and mobile), that will only be the case in the next version of the web-build.

[GeekCornerGH]

I assume this is finished? Maybe rebase your changes? I'll gladly test your changes once this is done

[quexten]

@GeekCornerGH, yes, pretty much. It is now up-to-date with the latest main branch. I still had to test on mariadb and postgres, which is now done. The postgres migration had an incorrect type, which is now fixed. Other than that it is also working on mariadb / postgres.

[GeekCornerGH]

Tested against Chrome Extension v2023.5.1 and Windows Desktop app v2023.5.1

[TheManchineel]

@dani-garcia does this look good to you?

[qymab]

I think what you describe should work, although I have not tested with Apache yet. I have removed my WebSocket proxy rule from traefik, and set WEBSOCKET_ENABLED to false and the notifications still work.

Maybe we should add a wiki page for that, because these 2 ways of doing notifications is a bit strange when we use both of them at the same time.

Well, there is https://github.com/dani-garcia/vaultwarden/wiki/Enabling-WebSocket-notifications, but I think the page can be removed entirely in the future since WebSockets do not require special proxy configuration anymore.

mind sharing which traefik rules you remove ? is this enough ?

- traefik.http.routers.bwws.middlewares=redirect-to-https
- traefik.http.routers.bwws.service=bw-ws
- traefik.http.services.bw-ws.loadbalancer.server.port=3012

[tessus]

@qymab I created a discussion How to reverse proxy websockets?

People might rather read discussions than PR comments, so I believe you have a better chance to get an answer there.

I haven't used traefik for a while, so I won't be of much help. I don't know if traefik needs a directive to upgrade connections or if this is done automatically. But if you are only removing these 3 lines, websockets still only exist for path /notifications/hub and afaik it should not be restricted to that path anymore.

[BlackDex]

Ok, I have done some checks, and it seems to work for the most part.
There are some changes which can be removed, and there are some additions needed.

I have not looked yet at the layout of the code if there could be some improvements there.
But the changes i mentioned can be addressed already i think :).

[BlackDex]

Ok, I found a few more items.
I also looked if we could change the custom from_i32 to use num_derive, but that seems to only use i64, while that can also be casted to an i32 again, it might be a bit overkill. But it would safe manual work if there are new types.

Anyway, not that important i think.

Looks good overall from my opinion.
With the changes requested above i think it's a go for me.

[BlackDex]

@quexten, Ok this looks good to me.

The only thing I would like is to have this PR squashed into one commit to keep the history a bit cleaner.
Once that is done, it has a go for me.

[quexten]

@quexten, Ok this looks good to me.

The only thing I would like is to have this PR squashed into one commit to keep the history a bit cleaner. Once that is done, it has a go for me.

Done.

[fxzxmic]

I have no idea.

[stefan0xC]

@fxzxmic No idea what your error message says either, but can you try testing as it includes this fix #3831 (which hopefully fixes the issue).

[fxzxmic]

@stefan0xC I don't know where the problem is either. There will be the error mentioned above on the phone, and there will be no error reported on the desktop, but the login has not been successful.
Then I'll wait first. I haven't used Docker, so using the testing version is quite troublesome.

[Tyree]

I'm just testing Login with Device. The first attempt to log in on my browser, no notification came up on my phone. I had to go into settings and into Pending Login Requests. I confirmed and it let me in.
All other tests have, whether I Confirm or Deny, failed with the error, "We were unable to process the request. Please try again or contact us."
My phone is communicating with the server. Requests come in and I can sync the vault. But these approvals don't work anymore.

[BobWs]

I apologize in advance before "someone" bites my head off. I don't know if this is the right place to point it out given the many duplicate reports surrounding this topic, but I'll try anyway.

I have noticed that "login with device" does not work if your device is still on iOS 14. In fact I still have my iPhone running iOS 14 and there "login with device" does not work.
I do get the push notification but when I click "confirm login" I get the same error message as previously stated by others "We were unable...etc".

If I then try this with a device that has iOS 16, and with the Bitwarden app reinstalled it does work. But if I then want to use this again the next day it no longer works.

When I log out of the iOS 16 app and log back in then it works again. By the way this is not the case with iOS 14, there this does not help, neither log out or reinstall the app.

[BlackDex]

I apologize in advance before "someone" bites my head off. I don't know if this is the right place to point it out given the many duplicate reports surrounding this topic, but I'll try anyway.

I have noticed that "login with device" does not work if your device is still on iOS 14. In fact I still have my iPhone running iOS 14 and there "login with device" does not work. I do get the push notification but when I click "confirm login" I get the same error message as previously stated by others "We were unable...etc".

If I then try this with a device that has iOS 16, and with the Bitwarden app reinstalled it does work. But if I then want to use this again the next day it no longer works.

When I log out of the iOS 16 app and log back in then it works again. By the way this is not the case with iOS 14, there this does not help, neither log out or reinstall the app.

If you would have read all those duplicate reports, you would have seen that updating to the testing tagged docker image will solve your issues.

[BobWs]

If you would have read all those duplicate reports, you would have seen that updating to the testing tagged docker image will solve your issues.

HAHAHA I didn't expect any other answer from you....

[ados8]

Amazing work implementing device login so quick.
I did run into the approve bug and using master password to login once resolves the approve request issue.
Further testing shows that MFA is still required and I was hoping to have an option to bypass should the user approve via device.
It could be without device login: email + password + MFA = successful.
But should the user use device login it then would be: email + device login = successful
This shouldn't lower security as it only works for known devices and still uses a 2 factor method/physical device.
Currently the "workaround" is to have MFA remember allowed but I don't want that because without device login that lowers security.
Are there any plans for such a feature?

[quexten]

Amazing work implementing device login so quick.
I did run into the approve bug and using master password to login once resolves the approve request issue.
Further testing shows that MFA is still required and I was hoping to have an option to bypass should the user approve via device.
It could be without device login: email + password + MFA = successful.
But should the user use device login it then would be: email + device login = successful
This shouldn't lower security as it only works for known devices and still uses a 2 factor method/physical device.
Currently the "workaround" is to have MFA remember allowed but I don't want that because without device login that lowers security.
Are there any plans for such a feature?

This is behaviour is dictated by upstream (bitwarden). It works the same on vault.bitwarden.com. Since vaultwarden aims to be compatible with upstream, if you want this feature you would need to bring it up with bitwarden instead.

[mada199122]

When I click "Confirm login" on the desktop app to approve the login request made for example from the web vault, it fails and in the log I see this line

[2023-10-09 11:39:52.112][vaultwarden::api::core::accounts::_][WARN] Data guard `Json < AuthResponseRequest >` failed: Parse("{\"key\":\"4.dAa5C9CAzqysrFu7iQFjDyp9x0vzyZSCP/SNLNLQUYJSgCvqM1/qzidjUjBIsBSQPssLo4JuYB6lEHTCG0BAr1cBCexgvtqExzMi4mMbqVqAVNHMI2pbdMHQ/t09sWYLmEaTaYzZRxFiVlymLDApcKUErmvxVfgES3LbGXV9WIUdrdYOKcXu0KiNM1hSzIPlUU/XNKgwpegYDjKfeMrOnT+LKDQpgxJOba2Hh1I7Bmedd8BZI4lE5aOgkjzipkXuvx5fDwLX5yvgQ6qFeqA+/5eGBhfZuMpLEnVw/SBP65ich9qIKDtuw11OisGKhCs5KHME0kwt0iLo564dCNAR0Q==\",\"deviceIdentifier\":\"c2dcb8bc-82cd-485f-9cb3-ea0b45ad8659\",\"requestApproved\":true}", Error("missing field `masterPasswordHash`", line: 1, column: 437)).

[stefan0xC]

@mada199122 this has already been fixed in #3831 (currently available in testing)

[mada199122]

Ah ok thank you!!!