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

Add: enable .well-known/acme-challenge when https #325

Merged
merged 1 commit into from
Mar 1, 2023

Conversation

300481
Copy link
Contributor

@300481 300481 commented Jan 24, 2023

Hello,

I've added a small change for the NGINX template:

Enable the NGINX route .well-known/acme-challenge only,
when using LetsEncrypt and activating HTTPS-protocol.

Why?

When using an external reverse proxy which is terminating TLS and itself is using
LetsEncrypt with a http based challenge it gets in conflict with the NGINX running in the container.
It looks like the containers NGINX will get the traffic before the proxy in front of it.

Additionally when running the container with http only, this would unnecessary
increase the NGINX configuration in a confusing way.

Enable the NGINX route .well-known/acme-challenge only,
when using LetsEncrypt and activating HTTPS-protocol
@undergroundwires
Copy link

This is great, I think this PR can also be considered as a bug fix.

#324, #325 together would enable us to consume Seafile Docker with our own proxy, giving us more capabilities for security among others. My searches show that the community has been discussing and asking for it on many occasions and coming up with workarounds, and even own container images for this. It's almost a standard that Docker images support having TLS-supporting proxy with solutions similar to Traefik and Caddy. I think the feature flag SEAFILE_SERVER_LETSENCRYPT should make TLS an "an addition", not the only way where other way around is broken.

Hope there's no blocker for these PRs as they provide wished functionality from community that will support using Seafile in more use-cases, and will do that without breaking any current functionality, or increasing the complexity of the current design.

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

Successfully merging this pull request may close these issues.

4 participants