Look at X-Forwarded-Host when inferring site-url (#12529)

src/metabase/middleware/misc.clj

@@ -36,15 +36,16 @@
 ;;; ------------------------------------------------ SETTING SITE-URL ------------------------------------------------
 
 ;; It's important for us to know what the site URL is for things like returning links, etc. this is stored in the
-;; `site-url` Setting; we can set it automatically by looking at the `Origin` or `Host` headers sent with a request.
+;; `site-url` Setting; we can set it automatically by looking at the `Origin`, `X-Forwarded-Host`, or `Host` headers
+;; sent with a request.
+;;
 ;; Effectively the very first API request that gets sent to us (usually some sort of setup request) ends up setting
 ;; the (initial) value of `site-url`
-
-(defn- maybe-set-site-url* [{{:strs [origin host] :as headers} :headers, :as request}]
+(defn- maybe-set-site-url* [{{:strs [origin x-forwarded-host host] :as headers} :headers, :as request}]
   (when (and (mdb/db-is-setup?)
              (not (public-settings/site-url))
              api/*current-user*)
-    (when-let [site-url (or origin host)]
+    (when-let [site-url (or origin x-forwarded-host host)]
       (log/info (trs "Setting Metabase site URL to {0}" site-url))
       (try
         (public-settings/site-url site-url)

test/metabase/middleware/misc_test.clj

+(ns metabase.middleware.misc-test
+  (:require [clojure.test :refer :all]
+            [medley.core :as m]
+            [metabase
+             [public-settings :as public-settings]
+             [test :as mt]]
+            [metabase.middleware.misc :as mw.misc]
+            [ring.mock.request :as ring.mock]))
+
+(deftest maybe-set-site-url-test
+  (testing "Make sure `maybe-set-site-url` middleware looks at the correct headers in the correct order (#12528)"
+    (let [handler            (fn [request respond _]
+                               (respond request))
+          maybe-set-site-url (fn [request]
+                               ((mw.misc/maybe-set-site-url handler) request identity (fn [e] (throw e))))]
+      (doseq [origin-header           ["https://mb1.example.com" nil]
+              x-forwarded-host-header ["https://mb2.example.com" nil]
+              host-header             ["https://mb3.example.com" nil]
+              :let                    [request (cond-> (m/dissoc-in (ring.mock/request :get "/") [:headers "host"])
+                                                 origin-header           (ring.mock/header "Origin" origin-header)
+                                                 x-forwarded-host-header (ring.mock/header "X-Forwarded-Host" x-forwarded-host-header)
+                                                 host-header             (ring.mock/header "Host" host-header))]]
+        (testing (format "headers = %s" (pr-str (:headers request)))
+          (mt/with-temporary-setting-values [site-url nil]
+            (maybe-set-site-url request)
+            (is (= (or origin-header x-forwarded-host-header host-header)
+                   (public-settings/site-url)))))))))