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

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

Open
thatguyatgithub opened this issue Sep 6, 2024 · 3 comments

Comments

@thatguyatgithub
Copy link

thatguyatgithub commented Sep 6, 2024

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
Copy link
Owner

guerzon commented Sep 10, 2024

Hello @thatguyatgithub, thanks for raising this.

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

@thatguyatgithub
Copy link
Author

Ouch, somehow I missed this notification, quite sorry!

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

@guerzon
Copy link
Owner

guerzon commented Oct 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants