Merge pull request #587 from mfrances17/fix-bind-type

LDAP Providers - do not show DN and Credentials when bind type is none
2021-05-14 16:14:37 -04:00 · 2021-05-14 16:14:37 -04:00 · 07e6628040
commit 07e6628040
parent 8e1a1c57bd df2263d11b
6 changed files with 271 additions and 136 deletions
--- a/cypress/integration/user_fed_ldap_test.spec.ts
+++ b/cypress/integration/user_fed_ldap_test.spec.ts
@ -32,8 +32,6 @@ const secondLdapName = `${firstLdapName}-2`;
 const secondLdapVendor = "Other";

 const secondBindType = "none";
-const secondBindDn = "user-2";
-const secondBindCreds = "password2";

 const secondUsersDn = "user-dn-2";
 const secondUserLdapAtt = "cn";
@ -165,8 +163,6 @@ describe("User Fed LDAP tests", () => {
    providersPage.fillLdapRequiredConnectionData(
      connectionUrl,
      secondBindType,
-      secondBindDn,
-      secondBindCreds
    );
    providersPage.fillLdapRequiredSearchingData(
      secondUsersDn,
--- a/cypress/support/pages/admin_console/manage/providers/ProviderPage.ts
+++ b/cypress/support/pages/admin_console/manage/providers/ProviderPage.ts
@ -129,8 +129,8 @@ export default class ProviderPage {
  fillLdapRequiredConnectionData(
    connectionUrl: string,
    bindType: string,
-    bindDn: string,
-    bindCreds: string
+    bindDn?: string,
+    bindCreds?: string
  ) {
    if (connectionUrl) {
      cy.get(`[${this.ldapConnectionUrlInput}]`).type(connectionUrl);
--- a/src/user-federation/UserFederationLdapSettings.tsx
+++ b/src/user-federation/UserFederationLdapSettings.tsx
@ -26,7 +26,7 @@ import ComponentRepresentation from "keycloak-admin/lib/defs/componentRepresenta

 import { Controller, useForm } from "react-hook-form";
 import { useConfirmDialog } from "../components/confirm-dialog/ConfirmDialog";
-import { useAdminClient } from "../context/auth/AdminClient";
+import { asyncStateFetch, useAdminClient } from "../context/auth/AdminClient";
 import { useAlerts } from "../components/alert/Alerts";
 import { useTranslation } from "react-i18next";
 import { ViewHeader } from "../components/view-header/ViewHeader";
@ -35,6 +35,14 @@ import { ScrollForm } from "../components/scroll-form/ScrollForm";

 import { KeycloakTabs } from "../components/keycloak-tabs/KeycloakTabs";
 import { LdapMapperList } from "./ldap/mappers/LdapMapperList";
+import { useErrorHandler } from "react-error-boundary";
+
+type ldapComponentRepresentation = ComponentRepresentation & {
+  config?: {
+    periodicChangedUsersSync?: boolean;
+    periodicFullSync?: boolean;
+  };
+};

 type LdapSettingsHeaderProps = {
  onChange: (value: string) => void;
@ -167,33 +175,45 @@ const LdapSettingsHeader = ({

 export const UserFederationLdapSettings = () => {
  const { t } = useTranslation("user-federation");
-  const form = useForm<ComponentRepresentation>();
+  const form = useForm<ComponentRepresentation>({ mode: "onChange" });
  const history = useHistory();
  const adminClient = useAdminClient();
  const { realm } = useRealm();
+  const errorHandler = useErrorHandler();

  const { id } = useParams<{ id: string }>();
  const { addAlert } = useAlerts();

  useEffect(() => {
-    (async () => {
    if (id) {
-        const fetchedComponent = await adminClient.components.findOne({ id });
+      return asyncStateFetch(
+        () => adminClient.components.findOne({ id }),
+        (fetchedComponent) => {
          if (fetchedComponent) {
            setupForm(fetchedComponent);
          }
+        },
+        errorHandler
+      );
    }
-    })();
  }, []);

  const setupForm = (component: ComponentRepresentation) => {
-    form.reset();
    Object.entries(component).map((entry) => {
      if (entry[0] === "config") {
+        form.setValue(
+          "config.periodicChangedUsersSync",
+          entry[1].changedSyncPeriod[0] !== "-1"
+        );
+
+        form.setValue(
+          "config.periodicFullSync",
+          entry[1].fullSyncPeriod[0] !== "-1"
+        );
+
        convertToFormValues(entry[1], "config", form.setValue);
-      } else {
-        form.setValue(entry[0], entry[1]);
      }
+      form.setValue(entry[0], entry[1]);
    });
  };

@ -208,7 +228,19 @@ export const UserFederationLdapSettings = () => {
    }
  };

-  const save = async (component: ComponentRepresentation) => {
+  const save = async (component: ldapComponentRepresentation) => {
+    if (component?.config?.periodicChangedUsersSync !== null) {
+      if (component?.config?.periodicChangedUsersSync === false) {
+        component.config.changedSyncPeriod = ["-1"];
+      }
+      delete component?.config?.periodicChangedUsersSync;
+    }
+    if (component?.config?.periodicFullSync !== null) {
+      if (component?.config?.periodicFullSync === false) {
+        component.config.fullSyncPeriod = ["-1"];
+      }
+      delete component?.config?.periodicFullSync;
+    }
    try {
      if (!id) {
        await adminClient.components.create(component);
@ -216,11 +248,10 @@ export const UserFederationLdapSettings = () => {
      } else {
        await adminClient.components.update({ id }, component);
      }
-      setupForm(component as ComponentRepresentation);
      addAlert(t(id ? "saveSuccess" : "createSuccess"), AlertVariant.success);
    } catch (error) {
      addAlert(
-        `${t(id ? "saveError" : "createError")} '${error}'`,
+        t(id ? "saveError" : "createError", { error }),
        AlertVariant.danger
      );
    }
--- a/src/user-federation/ldap/LdapSettingsConnection.tsx
+++ b/src/user-federation/ldap/LdapSettingsConnection.tsx
@ -1,4 +1,5 @@
 import {
+  AlertVariant,
  Button,
  FormGroup,
  Select,
@ -6,14 +7,21 @@ import {
  SelectVariant,
  Switch,
  TextInput,
+  ValidatedOptions,
 } from "@patternfly/react-core";
 import { useTranslation } from "react-i18next";
 import React, { useState } from "react";
+import _ from "lodash";
+
+import TestLdapConnectionRepresentation from "keycloak-admin/lib/defs/testLdapConnection";
 import { HelpItem } from "../../components/help-enabler/HelpItem";
-import { Controller, UseFormMethods } from "react-hook-form";
+import { Controller, UseFormMethods, useWatch } from "react-hook-form";
 import { FormAccess } from "../../components/form-access/FormAccess";
 import { WizardSectionHeader } from "../../components/wizard-section-header/WizardSectionHeader";
 import { PasswordInput } from "../../components/password-input/PasswordInput";
+import { useAdminClient } from "../../context/auth/AdminClient";
+import { useRealm } from "../../context/realm-context/RealmContext";
+import { useAlerts } from "../../components/alert/Alerts";

 export type LdapSettingsConnectionProps = {
  form: UseFormMethods;
@ -21,13 +29,45 @@ export type LdapSettingsConnectionProps = {
  showSectionDescription?: boolean;
 };

+const testLdapProperties: Array<keyof TestLdapConnectionRepresentation> = [
+  "connectionUrl",
+  "bindDn",
+  "bindCredential",
+  "useTruststoreSpi",
+  "connectionTimeout",
+  "startTls",
+  "authType",
+];
+
 export const LdapSettingsConnection = ({
  form,
  showSectionHeading = false,
  showSectionDescription = false,
 }: LdapSettingsConnectionProps) => {
  const { t } = useTranslation("user-federation");
-  const helpText = useTranslation("user-federation-help").t;
+  const { t: helpText } = useTranslation("user-federation-help");
+  const adminClient = useAdminClient();
+  const { realm } = useRealm();
+  const { addAlert } = useAlerts();
+
+  const testLdap = async () => {
+    try {
+      const settings: TestLdapConnectionRepresentation = {};
+
+      testLdapProperties.forEach((key) => {
+        const value = _.get(form.getValues(), `config.${key}`);
+        settings[key] = _.isArray(value) ? value[0] : "";
+      });
+      await adminClient.realms.testLDAPConnection(
+        { realm },
+        { ...settings, action: "testConnection" }
+      );
+      addAlert(t("testSuccess"), AlertVariant.success);
+    } catch (error) {
+      addAlert(t("testError"), AlertVariant.danger);
+      console.error(error.response?.data?.errorMessage);
+    }
+  };

  const [
    isTruststoreSpiDropdownOpen,
@ -36,6 +76,11 @@ export const LdapSettingsConnection = ({

  const [isBindTypeDropdownOpen, setIsBindTypeDropdownOpen] = useState(false);

+  const ldapBindType = useWatch({
+    control: form.control,
+    name: "config.authType",
+  });
+
  return (
    <>
      {showSectionHeading && (
@ -213,7 +258,7 @@ export const LdapSettingsConnection = ({
        >
          <Controller
            name="config.authType[0]"
-            defaultValue=""
+            defaultValue="none"
            control={form.control}
            render={({ onChange, value }) => (
              <Select
@ -231,12 +276,15 @@ export const LdapSettingsConnection = ({
                variant={SelectVariant.single}
                data-testid="ldap-bind-type"
              >
-                <SelectOption key={3} value="simple" />
-                <SelectOption key={4} value="none" />
+                <SelectOption key={0} value="simple" />
+                <SelectOption key={1} value="none" isPlaceholder />
              </Select>
            )}
          ></Controller>
        </FormGroup>
+
+        {_.isEqual(ldapBindType, ["simple"]) && (
+          <>
            <FormGroup
              label={t("bindDn")}
              labelIcon={
@ -247,6 +295,12 @@ export const LdapSettingsConnection = ({
                />
              }
              fieldId="kc-console-bind-dn"
+              helperTextInvalid={t("validateBindDn")}
+              validated={
+                form.errors.config?.bindDn
+                  ? ValidatedOptions.error
+                  : ValidatedOptions.default
+              }
              isRequired
            >
              <TextInput
@ -254,20 +308,8 @@ export const LdapSettingsConnection = ({
                id="kc-console-bind-dn"
                data-testid="ldap-bind-dn"
                name="config.bindDn[0]"
-            ref={form.register({
-              required: {
-                value: true,
-                message: `${t("validateBindDn")}`,
-              },
-            })}
+                ref={form.register({ required: true })}
              />
-          {form.errors.config &&
-            form.errors.config.bindDn &&
-            form.errors.config.bindDn[0] && (
-              <div className="error">
-                {form.errors.config.bindDn[0].message}
-              </div>
-            )}
            </FormGroup>
            <FormGroup
              label={t("bindCredentials")}
@ -279,6 +321,12 @@ export const LdapSettingsConnection = ({
                />
              }
              fieldId="kc-console-bind-credentials"
+              helperTextInvalid={t("validateBindCredentials")}
+              validated={
+                form.errors.config?.bindCredential
+                  ? ValidatedOptions.error
+                  : ValidatedOptions.default
+              }
              isRequired
            >
              <PasswordInput
@ -287,27 +335,22 @@ export const LdapSettingsConnection = ({
                data-testid="ldap-bind-credentials"
                name="config.bindCredential[0]"
                ref={form.register({
-              required: {
-                value: true,
-                message: `${t("validateBindCredentials")}`,
-              },
+                  required: true,
                })}
              />
-          {form.errors.config &&
-            form.errors.config.bindCredential &&
-            form.errors.config.bindCredential[0] && (
-              <div className="error">
-                {form.errors.config.bindCredential[0].message}
-              </div>
-            )}
            </FormGroup>
            <FormGroup fieldId="kc-test-button">
-          {" "}
-          {/* TODO: whatever this button is supposed to do */}
-          <Button variant="secondary" id="kc-test-button">
+              <Button
+                isDisabled={!form.formState.isValid}
+                variant="secondary"
+                id="kc-test-button"
+                onClick={() => testLdap()}
+              >
                {t("common:test")}
              </Button>
            </FormGroup>
+          </>
+        )}
      </FormAccess>
    </>
  );
--- a/src/user-federation/ldap/LdapSettingsSynchronization.tsx
+++ b/src/user-federation/ldap/LdapSettingsSynchronization.tsx
@ -20,6 +20,9 @@ export const LdapSettingsSynchronization = ({
  const { t } = useTranslation("user-federation");
  const helpText = useTranslation("user-federation-help").t;

+  const watchPeriodicSync = form.watch("config.periodicFullSync", false);
+  const watchChangedSync = form.watch("config.periodicChangedUsersSync", false);
+
  return (
    <>
      {showSectionHeading && (
@ -78,8 +81,36 @@ export const LdapSettingsSynchronization = ({
            ref={form.register}
          />
        </FormGroup>
-
-        {/* Enter -1 to switch off, otherwise enter value */}
+        <FormGroup
+          label={"Periodic full sync"}
+          labelIcon={
+            <HelpItem
+              helpText={helpText("periodicFullSyncHelp")}
+              forLabel={"periodicFullSync"}
+              forID="kc-periodic-full-sync"
+            />
+          }
+          fieldId="kc-periodic-full-sync"
+          hasNoPaddingTop
+        >
+          <Controller
+            name="config.periodicFullSync"
+            defaultValue={false}
+            control={form.control}
+            render={({ onChange, value }) => (
+              <Switch
+                id={"kc-periodic-full-sync"}
+                isDisabled={false}
+                onChange={(value) => onChange(value)}
+                isChecked={value === true}
+                label={t("common:on")}
+                labelOff={t("common:off")}
+                ref={form.register}
+              />
+            )}
+          ></Controller>
+        </FormGroup>
+        {watchPeriodicSync && (
          <FormGroup
            hasNoPaddingTop
            label={t("fullSyncPeriod")}
@ -95,13 +126,43 @@ export const LdapSettingsSynchronization = ({
            <TextInput
              type="number"
              min={-1}
+              defaultValue={604800}
              id="kc-full-sync-period"
              name="config.fullSyncPeriod[0]"
              ref={form.register}
            />
          </FormGroup>
-
-        {/* Enter -1 to switch off, otherwise enter value */}
+        )}
+        <FormGroup
+          label={"Periodic Changed Users Sync"}
+          labelIcon={
+            <HelpItem
+              helpText={helpText("periodicChangedUsersSyncHelp")}
+              forLabel={"periodicChangedUsersSync"}
+              forID="kc-periodic-changed-users-sync"
+            />
+          }
+          fieldId="kc-periodic-changed-users-sync"
+          hasNoPaddingTop
+        >
+          <Controller
+            name="config.periodicChangedUsersSync"
+            defaultValue={false}
+            control={form.control}
+            render={({ onChange, value }) => (
+              <Switch
+                id={"kc-periodic-changed-users-sync"}
+                isDisabled={false}
+                onChange={(value) => onChange(value)}
+                isChecked={value === true}
+                label={t("common:on")}
+                labelOff={t("common:off")}
+                ref={form.register}
+              />
+            )}
+          ></Controller>
+        </FormGroup>
+        {watchChangedSync && (
          <FormGroup
            label={t("changedUsersSyncPeriod")}
            labelIcon={
@ -117,11 +178,13 @@ export const LdapSettingsSynchronization = ({
            <TextInput
              type="number"
              min={-1}
+              defaultValue={86400}
              id="kc-changed-users-sync-period"
              name="config.changedSyncPeriod[0]"
              ref={form.register}
            />
          </FormGroup>
+        )}
      </FormAccess>
    </>
  );
--- a/src/user-federation/messages.json
+++ b/src/user-federation/messages.json
@ -81,9 +81,11 @@
    "subtree": "Subtree",

    "saveSuccess": "User federation provider successfully saved",
-    "saveError": "User federation provider could not be saved: {error}",
+    "saveError": "User federation provider could not be saved: {{error}}",
    "createSuccess": "User federation provider successfully created",
-    "createError": "User federation provider could not be created: {error}",
+    "createError": "User federation provider could not be created: {{error}}",
+    "testSuccess": "Successfully connected to LDAP",
+    "testError": "Error when trying to connect to LDAP. See server.log for details.",

    "learnMore": "Learn more",
    "addNewProvider": "Add new provider",