vaultwarden icon indicating copy to clipboard operation
vaultwarden copied to clipboard

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

Open thatguyatgithub opened this issue 1 year ago • 2 comments

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;
      }

thatguyatgithub avatar Sep 06 '24 17:09 thatguyatgithub

Hello @thatguyatgithub, thanks for raising this.

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

guerzon avatar Sep 10 '24 06:09 guerzon

Ouch, somehow I missed this notification, quite sorry!

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

thatguyatgithub avatar Sep 25 '24 12:09 thatguyatgithub

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

guerzon avatar Oct 30 '24 03:10 guerzon

The issue is coming from this line:

nginx.ingress.kubernetes.io/connection-proxy-header: "keep-alive"

It overrides default value for the Connection header which is $connection_upgrade;:

# See https://www.nginx.com/blog/websocket-nginx
map $http_upgrade $connection_upgrade {
        default          upgrade;
        # See http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
        '''';
 }

after manually removing said annotation, I was able to establish a websocket connection. Not sure what was the reason for adding the annotation, though.

rtim75 avatar Jan 13 '25 16:01 rtim75

Just want to verify that this solved it for me, too. Given the blame the annotation is there since a long time.

after manually removing said annotation, I was able to establish a websocket connection. Not sure what was the reason for adding the annotation, though.

nuggie avatar Jan 22 '25 22:01 nuggie

@guerzon do you remember the reason for this annotation? I don't see a reason for it, and I think it can be safely removed. But maybe you can provide more insight.

rtim75 avatar Feb 17 '25 11:02 rtim75

Hi all, I have removed the annotation. Let me know how it goes.

Lester

guerzon avatar Mar 15 '25 02:03 guerzon