From caf1d3f95cceb643fd60069a9d5686ab1e39ba81 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 13 Dec 2023 13:45:04 +0530 Subject: [PATCH] chore: Auto-fix invalid custom domain (#29550) Defining custom domain as `https://example.com/` is invalid. It should be just the domain, just `example.com`. But turns out a lot of our users have the incorrect configuration, and our previous stack of NGINX+Certbot was able to ignore this and serve without HTTPS. This PR brings that behaviour back. ## Test performed Have Appsmith running on an EC2 instance, and a domain `correct.com` with an A-record pointed to this EC2 instance. In the instance, we run Appsmith with `APPSMITH_CUSTOM_DOMAIN` set to `wrong.com`. Caddy will obviously fail to provision the cert, and so we expect it to accept connections on just HTTP. So hitting `curl -i http://correct.com` produced a 200 with the HTML response, and not a 308 with a redirect. Before the changes from this PR, the same curl command produced a 308 with a redirect to `https://correct.com`, which fails with a certificate error. Next up, we run Appsmith with `APPSMITH_CUSTOM_DOMAIN` set to `correct.com`. Caddy will succeed in provisioning a cert, and so we expect HTTP URLs to be redirected to HTTPS. So hitting `curl -i http://correct.com` produces a 308 redirect to `http://correct.com` which then works fine, since Caddy now has the cert for the domain. --- deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs b/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs index 02d7c121aa..2c3d37c123 100644 --- a/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs +++ b/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs @@ -33,6 +33,14 @@ const tlsConfig = certLocation == null ? "" : `tls ${certLocation}/fullchain.pem const frameAncestorsPolicy = (process.env.APPSMITH_ALLOWED_FRAME_ANCESTORS || "'self'") .replace(/;.*$/, "") +const bind = [ + // The custom domain is expected to only have the domain. So if it has protocol or trailing slash, we remove it. + (APPSMITH_CUSTOM_DOMAIN || "").replace(/^https?:\/\//, "").replace(/\/$/, ""), + // Also bind to http on 80, so that if the cert provisioning fails, we can still serve on http. + // But this still means that if cert provisioning is successful, http will be redirected to https. + ":80", +].join(" ") + const parts = [] parts.push(` @@ -121,7 +129,7 @@ localhost:80 127.0.0.1:80 { import all-config } -${APPSMITH_CUSTOM_DOMAIN || ":80"} { +${bind} { import all-config ${tlsConfig} }