janeczku / calibre-web

:books: Web app for browsing, reading and downloading eBooks stored in a Calibre database
GNU General Public License v3.0
13.22k stars 1.41k forks source link

Reverse Proxy Login broken in version 0.6.10 - 9418045 #1820

Closed rg9400 closed 3 years ago

rg9400 commented 3 years ago

Describe the bug/problem Previously, you could set a header for reverse proxy login, and if sent via a request, Calibre-Web would auto log you in based on that header. After updating to version 1/17/21, 9:28 AM, this no longer seems to work. I get the login screen no matter what despite not changing the headers at all.

To Reproduce Steps to reproduce the behavior:

  1. Enable Reverse Proxy login
  2. Set a reverse proxy header
  3. Send such header in a request to calibre while logged out, where the header matches the username of an existing user
  4. Notice that Calibre does not log you in anymore automatically.
  5. Revert to an older version of CW, repeat 1-4, notice that it logs you in fine.

Logfile Doesn't seem to log the failure to use the header

Expected behavior Should log you in automatically based on the header. Even before, it would log in, but it usually required a refresh of the page vs logging in the user automatically on their first visit.

Screenshots N/A

Environment (please complete the following information):

Additional context Using SWAG for reverse proxy and reverse proxy headers. Location block below

location ^~ /calibre {
    auth_request /auth-4;
    include /config/nginx/proxy.conf;
    set $theme_app calibreweb;
    set $upstream_app calibre-web;
    set $upstream_port 8083;
    set $upstream_proto http;
    proxy_pass $upstream_proto://$upstream_app:$upstream_port;
    proxy_set_header    X-Scheme            $scheme;
    proxy_set_header    X-Script-Name       /calibre;
    auth_request_set $auth_user $upstream_http_x_organizr_user;
    proxy_set_header myheader $auth_user;
    auth_request_set $auth_email $upstream_http_x_organizr_email;
    proxy_set_header  myemailheader $auth_email;
    include /config/nginx/themes/themepark.conf;
}
OzzieIsaacs commented 3 years ago

Could you please tell me, which version you think is working. As I'm not aware of any change regarding this. It would me help to figure out what to look for. (Looks like I do no tests regarding this feature up to now, so indeed could be totally broken)

rg9400 commented 3 years ago

It is definitely working in 0.6.7 - 0735fb1e929fd673397acdd7e2c1c8be0d510d3f. It also seems to be broken in 0.6.8 - 2ad329e64c0cfa67adc4a9f594481379ab8bb3ac.

So I guess it broke a while ago, but I only recently noticed

OzzieIsaacs commented 3 years ago

I could tear it down to the merge of the development branch on October 10th 2020 (0.6.9 and lower should work)

OzzieIsaacs commented 3 years ago

The commit to store UI settings in flask session for guest user seems to break it (September 27th 7pm on develop). Looks like it's only not working if there is a request with a invalid username in header before. This is going to be funny...

OzzieIsaacs commented 3 years ago

I think I fixed it, but I need some time to do some more tests

scrapix commented 3 years ago

noticed the same today when I wanted to get the new docker container. 0.6.9 works well for me. thank you a lot @OzzieIsaacs !

rg9400 commented 3 years ago

Mine doesn't seem to be fixed on 0.6.11 Beta - 0f83f9992c13c666e3e6db8fa25d6c0e954ad44c. The way I am testing is by logging out. If RP login was working, it wouldn't actually let me log out and go to the login page, it would keep logging me back in. Indeed, that's how 0.6.7 - 0735fb1e929fd673397acdd7e2c1c8be0d510d3f was working, as well as how RP login works in Grafana.

So it still seems like something is messing with it.

OzzieIsaacs commented 3 years ago

I tried it with Firefox and also automated with python-requests on the newest commit. It works as you described it (both versions). After clicking on logout get back logged in and you are on the main start page. I used Modify Header Value (HTTP Headers) addon to add up the needed header value

kenjibailly commented 3 years ago

I'm trying in Firefox and Microsoft Edge and unfortunately cannot get it to work after trying several hours. Everything worked fine before updating to 0.6.10

OzzieIsaacs commented 3 years ago

@kenjibailly which version you are using? 0.6.10 isn‘t working, 0.6.11 Beta should work

rg9400 commented 3 years ago

Yeah I cannot get it to work, tried multiple different browsers (Firefox, Chrome, Edge) as well as hardcoding the header for my username into the reverse proxy. If I revert to https://github.com/janeczku/calibre-web/commit/0735fb1e929fd673397acdd7e2c1c8be0d510d3f, it works perfectly, and it breaks at 2ad329e64c0cfa67adc4a9f594481379ab8bb3ac. Looking through the commit history, there is a lot that happened between those two releases, including stuff related to hardening the security. I wonder if this is preventing the headers from being set properly in the first place?

I am trying to add it via NGINX by adding something like this in my reverse proxy location block:

location ^~ /calibre {
    auth_request /auth-4;
    include /config/nginx/proxy.conf;
    set $theme_app calibreweb;
    set $upstream_app calibre-web;
    set $upstream_port 8083;
    set $upstream_proto http;
    proxy_pass $upstream_proto://$upstream_app:$upstream_port;
    proxy_set_header    X-Scheme            $scheme;
    proxy_set_header    X-Script-Name       /calibre;
    auth_request_set $auth_user $upstream_http_x_organizr_user;
    proxy_set_header X-CW-USER $auth_user;
    # auth_request_set $auth_email $upstream_http_x_organizr_email;
    # proxy_set_header X-CW-EMAIL $auth_email;
    include /config/nginx/themes/themepark.conf;
}

It fails even if I hardcode the header to my username vs trying to grab it via the variable from the upstream auth_request.

Currently on 0.6.11 Beta - 88078d65e9b17733001d80bce1c08fa015fddb00

kenjibailly commented 3 years ago

@OzzieIsaacs I had a problem when selecting nightly as update channel the container would go to a server internal error state. Seems like I fixed that by updating to 0.6.10 first, then changing the setting and then updating to the beta version. I can confirm it works without extra configuration or locations to the nginx proxy manager.

@rg9400 Are you perhaps using Cloudflare with a proxied CNAME? That's what didn't work for me at least, I had to set it to DNS Only.

rg9400 commented 3 years ago

I am using Cloudflare and their CDN, but the issue remained after switching it to DNS only.

It does seem like there is something with the way I have implemented it that is causing potential issues, but the fact that it works with an earlier version (and with other apps like Grafana), indicates that it is related to something that was added in between 0.6.7 and 0.6.8. Could you maybe share how you are doing it using NGINX proxy manager?

kenjibailly commented 3 years ago

@rg9400 Sure, what I did in nginx proxy manager is very simple actually. Scheme: http forward hostname/ip: gateway ip of the shared docker network (mine looks something like this 172.xx.xx.1) If you don't know where to find it and you have portainer, click on any container in the network, go to the bottom of the page and the internal ip and gateway ip will be shown there. Forward port: exposed port you setup for yourls (8080 for example) Setup new SSL certificate for subdomain. Set CNAME subdomain in cloudflare with DNS only.

That's it, if you cannot get it to work I can help you over Discord. Xyroxis#6703

rg9400 commented 3 years ago

How are you actually forwarding the reverse proxy login details? I can get Calibre to work behind reverse proxy (using Swag), but cannot get it to auto login based on the headers. I tried subdomain as well (was using subfolder), but still no luck. Might ping you on discord

rg9400 commented 3 years ago

After doing some further testing, this works on subdomain (calibre.domain.com), but fails on subdirectories (domain.com/calibre). Nothing else is changed between the two except for the below header which is required for subdir.

proxy_set_header X-Script-Name /calibre;

Both work fine on 0.6.7

kenjibailly commented 3 years ago

I just tried it and it also fails for me on subdirectories. Subdomain is fine.

rg9400 commented 3 years ago

@OzzieIsaacs any plan to fix this for subdirectory, or is the recommendation to just switch to subdomain? Subdomains work for me, just have some slight nuances, so just wanted to verify before switching over.

rg9400 commented 3 years ago

This seems to be fixed on 0.6.12 Beta - bc876a159e571b0337c3a327648a5aa097e80dc9 - 2021-02-18T17:02:58+01:00. Appreciate the help!

twnk commented 3 years ago

For what it's worth, I think there's actually a bug in the programme logic of Flask Login (which this fix works around)

https://github.com/maxcountryman/flask-login/issues/569