From 763163603c0541505d564caa852ba047dc738fe5 Mon Sep 17 00:00:00 2001
From: linoman <2051016+linoman@users.noreply.github.com>
Date: Wed, 2 Oct 2024 11:42:23 +0200
Subject: [PATCH] SSO LDAP: Bug-bashing follow-up changes (#94093)

* fix html encoding rendering

* Redirect to providers page

* Fix cert isEmpty

* Rework input fields into multiselect

* add disable button

* Rework MultiSelect design

* Remove prompt modal
---
 public/app/features/admin/ldap/LdapDrawer.tsx | 94 +++++++++++--------
 .../features/admin/ldap/LdapSettingsPage.tsx  | 69 +++++++-------
 public/locales/en-US/grafana.json             | 18 ++--
 public/locales/pseudo-LOCALE/grafana.json     | 18 ++--
 4 files changed, 102 insertions(+), 97 deletions(-)

diff --git a/public/app/features/admin/ldap/LdapDrawer.tsx b/public/app/features/admin/ldap/LdapDrawer.tsx
index caa5edefa1f..a42cb0c08a0 100644
--- a/public/app/features/admin/ldap/LdapDrawer.tsx
+++ b/public/app/features/admin/ldap/LdapDrawer.tsx
@@ -17,7 +17,6 @@ import {
   Select,
   Stack,
   Switch,
-  Text,
   TextLink,
   Tooltip,
   RadioButtonGroup,
@@ -73,6 +72,18 @@ export const LdapDrawerComponent = ({
     return value;
   };
 
+  const attributesLabel = (
+    <Label
+      className={styles.sectionLabel}
+      description={t(
+        'ldap-drawer.attributes-section.description',
+        "Specify the LDAP attributes that map to the user's given name, surname, and email address, ensuring the application correctly retrieves and displays user information."
+      )}
+    >
+      <Trans i18nKey="ldap-drawer.attributes-section.label">Attributes</Trans>
+    </Label>
+  );
+
   const groupMappingsLabel = (
     <Label
       className={styles.sectionLabel}
@@ -153,13 +164,7 @@ export const LdapDrawerComponent = ({
           />
         </Field>
       </CollapsableSection>
-      <CollapsableSection label={t('ldap-drawer.attributes-section.label', 'Attributes')} isOpen={true}>
-        <Text color="secondary">
-          <Trans i18nKey="ldap-drawer.attributes-section.description">
-            Specify the LDAP attributes that map to the user&lsquo;s given name, surname, and email address, ensuring
-            the application correctly retrieves and displays user information.
-          </Trans>
-        </Text>
+      <CollapsableSection label={attributesLabel} isOpen={true}>
         <Field label={t('ldap-drawer.attributes-section.name-label', 'Name')}>
           <Input id={nameId} {...register(`${serverConfig}.attributes.name`)} />
         </Field>
@@ -197,17 +202,24 @@ export const LdapDrawerComponent = ({
         >
           <Input id="group-search-filter" {...register(`${serverConfig}.group_search_filter`)} />
         </Field>
-        <Field
-          htmlFor="group-search-base-dns"
-          label={t('ldap-drawer.group-mapping-section.group-search-base-dns-label', 'Group search base DNS')}
-          description={t(
-            'ldap-drawer.group-mapping-section.group-search-base-dns-description',
-            'Separate by commas or spaces'
-          )}
-        >
-          <Input
-            id="group-search-base-dns"
-            onChange={({ currentTarget: { value } }) => setValue(`${serverConfig}.group_search_base_dns`, [value])}
+        <Field label={t('ldap-drawer.group-mapping-section.group-search-base-dns-label', 'Group search base DNS')}>
+          <Controller
+            name={`${serverConfig}.group_search_base_dns`}
+            control={control}
+            render={({ field: { onChange, ref, value, ...field } }) => (
+              <MultiSelect
+                {...field}
+                allowCustomValue
+                className={styles.multiSelect}
+                noOptionsMessage=""
+                placeholder={t(
+                  'ldap-drawer.group-mapping-section.group-search-base-dns-placeholder',
+                  'example: ou=groups,dc=example,dc=com'
+                )}
+                onChange={(v) => onChange(v.map(({ value }) => String(value)))}
+                value={value?.map((v) => ({ label: v, value: v }))}
+              />
+            )}
           />
         </Field>
         <Field
@@ -278,26 +290,24 @@ export const LdapDrawerComponent = ({
                 onChange={({ value }) => setValue(`${serverConfig}.min_tls_version`, value)}
               />
             </Field>
-            <Field
-              label={t('ldap-drawer.extra-security-section.tls-ciphers-label', 'TLS ciphers')}
-              description={t(
-                'ldap-drawer.extra-security-section.tls-ciphers-description',
-                'List of comma- or space-separated ciphers'
-              )}
-            >
-              <Input
-                id="tls-ciphers"
-                placeholder={t(
-                  'ldap-drawer.extra-security-section.tls-ciphers-placeholder',
-                  'e.g. ["TLS_AES_256_GCM_SHA384"]'
+            <Field label={t('ldap-drawer.extra-security-section.tls-ciphers-label', 'TLS ciphers')}>
+              <Controller
+                name={`${serverConfig}.tls_ciphers`}
+                control={control}
+                render={({ field: { onChange, ref, value, ...field } }) => (
+                  <MultiSelect
+                    {...field}
+                    allowCustomValue
+                    className={styles.multiSelect}
+                    noOptionsMessage=""
+                    placeholder={t(
+                      'ldap-drawer.extra-security-section.tls-ciphers-placeholder',
+                      'example: TLS_AES_256_GCM_SHA384'
+                    )}
+                    onChange={(v) => onChange(v.map(({ value }) => String(value)))}
+                    value={value?.map((v) => ({ label: v, value: v }))}
+                  />
                 )}
-                value={watch(`${serverConfig}.tls_ciphers`) || ''}
-                onChange={({ currentTarget: { value } }) =>
-                  setValue(
-                    `${serverConfig}.tls_ciphers`,
-                    value?.split(/,|\s/).map((v) => v.trim())
-                  )
-                }
               />
             </Field>
             <Field
@@ -345,7 +355,11 @@ export const LdapDrawerComponent = ({
                         {...field}
                         allowCustomValue
                         className={styles.multiSelect}
-                        noOptionsMessage={''}
+                        noOptionsMessage=""
+                        placeholder={t(
+                          'ldap-drawer.extra-security-section.root-ca-cert-value-placeholder',
+                          'example: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t'
+                        )}
                         onChange={(v) => onChange(v.map(({ value }) => String(value)))}
                         value={value?.map((v) => ({ label: renderMultiSelectLabel(v), value: v }))}
                       />
@@ -452,7 +466,7 @@ function getStyles(theme: GrafanaTheme2) {
       marginBottom: theme.spacing(4),
     }),
     multiSelect: css({
-      svg: {
+      'div:last-of-type > svg': {
         display: 'none',
       },
     }),
diff --git a/public/app/features/admin/ldap/LdapSettingsPage.tsx b/public/app/features/admin/ldap/LdapSettingsPage.tsx
index 736a40a76c5..1132b95265f 100644
--- a/public/app/features/admin/ldap/LdapSettingsPage.tsx
+++ b/public/app/features/admin/ldap/LdapSettingsPage.tsx
@@ -4,7 +4,7 @@ import { Controller, FormProvider, useForm } from 'react-hook-form';
 import { connect } from 'react-redux';
 
 import { AppEvents, GrafanaTheme2, NavModelItem } from '@grafana/data';
-import { getBackendSrv, getAppEvents, reportInteraction } from '@grafana/runtime';
+import { getBackendSrv, getAppEvents, locationService, reportInteraction } from '@grafana/runtime';
 import {
   useStyles2,
   Alert,
@@ -15,7 +15,6 @@ import {
   Input,
   LinkButton,
   Menu,
-  Modal,
   Stack,
   Text,
   TextLink,
@@ -49,6 +48,8 @@ const pageNav: NavModelItem = {
 
 const serverConfig = 'settings.config.servers.0';
 
+const isOptionDefined = (option: string | undefined) => option !== undefined && option !== '';
+
 const emptySettings: LdapPayload = {
   id: '',
   provider: '',
@@ -96,7 +97,6 @@ const emptySettings: LdapPayload = {
 export const LdapSettingsPage = () => {
   const [isLoading, setIsLoading] = useState(true);
   const [isDrawerOpen, setIsDrawerOpen] = useState(false);
-  const [isModalOpen, setIsModalOpen] = useState(false);
 
   const [mapKeyCertConfigured, setMapKeyCertConfigured] = useState<MapKeyCertConfigured>({
     // values
@@ -131,11 +131,11 @@ export const LdapSettingsPage = () => {
       }
       setMapKeyCertConfigured({
         rootCaCertValue: serverConfig.root_ca_cert_value?.length > 0,
-        clientCertValue: serverConfig.client_cert_value !== '',
-        clientKeyCertValue: serverConfig.client_key_value !== '',
-        rootCaCertPath: serverConfig.root_ca_cert !== '',
-        clientCertPath: serverConfig.client_cert !== '',
-        clientKeyCertPath: serverConfig.client_key !== '',
+        clientCertValue: isOptionDefined(serverConfig.client_cert_value),
+        clientKeyCertValue: isOptionDefined(serverConfig.client_key_value),
+        rootCaCertPath: isOptionDefined(serverConfig.root_ca_cert),
+        clientCertPath: isOptionDefined(serverConfig.client_cert),
+        clientKeyCertPath: isOptionDefined(serverConfig.client_key),
       });
 
       reset(payload);
@@ -199,6 +199,11 @@ export const LdapSettingsPage = () => {
         payload: [t('ldap-settings-page.alert.saved', 'LDAP settings saved')],
       });
       reset(await getSettings());
+
+      // Delay redirect so the form state can update
+      setTimeout(() => {
+        locationService.push(`/admin/authentication`);
+      }, 300);
     } catch (error) {
       appEvents.publish({
         type: AppEvents.alertError.name,
@@ -218,7 +223,7 @@ export const LdapSettingsPage = () => {
    * Button's Actions
    */
   const submitAndEnableLdapSettings = async (payload: LdapPayload) => {
-    payload.settings.enabled = true;
+    payload.settings.enabled = !payload.settings.enabled;
     await putPayload(payload);
     reportInteraction('authentication_ldap_enabled');
   };
@@ -237,6 +242,10 @@ export const LdapSettingsPage = () => {
       });
       reset(payload);
       reportInteraction('authentication_ldap_deleted');
+
+      setTimeout(() => {
+        locationService.push(`/admin/authentication`);
+      }, 300);
     } catch (error) {
       appEvents.publish({
         type: AppEvents.alertError.name,
@@ -351,11 +360,12 @@ export const LdapSettingsPage = () => {
                         {...field}
                         allowCustomValue
                         className={styles.multiSelect}
-                        noOptionsMessage={''}
+                        noOptionsMessage=""
+                        placeholder={t('ldap-settings-page.search-base-dns.placeholder', 'example: dc=grafana,dc=org')}
                         onChange={(v) => onChange(v.map(({ value }) => String(value)))}
                       />
                     )}
-                  ></Controller>
+                  />
                 </Field>
                 <Box borderColor="strong" borderStyle="solid" padding={2} width={68}>
                   <Stack alignItems={'center'} direction={'row'} gap={2} justifyContent={'space-between'}>
@@ -376,9 +386,18 @@ export const LdapSettingsPage = () => {
                 </Box>
                 <Box display="flex" gap={2} marginTop={5}>
                   <Stack alignItems="center" gap={2}>
-                    <Button type="submit">
-                      <Trans i18nKey="ldap-settings-page.buttons-section.save-and-enable-button">Save and enable</Trans>
-                    </Button>
+                    {!watch('settings.enabled') && (
+                      <Button type="submit">
+                        <Trans i18nKey="ldap-settings-page.buttons-section.save-and-enable-button">
+                          Save and enable
+                        </Trans>
+                      </Button>
+                    )}
+                    {watch('settings.enabled') && (
+                      <Button variant="secondary" type="submit">
+                        <Trans i18nKey="ldap-settings-page.buttons-section.disable-button">Disable</Trans>
+                      </Button>
+                    )}
                     <Button variant="secondary" onClick={saveForm}>
                       <Trans i18nKey="ldap-settings-page.buttons-section.save-button">Save</Trans>
                     </Button>
@@ -416,26 +435,6 @@ export const LdapSettingsPage = () => {
           </form>
         </FormProvider>
       </Page.Contents>
-      <Modal
-        title={t('ldap-settings-page.discard-modal.title', 'Leave LDAP configuration?')}
-        isOpen={isModalOpen}
-        onDismiss={() => setIsModalOpen(false)}
-      >
-        <Stack direction={'column'} gap={2}>
-          <Trans i18nKey="ldap-settings-page.discard-modal.description">
-            Are you sure you want to abandon the changes you&lsquo;ve made to the LDAP configuration? All changes will
-            be lost.
-          </Trans>
-          <Stack direction="row" gap={2} justifyContent="flex-end">
-            <Button variant="secondary">
-              <Trans i18nKey="ldap-settings-page.discard-modal.cancel-button">Back to editing</Trans>
-            </Button>
-            <Button variant="destructive">
-              <Trans i18nKey="ldap-settings-page.discard-modal.discard-button">Abandon LDAP</Trans>
-            </Button>
-          </Stack>
-        </Stack>
-      </Modal>
     </Page>
   );
 };
@@ -446,7 +445,7 @@ function getStyles(theme: GrafanaTheme2) {
       width: theme.spacing(68),
     }),
     multiSelect: css({
-      svg: {
+      'div:last-of-type > svg': {
         display: 'none',
       },
     }),
diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json
index 3d5fff7e31a..2d48513aa12 100644
--- a/public/locales/en-US/grafana.json
+++ b/public/locales/en-US/grafana.json
@@ -1102,7 +1102,7 @@
   },
   "ldap-drawer": {
     "attributes-section": {
-      "description": "Specify the LDAP attributes that map to the user&lsquo;s given name, surname, and email address, ensuring the application correctly retrieves and displays user information.",
+      "description": "Specify the LDAP attributes that map to the user's given name, surname, and email address, ensuring the application correctly retrieves and displays user information.",
       "email-label": "Email",
       "label": "Attributes",
       "member-of-label": "Member Of",
@@ -1129,11 +1129,11 @@
       "root-ca-cert-label": "Root CA certificate path",
       "root-ca-cert-placeholder": "/path/to/root_ca_cert.pem",
       "root-ca-cert-value-label": "Root CA certificate content",
+      "root-ca-cert-value-placeholder": "example: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t",
       "start-tls-description": "If set to true, use LDAP with STARTTLS instead of LDAPS",
       "start-tls-label": "Start TLS",
-      "tls-ciphers-description": "List of comma- or space-separated ciphers",
       "tls-ciphers-label": "TLS ciphers",
-      "tls-ciphers-placeholder": "e.g. [\"TLS_AES_256_GCM_SHA384\"]",
+      "tls-ciphers-placeholder": "example: TLS_AES_256_GCM_SHA384",
       "use-ssl-description": "Set to true if LDAP server should use TLS connection (either with STARTTLS or LDAPS)",
       "use-ssl-label": "Use SSL",
       "use-ssl-tooltip": "For a complete list of supported ciphers and TLS versions, refer to:"
@@ -1163,8 +1163,8 @@
         "button": "Add group mapping"
       },
       "description": "Map LDAP groups to Grafana org roles",
-      "group-search-base-dns-description": "Separate by commas or spaces",
       "group-search-base-dns-label": "Group search base DNS",
+      "group-search-base-dns-placeholder": "example: ou=groups,dc=example,dc=com",
       "group-search-filter-description": "Used to filter and identify group entries within the directory",
       "group-search-filter-label": "Group search filter",
       "group-search-filter-user-attribute-description": "Identifies users within group entries for filtering purposes",
@@ -1207,16 +1207,11 @@
       "label": "Bind password"
     },
     "buttons-section": {
+      "disable-button": "Disable",
       "discard-button": "Discard",
       "save-and-enable-button": "Save and enable",
       "save-button": "Save"
     },
-    "discard-modal": {
-      "cancel-button": "Back to editing",
-      "description": "Are you sure you want to abandon the changes you&lsquo;ve made to the LDAP configuration? All changes will be lost.",
-      "discard-button": "Abandon LDAP",
-      "title": "Leave LDAP configuration?"
-    },
     "documentation": "documentation",
     "host": {
       "description": "Hostname or IP address of the LDAP server you wish to connect to.",
@@ -1234,7 +1229,8 @@
     },
     "search-base-dns": {
       "description": "An array of base dns to search through.",
-      "label": "Search base DNS *"
+      "label": "Search base DNS *",
+      "placeholder": "example: dc=grafana,dc=org"
     },
     "subtitle": "The LDAP integration in Grafana allows your Grafana users to log in with their LDAP credentials. Find out more in our <2><0>documentation</0></2>.",
     "title": "Basic Settings"
diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json
index 60eb86a1b52..b9816e154bf 100644
--- a/public/locales/pseudo-LOCALE/grafana.json
+++ b/public/locales/pseudo-LOCALE/grafana.json
@@ -1102,7 +1102,7 @@
   },
   "ldap-drawer": {
     "attributes-section": {
-      "description": "Ŝpęčįƒy ŧĥę ĿĐÅP äŧŧřįþūŧęş ŧĥäŧ mäp ŧő ŧĥę ūşęř&ľşqūő;ş ģįvęʼn ʼnämę, şūřʼnämę, äʼnđ ęmäįľ äđđřęşş, ęʼnşūřįʼnģ ŧĥę äppľįčäŧįőʼn čőřřęčŧľy řęŧřįęvęş äʼnđ đįşpľäyş ūşęř įʼnƒőřmäŧįőʼn.",
+      "description": "Ŝpęčįƒy ŧĥę ĿĐÅP äŧŧřįþūŧęş ŧĥäŧ mäp ŧő ŧĥę ūşęř'ş ģįvęʼn ʼnämę, şūřʼnämę, äʼnđ ęmäįľ äđđřęşş, ęʼnşūřįʼnģ ŧĥę äppľįčäŧįőʼn čőřřęčŧľy řęŧřįęvęş äʼnđ đįşpľäyş ūşęř įʼnƒőřmäŧįőʼn.",
       "email-label": "Ēmäįľ",
       "label": "Åŧŧřįþūŧęş",
       "member-of-label": "Męmþęř ؃",
@@ -1129,11 +1129,11 @@
       "root-ca-cert-label": "Ŗőőŧ CÅ čęřŧįƒįčäŧę päŧĥ",
       "root-ca-cert-placeholder": "/päŧĥ/ŧő/řőőŧ_čä_čęřŧ.pęm",
       "root-ca-cert-value-label": "Ŗőőŧ CÅ čęřŧįƒįčäŧę čőʼnŧęʼnŧ",
+      "root-ca-cert-value-placeholder": "ęχämpľę: ĿŜ0ŧĿŜ1CŖŮđĴŦįßĐŖVĴŮŜŮŻĴQ0FŮŖŜ0ŧĿŜ0ŧ",
       "start-tls-description": "Ĩƒ şęŧ ŧő ŧřūę, ūşę ĿĐÅP ŵįŧĥ ŜŦÅŖŦŦĿŜ įʼnşŧęäđ őƒ ĿĐÅPŜ",
       "start-tls-label": "Ŝŧäřŧ ŦĿŜ",
-      "tls-ciphers-description": "Ŀįşŧ őƒ čőmmä- őř şpäčę-şępäřäŧęđ čįpĥęřş",
       "tls-ciphers-label": "ŦĿŜ čįpĥęřş",
-      "tls-ciphers-placeholder": "ę.ģ. [\"ŦĿŜ_ÅĒŜ_256_ĞCM_ŜĦÅ384\"]",
+      "tls-ciphers-placeholder": "ęχämpľę: ŦĿŜ_ÅĒŜ_256_ĞCM_ŜĦÅ384",
       "use-ssl-description": "Ŝęŧ ŧő ŧřūę įƒ ĿĐÅP şęřvęř şĥőūľđ ūşę ŦĿŜ čőʼnʼnęčŧįőʼn (ęįŧĥęř ŵįŧĥ ŜŦÅŖŦŦĿŜ őř ĿĐÅPŜ)",
       "use-ssl-label": "Ůşę ŜŜĿ",
       "use-ssl-tooltip": "Főř ä čőmpľęŧę ľįşŧ őƒ şūppőřŧęđ čįpĥęřş äʼnđ ŦĿŜ vęřşįőʼnş, řęƒęř ŧő:"
@@ -1163,8 +1163,8 @@
         "button": "Åđđ ģřőūp mäppįʼnģ"
       },
       "description": "Mäp ĿĐÅP ģřőūpş ŧő Ğřäƒäʼnä őřģ řőľęş",
-      "group-search-base-dns-description": "Ŝępäřäŧę þy čőmmäş őř şpäčęş",
       "group-search-base-dns-label": "Ğřőūp şęäřčĥ þäşę ĐŃŜ",
+      "group-search-base-dns-placeholder": "ęχämpľę: őū=ģřőūpş,đč=ęχämpľę,đč=čőm",
       "group-search-filter-description": "Ůşęđ ŧő ƒįľŧęř äʼnđ įđęʼnŧįƒy ģřőūp ęʼnŧřįęş ŵįŧĥįʼn ŧĥę đįřęčŧőřy",
       "group-search-filter-label": "Ğřőūp şęäřčĥ ƒįľŧęř",
       "group-search-filter-user-attribute-description": "Ĩđęʼnŧįƒįęş ūşęřş ŵįŧĥįʼn ģřőūp ęʼnŧřįęş ƒőř ƒįľŧęřįʼnģ pūřpőşęş",
@@ -1207,16 +1207,11 @@
       "label": "ßįʼnđ päşşŵőřđ"
     },
     "buttons-section": {
+      "disable-button": "Đįşäþľę",
       "discard-button": "Đįşčäřđ",
       "save-and-enable-button": "Ŝävę äʼnđ ęʼnäþľę",
       "save-button": "Ŝävę"
     },
-    "discard-modal": {
-      "cancel-button": "ßäčĸ ŧő ęđįŧįʼnģ",
-      "description": "Åřę yőū şūřę yőū ŵäʼnŧ ŧő äþäʼnđőʼn ŧĥę čĥäʼnģęş yőū&ľşqūő;vę mäđę ŧő ŧĥę ĿĐÅP čőʼnƒįģūřäŧįőʼn? Åľľ čĥäʼnģęş ŵįľľ þę ľőşŧ.",
-      "discard-button": "Åþäʼnđőʼn ĿĐÅP",
-      "title": "Ŀęävę ĿĐÅP čőʼnƒįģūřäŧįőʼn?"
-    },
     "documentation": "đőčūmęʼnŧäŧįőʼn",
     "host": {
       "description": "Ħőşŧʼnämę őř ĨP äđđřęşş őƒ ŧĥę ĿĐÅP şęřvęř yőū ŵįşĥ ŧő čőʼnʼnęčŧ ŧő.",
@@ -1234,7 +1229,8 @@
     },
     "search-base-dns": {
       "description": "Åʼn äřřäy őƒ þäşę đʼnş ŧő şęäřčĥ ŧĥřőūģĥ.",
-      "label": "Ŝęäřčĥ þäşę ĐŃŜ *"
+      "label": "Ŝęäřčĥ þäşę ĐŃŜ *",
+      "placeholder": "ęχämpľę: đč=ģřäƒäʼnä,đč=őřģ"
     },
     "subtitle": "Ŧĥę ĿĐÅP įʼnŧęģřäŧįőʼn įʼn Ğřäƒäʼnä äľľőŵş yőūř Ğřäƒäʼnä ūşęřş ŧő ľőģ įʼn ŵįŧĥ ŧĥęįř ĿĐÅP čřęđęʼnŧįäľş. Fįʼnđ őūŧ mőřę įʼn őūř <2><0>đőčūmęʼnŧäŧįőʼn</0></2>.",
     "title": "ßäşįč Ŝęŧŧįʼnģş"