Don't notify admins of new SSO users when setting is disabled (#29366)

* initial changes

* styling change

* address comments

* fix sso settings

* update test

* update test

* remove warning

* fix test

enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj

@@ -3,7 +3,9 @@
   the SSO backends and the generic routing code used to determine which SSO backend to use need this
   information. Separating out this information creates a better dependency graph and avoids circular dependencies."
   (:require
+   [metabase.integrations.common :as integrations.common]
    [metabase.models.setting :as setting :refer [defsetting]]
+   [metabase.models.setting.multi-setting :refer [define-multi-setting-impl]]
    [metabase.util.i18n :refer [deferred-tru trs tru]]
    [metabase.util.log :as log]
    [metabase.util.schema :as su]
@@ -160,10 +162,9 @@ on your IdP, this usually looks something like http://www.example.com/141xkex604
                (setting/get-value-of-type :boolean :jwt-enabled)
                false)))
 
-(defsetting send-new-sso-user-admin-email?
-  (deferred-tru "Should new email notifications be sent to admins, for all new SSO users?")
-  :type :boolean
-  :default true)
+(define-multi-setting-impl integrations.common/send-new-sso-user-admin-email? :ee
+  :getter (fn [] (setting/get-value-of-type :boolean :send-new-sso-user-admin-email?))
+  :setter (fn [send-emails] (setting/set-value-of-type! :boolean :send-new-sso-user-admin-email? send-emails)))
 
 (defsetting other-sso-enabled?
   "Are we using an SSO integration other than LDAP or Google Auth? These integrations use the `/auth/sso` endpoint for

enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj

 (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.integrations.common :as integrations.common]
    [metabase.models.user :refer [User]]
    [metabase.public-settings :as public-settings]
    [metabase.util :as u]
@@ -36,7 +36,7 @@
   (u/prog1 (db/insert! User (merge user {:password (str (UUID/randomUUID))}))
     (log/info (trs "New SSO user created: {0} ({1})" (:common_name <>) (:email <>)))
     ;; send an email to everyone including the site admin if that's set
-    (when (sso-settings/send-new-sso-user-admin-email?)
+    (when (integrations.common/send-new-sso-user-admin-email?)
       (messages/send-user-joined-admin-notification-email! <>, :google-auth? true))))
 
 (defn fetch-and-update-login-attributes!

src/metabase/integrations/common.clj

@@ -7,8 +7,12 @@
    [metabase.models.permissions-group-membership
     :as perms-group-membership
     :refer [PermissionsGroupMembership]]
+   [metabase.models.setting.multi-setting :refer [define-multi-setting
+                                                  define-multi-setting-impl]]
+   [metabase.public-settings.premium-features :as premium-features]
    [metabase.util :as u]
-   [metabase.util.i18n :refer [trs]]
+   [metabase.util.i18n :refer [deferred-tru
+                               trs]]
    [metabase.util.log :as log]
    [toucan.db :as db]
    [toucan2.core :as t2]))
@@ -52,3 +56,11 @@
         (db/insert! PermissionsGroupMembership :group_id id, :user_id user-id)
         (catch Throwable e
           (log/error e (trs "Error adding User {0} to Group {1}" user-id id)))))))
+
+(define-multi-setting send-new-sso-user-admin-email?
+  (deferred-tru "Should new email notifications be sent to admins, for all new SSO users?")
+  (fn [] (if (premium-features/enable-sso?) :ee :oss)))
+
+(define-multi-setting-impl send-new-sso-user-admin-email? :oss
+  :getter (fn [] (constantly true))
+  :setter :none)

src/metabase/models/user.clj

@@ -3,6 +3,7 @@
    [clojure.data :as data]
    [clojure.string :as str]
    [metabase.db.query :as mdb.query]
+   [metabase.integrations.common :as integrations.common]
    [metabase.models.collection :as collection]
    [metabase.models.interface :as mi]
    [metabase.models.permissions :as perms]
@@ -332,8 +333,9 @@
   [new-user :- NewUser]
   (u/prog1 (insert-new-user! (assoc new-user :google_auth true))
     ;; send an email to everyone including the site admin if that's set
-    (classloader/require 'metabase.email.messages)
-    ((resolve 'metabase.email.messages/send-user-joined-admin-notification-email!) <>, :google-auth? true)))
+    (when (integrations.common/send-new-sso-user-admin-email?)
+      (classloader/require 'metabase.email.messages)
+      ((resolve 'metabase.email.messages/send-user-joined-admin-notification-email!) <>, :google-auth? true))))
 
 (schema/defn create-new-ldap-auth-user!
   "Convenience for creating a new user via LDAP. This account is considered active immediately; thus all active admins

test/metabase/models/user_test.clj

@@ -3,19 +3,12 @@
    [clojure.set :as set]
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [metabase.config :as config]
    [metabase.http-client :as client]
    [metabase.integrations.google]
    [metabase.models
-    :refer [Collection
-            Database
-            PermissionsGroup
-            PermissionsGroupMembership
-            Pulse
-            PulseChannel
-            PulseChannelRecipient
-            Session
-            Table
-            User]]
+    :refer [Collection Database PermissionsGroup PermissionsGroupMembership
+            Pulse PulseChannel PulseChannelRecipient Session Table User]]
    [metabase.models.collection :as collection]
    [metabase.models.collection-test :as collection-test]
    [metabase.models.permissions :as perms]
@@ -23,6 +16,7 @@
    [metabase.models.permissions-test :as perms-test]
    [metabase.models.serialization :as serdes]
    [metabase.models.user :as user]
+   [metabase.public-settings.premium-features-test :as premium-features-test]
    [metabase.test :as mt]
    [metabase.test.data.users :as test.users]
    [metabase.test.integrations.ldap :as ldap.test]
@@ -185,11 +179,20 @@
                  (-> (invite-user-accept-and-check-inboxes! :google-auth? true)
                      (select-keys ["crowberto@metabase.com" "some_other_admin@metabase.com" "cam2@metabase.com"]))))))
 
-      (testing "...unless they are inactive"
+      (testing "...unless they are inactive..."
         (mt/with-temp User [user {:is_superuser true, :is_active false}]
           (is (= {"crowberto@metabase.com" ["<New User> created a Metabase account"]}
                  (-> (invite-user-accept-and-check-inboxes! :google-auth? true)
-                     (select-keys ["crowberto@metabase.com" (:email user)]))))))))
+                     (select-keys ["crowberto@metabase.com" (:email user)])))))
+
+        (testing "...or if setting is disabled"
+          (premium-features-test/with-premium-features #{:sso}
+            (mt/with-temporary-raw-setting-values [send-new-sso-user-admin-email? "false"]
+              (mt/with-temp User [_ {:is_superuser true, :email "some_other_admin@metabase.com"}]
+                (is (= (if config/ee-available? {} {"crowberto@metabase.com" ["<New User> created a Metabase account"],
+                                                    "some_other_admin@metabase.com" ["<New User> created a Metabase account"]})
+                       (-> (invite-user-accept-and-check-inboxes! :google-auth? true)
+                           (select-keys ["crowberto@metabase.com" "some_other_admin@metabase.com"])))))))))))
 
   (testing "if sso enabled and password login is disabled, email should send a link to sso login"
     (mt/with-temporary-setting-values [enable-password-login false]