From 43ff98be7f11d7331d91a92635367890651d8af2 Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Mon, 14 Aug 2023 12:35:21 -0400
Subject: [PATCH] Fix redirect URI validation for relative paths (#33100)
* don't throw when host is nil
* fix test ns name
* Update enterprise/backend/test/metabase_enterprise/sso/integrations/sso_utils_test.clj
Co-authored-by: Cam Saul <1455846+camsaul@users.noreply.github.com>
---------
Co-authored-by: Cam Saul <1455846+camsaul@users.noreply.github.com>
---
.../sso/integrations/sso_utils.clj | 2 +-
.../sso/integrations/jwt_test.clj | 2 +-
.../sso/integrations/sso_utils_test.clj | 17 +++++++++++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 enterprise/backend/test/metabase_enterprise/sso/integrations/sso_utils_test.clj
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
index c3e8665c6ee..791ce4f1eea 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
@@ -59,7 +59,7 @@
(let [decoded-url (some-> ^String redirect-url (URLDecoder/decode "UTF-8"))
host (some-> decoded-url (URI.) (.getHost))
our-host (some-> (public-settings/site-url) (URI.) (.getHost))]
- (api/check-400 (or (nil? decoded-url) (= host our-host))))
+ (api/check-400 (or (nil? decoded-url) (nil? host) (= host our-host))))
(catch Exception e
(log/error e "Invalid redirect URL")
(throw (ex-info (tru "Invalid redirect URL")
diff --git a/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj b/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj
index e91d1f9f6d4..7fac090d403 100644
--- a/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj
@@ -31,7 +31,7 @@
(use-fixtures :each disable-other-sso-types)
(def ^:private default-idp-uri "http://test.idp.metabase.com")
-(def ^:private default-redirect-uri "http://localhost:3000/test")
+(def ^:private default-redirect-uri "/")
(def ^:private default-jwt-secret (crypto-random/hex 32))
(defmacro with-sso-jwt-token
diff --git a/enterprise/backend/test/metabase_enterprise/sso/integrations/sso_utils_test.clj b/enterprise/backend/test/metabase_enterprise/sso/integrations/sso_utils_test.clj
new file mode 100644
index 00000000000..8154a2c66ec
--- /dev/null
+++ b/enterprise/backend/test/metabase_enterprise/sso/integrations/sso_utils_test.clj
@@ -0,0 +1,17 @@
+(ns metabase-enterprise.sso.integrations.sso-utils-test
+ (:require [clojure.test :refer :all]
+ [metabase-enterprise.sso.integrations.sso-utils :as sso-utils]))
+
+(deftest ^:parallel check-sso-redirect-test
+ (testing "check-sso-redirect properly validates redirect URIs"
+ (are [uri] (sso-utils/check-sso-redirect uri)
+ "/"
+ "/test"
+ "localhost"
+ "localhost:3000"
+ "http://localhost:3000"))
+
+ (testing "check-sso-redirect- throws an error for invalid redirect URIs"
+ (are [uri] (thrown-with-msg? clojure.lang.ExceptionInfo #"Invalid redirect URL" (sso-utils/check-sso-redirect uri))
+ "http://example.com"
+ "//example.com")))
--
GitLab