From c6c8ec0d3c11b63b5947f1b249a09924bdfaa73f Mon Sep 17 00:00:00 2001
From: Sloan Sparger <sloansparger@users.noreply.github.com>
Date: Wed, 27 Mar 2024 14:39:00 -0500
Subject: [PATCH] Correct issue where EE only LDAP user provisioning feature is
breaking OSS instances (#40685)
---
e2e/test/scenarios/admin-2/sso/jwt.cy.spec.js | 2 +-
.../scenarios/admin-2/sso/ldap.cy.spec.js | 24 +++--
.../scenarios/admin-2/sso/saml.cy.spec.js | 2 +-
.../scenarios/admin-2/sso/shared/helpers.js | 11 ++-
.../src/metabase-enterprise/auth/index.js | 35 +++++++
.../metabase/admin/settings/auth/constants.ts | 3 +-
.../settings/components/SettingsLdapForm.tsx | 93 ++++++++++---------
.../components/SettingsLdapForm.unit.spec.tsx | 12 ---
.../src/metabase/plugins/builtin/auth/ldap.js | 7 --
frontend/src/metabase/plugins/index.ts | 23 +++++
10 files changed, 136 insertions(+), 76 deletions(-)
diff --git a/e2e/test/scenarios/admin-2/sso/jwt.cy.spec.js b/e2e/test/scenarios/admin-2/sso/jwt.cy.spec.js
index 59cbac97219..6d45129697e 100644
--- a/e2e/test/scenarios/admin-2/sso/jwt.cy.spec.js
+++ b/e2e/test/scenarios/admin-2/sso/jwt.cy.spec.js
@@ -52,7 +52,7 @@ describeEE("scenarios > admin > settings > SSO > JWT", () => {
setupJwt();
cy.visit("/admin/settings/authentication/jwt");
- getUserProvisioningInput().click();
+ getUserProvisioningInput().label.click();
cy.button("Save changes").click();
cy.wait("@updateSettings");
diff --git a/e2e/test/scenarios/admin-2/sso/ldap.cy.spec.js b/e2e/test/scenarios/admin-2/sso/ldap.cy.spec.js
index 09efbac8307..40de617a420 100644
--- a/e2e/test/scenarios/admin-2/sso/ldap.cy.spec.js
+++ b/e2e/test/scenarios/admin-2/sso/ldap.cy.spec.js
@@ -65,15 +65,13 @@ describe(
getLdapCard().findByText("Active").should("exist");
});
- it("should allow the user to enable/disable user provisioning", () => {
+ it("should not show the user provision UI to OSS users", () => {
setupLdap();
cy.visit("/admin/settings/authentication/ldap");
- getUserProvisioningInput().click();
- cy.button("Save changes").click();
- cy.wait("@updateLdapSettings");
-
- getSuccessUi().should("exist");
+ cy.findByTestId("admin-layout-content")
+ .findByText("User Provisioning")
+ .should("not.exist");
});
it("should allow to reset ldap settings", () => {
@@ -172,6 +170,20 @@ describeEE(
restore();
cy.signInAsAdmin();
setTokenFeatures("all");
+ cy.intercept("PUT", "/api/ldap/settings").as("updateLdapSettings");
+ });
+
+ it("should allow the user to enable/disable user provisioning", () => {
+ setupLdap();
+ cy.visit("/admin/settings/authentication/ldap");
+
+ const { label, input } = getUserProvisioningInput();
+ input.should("be.checked");
+ label.click();
+ cy.button("Save changes").click();
+ cy.wait("@updateLdapSettings");
+
+ getSuccessUi().should("exist");
});
it("should show the login form when ldap is enabled but password login isn't (metabase#25661)", () => {
diff --git a/e2e/test/scenarios/admin-2/sso/saml.cy.spec.js b/e2e/test/scenarios/admin-2/sso/saml.cy.spec.js
index 7077fd95529..7dc00a66797 100644
--- a/e2e/test/scenarios/admin-2/sso/saml.cy.spec.js
+++ b/e2e/test/scenarios/admin-2/sso/saml.cy.spec.js
@@ -81,7 +81,7 @@ describeEE("scenarios > admin > settings > SSO > SAML", () => {
setupSaml();
cy.visit("/admin/settings/authentication/saml");
- getUserProvisioningInput().click();
+ getUserProvisioningInput().label.click();
cy.button("Save changes").click();
cy.wait("@updateSamlSettings");
diff --git a/e2e/test/scenarios/admin-2/sso/shared/helpers.js b/e2e/test/scenarios/admin-2/sso/shared/helpers.js
index c9846625659..580d2f7747a 100644
--- a/e2e/test/scenarios/admin-2/sso/shared/helpers.js
+++ b/e2e/test/scenarios/admin-2/sso/shared/helpers.js
@@ -1,7 +1,12 @@
export function getUserProvisioningInput() {
- return cy
- .findByTestId("admin-layout-content")
- .findByText("User Provisioning");
+ return {
+ label: cy
+ .findByTestId("admin-layout-content")
+ .findByText("User Provisioning"),
+ input: cy
+ .findByTestId("admin-layout-content")
+ .findByLabelText("User Provisioning"),
+ };
}
export function getSuccessUi() {
diff --git a/enterprise/frontend/src/metabase-enterprise/auth/index.js b/enterprise/frontend/src/metabase-enterprise/auth/index.js
index e2b46f6daf2..d19d714bcae 100644
--- a/enterprise/frontend/src/metabase-enterprise/auth/index.js
+++ b/enterprise/frontend/src/metabase-enterprise/auth/index.js
@@ -1,16 +1,23 @@
+/* eslint-disable react/prop-types */
+
import { updateIn } from "icepick";
import { t } from "ttag";
import _ from "underscore";
+import * as Yup from "yup";
+import SettingHeader from "metabase/admin/settings/components/SettingHeader";
import GroupMappingsWidget from "metabase/admin/settings/containers/GroupMappingsWidget";
import { LOGIN, LOGIN_GOOGLE } from "metabase/auth/actions";
+import { FormSwitch } from "metabase/forms";
import MetabaseSettings from "metabase/lib/settings";
import {
PLUGIN_ADMIN_SETTINGS_UPDATES,
PLUGIN_AUTH_PROVIDERS,
+ PLUGIN_LDAP_FORM_FIELDS,
PLUGIN_IS_PASSWORD_USER,
PLUGIN_REDUX_MIDDLEWARES,
} from "metabase/plugins";
+import { Stack } from "metabase/ui";
import SessionTimeoutSetting from "metabase-enterprise/auth/components/SessionTimeoutSetting";
import {
hasAnySsoPremiumFeature,
@@ -270,8 +277,36 @@ if (hasPremiumFeature("disable_password_login")) {
}
if (hasPremiumFeature("sso_ldap")) {
+ Object.assign(PLUGIN_LDAP_FORM_FIELDS, {
+ formFieldAttributes: ["ldap-user-provisioning-enabled?"],
+ defaultableFormFieldAttributes: ["ldap-user-provisioning-enabled?"],
+ formFieldsSchemas: {
+ "ldap-user-provisioning-enabled?": Yup.boolean().default(null),
+ },
+ UserProvisioning: ({ fields, settings }) => (
+ <Stack spacing="0.75rem" m="2.5rem 0">
+ <SettingHeader
+ id="ldap-user-provisioning-enabled?"
+ setting={settings["ldap-user-provisioning-enabled?"]}
+ />
+ <FormSwitch
+ id="ldap-user-provisioning-enabled?"
+ name={fields["ldap-user-provisioning-enabled?"].name}
+ defaultChecked={fields["ldap-user-provisioning-enabled?"].default}
+ />
+ </Stack>
+ ),
+ });
+
PLUGIN_ADMIN_SETTINGS_UPDATES.push(sections =>
updateIn(sections, ["authentication/ldap", "settings"], settings => [
+ {
+ key: "ldap-user-provisioning-enabled?",
+ display_name: t`User Provisioning`,
+ // eslint-disable-next-line no-literal-metabase-strings -- This string only shows for admins.
+ description: t`When a user logs in via LDAP, create a Metabase account for them automatically if they don't have one.`,
+ type: "boolean",
+ },
...settings,
{
key: "ldap-group-membership-filter",
diff --git a/frontend/src/metabase/admin/settings/auth/constants.ts b/frontend/src/metabase/admin/settings/auth/constants.ts
index ffbf8f87e19..374f7993c11 100644
--- a/frontend/src/metabase/admin/settings/auth/constants.ts
+++ b/frontend/src/metabase/admin/settings/auth/constants.ts
@@ -1,6 +1,7 @@
import * as Yup from "yup";
import * as Errors from "metabase/lib/errors";
+import { PLUGIN_LDAP_FORM_FIELDS } from "metabase/plugins";
import type { SettingDefinition } from "metabase-types/api";
const REQUIRED_SCHEMA = {
@@ -21,8 +22,8 @@ export const GOOGLE_SCHEMA = Yup.object({
});
export const LDAP_SCHEMA = Yup.object({
+ ...PLUGIN_LDAP_FORM_FIELDS.formFieldsSchemas,
"ldap-enabled": Yup.boolean().nullable().default(false),
- "ldap-user-provisioning-enabled?": Yup.boolean().default(null),
"ldap-host": Yup.string().nullable().default(null),
"ldap-port": Yup.number().nullable().default(null),
"ldap-security": Yup.string().nullable().default("none"),
diff --git a/frontend/src/metabase/admin/settings/components/SettingsLdapForm.tsx b/frontend/src/metabase/admin/settings/components/SettingsLdapForm.tsx
index 97c8a9f75a4..ebe08a59394 100644
--- a/frontend/src/metabase/admin/settings/components/SettingsLdapForm.tsx
+++ b/frontend/src/metabase/admin/settings/components/SettingsLdapForm.tsx
@@ -5,7 +5,6 @@ import _ from "underscore";
import type { TestConfig } from "yup";
import * as Yup from "yup";
-import SettingHeader from "metabase/admin/settings/components/SettingHeader";
import GroupMappingsWidget from "metabase/admin/settings/containers/GroupMappingsWidget";
import { updateLdapSettings } from "metabase/admin/settings/settings";
import type { SettingElement } from "metabase/admin/settings/types";
@@ -20,6 +19,7 @@ import {
FormSwitch,
FormTextInput,
} from "metabase/forms";
+import { PLUGIN_LDAP_FORM_FIELDS } from "metabase/plugins";
import { Group, Radio, Stack } from "metabase/ui";
import type { SettingValue } from "metabase-types/api";
@@ -82,9 +82,46 @@ export const SettingsLdapFormView = ({
}));
}, [settings]);
+ const defaultableAttrs = useMemo(() => {
+ return new Set(PLUGIN_LDAP_FORM_FIELDS.defaultableFormFieldAttributes);
+ }, []);
+
+ const ldapAttributes = useMemo(
+ () => [
+ ...PLUGIN_LDAP_FORM_FIELDS.formFieldAttributes,
+
+ // Server Settings
+ "ldap-host",
+ "ldap-port",
+ "ldap-security",
+ "ldap-bind-dn",
+ "ldap-password",
+
+ // User Schema
+ "ldap-user-base",
+ "ldap-user-filter",
+
+ // Attributes
+ "ldap-attribute-email",
+ "ldap-attribute-firstname",
+ "ldap-attribute-lastname",
+
+ // Group Schema
+ "ldap-group-sync",
+ "ldap-group-base",
+ "ldap-group-membership-filter",
+ "ldap-sync-admin-group",
+ ],
+ [],
+ );
const attributeValues = useMemo(() => {
- return getAttributeValues(settings, settingValues);
- }, [settings, settingValues]);
+ return getAttributeValues(
+ ldapAttributes,
+ settings,
+ settingValues,
+ defaultableAttrs,
+ );
+ }, [settings, settingValues, ldapAttributes, defaultableAttrs]);
const handleSubmit = useCallback(
values => {
@@ -113,17 +150,10 @@ export const SettingsLdapFormView = ({
[t`LDAP`],
]}
/>
- <Stack spacing="0.75rem" m="2.5rem 0">
- <SettingHeader
- id="ldap-user-provisioning-enabled?"
- setting={settings["ldap-user-provisioning-enabled?"]}
- />
- <FormSwitch
- id="ldap-user-provisioning-enabled?"
- name={fields["ldap-user-provisioning-enabled?"].name}
- defaultChecked={fields["ldap-user-provisioning-enabled?"].default}
- />
- </Stack>
+ <PLUGIN_LDAP_FORM_FIELDS.UserProvisioning
+ fields={fields}
+ settings={settings}
+ />
<FormSection title={"Server Settings"}>
<Stack spacing="md">
<FormTextInput {...fields["ldap-host"]} />
@@ -188,43 +218,16 @@ export const SettingsLdapFormView = ({
);
};
-const LDAP_ATTRS = [
- // User Provision Settings
- "ldap-user-provisioning-enabled?",
-
- // Server Settings
- "ldap-host",
- "ldap-port",
- "ldap-security",
- "ldap-bind-dn",
- "ldap-password",
-
- // User Schema
- "ldap-user-base",
- "ldap-user-filter",
-
- // Attributes
- "ldap-attribute-email",
- "ldap-attribute-firstname",
- "ldap-attribute-lastname",
-
- // Group Schema
- "ldap-group-sync",
- "ldap-group-base",
- "ldap-group-membership-filter",
- "ldap-sync-admin-group",
-];
-
-const DEFAULTABLE_LDAP_ATTRS = new Set(["ldap-user-provisioning-enabled?"]);
-
const getAttributeValues = (
+ ldapAttributes: string[],
settings: Record<string, LdapFormSettingElement>,
values: SettingValues,
+ defaultableAttrs: Set<string>,
) => {
return Object.fromEntries(
- LDAP_ATTRS.map(key => [
+ ldapAttributes.map(key => [
key,
- DEFAULTABLE_LDAP_ATTRS.has(key)
+ defaultableAttrs.has(key)
? values[key] ?? settings[key]?.default
: values[key],
]),
diff --git a/frontend/src/metabase/admin/settings/components/SettingsLdapForm.unit.spec.tsx b/frontend/src/metabase/admin/settings/components/SettingsLdapForm.unit.spec.tsx
index e5efa6093b1..4cbdd654d6a 100644
--- a/frontend/src/metabase/admin/settings/components/SettingsLdapForm.unit.spec.tsx
+++ b/frontend/src/metabase/admin/settings/components/SettingsLdapForm.unit.spec.tsx
@@ -28,16 +28,6 @@ const elements = [
display_name: "LDAP Authentication",
type: "boolean",
},
- {
- key: "ldap-user-provisioning-enabled?",
- value: null,
- is_env_setting: false,
- env_name: "MB_LDAP_USER_PROVISIONING_ENABLED",
- display_name: "User Provisioning",
- description:
- "When we enable LDAP user provisioning, we automatically create a Metabase account on LDAP signin for users who\ndon't have one.",
- default: true,
- },
{
placeholder: "ldap.yourdomain.org",
key: "ldap-host",
@@ -247,7 +237,6 @@ describe("SettingsLdapForm", () => {
});
const ATTRS = {
- "ldap-user-provisioning-enabled?": false,
"ldap-host": "example.com",
"ldap-port": "123",
"ldap-security": "ssl",
@@ -265,7 +254,6 @@ describe("SettingsLdapForm", () => {
"ldap-sync-admin-group": true,
};
- userEvent.click(screen.getByLabelText(/User Provisioning/));
userEvent.type(
await screen.findByRole("textbox", { name: /LDAP Host/ }),
ATTRS["ldap-host"],
diff --git a/frontend/src/metabase/plugins/builtin/auth/ldap.js b/frontend/src/metabase/plugins/builtin/auth/ldap.js
index 829cf30ed7d..a54ea7171e9 100644
--- a/frontend/src/metabase/plugins/builtin/auth/ldap.js
+++ b/frontend/src/metabase/plugins/builtin/auth/ldap.js
@@ -33,13 +33,6 @@ PLUGIN_ADMIN_SETTINGS_UPDATES.push(
type: "boolean",
getHidden: () => true,
},
- {
- key: "ldap-user-provisioning-enabled?",
- display_name: t`User Provisioning`,
- // eslint-disable-next-line no-literal-metabase-strings -- This string only shows for admins.
- description: t`When a user logs in via LDAP, create a Metabase account for them automatically if they don't have one.`,
- type: "boolean",
- },
{
key: "ldap-host",
display_name: t`LDAP Host`,
diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts
index ed404864feb..35c3437a3ab 100644
--- a/frontend/src/metabase/plugins/index.ts
+++ b/frontend/src/metabase/plugins/index.ts
@@ -1,5 +1,6 @@
import type { ComponentType, HTMLAttributes, ReactNode } from "react";
import { t } from "ttag";
+import type { AnySchema } from "yup";
import { UNABLE_TO_CHANGE_ADMIN_PERMISSIONS } from "metabase/admin/permissions/constants/messages";
import type {
@@ -126,6 +127,28 @@ export const PLUGIN_ADMIN_USER_MENU_ROUTES = [];
// authentication providers
export const PLUGIN_AUTH_PROVIDERS: GetAuthProviders[] = [];
+export const PLUGIN_LDAP_FORM_FIELDS = {
+ formFieldAttributes: [] as string[],
+ defaultableFormFieldAttributes: [] as string[],
+ formFieldsSchemas: {} as Record<string, AnySchema>,
+ UserProvisioning: (() => null) as ComponentType<{
+ settings: {
+ [setting: string]: {
+ display_name?: string | undefined;
+ warningMessage?: string | undefined;
+ description?: string | undefined;
+ note?: string | undefined;
+ };
+ };
+ fields: {
+ [field: string]: {
+ name: string;
+ default: boolean;
+ };
+ };
+ }>,
+};
+
// Only show the password tab in account settings if these functions all return true.
// Otherwise, the user is logged in via SSO and should hide first name, last name, and email field in profile settings metabase#23298.
export const PLUGIN_IS_PASSWORD_USER: ((user: User) => boolean)[] = [];
--
GitLab