guerzon / vaultwarden

Helm chart for Vaultwarden, the (unofficial) Bitwarden-compatible server written in Rust, formerly known as bitwarden_rs
MIT License
151 stars 72 forks source link

/notifications/hub broken on nginx as ingress: Websocket Missing Upgrade section #117

Open thatguyatgithub opened 2 months ago

thatguyatgithub commented 2 months ago

First of all, thanks @guerzon for the awesome work you've done with this chart, It's simply amazing!

Meanwhile I'm testing the patch of the chart, I wanted to drop here the issue with a kind of workaround for anyone out there that might experience the same problem.

The websocket notification part, used for "Login as device" feature, is broken when using Nginx as Ingress, because Nginx will not handle by default do "Upgrade" header, and instead close the connection immediately, which is kind of confusing/misleading when you read vaultwarden logs, getting a "GET /notifications/anonymous-hub?$TOKEN (web_files) GET /<p..> [10] => 404 Not Found as part of that issue having had the Websocket connection previously closed facing the user.

I found that If handled as an nginx ingress snipped, inside the Ingress k8s object, you can basically patch that section


    nginx.ingress.kubernetes.io/server-snippet: |
      location /notifications {
        proxy_http_version 1.1;
        proxy_set_header Upgrade $http_upgrade;
        proxy_set_header Connection $http_connection;

        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;

        proxy_pass http://vaultwarden.vaultwarden.svc:80;
      }
guerzon commented 2 months ago

Hello @thatguyatgithub, thanks for raising this.

Just to clarify, you're using a recent chart version hence the HTTP-integrated websocket notification, right?

thatguyatgithub commented 2 months ago

Ouch, somehow I missed this notification, quite sorry!

Yes, indeed that's the case, using the latest chart with HTTP-integrated websocket notification.

guerzon commented 3 weeks ago

Hi, I will take a look into this, but a PR would also be welcome.