Open Raruto opened 2 years ago
@Raruto I checked every point and tested, if you don't want propose new changes for me it possible merge it on dev.
The if statement I think is necessary, otherwise we would have a 'loop' of constant redirections. With if we have redirection only on http protocol.
@wlorenzetti given the declarative nature of nginx confing it is not immediately clear whether the ssl options are also sent to the browser when requesting the page as http
version:
ssl_certificate /etc/letsencrypt/live/$WEBGIS_PUBLIC_HOSTNAME/fullchain.pem;
ssl_certificate_key /etc/letsencrypt/live/$WEBGIS_PUBLIC_HOSTNAME/privkey.pem;
include /etc/letsencrypt/options-ssl-nginx.conf;
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;
resolver 8.8.8.8;
# written like this its doubtful if this condition is executed before or after the previous ssl options
if ($scheme = http) {
return 301 https://$server_name$request_uri;
}
Probably an optimal solution (also in terms of future customizations) could be to keep two separate server sections and share the common portions through the include
directive:
server {
listen 80;
server_name example.org;
include mime_types;
include default_routes;
location / {
return 301 https://$host$request_uri;
}
}
server {
listen 443 ssl;
server_name example.org;
include mime_types;
include default_routes;
include /etc/letsencrypt/options-ssl-nginx.conf;
....
location / {
proxy_pass http://example.org; #for demo purposes
}
}
The if statement I think is necessary, otherwise we would have a 'loop' of constant redirections. With if we have redirection only on http protocol.
In https://github.com/g3w-suite/g3w-suite-docker/pull/77#discussion_r979963606 i was thinking if it could be feasible to replace the if condition with the default_server
directive (see also: How nginx processes a request), anyway, as you can see I still don't have a very clear idea about it..
BTW, another solution, which would not require changes to the nginx configuration files, could be to always activate ports 80 and 443 inside the container and expose
them to the public only when needed (e.g. via docker-compose.override.yml
, ref: https://github.com/g3w-suite/g3w-suite-docker/pull/79)
So if there is no rush, I would wait a moment longer before merging this.
List of changes:
config/_nginx/django_ssl.conf
config/nginx/django.conf
accordingly in order to use a single HTTP/HTTPS serverWEBGIS_PUBLIC_HOSTNAME
environment variable tonginx
container (docker-compose.yml
)WEBGIS_PUBLIC_HOSTNAME
environment variable tonginx
container (docker-compose-consumer.yml
)README.md
section related toHTTPS additional setup
run_certbot.sh
(to improve readability and terminal output)Closes: https://github.com/g3w-suite/g3w-suite-docker/issues/67, closes: https://github.com/g3w-suite/g3w-suite-docker/issues/68