From 8ad2a85b94a656ecdab1c7399a55e7a86753fea6 Mon Sep 17 00:00:00 2001 From: Gustavo Saiani <gus@metabase.com> Date: Fri, 17 Feb 2023 20:06:15 -0300 Subject: [PATCH] Update UX of authentication methods' Group Mappings Widget (#28108) --- .../SettingsSAMLForm/SettingsSAMLForm.jsx | 4 +- .../SettingsSAMLForm.styled.tsx | 8 + .../admin/people/components/GroupSelect.jsx | 2 +- .../widgets/GroupMappingsWidget.jsx | 283 ------------------ .../AddMappingRow/AddMappingRow.tsx | 76 +++++ .../AddMappingRow/index.tsx | 1 + .../DeleteGroupMappingModal.styled.tsx | 14 + .../DeleteGroupMappingModal.tsx | 106 +++++++ .../DeleteGroupMappingModal.unit.spec.tsx | 100 +++++++ .../DeleteGroupMappingModal/index.tsx | 1 + .../GroupMappingsWidget.jsx | 201 +++++++++++++ .../GroupMappingsWidget.styled.tsx | 74 +++++ .../GroupMappingsWidget.unit.spec.tsx | 152 ++++++++++ .../GroupSelect/GroupSelect.tsx | 31 ++ .../GroupMappingsWidget/GroupSelect/index.tsx | 1 + .../MappingRow/MappingRow.styled.tsx | 12 + .../MappingRow/MappingRow.tsx | 175 +++++++++++ .../GroupMappingsWidget/MappingRow/index.tsx | 1 + .../widgets/GroupMappingsWidget/index.tsx | 1 + .../widgets/GroupMappingsWidget/types.ts | 5 + .../components/widgets/SettingToggle.jsx | 13 +- frontend/src/metabase/css/admin.css | 5 + .../src/metabase/plugins/builtin/auth/ldap.js | 1 - frontend/src/metabase/services.js | 1 + .../admin/settings/sso/ldap.cy.spec.js | 88 ++++++ 25 files changed, 1066 insertions(+), 290 deletions(-) delete mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/AddMappingRow.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/index.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.styled.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.unit.spec.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/index.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.jsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.styled.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.unit.spec.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/GroupSelect.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/index.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.styled.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/index.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/index.tsx create mode 100644 frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/types.ts diff --git a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.jsx b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.jsx index 1125d1d67ea..5ffda593453 100644 --- a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.jsx +++ b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.jsx @@ -55,7 +55,6 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => { return ( <Form className="mx2" - style={{ maxWidth: 520 }} initialValues={{ ...settingValues, ...attributeValues }} disablePristineSubmit overwriteOnInitialValuesChange @@ -152,7 +151,7 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => { </FormSection> </SAMLFormSection> - <SAMLFormSection> + <SAMLFormSection wide> <h3 className="mb0">{t`Synchronize group membership with your SSO`}</h3> <p className="mb4 mt1 text-medium"> {t`To enable this, you'll need to create mappings to tell Metabase which group(s) your users should @@ -160,7 +159,6 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => { </p> <FormField {...fields["saml-group-sync"]} - title={t`Synchronize group memberships`} type={({ field: { value, onChange } }) => ( <GroupMappingsWidget // map to legacy setting props diff --git a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.styled.tsx b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.styled.tsx index 3e56ec53bff..8705d52e6c7 100644 --- a/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.styled.tsx +++ b/enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.styled.tsx @@ -3,6 +3,7 @@ import { color } from "metabase/lib/colors"; export interface SAMLFormSectionProps { isSSLSection?: boolean; + wide?: boolean; } export const SAMLFormSection = styled.div<SAMLFormSectionProps>` @@ -10,6 +11,13 @@ export const SAMLFormSection = styled.div<SAMLFormSectionProps>` margin-bottom: 1rem; border: 1px solid ${color("border")}; border-radius: 0.5rem; + // The section containing the GroupMappingsWidget needs to be wider + width: ${props => (props.wide ? "780px" : "520px")}; + + // Even in a wide section, the input is better if same width as elsewhere + input { + max-width: 460px; + } `; export const SAMLFormCaption = styled.div` diff --git a/frontend/src/metabase/admin/people/components/GroupSelect.jsx b/frontend/src/metabase/admin/people/components/GroupSelect.jsx index a9b82ec24b1..9972506be0e 100644 --- a/frontend/src/metabase/admin/people/components/GroupSelect.jsx +++ b/frontend/src/metabase/admin/people/components/GroupSelect.jsx @@ -17,7 +17,7 @@ import GroupSummary from "./GroupSummary"; export const GroupSelect = ({ groups, - selectedGroupIds = new Set(), + selectedGroupIds = [], onGroupChange, isCurrentUser = false, emptyListMessage = t`No groups`, diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx deleted file mode 100644 index 064ccbc2614..00000000000 --- a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx +++ /dev/null @@ -1,283 +0,0 @@ -/* eslint-disable react/prop-types */ -import React from "react"; - -import { t } from "ttag"; -import _ from "underscore"; -import { ModalFooter } from "metabase/components/ModalContent"; -import AdminContentTable from "metabase/components/AdminContentTable"; -import Button from "metabase/core/components/Button"; -import GroupSelect from "metabase/admin/people/components/GroupSelect"; -import LoadingSpinner from "metabase/components/LoadingSpinner"; -import Modal from "metabase/components/Modal"; -import { PermissionsApi, SettingsApi } from "metabase/services"; -import { isDefaultGroup } from "metabase/lib/groups"; - -import SettingToggle from "./SettingToggle"; - -const groupIsMappable = group => !isDefaultGroup(group); - -export default class GroupMappingsWidget extends React.Component { - constructor(props, context) { - super(props, context); - this.state = { - showEditModal: false, - showAddRow: false, - groups: null, - mappings: {}, - saveError: null, - }; - } - - _showEditModal = async e => { - e.preventDefault(); - // just load the setting again to make sure it's up to date - const setting = _.findWhere(await SettingsApi.list(), { - key: this.props.mappingSetting, - }); - this.setState({ - mappings: (setting && setting.value) || {}, - showEditModal: true, - }); - PermissionsApi.groups().then(groups => - this.setState({ groups: groups.filter(groupIsMappable) }), - ); - }; - - _showAddRow = e => { - e.preventDefault(); - this.setState({ showAddRow: true }); - }; - - _hideAddRow = () => { - this.setState({ showAddRow: false }); - }; - - _addMapping = dn => { - this.setState(prevState => ({ - mappings: { ...prevState.mappings, [dn]: [] }, - showAddRow: false, - })); - }; - - _changeMapping = dn => (group, selected) => { - if (selected) { - this.setState(prevState => ({ - mappings: { - ...prevState.mappings, - [dn]: [...prevState.mappings[dn], group.id], - }, - })); - } else { - this.setState(prevState => ({ - mappings: { - ...prevState.mappings, - [dn]: prevState.mappings[dn].filter(id => id !== group.id), - }, - })); - } - }; - - _deleteMapping = dn => e => { - e.preventDefault(); - this.setState(prevState => ({ - mappings: _.omit(prevState.mappings, dn), - })); - }; - - _cancelClick = e => { - e.preventDefault(); - this.setState({ showEditModal: false, showAddRow: false }); - }; - - _saveClick = e => { - e.preventDefault(); - const { - state: { mappings }, - props: { onChangeSetting }, - } = this; - SettingsApi.put({ key: this.props.mappingSetting, value: mappings }).then( - () => { - onChangeSetting(this.props.mappingSetting, mappings); - this.setState({ - showEditModal: false, - showAddRow: false, - saveError: null, - }); - }, - e => this.setState({ saveError: e }), - ); - }; - - render() { - const { showEditModal, showAddRow, groups, mappings, saveError } = - this.state; - - return ( - <div className="flex align-center"> - <SettingToggle {...this.props} /> - <div className="flex align-center pt1"> - <Button - type="button" - className="ml1" - medium - onClick={this._showEditModal} - >{t`Edit Mappings`}</Button> - </div> - {showEditModal ? ( - <Modal wide> - <div> - <div className="pt4 px4"> - <h2>{t`Group Mappings`}</h2> - </div> - <div className="px4"> - <Button - className="float-right" - primary - onClick={this._showAddRow} - >{t`Create a mapping`}</Button> - <p className="text-measure"> - {t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the - directory server. Users are only ever added to or removed from mapped groups.`} - </p> - <AdminContentTable - columnTitles={[this.props.groupHeading, t`Groups`, ""]} - > - {showAddRow ? ( - <AddMappingRow - mappings={mappings} - onCancel={this._hideAddRow} - onAdd={this._addMapping} - placeholder={this.props.groupPlaceholder} - /> - ) : null} - {Object.entries(mappings).map(([dn, ids]) => ( - <MappingRow - key={dn} - dn={dn} - groups={groups || []} - selectedGroups={ids} - onChange={this._changeMapping(dn)} - onDelete={this._deleteMapping(dn)} - /> - ))} - </AdminContentTable> - </div> - <ModalFooter> - {saveError && saveError.data && saveError.data.message ? ( - <span className="text-error text-bold"> - {saveError.data.message} - </span> - ) : null} - <Button - type="button" - onClick={this._cancelClick} - >{t`Cancel`}</Button> - <Button primary onClick={this._saveClick}>{t`Save`}</Button> - </ModalFooter> - </div> - </Modal> - ) : null} - </div> - ); - } -} - -class AddMappingRow extends React.Component { - constructor(props, context) { - super(props, context); - this.state = { - value: "", - }; - } - - _handleSubmit = e => { - e.preventDefault(); - const { onAdd } = this.props; - onAdd && onAdd(this.state.value); - this.setState({ value: "" }); - }; - - _handleCancelClick = e => { - e.preventDefault(); - const { onCancel } = this.props; - onCancel && onCancel(); - this.setState({ value: "" }); - }; - - render() { - const { value } = this.state; - - const isValid = value && this.props.mappings[value] === undefined; - - return ( - <tr> - <td colSpan="3" style={{ padding: 0 }}> - <form - className="my2 pl1 p1 bordered border-brand rounded relative flex align-center" - onSubmit={isValid ? this._handleSubmit : undefined} - > - <input - className="input--borderless h3 ml1 flex-full" - type="text" - value={value} - placeholder={this.props.placeholder} - autoFocus - onChange={e => this.setState({ value: e.target.value })} - /> - <span - className="link no-decoration cursor-pointer" - onClick={this._handleCancelClick} - >{t`Cancel`}</span> - <Button - className="ml2" - type="submit" - primary={!!isValid} - disabled={!isValid} - >{t`Add`}</Button> - </form> - </td> - </tr> - ); - } -} - -class MappingGroupSelect extends React.Component { - render() { - const { groups, selectedGroups, onGroupChange } = this.props; - - if (!groups) { - return <LoadingSpinner />; - } - - return ( - <GroupSelect - groups={groups} - selectedGroupIds={selectedGroups} - onGroupChange={onGroupChange} - emptyListMessage={t`No mappable groups`} - /> - ); - } -} - -class MappingRow extends React.Component { - render() { - const { dn, groups, selectedGroups, onChange, onDelete } = this.props; - - return ( - <tr> - <td>{dn}</td> - <td> - <MappingGroupSelect - groups={groups} - selectedGroups={selectedGroups} - onGroupChange={onChange} - /> - </td> - <td className="Table-actions"> - <Button warning onClick={onDelete}>{t`Remove`}</Button> - </td> - </tr> - ); - } -} diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/AddMappingRow.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/AddMappingRow.tsx new file mode 100644 index 00000000000..03484585c00 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/AddMappingRow.tsx @@ -0,0 +1,76 @@ +import React, { useState } from "react"; + +import { t } from "ttag"; + +import Button from "metabase/core/components/Button"; + +import type { MappingsType } from "../types"; + +type AddMappingRowProps = { + mappings: MappingsType; + placeholder: string; + onCancel: () => void; + onAdd: (value: string) => void; +}; + +function AddMappingRow({ + mappings, + placeholder, + onCancel, + onAdd, +}: AddMappingRowProps) { + const [value, setValue] = useState(""); + + const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { + // Enter key + if (e.keyCode === 13) { + handleSubmit(); + } + }; + + const handleSubmit = (e?: React.FormEvent<HTMLFormElement>) => { + e?.preventDefault(); + onAdd(value); + setValue(""); + }; + + const handleCancelClick = () => { + onCancel(); + setValue(""); + }; + + const isMappingNameUnique = value && mappings[value] === undefined; + + return ( + <tr> + <td colSpan={3} style={{ padding: 0 }}> + <form + className="m2 p1 bordered border-brand justify-between rounded relative flex align-center" + onSubmit={isMappingNameUnique ? handleSubmit : undefined} + > + <input + aria-label="new-group-mapping-name-input" + className="input--borderless h3 ml1 flex-full" + type="text" + value={value} + placeholder={placeholder} + autoFocus + onChange={e => setValue(e.target.value)} + onKeyDown={handleKeyDown} + /> + <div> + <Button borderless onClick={handleCancelClick}>{t`Cancel`}</Button> + <Button + className="ml2" + type="submit" + primary={!!isMappingNameUnique} + disabled={!isMappingNameUnique} + >{t`Add`}</Button> + </div> + </form> + </td> + </tr> + ); +} + +export default AddMappingRow; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/index.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/index.tsx new file mode 100644 index 00000000000..4d0a293c0d8 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/AddMappingRow/index.tsx @@ -0,0 +1 @@ +export { default } from "./AddMappingRow"; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.styled.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.styled.tsx new file mode 100644 index 00000000000..4221a9a3b07 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.styled.tsx @@ -0,0 +1,14 @@ +import styled from "@emotion/styled"; +import { space } from "metabase/styled-components/theme"; + +export const ModalHeader = styled.h2` + padding: ${space(3)}; +`; + +export const ModalSubtitle = styled.p` + padding: 0 ${space(3)}; +`; + +export const ModalRadioRoot = styled.div` + padding: ${space(0)} ${space(3)}; +`; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.tsx new file mode 100644 index 00000000000..a404b35b753 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.tsx @@ -0,0 +1,106 @@ +import React, { useState } from "react"; +import { t } from "ttag"; + +import Modal from "metabase/components/Modal"; +import { ModalFooter } from "metabase/components/ModalContent"; +import Radio from "metabase/core/components/Radio"; +import Button from "metabase/core/components/Button"; + +import type { DeleteMappingModalValueType, GroupIds } from "../types"; +import { + ModalHeader, + ModalSubtitle, + ModalRadioRoot, +} from "./DeleteGroupMappingModal.styled"; + +export type DeleteGroupMappingModalProps = { + name: string; + groupIds: GroupIds; + onConfirm: ( + value: DeleteMappingModalValueType, + groupIds: GroupIds, + name: string, + ) => void; + onHide: () => void; +}; + +const DeleteGroupMappingModal = ({ + name, + groupIds, + onConfirm, + onHide, +}: DeleteGroupMappingModalProps) => { + const [value, setValue] = useState<DeleteMappingModalValueType>("nothing"); + + const handleChange = (newValue: DeleteMappingModalValueType) => { + setValue(newValue); + }; + + const handleConfirm = () => { + onConfirm(value, groupIds, name); + }; + + const submitButtonLabels: Record<DeleteMappingModalValueType, string> = { + nothing: t`Remove mapping`, + clear: t`Remove mapping and members`, + delete: + groupIds.length > 1 + ? t`Remove mapping and delete groups` + : t`Remove mapping and delete group`, + }; + + const subtitle = + groupIds.length > 1 + ? t`These groups' user memberships will no longer be synced with the directory server.` + : t`This group's user membership will no longer be synced with the directory server.`; + + const whatShouldHappenText = + groupIds.length > 1 + ? t`What should happen with the groups themselves in Metabase?` + : t`What should happen with the group itself in Metabase?`; + + return ( + <Modal> + <div> + <ModalHeader>{t`Remove this group mapping?`}</ModalHeader> + <ModalSubtitle>{subtitle}</ModalSubtitle> + <ModalRadioRoot> + <p>{whatShouldHappenText}</p> + + <Radio + className="ml2" + vertical + value={value as DeleteMappingModalValueType | undefined} + options={[ + { + name: t`Nothing, just remove the mapping`, + value: "nothing", + }, + { + name: t`Also remove all group members (except from Admin)`, + value: "clear", + }, + { + name: + groupIds.length > 1 + ? t`Also delete the groups (except Admin)` + : t`Also delete the group`, + value: "delete", + }, + ]} + showButtons + onChange={handleChange} + /> + </ModalRadioRoot> + <ModalFooter fullPageModal={false} formModal={true}> + <Button onClick={onHide}>{t`Cancel`}</Button> + <Button danger onClick={handleConfirm}> + {submitButtonLabels[value as DeleteMappingModalValueType]} + </Button> + </ModalFooter> + </div> + </Modal> + ); +}; + +export default DeleteGroupMappingModal; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.unit.spec.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.unit.spec.tsx new file mode 100644 index 00000000000..37b4ca9379e --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.unit.spec.tsx @@ -0,0 +1,100 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import DeleteGroupMappingModal, { + DeleteGroupMappingModalProps, +} from "./DeleteGroupMappingModal"; + +type SetupOpts = Partial<DeleteGroupMappingModalProps>; + +const DEFAULT_PROPS = { + name: "cn=People", + groupIds: [1], + onConfirm: jest.fn(), + onHide: jest.fn(), +}; + +const setup = (props?: SetupOpts) => { + render(<DeleteGroupMappingModal {...DEFAULT_PROPS} {...props} />); +}; + +describe("DeleteGroupMappingModal", () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it("shows options for when mapping is linked to just one group", () => { + setup(); + + expect( + screen.getByText("Nothing, just remove the mapping"), + ).toBeInTheDocument(); + + expect( + screen.getByText("Also remove all group members (except from Admin)"), + ).toBeInTheDocument(); + + expect(screen.getByText("Also delete the group")).toBeInTheDocument(); + }); + + it("shows options for when mapping is linked to more than one group", () => { + setup({ groupIds: [1, 2] }); + + expect( + screen.getByText("Nothing, just remove the mapping"), + ).toBeInTheDocument(); + + expect( + screen.getByText("Also remove all group members (except from Admin)"), + ).toBeInTheDocument(); + + expect( + screen.getByText("Also delete the groups (except Admin)"), + ).toBeInTheDocument(); + }); + + it("starts with 'Nothing' option checked", () => { + setup(); + + expect( + screen.getByLabelText("Nothing, just remove the mapping"), + ).toBeChecked(); + }); + + it("confirms when clearing members", () => { + setup(); + + userEvent.click( + screen.getByLabelText( + "Also remove all group members (except from Admin)", + ), + ); + + userEvent.click( + screen.getByRole("button", { name: "Remove mapping and members" }), + ); + + expect(DEFAULT_PROPS.onConfirm).toHaveBeenCalledWith( + "clear", + DEFAULT_PROPS.groupIds, + DEFAULT_PROPS.name, + ); + }); + + it("confirms when deleting groups", () => { + setup(); + + userEvent.click(screen.getByLabelText("Also delete the group")); + + userEvent.click( + screen.getByRole("button", { name: "Remove mapping and delete group" }), + ); + + expect(DEFAULT_PROPS.onConfirm).toHaveBeenCalledWith( + "delete", + DEFAULT_PROPS.groupIds, + DEFAULT_PROPS.name, + ); + }); +}); diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/index.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/index.tsx new file mode 100644 index 00000000000..c02c59c1080 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/index.tsx @@ -0,0 +1 @@ +export { default } from "./DeleteGroupMappingModal"; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.jsx new file mode 100644 index 00000000000..5f33db08cc8 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.jsx @@ -0,0 +1,201 @@ +/* eslint-disable react/prop-types */ +import React, { useState, useEffect } from "react"; +import { t } from "ttag"; +import _ from "underscore"; + +import AdminContentTable from "metabase/components/AdminContentTable"; +import { PermissionsApi, SettingsApi } from "metabase/services"; +import { isDefaultGroup } from "metabase/lib/groups"; + +import Icon from "metabase/components/Icon"; +import Tooltip from "metabase/core/components/Tooltip"; +import SettingToggle from "../SettingToggle"; +import AddMappingRow from "./AddMappingRow"; +import { + GroupMappingsWidgetAndErrorRoot as WidgetAndErrorRoot, + GroupMappingsWidgetRoot as Root, + GroupMappingsWidgetHeader as Header, + GroupMappingsWidgetToggleRoot as ToggleRoot, + GroupMappingsWidgetAbout as About, + GroupMappingsWidgetAboutContentRoot as AboutContentRoot, + AddMappingButton, +} from "./GroupMappingsWidget.styled"; +import MappingRow from "./MappingRow"; + +const groupIsMappable = group => !isDefaultGroup(group); + +const loadGroupsAndMappings = async mappingSetting => { + const [settings, groupsIncludingDefaultGroup] = await Promise.all([ + SettingsApi.list(), + PermissionsApi.groups(), + ]); + + const setting = _.findWhere(settings, { + key: mappingSetting, + }); + + return { + groups: groupsIncludingDefaultGroup.filter(groupIsMappable), + mappings: setting?.value || {}, + }; +}; + +function GroupMappingsWidget({ mappingSetting, ...props }) { + const [showAddRow, setShowAddRow] = useState(false); + const [groups, setGroups] = useState([]); + const [mappings, setMappings] = useState({}); + const [saveError, setSaveError] = useState({}); + + useEffect(() => { + async function fetchData() { + const { groups, mappings } = await loadGroupsAndMappings(mappingSetting); + + setMappings(mappings); + setGroups(groups); + } + + fetchData(); + }, [mappingSetting]); + + const handleShowAddRow = () => { + setShowAddRow(true); + }; + + const handleHideAddRow = () => { + setShowAddRow(false); + }; + + const handleAddMapping = name => { + const mappingsPlusNewMapping = { ...mappings, [name]: [] }; + + SettingsApi.put({ + key: mappingSetting, + value: mappingsPlusNewMapping, + }).then( + () => { + props.onChangeSetting(mappingSetting, mappingsPlusNewMapping); + + setMappings(mappingsPlusNewMapping); + setShowAddRow(false); + setSaveError(null); + }, + e => setSaveError(e), + ); + }; + + const handleChangeMapping = name => (group, selected) => { + const updatedMappings = selected + ? { ...mappings, [name]: [...mappings[name], group.id] } + : { + ...mappings, + [name]: mappings[name].filter(id => id !== group.id), + }; + + SettingsApi.put({ + key: mappingSetting, + value: updatedMappings, + }).then( + () => { + props.onChangeSetting(mappingSetting, updatedMappings); + setMappings(updatedMappings); + + setSaveError(null); + }, + e => setSaveError(e), + ); + }; + + const handleDeleteMapping = ({ name, onSuccess, groupIdsToDelete = [] }) => { + const mappingsMinusDeletedMapping = _.omit(mappings, name); + + SettingsApi.put({ + key: mappingSetting, + value: mappingsMinusDeletedMapping, + }).then( + async () => { + props.onChangeSetting(mappingSetting, mappingsMinusDeletedMapping); + + onSuccess && (await onSuccess()); + + const { groups, mappings } = await loadGroupsAndMappings( + mappingSetting, + ); + + setGroups(groups); + setMappings(mappings); + setSaveError(null); + }, + e => setSaveError(e), + ); + }; + + return ( + <WidgetAndErrorRoot> + <Root> + <Header> + <ToggleRoot> + <span>{t`Synchronize Group Memberships`}</span> + <SettingToggle {...props} hideLabel /> + </ToggleRoot> + <About> + <Tooltip + tooltip={t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server. If a group isn‘t mapped, its membership won‘t be synced.`} + placement="top" + > + <AboutContentRoot> + <Icon name="info" /> + <span>{t`About mappings`}</span> + </AboutContentRoot> + </Tooltip> + </About> + </Header> + + <div> + <div> + {!showAddRow && ( + <AddMappingButton primary small onClick={handleShowAddRow}> + {t`New mapping`} + </AddMappingButton> + )} + <AdminContentTable + columnTitles={[props.groupHeading, t`Groups`, ""]} + > + {showAddRow && ( + <AddMappingRow + mappings={mappings} + placeholder={props.groupPlaceholder} + onCancel={handleHideAddRow} + onAdd={handleAddMapping} + /> + )} + {Object.keys(mappings).length === 0 && !showAddRow && ( + <tr> + <td> </td> + <td> {t`No mappings yet`}</td> + <td> </td> + </tr> + )} + {Object.entries(mappings).map(([name, selectedGroupIds]) => { + return groups.length > 0 ? ( + <MappingRow + key={name} + name={name} + groups={groups} + selectedGroupIds={selectedGroupIds} + onChange={handleChangeMapping(name)} + onDeleteMapping={handleDeleteMapping} + /> + ) : null; + })} + </AdminContentTable> + </div> + </div> + </Root> + {saveError?.data?.message && ( + <div className="text-error text-bold m1">{saveError.data.message}</div> + )} + </WidgetAndErrorRoot> + ); +} + +export default GroupMappingsWidget; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.styled.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.styled.tsx new file mode 100644 index 00000000000..de8204d5a81 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.styled.tsx @@ -0,0 +1,74 @@ +import styled from "@emotion/styled"; + +import { color } from "metabase/lib/colors"; +import { space } from "metabase/styled-components/theme"; +import Button from "metabase/core/components/Button"; + +import SettingToggle from "../SettingToggle"; + +export const GroupMappingsWidgetAndErrorRoot = styled.div` + display: flex; + flex-direction: column; + width: 100%; +`; + +export const GroupMappingsWidgetRoot = styled.div` + border: 1px solid ${color("border")}; + border-radius: 8px; + display: flex; + flex-direction: column; + max-width: 720px; + width: 100%; +`; + +export const GroupMappingsWidgetHeader = styled.div` + background-color: ${color("bg-light")}; + border-top-left-radius: 8px; + border-top-right-radius: 8px; + display: flex; + justify-content: space-between; + margin-bottom: ${space(2)}; + padding: ${space(1)} ${space(2)}; + + span { + font-weight: 700; + } +`; + +export const GroupMappingsWidgetToggleRoot = styled.div` + align-items: center; + display: flex; + + > * { + color: ${color("text-dark")}; + padding-right: ${space(1)}; + padding-top: 0; + } +`; + +export const GroupMappingsToggle = styled(SettingToggle)` + padding-top: 0; + opacity: 0.5; +`; + +export const GroupMappingsWidgetAbout = styled.div` + align-items: center; + color: ${color("text-medium")}; + display: flex; + flex-direction: row; + + span { + padding-left: ${space(1)}; + } +`; + +export const GroupMappingsWidgetAboutContentRoot = styled.div` + display: flex; + align-items: center; +`; + +export const AddMappingButton = styled(Button)` + float: right; + margin-right: ${space(2)}; + margin-bottom: -40px; +`; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.unit.spec.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.unit.spec.tsx new file mode 100644 index 00000000000..406644c429a --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.unit.spec.tsx @@ -0,0 +1,152 @@ +import React from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import nock from "nock"; + +import GroupMappingsWidget from "./GroupMappingsWidget"; + +const setup = ({ + mappingSetting = "ldap-group-mappings", + onChange = jest.fn(), + onChangeSetting = jest.fn(), + onSuccess = jest.fn(), + setting = { value: true }, +} = {}) => { + render( + <GroupMappingsWidget + mappingSetting={mappingSetting} + setting={setting} + onChangeSetting={onChangeSetting} + onSuccess={onSuccess} + />, + ); +}; + +describe("GroupMappingsWidget", () => { + afterEach(() => { + nock.cleanAll(); + }); + + describe("when a mapping is set for admin group", () => { + const settingBody = [ + { + key: "ldap-group-mappings", + value: { "cn=People": [2] }, + }, + ]; + + const groupsBody = [{ id: 2, name: "Administrators", member_count: 1 }]; + + beforeEach(() => { + nock(location.origin) + .get("/api/setting") + .times(2) + .reply(200, settingBody); + nock(location.origin) + .get("/api/permissions/group") + .times(2) + .reply(200, groupsBody); + + nock(location.origin).put("/api/setting/ldap-group-mappings").reply(200); + }); + + it("handles deleting mapping", async () => { + const onChangeSettingSpy = jest.fn(); + setup({ onChangeSetting: onChangeSettingSpy }); + + expect(await screen.findByText("cn=People")).toBeInTheDocument(); + expect(await screen.findByText("Admin")).toBeInTheDocument(); + + // Click on button to delete mapping + userEvent.click(await screen.findByLabelText("close icon")); + + // Confirm remove + userEvent.click(screen.getByText("Yes")); + + await waitFor(() => { + expect(onChangeSettingSpy).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe("when a mapping is set for more than one group", () => { + const settingBody = [ + { + key: "ldap-group-mappings", + value: { "cn=People": [3, 4] }, + }, + ]; + + const groupsBody = [ + { id: 1, name: "All Users", member_count: 5 }, + { id: 2, name: "Administrators", member_count: 1 }, + { id: 3, name: "Group 1", member_count: 2 }, + { id: 4, name: "Group 2", member_count: 2 }, + ]; + + beforeEach(() => { + nock(location.origin).get("/api/setting").reply(200, settingBody); + nock(location.origin).get("/api/setting").reply(200, []); + + nock(location.origin).put("/api/setting/ldap-group-mappings").reply(200); + + nock(location.origin) + .get("/api/permissions/group") + .times(2) + .reply(200, groupsBody); + }); + + it("handles clearing mapped groups after deleting mapping", async () => { + nock(location.origin) + .put("/api/permissions/membership/3/clear") + .reply(200); + + nock(location.origin) + .put("/api/permissions/membership/4/clear") + .reply(200); + + setup(); + + expect(await screen.findByText("cn=People")).toBeInTheDocument(); + expect(await screen.findByText("2 other groups")).toBeInTheDocument(); + + // Click on button to delete mapping + userEvent.click(await screen.findByLabelText("close icon")); + + userEvent.click(screen.getByLabelText(/Also remove all group members/i)); + userEvent.click( + await screen.findByRole("button", { + name: "Remove mapping and members", + }), + ); + + expect(await screen.findByText("No mappings yet")).toBeInTheDocument(); + + expect(nock.isDone()).toBeTruthy(); + }); + + it("handles deleting mapped groups after deleting mapping", async () => { + nock(location.origin).delete("/api/permissions/group/3").reply(200); + + nock(location.origin).delete("/api/permissions/group/4").reply(200); + setup(); + + expect(await screen.findByText("cn=People")).toBeInTheDocument(); + expect(await screen.findByText("2 other groups")).toBeInTheDocument(); + + // Click on button to delete mapping + userEvent.click(await screen.findByLabelText("close icon")); + + userEvent.click(screen.getByLabelText(/Also delete the groups/i)); + userEvent.click( + await screen.findByRole("button", { + name: "Remove mapping and delete groups", + }), + ); + + expect(await screen.findByText("No mappings yet")).toBeInTheDocument(); + + expect(nock.isDone()).toBeTruthy(); + }); + }); +}); diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/GroupSelect.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/GroupSelect.tsx new file mode 100644 index 00000000000..7015d9bc508 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/GroupSelect.tsx @@ -0,0 +1,31 @@ +import React from "react"; +import { t } from "ttag"; + +import LoadingSpinner from "metabase/components/LoadingSpinner"; +import GroupSelect from "metabase/admin/people/components/GroupSelect"; + +import type { GroupIds, UserGroupsType } from "../types"; + +type Props = { + groups: UserGroupsType; + selectedGroupIds: GroupIds; + onGroupChange: () => void; +}; + +const GroupMappingsWidgetGroupSelect = ({ + groups, + selectedGroupIds, + onGroupChange, +}: Props) => + groups ? ( + <GroupSelect + groups={groups} + selectedGroupIds={selectedGroupIds} + onGroupChange={onGroupChange} + emptyListMessage={t`No mappable groups`} + /> + ) : ( + <LoadingSpinner /> + ); + +export default GroupMappingsWidgetGroupSelect; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/index.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/index.tsx new file mode 100644 index 00000000000..f30bebc756f --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/index.tsx @@ -0,0 +1 @@ +export { default } from "./GroupSelect"; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.styled.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.styled.tsx new file mode 100644 index 00000000000..4268e82a2a7 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.styled.tsx @@ -0,0 +1,12 @@ +import styled from "@emotion/styled"; +import { color } from "metabase/lib/colors"; + +import IconButtonWrapper from "metabase/components/IconButtonWrapper"; + +export const DeleteMappingButton = styled(IconButtonWrapper)` + color: ${color("text-dark")}; + + &:hover { + color: ${color("danger")}; + } +`; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.tsx new file mode 100644 index 00000000000..c7a44d99e9e --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.tsx @@ -0,0 +1,175 @@ +import React, { useState } from "react"; +import { t } from "ttag"; +import _ from "underscore"; + +import { isAdminGroup } from "metabase/lib/groups"; +import { PermissionsApi } from "metabase/services"; +import Tooltip from "metabase/core/components/Tooltip"; +import Icon from "metabase/components/Icon"; +import Confirm from "metabase/components/Confirm"; + +import Selectbox from "../GroupSelect"; +import DeleteGroupMappingModal from "../DeleteGroupMappingModal"; + +import type { + DeleteMappingModalValueType, + GroupIds, + UserGroupsType, +} from "../types"; + +import { DeleteMappingButton } from "./MappingRow.styled"; + +type OnDeleteMappingType = (arg: { + name: string; + groupIdsToDelete?: GroupIds; + onSuccess?: () => void; +}) => void; + +type MappingRowProps = { + name: string; + groups: UserGroupsType; + selectedGroupIds: GroupIds; + onChange: () => void; + onDeleteMapping: OnDeleteMappingType; +}; + +const MappingRow = ({ + name, + groups, + selectedGroupIds, + onChange, + onDeleteMapping, +}: MappingRowProps) => { + const [showDeleteMappingModal, setShowDeleteMappingModal] = useState(false); + + // Mappings may receive group ids even from the back-end + // if the groups themselves have been deleted. + // Let's ensure this row works with the ones that exist. + const selectedGroupIdsFromGroupsThatExist = selectedGroupIds.filter(id => + _.findWhere(groups, { id: id }), + ); + + const handleShowDeleteMappingModal = () => { + setShowDeleteMappingModal(true); + }; + + const handleHideDeleteMappingModal = () => { + setShowDeleteMappingModal(false); + }; + + const handleConfirmDeleteMapping = ( + whatToDoAboutGroups: DeleteMappingModalValueType, + groups: GroupIds, + ) => { + const onSuccess = getCallbackForGroupsAfterDeletingMapping( + whatToDoAboutGroups, + groups, + ); + + const groupIdsToDelete = + whatToDoAboutGroups === "delete" ? selectedGroupIds : []; + + onDeleteMapping({ name, onSuccess, groupIdsToDelete }); + }; + + const getCallbackForGroupsAfterDeletingMapping = ( + whatToDoAboutGroups: DeleteMappingModalValueType, + groupIds: GroupIds, + ) => { + switch (whatToDoAboutGroups) { + case "clear": + return () => + Promise.all( + groupIds.map(async id => { + try { + if (!isAdminGroup(groups.find(group => group.id === id))) { + await PermissionsApi.clearGroupMembership({ id }); + } + } catch (error) { + console.error(error); + } + }), + ); + case "delete": + return () => + Promise.all( + groupIds.map(async id => { + try { + if (!isAdminGroup(groups.find(group => group.id === id))) { + await PermissionsApi.deleteGroup({ id }); + } + } catch (error) { + console.error(error); + } + }), + ); + default: + return () => null; + } + }; + + const firstGroupInMapping = groups.find( + group => group.id === selectedGroupIdsFromGroupsThatExist[0], + ); + + const isMappingLinkedOnlyToAdminGroup = + groups.length > 0 && + selectedGroupIdsFromGroupsThatExist.length === 1 && + isAdminGroup(firstGroupInMapping); + + const shouldUseDeleteMappingModal = + selectedGroupIdsFromGroupsThatExist.length > 0 && + !isMappingLinkedOnlyToAdminGroup; + + const onDelete = shouldUseDeleteMappingModal + ? () => handleShowDeleteMappingModal() + : () => onDeleteMapping({ name }); + + return ( + <> + <tr> + <td>{name}</td> + <td> + <Selectbox + groups={groups} + selectedGroupIds={selectedGroupIdsFromGroupsThatExist} + onGroupChange={onChange} + /> + </td> + <td className="Table-actions"> + <div className="float-right mr1"> + {shouldUseDeleteMappingModal ? ( + <DeleteButton onDelete={onDelete} /> + ) : ( + <Confirm action={onDelete} title={t`Delete this mapping?`}> + <DeleteButton /> + </Confirm> + )} + </div> + </td> + </tr> + {showDeleteMappingModal && ( + <DeleteGroupMappingModal + name={name} + groupIds={selectedGroupIds} + onHide={handleHideDeleteMappingModal} + onConfirm={handleConfirmDeleteMapping} + /> + )} + </> + ); +}; + +const DeleteButton = ({ + onDelete, +}: { + onDelete?: React.MouseEventHandler<HTMLButtonElement>; +}) => ( + <Tooltip tooltip={t`Remove mapping`} placement="top"> + <DeleteMappingButton onClick={onDelete}> + <Icon name="close" /> + </DeleteMappingButton> + </Tooltip> +); + +export default MappingRow; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/index.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/index.tsx new file mode 100644 index 00000000000..bd43766a898 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/index.tsx @@ -0,0 +1 @@ +export { default } from "./MappingRow"; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/index.tsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/index.tsx new file mode 100644 index 00000000000..97bf61865c1 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/index.tsx @@ -0,0 +1 @@ +export { default } from "./GroupMappingsWidget"; diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/types.ts b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/types.ts new file mode 100644 index 00000000000..0cee6ac4101 --- /dev/null +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/types.ts @@ -0,0 +1,5 @@ +export type MappingsType = Record<string, number[]>; +export type GroupIds = number[]; +export type DeleteMappingModalValueType = "nothing" | "clear" | "delete"; +export type UserGroupType = { id: number; name: string; member_count: number }; +export type UserGroupsType = UserGroupType[]; diff --git a/frontend/src/metabase/admin/settings/components/widgets/SettingToggle.jsx b/frontend/src/metabase/admin/settings/components/widgets/SettingToggle.jsx index 87a6426b4e0..50e6517e2c4 100644 --- a/frontend/src/metabase/admin/settings/components/widgets/SettingToggle.jsx +++ b/frontend/src/metabase/admin/settings/components/widgets/SettingToggle.jsx @@ -4,7 +4,14 @@ import { t } from "ttag"; import Toggle from "metabase/core/components/Toggle"; import Tooltip from "metabase/core/components/Tooltip"; -const SettingToggle = ({ id, setting, onChange, disabled, tooltip }) => { +const SettingToggle = ({ + disabled, + hideLabel, + id, + setting, + tooltip, + onChange, +}) => { const value = setting.value == null ? setting.default : setting.value; const on = value === true || value === "true"; return ( @@ -17,7 +24,9 @@ const SettingToggle = ({ id, setting, onChange, disabled, tooltip }) => { disabled={disabled} /> </Tooltip> - <span className="text-bold mx1">{on ? t`Enabled` : t`Disabled`}</span> + {!hideLabel && ( + <span className="text-bold mx1">{on ? t`Enabled` : t`Disabled`}</span> + )} </div> ); }; diff --git a/frontend/src/metabase/css/admin.css b/frontend/src/metabase/css/admin.css index da271ff9cbd..0556e045c71 100644 --- a/frontend/src/metabase/css/admin.css +++ b/frontend/src/metabase/css/admin.css @@ -45,6 +45,11 @@ padding: 1em; } +.ContentTable th { + color: var(--color-text-dark); + padding: 1em; +} + /* TODO: remove this and apply AdminHoverItem to content rows */ .ContentTable tbody tr:hover { background-color: color-mod(var(--color-brand) alpha(-96%)); diff --git a/frontend/src/metabase/plugins/builtin/auth/ldap.js b/frontend/src/metabase/plugins/builtin/auth/ldap.js index cc6a9860770..9a3dfbbe488 100644 --- a/frontend/src/metabase/plugins/builtin/auth/ldap.js +++ b/frontend/src/metabase/plugins/builtin/auth/ldap.js @@ -101,7 +101,6 @@ PLUGIN_ADMIN_SETTINGS_UPDATES.push( }, { key: "ldap-group-sync", - display_name: t`Synchronize group memberships`, description: null, widget: GroupMappingsWidget, props: { diff --git a/frontend/src/metabase/services.js b/frontend/src/metabase/services.js index 6349aa7f0ed..f4aa3082d5b 100644 --- a/frontend/src/metabase/services.js +++ b/frontend/src/metabase/services.js @@ -382,6 +382,7 @@ export const PermissionsApi = { createMembership: POST("/api/permissions/membership"), deleteMembership: DELETE("/api/permissions/membership/:id"), updateMembership: PUT("/api/permissions/membership/:id"), + clearGroupMembership: PUT("/api/permissions/membership/:id/clear"), updateGroup: PUT("/api/permissions/group/:id"), deleteGroup: DELETE("/api/permissions/group/:id"), }; diff --git a/frontend/test/metabase/scenarios/admin/settings/sso/ldap.cy.spec.js b/frontend/test/metabase/scenarios/admin/settings/sso/ldap.cy.spec.js index 9a59f07f7f6..c0294cd104c 100644 --- a/frontend/test/metabase/scenarios/admin/settings/sso/ldap.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/settings/sso/ldap.cy.spec.js @@ -106,6 +106,61 @@ describe( cy.findByText("Username or email address").should("be.visible"); cy.findByText("Password").should("be.visible"); }); + + describe("Group Mappings Widget", () => { + it("should allow deleting mappings along with deleting, or clearing users of, mapped groups", () => { + cy.visit("/admin/settings/authentication/ldap"); + + // Create mapping, then delete it along with its groups + createMapping("cn=People1"); + addGroupsToMapping("cn=People1", ["Administrators", "data", "nosql"]); + deleteMappingWithGroups("cn=People1"); + + // Create mapping, then clear its groups of members + createMapping("cn=People2"); + addGroupsToMapping("cn=People2", ["collection", "readonly"]); + // Groups deleted along with first mapping should not be offered + cy.findByText("data").should("not.exist"); + cy.findByText("nosql").should("not.exist"); + + cy.icon("close").click({ force: true }); + cy.findByText(/remove all group members/i).click(); + cy.button("Remove mapping and members").click(); + + cy.visit("/admin/people/groups"); + cy.findByText("data").should("not.exist"); + cy.findByText("nosql").should("not.exist"); + + checkThatGroupHasNoMembers("collection"); + checkThatGroupHasNoMembers("readonly"); + }); + + it("should allow deleting mappings with groups, while keeping remaining mappings consistent with their undeleted groups", () => { + cy.visit("/admin/settings/authentication/ldap"); + + createMapping("cn=People1"); + addGroupsToMapping("cn=People1", ["Administrators", "data", "nosql"]); + + createMapping("cn=People2"); + addGroupsToMapping("cn=People2", ["data", "collection"]); + + createMapping("cn=People3"); + addGroupsToMapping("cn=People3", ["collection", "readonly"]); + + deleteMappingWithGroups("cn=People2"); + + // cn=People1 will have Admin and nosql as groups + cy.findByText("1 other group"); + + // cn=People3 will have readonly as group + cy.findByText("readonly"); + + // Ensure mappings are as expected after a page reload + cy.visit("/admin/settings/authentication/ldap"); + cy.findByText("1 other group"); + cy.findByText("readonly"); + }); + }); }, ); @@ -124,3 +179,36 @@ const enterLdapSettings = () => { typeAndBlurUsingLabel("Password", "admin"); typeAndBlurUsingLabel("User search base", "dc=example,dc=org"); }; + +const createMapping = name => { + cy.button("New mapping").click(); + cy.focused().type(name); + cy.button("Add").click(); +}; + +const addGroupsToMapping = (mappingName, groups) => { + cy.findByText(mappingName) + .closest("tr") + .within(() => { + cy.findByText("Default").click(); + }); + + groups.forEach(group => cy.findByText(group).click()); +}; + +const deleteMappingWithGroups = mappingName => { + cy.findByText(mappingName) + .closest("tr") + .within(() => { + cy.icon("close").click({ force: true }); + }); + + cy.findByText(/delete the groups/i).click(); + cy.button("Remove mapping and delete groups").click(); +}; + +const checkThatGroupHasNoMembers = name => { + cy.findByText(name) + .closest("tr") + .within(() => cy.findByText("0")); +}; -- GitLab