From a9b9c0fe7d6bba245d8b15644ba4a44db563310c Mon Sep 17 00:00:00 2001
From: Jerry Huang <34140255+qwef@users.noreply.github.com>
Date: Thu, 18 Jan 2024 17:45:00 -0500
Subject: [PATCH] Add user-provisioning-enabled? setting and logic (#37770)
* add user-provisioning code
* sort namespace
* fix linter
* fix linter
* swap to ee
* fix logic
* add tests
* address comments
* fix linter
* update tests, address comments
---
.clj-kondo/hooks/metabase/models/setting.clj | 3 +++
.../enhancements/integrations/ldap.clj | 2 ++
.../sso/integrations/jwt.clj | 1 +
.../sso/integrations/saml.clj | 1 +
.../sso/integrations/sso_settings.clj | 23 ++++++++++++++++
.../sso/integrations/sso_utils.clj | 26 ++++++++++++++++++-
.../enhancements/integrations/ldap_test.clj | 13 ++++++++++
.../sso/integrations/jwt_test.clj | 13 ++++++++++
.../sso/integrations/saml_test.clj | 14 ++++++++++
9 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/.clj-kondo/hooks/metabase/models/setting.clj b/.clj-kondo/hooks/metabase/models/setting.clj
index 4ec17037ea6..4c64701a4bb 100644
--- a/.clj-kondo/hooks/metabase/models/setting.clj
+++ b/.clj-kondo/hooks/metabase/models/setting.clj
@@ -62,6 +62,7 @@
jwt-group-mappings
jwt-group-sync
jwt-identity-provider-uri
+ jwt-user-provisioning-enabled?
jwt-shared-secret
last-acknowledged-version
ldap-attribute-email
@@ -77,6 +78,7 @@
ldap-host
ldap-password
ldap-port
+ ldap-user-provisioning-enabled?
ldap-security
ldap-sync-user-attributes
ldap-sync-user-attributes-blacklist
@@ -128,6 +130,7 @@
saml-keystore-alias
saml-keystore-password
saml-keystore-path
+ saml-user-provisioning-enabled?
send-email-on-first-login-from-new-device
send-new-sso-user-admin-email?
session-cookie-samesite
diff --git a/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj b/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
index 01b284c4fa6..cae4a48bc4c 100644
--- a/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
+++ b/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
@@ -1,6 +1,7 @@
(ns metabase-enterprise.enhancements.integrations.ldap
"The Enterprise version of the LDAP integration is basically the same but also supports syncing user attributes."
(:require
+ [metabase-enterprise.sso.integrations.sso-utils :as sso-utils]
[metabase.integrations.common :as integrations.common]
[metabase.integrations.ldap.default-implementation :as default-impl]
[metabase.models.setting :as setting :refer [defsetting]]
@@ -83,6 +84,7 @@
[{:keys [first-name last-name email groups attributes], :as user-info} :- EEUserInfo
{:keys [sync-groups?], :as settings} :- default-impl/LDAPSettings]
(let [user (or (attribute-synced-user user-info)
+ (sso-utils/check-user-provisioning :ldap)
(-> (user/create-new-ldap-auth-user! {:first_name first-name
:last_name last-name
:email email
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
index 02045148b03..0fd300419a1 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
@@ -31,6 +31,7 @@
:sso_source :jwt
:login_attributes user-attributes}]
(or (sso-utils/fetch-and-update-login-attributes! user)
+ (sso-utils/check-user-provisioning :jwt)
(sso-utils/create-new-sso-user! user))))
(def ^:private ^{:arglists '([])} jwt-attribute-email (comp keyword sso-settings/jwt-attribute-email))
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
index 90ace6e59ca..fc1d4cc318a 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
@@ -87,6 +87,7 @@
:sso_source :saml
:login_attributes user-attributes}]
(when-let [user (or (sso-utils/fetch-and-update-login-attributes! new-user)
+ (sso-utils/check-user-provisioning :saml)
(sso-utils/create-new-sso-user! new-user))]
(sync-groups! user group-names)
(api.session/create-session! :sso user device-info))))
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
index e7ec136a2f5..5024e8bbf9f 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
@@ -23,6 +23,29 @@
(def ^:private ^{:arglists '([group-mappings])} validate-group-mappings
(mc/validator GroupMappings))
+(defsetting saml-user-provisioning-enabled?
+ (deferred-tru "When we enable SAML user provisioning, we automatically create a Metabase account on SAML signin for users who
+don''t have one.")
+ :type :boolean
+ :default true
+ :feature :sso-saml
+ :audit :getter)
+
+(defsetting jwt-user-provisioning-enabled?
+ (deferred-tru "When we enable JWT user provisioning, we automatically create a Metabase account on JWT signin for users who
+don''t have one.")
+ :type :boolean
+ :default true
+ :feature :sso-jwt
+ :audit :getter)
+
+(defsetting ldap-user-provisioning-enabled?
+ (deferred-tru "When we enable LDAP user provisioning, we automatically create a Metabase account on LDAP signin for users who
+don''t have one.")
+ :type :boolean
+ :default true
+ :audit :getter)
+
(defsetting saml-identity-provider-uri
(deferred-tru "This is the URL where your users go to log in to your identity provider. Depending on which IdP you''re
using, this usually looks like https://your-org-name.example.com or https://example.com/app/my_saml_app/abc123/sso/saml")
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 a65b122ee31..f471e8a7e2f 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
@@ -1,6 +1,7 @@
(ns metabase-enterprise.sso.integrations.sso-utils
"Functions shared by the various SSO implementations"
(:require
+ [metabase-enterprise.sso.integrations.sso-settings :as sso-settings]
[metabase.api.common :as api]
[metabase.email.messages :as messages]
[metabase.events :as events]
@@ -29,10 +30,33 @@
[:sso_source [:enum :saml :jwt]]
[:login_attributes [:maybe :map]]])
+(defn- maybe-throw-user-provisioning
+ [user-provisioning-type]
+ (when (not user-provisioning-type)
+ (throw (ex-info (trs "Sorry, but you''ll need a {0} account to view this page. Please contact your administrator."
+ (u/slugify (public-settings/site-name))) {}))))
+
+(defmulti check-user-provisioning
+ "If `user-provisioning-enabled?` is false, then we should throw an error when attempting to create a new user."
+ {:arglists '([model])}
+ keyword)
+
+(defmethod check-user-provisioning :saml
+ [_]
+ (maybe-throw-user-provisioning (sso-settings/saml-user-provisioning-enabled?)))
+
+(defmethod check-user-provisioning :ldap
+ [_]
+ (maybe-throw-user-provisioning (sso-settings/ldap-user-provisioning-enabled?)))
+
+(defmethod check-user-provisioning :jwt
+ [_]
+ (maybe-throw-user-provisioning (sso-settings/jwt-user-provisioning-enabled?)))
+
(mu/defn create-new-sso-user!
"This function is basically the same thing as the `create-new-google-auth-user` from `metabase.models.user`. We need
to refactor the `core_user` table structure and the function used to populate it so that the enterprise product can
- reuse it"
+ reuse it."
[user :- UserAttributes]
(try
(u/prog1 (first (t2/insert-returning-instances! User (merge user {:password (str (random-uuid))})))
diff --git a/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj b/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
index 01508c6c26c..9f9da6588e5 100644
--- a/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
@@ -2,8 +2,10 @@
(:require
[clojure.test :refer :all]
[metabase-enterprise.enhancements.integrations.ldap :as ldap-ee]
+ [metabase-enterprise.sso.integrations.sso-settings :as sso-settings]
[metabase.integrations.ldap :as ldap]
[metabase.models.user :as user :refer [User]]
+ [metabase.public-settings :as public-settings]
[metabase.test :as mt]
[metabase.test.integrations.ldap :as ldap.test]
[metabase.util.malli.schema :as ms]
@@ -247,3 +249,14 @@
:common_name "jane.miller@metabase.com"}
(select-keys (t2/select-one User :email "jane.miller@metabase.com") [:first_name :last_name :common_name]))))
(finally (t2/delete! User :email "jane.miller@metabase.com"))))))
+
+(deftest ldap-no-user-provisioning-test
+ (mt/with-premium-features #{:sso-ldap}
+ (ldap.test/with-ldap-server
+ (testing "an error is thrown when a new user is fetched and user provisioning is not enabled"
+ (with-redefs [sso-settings/ldap-user-provisioning-enabled? (constantly false)
+ public-settings/site-name (constantly "test")]
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Sorry, but you'll need a test account to view this page. Please contact your administrator."
+ (ldap/fetch-or-create-user! (ldap/find-user "jsmith1")))))))))
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 b545755fba8..e4dc77137cd 100644
--- a/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sso/integrations/jwt_test.clj
@@ -7,11 +7,13 @@
[crypto.random :as crypto-random]
[metabase-enterprise.sso.integrations.jwt :as mt.jwt]
[metabase-enterprise.sso.integrations.saml-test :as saml-test]
+ [metabase-enterprise.sso.integrations.sso-settings :as sso-settings]
[metabase.config :as config]
[metabase.models.permissions-group :refer [PermissionsGroup]]
[metabase.models.permissions-group-membership
:refer [PermissionsGroupMembership]]
[metabase.models.user :refer [User]]
+ [metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features :as premium-features]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
@@ -294,3 +296,14 @@
(is (= #{"All Users"
":metabase-enterprise.sso.integrations.jwt-test/my-group"}
(group-memberships (u/the-id (t2/select-one-pk User :email "newuser@metabase.com"))))))))))))
+
+(deftest create-new-jwt-user-no-user-provisioning-test
+ (testing "When user provisioning is disabled, throw an error if we attempt to create a new user."
+ (with-jwt-default-setup
+ (with-redefs [sso-settings/jwt-user-provisioning-enabled? (constantly false)
+ public-settings/site-name (constantly "test")]
+ (is
+ (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Sorry, but you'll need a test account to view this page. Please contact your administrator."
+ (#'mt.jwt/fetch-or-create-user! "Test" "User" "test1234@metabase.com" nil)))))))
diff --git a/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj b/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj
index 78052d99bde..cf9bbce18b9 100644
--- a/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sso/integrations/saml_test.clj
@@ -3,6 +3,7 @@
[clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer :all]
+ [metabase-enterprise.sso.integrations.saml :as saml.mt]
[metabase-enterprise.sso.integrations.sso-settings :as sso-settings]
[metabase.http-client :as client]
[metabase.models.permissions-group :refer [PermissionsGroup]]
@@ -636,3 +637,16 @@
(is (successful-login? response))
(is (= (str "http://localhost:3001/path" redirect-url)
(get-in response [:headers "Location"])))))))))))))))
+
+(deftest create-new-saml-user-no-user-provisioning-test
+ (testing "When user provisioning is disabled, throw an error if we attempt to create a new user."
+ (with-saml-default-setup
+ (with-redefs [sso-settings/saml-user-provisioning-enabled? (constantly false)
+ public-settings/site-name (constantly "test")]
+ (is
+ (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Sorry, but you'll need a test account to view this page. Please contact your administrator."
+ (#'saml.mt/fetch-or-create-user! {:first-name "Test"
+ :last-name "user"
+ :email "test1234@metabsae.com"})))))))
--
GitLab