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

Wrong scheme in download link still not fixed #97

Open
andymarden opened this issue Jan 29, 2024 · 13 comments
Open

Wrong scheme in download link still not fixed #97

andymarden opened this issue Jan 29, 2024 · 13 comments

Comments

@andymarden
Copy link
Contributor

#88

Simple use case:

Nginx is reverse proxy - fileshelter is accessed at https://fileshelter.example.com
Nginx does proxy pass through to http://internal-server:5091
Download links created as http://fileshelter.example.com

The way most docker containers deal with all this is to have an EXTERNAL_URL env variable set which would be http://fileshelter.example.com and the code uses this as the link prefix for everything.

Good because it doesn't matter how many proxies yo go through - you are in control.

Can we get that implemented? I may have a go if noone else does.

@epoupon
Copy link
Owner

epoupon commented Jan 29, 2024

Hmmm it should already work.
Have you tried the behind-reverse-proxy + trusted-proxies settings in the file shelter.conf file?
Fileshelter should eventually pick up the original url as seen by the client.

@andymarden
Copy link
Contributor Author

Aha - I see it is the same as another closed issue - the link is https but the text displayed is http. Surely those two can be constructed from the same data?

I don't know if it is because I am bouncing around nginx and authentik to put auth on the non-download urls. That's why, in addition to fixing this having an optional override may be simplest to guarantee it always works.

I am going to to a for and then pull-request over the next couple of days to remove the Create button from the download page so why don't I pick up this fix and the optional override?

@andymarden
Copy link
Contributor Author

I can see also that if I go in not through nginx reverse proxy, the displayed link test gets http but there is no prefix on the link itself. Anyway - going in....

@andymarden
Copy link
Contributor Author

OK - fixed it and creating pull request now.

The cause is a bit ridiculous - in wt4, a wLink object references something (X-Forwarded-Proto probably which is set in the reverse proxy) to embed the correct scheme in the link but the url() method does not contain the scheme and there is no way to get it out. Nice.

So you have to use the environment object - but this does not be default pay attention to X-Forwarded-Proto. There is apparently a way to make it do that but, rather than mess with the build, I felt it was clearer to get the X-Forwarded-Proto scheme explicitly in the code (and default to http if not set).

I'm sure this made sense to someone at some point in the wt4 world but what a song and dance just to get the full URL!

@andymarden
Copy link
Contributor Author

I don't seem to have permissions to create a Pull Request on the repo - can you enable that?

@epoupon
Copy link
Owner

epoupon commented Jan 31, 2024

You have to fork the project, create a branch (from the develop branch), push commits in that branch and then open a PR between your branch on your fork and the develop branch on the upstream repository

@andymarden
Copy link
Contributor Author

OK - so permissions are ok. I did a copy to my gitea server and perhaps that's the issue then. OK - let me see.

@andymarden
Copy link
Contributor Author

Need to ensure it works with no reverse proxy for https too - will fix that

@epoupon
Copy link
Owner

epoupon commented Feb 1, 2024

By the way, I use the exact same settings for the demo instance (nginx handling https, redirecting to a http instance), and it does display 'https' in the link...

@andymarden
Copy link
Contributor Author

andymarden commented Feb 2, 2024

Strange - I guess something somewhere is different. It could be how Wt4 is configured perhaps

@andymarden
Copy link
Contributor Author

done now - pr updated

@epoupon
Copy link
Owner

epoupon commented Feb 6, 2024

By the way did you see this comment? #88 (comment)
Did you edit the trusted proxy list to add the default one set by the docker setup?

@epoupon
Copy link
Owner

epoupon commented Feb 6, 2024

Maybe you missed it because the doc is still on the develop branch, see https://github.com/epoupon/fileshelter/blob/develop/INSTALL.md#reverse-proxy-settings

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