From c80b5bab7a056a14fb2749c6a0dc9e988167258d Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Wed, 9 Jun 2021 10:43:54 -0700
Subject: [PATCH] Refactor Google sign-in settings page to use Form component
(#16506)
---
.../auth/components/SettingsSAMLForm.jsx | 10 +-
.../components/SettingsGoogleForm.jsx | 89 +++++++++
.../components/SettingsSingleSignOnForm.jsx | 170 ------------------
frontend/src/metabase/admin/settings/utils.js | 11 ++
.../metabase/plugins/builtin/auth/google.js | 15 +-
.../admin/settings/settings.cy.spec.js | 3 +-
6 files changed, 113 insertions(+), 185 deletions(-)
create mode 100644 frontend/src/metabase/admin/settings/components/SettingsGoogleForm.jsx
delete mode 100644 frontend/src/metabase/admin/settings/components/SettingsSingleSignOnForm.jsx
diff --git a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm.jsx b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm.jsx
index c7a66ccbd06..420a087df1e 100644
--- a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm.jsx
@@ -6,6 +6,7 @@ import _ from "underscore";
import { Box } from "grid-styled";
import { updateSettings } from "metabase/admin/settings/settings";
+import { settingToFormField } from "metabase/admin/settings/utils";
import Form, {
FormField,
@@ -21,15 +22,6 @@ import GroupMappingsWidget from "metabase/admin/settings/components/widgets/Grou
import MetabaseSettings from "metabase/lib/settings";
-const settingToFormField = setting => ({
- name: setting.key,
- description: setting.description,
- placeholder: setting.is_env_setting
- ? t`Using ${setting.env_name}`
- : setting.placeholder || setting.default,
- validate: setting.required ? value => !value && "required" : null,
-});
-
@connect(
null,
{ updateSettings },
diff --git a/frontend/src/metabase/admin/settings/components/SettingsGoogleForm.jsx b/frontend/src/metabase/admin/settings/components/SettingsGoogleForm.jsx
new file mode 100644
index 00000000000..b805663c78e
--- /dev/null
+++ b/frontend/src/metabase/admin/settings/components/SettingsGoogleForm.jsx
@@ -0,0 +1,89 @@
+import React, { Component } from "react";
+import PropTypes from "prop-types";
+import { connect } from "react-redux";
+import { t, jt } from "ttag";
+import _ from "underscore";
+
+import Form, {
+ FormField,
+ FormSubmit,
+ FormMessage,
+} from "metabase/containers/Form";
+
+import { updateSettings } from "metabase/admin/settings/settings";
+import { settingToFormField } from "metabase/admin/settings/utils";
+import Breadcrumbs from "metabase/components/Breadcrumbs";
+import ExternalLink from "metabase/components/ExternalLink";
+
+const settingsGoogleFormPropTypes = {
+ elements: PropTypes.array,
+ settingValues: PropTypes.object,
+ updateSettings: PropTypes.func,
+};
+
+@connect(
+ null,
+ { updateSettings },
+)
+export default class SettingsGoogleForm extends Component {
+ render() {
+ const { elements, settingValues, updateSettings } = this.props;
+
+ const setting = name =>
+ _.findWhere(elements, { key: name }) || { key: name };
+ const settingField = name => settingToFormField(setting(name));
+
+ const initialValues = { ...settingValues };
+
+ return (
+ <Form
+ className="mx2"
+ style={{ maxWidth: 520 }}
+ initialValues={initialValues}
+ onSubmit={updateSettings}
+ >
+ <Breadcrumbs
+ crumbs={[
+ [t`Authentication`, "/admin/settings/authentication"],
+ [t`Google Sign-In`],
+ ]}
+ className="mb2"
+ />
+ <h2>{t`Sign in with Google`}</h2>
+ <p className="text-medium">
+ {t`Allows users with existing Metabase accounts to login with a Google account that matches their email address in addition to their Metabase username and password.`}
+ </p>
+ <p className="text-medium">
+ {jt`To allow users to sign in with Google you'll need to give Metabase a Google Developers console application client ID. It only takes a few steps and instructions on how to create a key can be found ${(
+ <ExternalLink
+ href="https://developers.google.com/identity/sign-in/web/devconsole-project"
+ target="_blank"
+ >
+ {t`here`}
+ </ExternalLink>
+ )}.`}
+ </p>
+ <FormField
+ {...settingField("google-auth-client-id")}
+ title={t`Client ID`}
+ description=""
+ placeholder={t`{your-client-id}.apps.googleusercontent.com`}
+ required
+ autoFocus
+ />
+ <FormField
+ {...settingField("google-auth-auto-create-accounts-domain")}
+ title={t`Domain`}
+ />
+ <div>
+ <FormMessage />
+ </div>
+ <div>
+ <FormSubmit>{t`Save changes`}</FormSubmit>
+ </div>
+ </Form>
+ );
+ }
+}
+
+SettingsGoogleForm.propTypes = settingsGoogleFormPropTypes;
diff --git a/frontend/src/metabase/admin/settings/components/SettingsSingleSignOnForm.jsx b/frontend/src/metabase/admin/settings/components/SettingsSingleSignOnForm.jsx
deleted file mode 100644
index d73a335a4a6..00000000000
--- a/frontend/src/metabase/admin/settings/components/SettingsSingleSignOnForm.jsx
+++ /dev/null
@@ -1,170 +0,0 @@
-import React, { Component } from "react";
-import PropTypes from "prop-types";
-import cx from "classnames";
-import _ from "underscore";
-import { t, jt } from "ttag";
-
-import Breadcrumbs from "metabase/components/Breadcrumbs";
-import InputBlurChange from "metabase/components/InputBlurChange";
-import ExternalLink from "metabase/components/ExternalLink";
-
-export default class SettingsSingleSignOnForm extends Component {
- constructor(props, context) {
- super(props, context);
- this.updateClientID = this.updateClientID.bind(this);
- this.updateDomain = this.updateDomain.bind(this);
- (this.onCheckboxClicked = this.onCheckboxClicked.bind(this)),
- (this.saveChanges = this.saveChanges.bind(this)),
- (this.clientIDChanged = this.clientIDChanged.bind(this)),
- (this.domainChanged = this.domainChanged.bind(this));
- }
-
- static propTypes = {
- elements: PropTypes.array,
- updateSetting: PropTypes.func.isRequired,
- };
-
- UNSAFE_componentWillMount() {
- const { elements } = this.props;
- const clientID = _.findWhere(elements, { key: "google-auth-client-id" });
- const domain = _.findWhere(elements, {
- key: "google-auth-auto-create-accounts-domain",
- });
-
- this.setState({
- clientID: clientID,
- domain: domain,
- clientIDValue: clientID.value,
- domainValue: domain.value,
- recentlySaved: false,
- });
- }
-
- updateClientID(newValue) {
- if (newValue === this.state.clientIDValue) {
- return;
- }
-
- this.setState({
- clientIDValue: newValue && newValue.length ? newValue : null,
- recentlySaved: false,
- });
- }
-
- updateDomain(newValue) {
- if (newValue === this.state.domain.value) {
- return;
- }
-
- this.setState({
- domainValue: newValue && newValue.length ? newValue : null,
- recentlySaved: false,
- });
- }
-
- clientIDChanged() {
- return this.state.clientID.value !== this.state.clientIDValue;
- }
-
- domainChanged() {
- return this.state.domain.value !== this.state.domainValue;
- }
-
- saveChanges() {
- const { clientID, clientIDValue, domain, domainValue } = this.state;
-
- if (this.clientIDChanged()) {
- this.props.updateSetting(clientID, clientIDValue);
- this.setState({
- clientID: {
- value: clientIDValue,
- },
- recentlySaved: true,
- });
- }
-
- if (this.domainChanged()) {
- this.props.updateSetting(domain, domainValue);
- this.setState({
- domain: {
- value: domainValue,
- },
- recentlySaved: true,
- });
- }
- }
-
- onCheckboxClicked() {
- // if domain is present, clear it out; otherwise if there's no domain try to set it back to what it was
- this.setState({
- domainValue: this.state.domainValue ? null : this.state.domain.value,
- recentlySaved: false,
- });
- }
-
- render() {
- const hasChanges = this.domainChanged() || this.clientIDChanged();
- const hasClientID = this.state.clientIDValue;
-
- return (
- <form noValidate>
- <div className="px2" style={{ maxWidth: "585px" }}>
- <Breadcrumbs
- crumbs={[
- [t`Authentication`, "/admin/settings/authentication"],
- [t`Google Sign-In`],
- ]}
- className="mb2"
- />
- <h2>{t`Sign in with Google`}</h2>
- <p className="text-medium">
- {t`Allows users with existing Metabase accounts to login with a Google account that matches their email address in addition to their Metabase username and password.`}
- </p>
- <p className="text-medium">
- {jt`To allow users to sign in with Google you'll need to give Metabase a Google Developers console application client ID. It only takes a few steps and instructions on how to create a key can be found ${(
- <ExternalLink
- href="https://developers.google.com/identity/sign-in/web/devconsole-project"
- target="_blank"
- >
- {t`here`}
- </ExternalLink>
- )}.`}
- </p>
- <p className="text-medium">
- {t`Be sure to include the full client ID, including the apps.googleusercontent.com suffix.`}
- </p>
- <InputBlurChange
- className="SettingsInput AdminInput bordered rounded h3"
- type="text"
- value={this.state.clientIDValue}
- placeholder={t`Your Google client ID`}
- onChange={event => this.updateClientID(event.target.value)}
- />
- <div className="py3">
- <div className="flex align-center">
- <p className="text-medium">{t`Allow users to sign up on their own if their Google account email address is from:`}</p>
- </div>
- <div className="mt1 bordered rounded inline-block">
- <div className="inline-block px2 h2">@</div>
- <InputBlurChange
- className="SettingsInput inline-block AdminInput h3 border-left"
- type="text"
- value={this.state.domainValue}
- onChange={event => this.updateDomain(event.target.value)}
- disabled={!hasClientID}
- />
- </div>
- </div>
-
- <button
- className={cx("Button mr2", { "Button--primary": hasChanges })}
- disabled={!hasChanges}
- onClick={this.saveChanges}
- >
- {this.state.recentlySaved ? t`Changes saved!` : t`Save Changes`}
- </button>
- </div>
- </form>
- );
- }
-}
diff --git a/frontend/src/metabase/admin/settings/utils.js b/frontend/src/metabase/admin/settings/utils.js
index 9316bd34f99..b5eab5a00e4 100644
--- a/frontend/src/metabase/admin/settings/utils.js
+++ b/frontend/src/metabase/admin/settings/utils.js
@@ -1,6 +1,17 @@
+import { t } from "ttag";
+
// in order to prevent collection of identifying information only fields
// that are explicitly marked as collectable or booleans should show the true value
export const prepareAnalyticsValue = setting =>
setting.allowValueCollection || setting.type === "boolean"
? setting.value
: "success";
+
+export const settingToFormField = setting => ({
+ name: setting.key,
+ description: setting.description,
+ placeholder: setting.is_env_setting
+ ? t`Using ${setting.env_name}`
+ : setting.placeholder || setting.default,
+ validate: setting.required ? value => !value && "required" : null,
+});
diff --git a/frontend/src/metabase/plugins/builtin/auth/google.js b/frontend/src/metabase/plugins/builtin/auth/google.js
index c81bc43bc70..538c9f00ed1 100644
--- a/frontend/src/metabase/plugins/builtin/auth/google.js
+++ b/frontend/src/metabase/plugins/builtin/auth/google.js
@@ -12,7 +12,7 @@ import MetabaseSettings from "metabase/lib/settings";
import GoogleButton from "metabase/auth/components/GoogleButton";
import AuthenticationOption from "metabase/admin/settings/components/widgets/AuthenticationOption";
-import SettingsSingleSignOnForm from "metabase/admin/settings/components/SettingsSingleSignOnForm";
+import SettingsGoogleForm from "metabase/admin/settings/components/SettingsGoogleForm";
const GOOGLE_PROVIDER = {
name: "google",
@@ -41,11 +41,18 @@ PLUGIN_ADMIN_SETTINGS_UPDATES.push(sections =>
PLUGIN_ADMIN_SETTINGS_UPDATES.push(sections => ({
...sections,
"authentication/google": {
- component: SettingsSingleSignOnForm,
+ component: SettingsGoogleForm,
sidebar: false,
settings: [
- { key: "google-auth-client-id" },
- { key: "google-auth-auto-create-accounts-domain" },
+ {
+ key: "google-auth-client-id",
+ },
+ {
+ key: "google-auth-auto-create-accounts-domain",
+ description:
+ "Allow users to sign up on their own if their Google account email address is from:",
+ placeholder: "mycompany.com",
+ },
],
},
}));
diff --git a/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js b/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
index 74e22fa6fdd..696ca13e31c 100644
--- a/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
@@ -62,8 +62,7 @@ describe("scenarios > admin > settings", () => {
cy.contains(
"To allow users to sign in with Google you'll need to give Metabase a Google Developers console application client ID.",
);
- // *** should be 'Save changes'
- cy.findByText("Save Changes");
+ cy.findByText("Save changes");
// SSO
cy.visit("/admin/settings/authentication");
--
GitLab