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

add support for push notifications on changes #126

Closed
fbartels opened this issue Aug 10, 2018 · 23 comments
Closed

add support for push notifications on changes #126

fbartels opened this issue Aug 10, 2018 · 23 comments
Labels
enhancement New feature or request low priority Won't fix anytime soon, but will accept PR if provided

Comments

@fbartels
Copy link
Contributor

Hi, I've been using bitwarden_rs for a few days to see if I should replace my KeePass setup with it. One thing I have noticed is that when I change a password on an android device or in the chrome extension I have to remember to sync the changes back to the server before they show up on other client.

Apparently Bitwarden uses a push service to notify clients when something has changed:

https://community.bitwarden.com/t/push-notifications-dont-work/1207

Was there already any though on making it possible for bitwarden_rs to use this service as well? Would the service be actually open to third party implementations (it works with the original self hosted version, though)?

@mprasil
Copy link
Contributor

mprasil commented Aug 10, 2018

The changes should be pushed to the server right away as you save them. You just need to run the sync on the other clients to get the changes there. So it's the other way around. Also the push only works for mobile apps, so it still won't help you with the browser extension for example.

Still though,maybe we could implement it at some stage? I'm not sure if we'll be able to use the API there and how well will it map to bitwarden_rs internal representation of client device.

@mprasil mprasil added the enhancement New feature or request label Aug 10, 2018
@dani-garcia
Copy link
Owner

I'm not sure if there is something we can do, I assume the push.bitwarden.com endpoint is only usable for first party apps and licensed self-hosted instances, and the mobile clients probably have it hardcoded, so the alternative would be recompiling the app with a new custom endpoint, and then I imagine we'd have to register the push service with each platforms service (Firebase for Android, whatever Apple uses...)

Unless I'm wrong, this is probably going to be a lot of work for the users to set up the service, which is kind of against the idea of this project in the first place,

I'll leave this open, and maybe someone will think of an easier solution.

@mprasil
Copy link
Contributor

mprasil commented Aug 10, 2018

Yeah, unless we would be able to use push.bitwarden.com endpoint directly it's basically impossible to do the notifications without distributing our own fork of the app in the app store.

@mprasil mprasil added the low priority Won't fix anytime soon, but will accept PR if provided label Aug 11, 2018
@fbartels
Copy link
Contributor Author

The changes should be pushed to the server right away as you save them

Ah, yes indeed. When saving items a sync seems to be triggered.

unless we would be able to use push.bitwarden.com endpoint directly it's basically impossible to do the notifications without distributing our own fork of the app in the app store

Totally agree, the goal would be to use the official push proxy, since even if the code for the proxy would be available (did a short check and this does not seem to be the case) using an own proxy would require to compile and distribute own apps.

@mprasil
Copy link
Contributor

mprasil commented Aug 12, 2018

When saving items a sync seems to be triggered.

It actually does the changes directly at the server side by calling appropriate API.

@ajostergaard
Copy link

Any plans to leverage the new WebSocket support in the clients?

@mprasil
Copy link
Contributor

mprasil commented Aug 25, 2018

Hi, there's some preliminary code, that was just added by @dani-garcia.

However I believe the WebSockets functionality is pending upstream support in Rocket.

@EnriqCG
Copy link

EnriqCG commented Aug 27, 2018

Is this https://blog.bitwarden.com/live-sync-bitwarden-apps-fb7a54569fea something new? It was has been posted today.

@ajostergaard
Copy link

@EnriqCG that's the WebSocket support I referred to above.

@dani-garcia
Copy link
Owner

dani-garcia commented Aug 30, 2018

An early version of the websockets support is available now in:
https://github.com/dani-garcia/bitwarden_rs/tree/ws.

For now only folder notifications are sent (create, rename, delete).
The notifications are only tested between two web-vault sessions in different browsers, mobile apps and browser extensions are untested.

The websocket server is exposed in port 3012, while the rocket server is exposed in another port (8000 by default). To make notifications work, both should be accessible in the same port, which requires a reverse proxy.

My testing is done with Caddy server, and the following config:

localhost:2015 {

    # The negotiation endpoint is also proxied to Rocket
    proxy /notifications/hub/negotiate 0.0.0.0:8000 {
        transparent
    }

    # Notifications redirected to the websockets server
    proxy /notifications/hub 0.0.0.0:3012 {
        websocket
    }

    # Proxy the Root directory to Rocket
    proxy / 0.0.0.0:8000 {
        transparent
    }
}

This exposes the service in port 2015.

@fbartels
Copy link
Contributor Author

Nice, will try to do some tests in the weekend. One thing though:

mobile apps [...] are untested.

Websockets are only implemented/supported in the Web vault and browser extension:

Live sync works by using a powerful technology called WebSockets. Live sync is now available in all Bitwarden apps, including mobile (mobile uses a different technology called Push Notifications).

@mprasil
Copy link
Contributor

mprasil commented Aug 30, 2018

If anyone want to give it a try, mprasil/bitwarden:beta-ws is available. Note, that you still need to add reverse proxy in front of it, the image doesn't contain any.

@ghost
Copy link

ghost commented Aug 31, 2018

@mprasil
Is is possible to add build-in reverse proxy in docker container in the near future?
I want to use notification but I don't want use any proxy layer for my containers because these are available in local network only.

@mprasil
Copy link
Contributor

mprasil commented Aug 31, 2018

@Haxy89 the best solution is to just start another container with the proxy and just reverse proxy it yourself. I'd like to avoid building reverse proxy inside as it would be less flexible solution - especially for people that already run the service behind some sort of proxy. (which is a common scenario from what I've seen so far)

Technically there should be no difference between proxy being inside the container and being in separate container. (in regards to local network access)

@ghost
Copy link

ghost commented Aug 31, 2018

@mprasil
"the best solution is to just start another container with the proxy "
Do you know any?

@mprasil
Copy link
Contributor

mprasil commented Aug 31, 2018

I have no experience with it myself (I use a custom built image) but for example you can use this one and configure it according to instructions from Dani.

@sitic
Copy link

sitic commented Sep 1, 2018

Here is my configuration using Docker Compose, it uses Caddy to automatically generate a Let's Encrypt certificate, so it is not intended for local environments. Only Caddy is accessible from the outside.

Create a docker-compose.yml file based on this:

#docker-compose.yml

version: "3"

services:
  bitwarden:
    image: mprasil/bitwarden:beta-ws
    restart: always
    volumes:
      - ./bw-data:/data
    environment:
      SIGNUPS_ALLOWED: "true" # set to false to disable signups

  caddy:
    image: abiosoft/caddy
    restart: always
    volumes:
      - ./Caddyfile:/etc/Caddyfile:ro
      - caddycerts:/root/.caddy
    ports:
      - 80:80 # needed for Let's Encrypt
      - 443:443
    environment:
      ACME_AGREE: "true" # agree to Let's Encrypt Subscriber Agreement
      DOMAIN: "bitwarden.example.org" # CHANGE THIS! Used for Auto Let's Encrypt SSL
      EMAIL: "bitwarden@example.org"  # CHANGE THIS! Optional, provided to Let's Encrypt
volumes:
  caddycerts:

and the corresponding Caddyfile (does not need to be modified):

#Caddyfile

{$DOMAIN} {
    tls {$EMAIL}

    header / {
        # Enable HTTP Strict Transport Security (HSTS)
        Strict-Transport-Security "max-age=31536000;"
        # Enable cross-site filter (XSS) and tell browser to block detected attacks
        X-XSS-Protection "1; mode=block"
        # Disallow the site to be rendered within a frame (clickjacking protection)
        X-Frame-Options "DENY"
    }

    # The negotiation endpoint is also proxied to Rocket
    proxy /notifications/hub/negotiate bitwarden:80 {
        transparent
    }

    # Notifications redirected to the websockets server
    proxy /notifications/hub bitwarden:3012 {
        websocket
    }

    # Proxy the Root directory to Rocket
    proxy / bitwarden:80 {
        transparent
    }
}

Run

docker-compose up -d

to create & start the containers. It creates a private network between the two containers for the reverse proxy, only caddy is exposed to the outside.

docker-compose down

stops and destroys the containers.

@fbartels
Copy link
Contributor Author

fbartels commented Sep 2, 2018

Hi @dani-garcia ,

I wanted to make some tests and just pulled the latest beta-ws image from the docker hub. I am running the docker container with Apache in front for proxying. I have adapted my configuration and I get a first response from /notifications/hub/negotiate which answers with an error and triggers the following logging:

Rocket has launched from http://0.0.0.0:80
Listening for new connections on 0.0.0.0:3012.

Accepted a new tcp connection from 172.17.42.1:47778.
WS: Connection made
Error: WS Error <Protocol>: Unable to parse WebSocket key.
thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', libcore/option.rs:345:21
note: Run with `RUST_BACKTRACE=1` for a backtrace.
WS: Connection lost
GET /favicon.ico image/webp:
    => Matched: GET /<p..>
    => Outcome: Success
    => Response succeeded.

I don't want to put my hand in the fire for my apache configuration, but the "WS: Connection made" makes me think the websocket part should work. Anyone able to do a quick "this is an error" or "you need to fix your webserver" from the above error?

EDIT: it was the webserver. The error is resolved and I can see folder renames be applied. will do some more testing.

the vhost (apart from ssl settings and hostname) now has the following lines, which makes the websocket connections succeed:

        RewriteEngine On
        RewriteCond %{HTTP:Upgrade} =websocket [NC]
        RewriteRule /(.*)           ws://127.0.0.1:3012/$1 [P,L]

        ProxyPass / http://127.0.0.1:9080/
        ProxyPassReverse / http://127.0.0.1:9080/

*EDIT2: did some basic test including special chars in folder names and all updates (new folder, folder rename, folder delete) seem to properly propagate between the web vault and the browser extension. when i create a new folder (or delete a folder) in the mobile application this also seems to trigger a sync and therefore an update in the web vault.

Looks good to me.

@dani-garcia
Copy link
Owner

dani-garcia commented Sep 2, 2018

It seems the connection is made successfully, but the websocket is expecting some values that aren't there. Does your config pass the Connection and Upgrade headers to the websocket server correctly?

Looking through the server's code, there needs to be a sec-websocket-key header during handshake, so maybe apache is not forwarding it?

fbartels added a commit to fbartels/univention-bitwarden that referenced this issue Sep 3, 2018
websockets will be introduced by dani-garcia/vaultwarden#126 (comment)

Signed-off-by: Felix Bartels <f.bartels@kopano.com>
@dani-garcia
Copy link
Owner

To keep the issue tracker more focused, I'm closing this issue in favor of the meta issue at #246.

@Selfmade-RuLeZ
Copy link

Push notifications could be interesting in the near future. Bitwarden decided to ditch the AzureNotificationHub and use a new provider which also supports new technologies like Web Push. Maybe they use a self hosted solution as they already mention in their documentation, that you can use your own push relay server instead of push.bitwarden.com.

https://contributing.bitwarden.com/architecture/adr/adopt-web-push/

For now I did a little research about Push Notifications on mobile devices. To be honest, more on iOS than on Android, as I don't have a Android phone with a compatible OS version anymore.

To make push notifications to iOS, you have to send a request to the Apple APNs Server with the bundle identifier (found in the Bitwarden Mobile Repo), the type of notification, the push target, the payload and your authentication token or certificate. For the last one, you need a apple developer account. I don't have one and can't test my approach, but theoretically this should be possible.

Maybe I gonna redo my apple developer subscription next month to try these things.

@BlackDex
Copy link
Collaborator

@Selfmade-RuLeZ , that is why the follow PR #3304 uses the push relay from Bitwarden. It you check the video in that PR you see it works.

@Selfmade-RuLeZ
Copy link

@BlackDex oh, I missed this one. My bad then, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority Won't fix anytime soon, but will accept PR if provided
Projects
None yet
Development

No branches or pull requests

8 participants