Improve invitation email when SSO is active (#23631)

* Update invitation email CTA link when SSO is active

* Update FE copy when inviting members with SSO configured

* Remove irrelevant comment

* Add relevant comments

* Match FE logic with BE

* Address feedback: Fix smtp setup

* Make BE function condition more obvious

* Address review

* Address feedback move code to settings.ts

* Fix the logic to not only send new email for EE but also OSS

* Fix SSO check logic to not require BE change.

* add a test for invitation email when sso is enabled and password login is disabled

Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>

enterprise/frontend/src/metabase-enterprise/auth/index.js

@@ -248,7 +248,7 @@ PLUGIN_AUTH_PROVIDERS.push(providers => {
   if (MetabaseSettings.get("other-sso-configured?")) {
     providers = [SSO_PROVIDER, ...providers];
   }
-  if (!MetabaseSettings.get("enable-password-login")) {
+  if (!MetabaseSettings.isPasswordLoginEnabled()) {
     providers = providers.filter(p => p.name !== "password");
   }
   return providers;
@@ -258,7 +258,7 @@ PLUGIN_IS_PASSWORD_USER.push(
   user =>
     !user.google_auth &&
     !user.ldap_auth &&
-    MetabaseSettings.get("enable-password-login"),
+    MetabaseSettings.isPasswordLoginEnabled(),
 );
 
 PLUGIN_ADMIN_SETTINGS_UPDATES.push(sections =>

frontend/src/metabase/admin/people/containers/UserSuccessModal.jsx

@@ -6,6 +6,7 @@ import _ from "underscore";
 import { connect } from "react-redux";
 import { push } from "react-router-redux";
 
+import MetabaseSettings from "metabase/lib/settings";
 import User from "metabase/entities/users";
 import { clearTemporaryPassword } from "../people";
 import { getUserTemporaryPassword } from "../selectors";
@@ -22,6 +23,9 @@ class UserSuccessModal extends React.Component {
   }
   render() {
     const { onClose, user, temporaryPassword } = this.props;
+    const isSsoConfigured =
+      MetabaseSettings.isSsoConfigured() &&
+      !MetabaseSettings.isPasswordLoginEnabled();
     return (
       <ModalContent
         title={t`${user.common_name} has been added`}
@@ -31,35 +35,32 @@ class UserSuccessModal extends React.Component {
         {temporaryPassword ? (
           <PasswordSuccess user={user} temporaryPassword={temporaryPassword} />
         ) : (
-          <EmailSuccess user={user} />
+          <EmailSuccess isEeSsoConfigured={isSsoConfigured} user={user} />
         )}
       </ModalContent>
     );
   }
 }
 
-export default _.compose(
-  User.load({
-    id: (state, props) => props.params.userId,
-  }),
-  connect(
-    (state, props) => ({
-      temporaryPassword: getUserTemporaryPassword(state, {
-        userId: props.params.userId,
-      }),
-    }),
-    {
-      onClose: () => push("/admin/people"),
-      clearTemporaryPassword,
-    },
-  ),
-)(UserSuccessModal);
-
-const EmailSuccess = ({ user }) => (
-  <div>{jt`We’ve sent an invite to ${(
-    <strong>{user.email}</strong>
-  )} with instructions to set their password.`}</div>
-);
+const EmailSuccess = ({ user, isEeSsoConfigured }) => {
+  if (isEeSsoConfigured) {
+    return (
+      <div>{jt`We’ve sent an invite to ${(
+        <strong>{user.email}</strong>
+      )} with instructions to log in. If this user is unable to authenticate then you can ${(
+        <Link
+          to={`/admin/people/${user.id}/reset`}
+          className="link"
+        >{t`reset their password.`}</Link>
+      )}`}</div>
+    );
+  }
+  return (
+    <div>{jt`We’ve sent an invite to ${(
+      <strong>{user.email}</strong>
+    )} with instructions to set their password.`}</div>
+  );
+};
 
 const PasswordSuccess = ({ user, temporaryPassword }) => (
   <div>
@@ -82,3 +83,20 @@ const PasswordSuccess = ({ user, temporaryPassword }) => (
     </div>
   </div>
 );
+
+export default _.compose(
+  User.load({
+    id: (state, props) => props.params.userId,
+  }),
+  connect(
+    (state, props) => ({
+      temporaryPassword: getUserTemporaryPassword(state, {
+        userId: props.params.userId,
+      }),
+    }),
+    {
+      onClose: () => push("/admin/people"),
+      clearTemporaryPassword,
+    },
+  ),
+)(UserSuccessModal);

frontend/src/metabase/auth/containers/ForgotPasswordApp/ForgotPasswordApp.tsx

@@ -5,8 +5,8 @@ import { forgotPassword } from "../../actions";
 
 const canResetPassword = () => {
   const isEmailConfigured = MetabaseSettings.isEmailConfigured();
-  const isLdapEnabled = MetabaseSettings.ldapEnabled();
-  return isEmailConfigured && !isLdapEnabled;
+  const isLdapConfigured = MetabaseSettings.isLdapConfigured();
+  return isEmailConfigured && !isLdapConfigured;
 };
 
 const mapStateToProps = (state: any, props: any) => ({

frontend/src/metabase/entities/users/forms.js

@@ -132,7 +132,7 @@ export default {
     ],
   }),
   login: () => {
-    const ldap = MetabaseSettings.ldapEnabled();
+    const ldap = MetabaseSettings.isLdapConfigured();
     const cookies = MetabaseSettings.get("session-cookies");
 
     return {

frontend/src/metabase/lib/settings.ts

@@ -76,6 +76,8 @@ export type SettingName =
   | "hide-embed-branding?"
   | "is-hosted?"
   | "ldap-configured?"
+  | "other-sso-configured?"
+  | "enable-password-login"
   | "map-tile-server-url"
   | "password-complexity"
   | "persisted-model-refresh-interval-hours"
@@ -167,10 +169,6 @@ class Settings {
     return this.get("cloud-gateway-ips") || [];
   }
 
-  googleAuthEnabled() {
-    return this.get("google-auth-client-id") != null;
-  }
-
   hasUserSetup() {
     return this.get("has-user-setup");
   }
@@ -179,10 +177,31 @@ class Settings {
     return this.get("hide-embed-branding?");
   }
 
-  ldapEnabled() {
+  isGoogleAuthConfigured() {
+    return this.get("google-auth-client-id") != null;
+  }
+
+  isLdapConfigured() {
     return this.get("ldap-configured?");
   }
 
+  // JWT or SAML is configured
+  isOtherSsoConfigured() {
+    return this.get("other-sso-configured?");
+  }
+
+  isSsoConfigured() {
+    return (
+      this.isGoogleAuthConfigured() ||
+      this.isLdapConfigured() ||
+      this.isGoogleAuthConfigured()
+    );
+  }
+
+  isPasswordLoginEnabled() {
+    return this.get("enable-password-login");
+  }
+
   searchTypeaheadEnabled() {
     return this.get("search-typeahead-enabled");
   }

frontend/src/metabase/plugins/builtin/auth/google.js

@@ -19,7 +19,7 @@ PLUGIN_AUTH_PROVIDERS.push(providers => {
     Button: require("metabase/auth/containers/GoogleButton").default,
   };
 
-  return MetabaseSettings.googleAuthEnabled()
+  return MetabaseSettings.isGoogleAuthConfigured()
     ? [googleProvider, ...providers]
     : providers;
 });

frontend/test/metabase/scenarios/admin/people/people.cy.spec.js

@@ -15,19 +15,19 @@ const TOTAL_USERS = Object.entries(USERS).length;
 const TOTAL_GROUPS = Object.entries(USER_GROUPS).length;
 const { ORDERS_ID } = SAMPLE_DATABASE;
 
+const TEST_USER = {
+  first_name: "Testy",
+  last_name: "McTestface",
+  email: `testy${Math.round(Math.random() * 100000)}@metabase.test`,
+  password: "12341234",
+};
+
 describe("scenarios > admin > people", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();
   });
 
-  const TEST_USER = {
-    first_name: "Testy",
-    last_name: "McTestface",
-    email: `testy${Math.round(Math.random() * 100000)}@metabase.test`,
-    password: "12341234",
-  };
-
   describe("user management", () => {
     it("should render (metabase-enterprise#210)", () => {
       cy.visit("/admin/people");
@@ -76,7 +76,6 @@ describe("scenarios > admin > people", () => {
       // first modal
       cy.findByLabelText("First name").type(first_name);
       cy.findByLabelText("Last name").type(last_name);
-      // bit of a hack since there are multiple "Email" nodes
       cy.findByLabelText("Email").type(email);
       clickButton("Create");
 
@@ -93,7 +92,6 @@ describe("scenarios > admin > people", () => {
       cy.visit("/admin/people");
       clickButton("Invite someone");
 
-      // bit of a hack since there are multiple "Email" nodes
       cy.findByLabelText("Email").type(email);
       clickButton("Create");
 
@@ -236,6 +234,78 @@ describe("scenarios > admin > people", () => {
       cy.findByText("readonly");
     });
 
+    describe("email configured", () => {
+      beforeEach(() => {
+        // Setup email server, since we show different modal message when email isn't configured
+        setupSMTP();
+
+        // Setup Google authentication
+        cy.request("PUT", "/api/setting", {
+          "google-auth-client-id": "fake-id.apps.googleusercontent.com",
+          "google-auth-auto-create-accounts-domain": "metabase.com",
+        });
+      });
+
+      it("invite member when SSO is not configured", () => {
+        const { first_name, last_name, email } = TEST_USER;
+        const FULL_NAME = `${first_name} ${last_name}`;
+        cy.visit("/admin/people");
+
+        clickButton("Invite someone");
+
+        // first modal
+        cy.findByLabelText("First name").type(first_name);
+        cy.findByLabelText("Last name").type(last_name);
+        cy.findByLabelText("Email").type(email);
+        clickButton("Create");
+
+        // second modal
+        cy.findByText(`${FULL_NAME} has been added`);
+        cy.contains(
+          `We’ve sent an invite to ${email} with instructions to set their password.`,
+        );
+        cy.findByText("Done").click();
+
+        cy.findByText(FULL_NAME);
+      });
+
+      it("invite member when SSO is configured metabase#23630", () => {
+        // Setup Google authentication
+        cy.request("PUT", "/api/setting", {
+          "enable-password-login": false,
+        });
+
+        const { first_name, last_name, email } = TEST_USER;
+        const FULL_NAME = `${first_name} ${last_name}`;
+        cy.visit("/admin/people");
+
+        clickButton("Invite someone");
+
+        // first modal
+        cy.findByLabelText("First name").type(first_name);
+        cy.findByLabelText("Last name").type(last_name);
+        cy.findByLabelText("Email").type(email);
+        clickButton("Create");
+
+        // second modal
+        cy.findByText(`${FULL_NAME} has been added`);
+        cy.contains(
+          `We’ve sent an invite to ${email} with instructions to log in. If this user is unable to authenticate then you can reset their password.`,
+        );
+        cy.url().then(url => {
+          const URL_REGEX = /\/admin\/people\/(?<userId>\d+)\/success/;
+          const { userId } = URL_REGEX.exec(url).groups;
+          assertLinkMatchesUrl(
+            "reset their password.",
+            `/admin/people/${userId}/reset`,
+          );
+        });
+        cy.findByText("Done").click();
+
+        cy.findByText(FULL_NAME);
+      });
+    });
+
     describe("pagination", () => {
       const NEW_USERS = 18;
       const NEW_TOTAL_USERS = TOTAL_USERS + NEW_USERS;
@@ -322,6 +392,9 @@ describeEE("scenarios > admin > people", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();
+  });
+
+  it("should unsubscribe a user from all subscriptions and alerts", () => {
     cy.getCurrentUser().then(({ body: { id: user_id } }) => {
       cy.createQuestionAndDashboard({
         questionDetails: getQuestionDetails(),
@@ -330,9 +403,6 @@ describeEE("scenarios > admin > people", () => {
         cy.createPulse(getPulseDetails({ card_id, dashboard_id }));
       });
     });
-  });
-
-  it("should unsubscribe a user from all subscriptions and alerts", () => {
     const { first_name, last_name } = admin;
     const fullName = `${first_name} ${last_name}`;
 
@@ -449,3 +519,9 @@ function getPulseDetails({ card_id, dashboard_id }) {
     ],
   };
 }
+
+function assertLinkMatchesUrl(text, url) {
+  cy.findByRole("link", { name: text })
+    .should("have.attr", "href")
+    .and("eq", url);
+}

src/metabase/models/user.clj

@@ -243,9 +243,13 @@
 (declare form-password-reset-url set-password-reset-token!)
 
 (defn- send-welcome-email! [new-user invitor sent-from-setup?]
-  (let [reset-token (set-password-reset-token! (u/the-id new-user))
-        ;; the new user join url is just a password reset with an indicator that this is a first time user
-        join-url    (str (form-password-reset-url reset-token) "#new")]
+  (let [reset-token               (set-password-reset-token! (u/the-id new-user))
+        should-link-to-login-page (and (public-settings/sso-configured?)
+                                       (not (public-settings/enable-password-login)))
+        join-url                  (if should-link-to-login-page
+                                    (str (public-settings/site-url) "/auth/login")
+                                    ;; NOTE: the new user join url is just a password reset with an indicator that this is a first time user
+                                    (str (form-password-reset-url reset-token) "#new"))]
     (classloader/require 'metabase.email.messages)
     ((resolve 'metabase.email.messages/send-new-user-email!) new-user invitor join-url sent-from-setup?)))
 

src/metabase/public_settings.clj

@@ -32,7 +32,7 @@
   (when-let [varr (resolve 'metabase-enterprise.sso.integrations.sso-settings/other-sso-configured?)]
     (varr)))
 
-(defn- sso-configured?
+(defn sso-configured?
   "Any SSO provider is configured"
   []
   (or (google-auth-configured?)

test/metabase/models/user_test.clj

@@ -23,6 +23,7 @@
             [metabase.models.user :as user]
             [metabase.test :as mt]
             [metabase.test.data.users :as test.users]
+            [metabase.test.integrations.ldap :as ldap.test]
             [metabase.util :as u]
             [metabase.util.password :as u.password]
             [toucan.db :as db]
@@ -181,7 +182,13 @@
         (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 "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]
+      (ldap.test/with-ldap-server
+        (invite-user-accept-and-check-inboxes! :invitor default-invitor , :accept-invite? false)
+        (is (not (empty? (mt/regex-email-bodies #"/auth/login"))))))))
 
 (deftest ldap-user-passwords-test
   (testing (str "LDAP users should not persist their passwords. Check that if somehow we get passed an LDAP user "