Only log session cookie SameSite=None warning when connection is insecure (#33386)

src/metabase/config.clj

@@ -139,7 +139,7 @@
   #_{:clj-kondo/ignore [:discouraged-var]}
   (let [same-site (str/lower-case (config-str :mb-session-cookie-samesite))]
     (when-not (#{"none", "lax", "strict"} same-site)
-      (throw (ex-info "Invalid value for MB_COOKIE_SAMESITE" {:mb-session-cookie-samesite same-site})))
+      (throw (ex-info "Invalid value for MB_SESSION_COOKIE_SAMESITE" {:mb-session-cookie-samesite same-site})))
     (keyword same-site)))
 
 (def ^Keyword mb-session-cookie-samesite

src/metabase/server/middleware/session.clj

@@ -156,7 +156,7 @@
                         ;; max-session age-is in minutes; Max-Age= directive should be in seconds
                         (when (use-permanent-cookies? request)
                           {:max-age (* 60 (config/config-int :max-session-age))}))]
-    (when (and (= config/mb-session-cookie-samesite :none) (request.u/https? request))
+    (when (and (= config/mb-session-cookie-samesite :none) (not (request.u/https? request)))
       (log/warn
        (str (deferred-trs "Session cookie's SameSite is configured to \"None\", but site is served over an insecure connection. Some browsers will reject cookies under these conditions.")
             " "

test/metabase/server/middleware/session_test.clj

@@ -49,7 +49,7 @@
            (with-redefs [env/env (assoc env/env :mb-session-cookie-samesite "NONE")]
              (#'config/mb-session-cookie-samesite*))))
 
-    (is (thrown-with-msg? ExceptionInfo #"Invalid value for MB_COOKIE_SAMESITE"
+    (is (thrown-with-msg? ExceptionInfo #"Invalid value for MB_SESSION_COOKIE_SAMESITE"
           (with-redefs [env/env (assoc env/env :mb-session-cookie-samesite "invalid value")]
             (#'config/mb-session-cookie-samesite*))))))
 
@@ -81,6 +81,24 @@
                  (-> (mw.session/set-session-cookies {:body {:remember true}} {} {:id uuid, :type :normal} request-time)
                      (get-in [:cookies "metabase.SESSION"])))))))))
 
+(deftest samesite-none-log-warning-test
+  (with-redefs [config/mb-session-cookie-samesite :none]
+    (let [session {:id   (random-uuid)
+                   :type :normal}
+          request-time (t/zoned-date-time "2022-07-06T02:00Z[UTC]")]
+         (testing "should log a warning if SameSite is configured to \"None\" and the site is served over an insecure connection."
+           (is (contains? (into #{}
+                                (map (fn [[_log-level _error message]] message))
+                                (mt/with-log-messages-for-level :warn
+                                  (mw.session/set-session-cookies {:headers {"x-forwarded-proto" "http"}} {} session request-time)))
+                          "Session cookies SameSite is configured to \"None\", but site is served over an insecure connection. Some browsers will reject cookies under these conditions. https://www.chromestatus.com/feature/5633521622188032")))
+         (testing "should not log a warning over a secure connection."
+           (is (not (contains? (into #{}
+                                     (map (fn [[_log-level _error message]] message))
+                                     (mt/with-log-messages-for-level :warn
+                                       (mw.session/set-session-cookies {:headers {"x-forwarded-proto" "https"}} {} session request-time)))
+                               "Session cookies SameSite is configured to \"None\", but site is served over an insecure connection. Some browsers will reject cookies under these conditions. https://www.chromestatus.com/feature/5633521622188032")))))))
+
 ;; if request is an HTTPS request then we should set `:secure true`. There are several different headers we check for
 ;; this. Make sure they all work.
 (deftest ^:parallel secure-cookie-test