From 78bdf2d2ecfd706cecbc4387afefc3197c73caa9 Mon Sep 17 00:00:00 2001
From: Erik Jan de Wit <erikjan.dewit@gmail.com>
Date: Tue, 29 Mar 2022 09:49:43 +0200
Subject: [PATCH] don't test ldap when form isn't valid (#2294)

* don't test ldap when form isn't valid

fixes: #1776

* changed to PF way of showing errors

* Update src/user-federation/kerberos/KerberosSettingsRequired.tsx

Co-authored-by: Jenny <32821331+jenny-s51@users.noreply.github.com>

* Update src/user-federation/kerberos/KerberosSettingsRequired.tsx

Co-authored-by: Jenny <32821331+jenny-s51@users.noreply.github.com>

* Update src/user-federation/kerberos/KerberosSettingsRequired.tsx

Co-authored-by: Jenny <32821331+jenny-s51@users.noreply.github.com>

Co-authored-by: Jenny <32821331+jenny-s51@users.noreply.github.com>
---
 .../attribute/AttributeGeneralSettings.tsx    |  6 +-
 .../kerberos/KerberosSettingsRequired.tsx     | 36 ++++----
 .../ldap/LdapSettingsConnection.tsx           | 23 +++--
 .../ldap/LdapSettingsGeneral.tsx              |  6 +-
 .../ldap/LdapSettingsKerberosIntegration.tsx  | 36 ++++----
 .../ldap/LdapSettingsSearching.tsx            | 86 ++++++++++++-------
 src/user-federation/user-federation.css       |  4 -
 7 files changed, 118 insertions(+), 79 deletions(-)

diff --git a/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx b/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
index fefb77d513..9b5b0423a1 100644
--- a/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
+++ b/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
@@ -65,6 +65,8 @@ export const AttributeGeneralSettings = () => {
         }
         fieldId="kc-attribute-name"
         isRequired
+        validated={form.errors.name ? "error" : "default"}
+        helperTextInvalid={form.errors.name?.message}
       >
         <TextInput
           isRequired
@@ -79,10 +81,8 @@ export const AttributeGeneralSettings = () => {
             },
           })}
           data-testid="attribute-name"
+          validated={form.errors.name ? "error" : "default"}
         />
-        {form.errors.name && (
-          <div className="error">{form.errors.name.message}</div>
-        )}
       </FormGroup>
       <FormGroup
         label={t("attributeDisplayName")}
diff --git a/src/user-federation/kerberos/KerberosSettingsRequired.tsx b/src/user-federation/kerberos/KerberosSettingsRequired.tsx
index 524c0e08f4..f029f657ac 100644
--- a/src/user-federation/kerberos/KerberosSettingsRequired.tsx
+++ b/src/user-federation/kerberos/KerberosSettingsRequired.tsx
@@ -70,6 +70,8 @@ export const KerberosSettingsRequired = ({
           }
           fieldId="kc-console-display-name"
           isRequired
+          validated={form.errors.name ? "error" : "default"}
+          helperTextInvalid={form.errors.name?.message}
         >
           {/* These hidden fields are required so data object written back matches data retrieved */}
           <TextInput
@@ -109,10 +111,8 @@ export const KerberosSettingsRequired = ({
               },
             })}
             data-testid="kerberos-name"
+            validated={form.errors.name ? "error" : "default"}
           />
-          {form.errors.name && (
-            <div className="error">{form.errors.name.message}</div>
-          )}
         </FormGroup>
 
         <FormGroup
@@ -125,6 +125,10 @@ export const KerberosSettingsRequired = ({
           }
           fieldId="kc-kerberos-realm"
           isRequired
+          validated={
+            form.errors.config?.kerberosRealm?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.kerberosRealm?.[0].message}
         >
           <TextInput
             isRequired
@@ -138,12 +142,10 @@ export const KerberosSettingsRequired = ({
               },
             })}
             data-testid="kerberos-realm"
+            validated={
+              form.errors.config?.kerberosRealm?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.kerberosRealm?.[0] && (
-            <div className="error">
-              {form.errors.config.kerberosRealm[0].message}
-            </div>
-          )}
         </FormGroup>
 
         <FormGroup
@@ -156,6 +158,10 @@ export const KerberosSettingsRequired = ({
           }
           fieldId="kc-server-principal"
           isRequired
+          validated={
+            form.errors.config?.serverPrincipal?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.serverPrincipal?.[0].message}
         >
           <TextInput
             isRequired
@@ -169,12 +175,10 @@ export const KerberosSettingsRequired = ({
               },
             })}
             data-testid="kerberos-principal"
+            validated={
+              form.errors.config?.serverPrincipal?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.serverPrincipal?.[0] && (
-            <div className="error">
-              {form.errors.config.serverPrincipal[0].message}
-            </div>
-          )}
         </FormGroup>
 
         <FormGroup
@@ -187,6 +191,8 @@ export const KerberosSettingsRequired = ({
           }
           fieldId="kc-key-tab"
           isRequired
+          validated={form.errors.config?.keyTab?.[0] ? "error" : "default"}
+          helperTextInvalid={form.errors.config?.keyTab?.[0].message}
         >
           <TextInput
             isRequired
@@ -200,10 +206,8 @@ export const KerberosSettingsRequired = ({
               },
             })}
             data-testid="kerberos-keytab"
+            validated={form.errors.config?.keyTab?.[0] ? "error" : "default"}
           />
-          {form.errors.config?.keyTab?.[0] && (
-            <div className="error">{form.errors.config.keyTab[0].message}</div>
-          )}
         </FormGroup>
 
         <FormGroup
diff --git a/src/user-federation/ldap/LdapSettingsConnection.tsx b/src/user-federation/ldap/LdapSettingsConnection.tsx
index c70126b22e..2277c7b858 100644
--- a/src/user-federation/ldap/LdapSettingsConnection.tsx
+++ b/src/user-federation/ldap/LdapSettingsConnection.tsx
@@ -68,6 +68,7 @@ export const LdapSettingsConnection = ({
   const { addAlert, addError } = useAlerts();
 
   const testLdap = async (testType: TestTypes) => {
+    if (!(await form.trigger())) return;
     try {
       const settings = convertFormToSettings(form);
       await adminClient.realms.testLDAPConnection(
@@ -113,6 +114,10 @@ export const LdapSettingsConnection = ({
           }
           fieldId="kc-console-connection-url"
           isRequired
+          validated={
+            form.errors.config?.connectionUrl?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.connectionUrl?.[0].message}
         >
           <TextInput
             isRequired
@@ -126,12 +131,10 @@ export const LdapSettingsConnection = ({
                 message: `${t("validateConnectionUrl")}`,
               },
             })}
+            validated={
+              form.errors.config?.connectionUrl?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.connectionUrl?.[0] && (
-            <div className="error">
-              {form.errors.config.connectionUrl[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("enableStartTls")}
@@ -312,6 +315,11 @@ export const LdapSettingsConnection = ({
                 data-testid="ldap-bind-dn"
                 name="config.bindDn[0]"
                 ref={form.register({ required: true })}
+                validated={
+                  form.errors.config?.bindDn
+                    ? ValidatedOptions.error
+                    : ValidatedOptions.default
+                }
               />
             </FormGroup>
             <FormGroup
@@ -340,6 +348,11 @@ export const LdapSettingsConnection = ({
                 ref={form.register({
                   required: true,
                 })}
+                validated={
+                  form.errors.config?.bindCredential
+                    ? ValidatedOptions.error
+                    : ValidatedOptions.default
+                }
               />
             </FormGroup>
           </>
diff --git a/src/user-federation/ldap/LdapSettingsGeneral.tsx b/src/user-federation/ldap/LdapSettingsGeneral.tsx
index 13c331b2f0..68a8726d55 100644
--- a/src/user-federation/ldap/LdapSettingsGeneral.tsx
+++ b/src/user-federation/ldap/LdapSettingsGeneral.tsx
@@ -113,6 +113,8 @@ export const LdapSettingsGeneral = ({
           }
           fieldId="kc-console-display-name"
           isRequired
+          validated={form.errors.name ? "error" : "default"}
+          helperTextInvalid={form.errors.name?.message}
         >
           {/* These hidden fields are required so data object written back matches data retrieved */}
           <TextInput hidden type="text" id="kc-console-id" name="id" />
@@ -153,10 +155,8 @@ export const LdapSettingsGeneral = ({
               },
             })}
             data-testid="ldap-name"
+            validated={form.errors.name ? "error" : "default"}
           />
-          {form.errors.name && (
-            <div className="error">{form.errors.name.message}</div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("vendor")}
diff --git a/src/user-federation/ldap/LdapSettingsKerberosIntegration.tsx b/src/user-federation/ldap/LdapSettingsKerberosIntegration.tsx
index b9ad751677..4e4a744c85 100644
--- a/src/user-federation/ldap/LdapSettingsKerberosIntegration.tsx
+++ b/src/user-federation/ldap/LdapSettingsKerberosIntegration.tsx
@@ -77,6 +77,10 @@ export const LdapSettingsKerberosIntegration = ({
               }
               fieldId="kc-kerberos-realm"
               isRequired
+              validated={
+                form.errors.config?.kerberosRealm?.[0] ? "error" : "default"
+              }
+              helperTextInvalid={form.errors.config?.kerberosRealm?.[0].message}
             >
               <TextInput
                 isRequired
@@ -90,12 +94,10 @@ export const LdapSettingsKerberosIntegration = ({
                   },
                 })}
                 data-testid="kerberos-realm"
+                validated={
+                  form.errors.config?.kerberosRealm?.[0] ? "error" : "default"
+                }
               />
-              {form.errors.config?.kerberosRealm?.[0] && (
-                <div className="error">
-                  {form.errors.config.kerberosRealm[0].message}
-                </div>
-              )}
             </FormGroup>
 
             <FormGroup
@@ -108,6 +110,12 @@ export const LdapSettingsKerberosIntegration = ({
               }
               fieldId="kc-server-principal"
               isRequired
+              validated={
+                form.errors.config?.serverPrincipal?.[0] ? "error" : "default"
+              }
+              helperTextInvalid={
+                form.errors.config?.serverPrincipal?.[0].message
+              }
             >
               <TextInput
                 isRequired
@@ -121,12 +129,10 @@ export const LdapSettingsKerberosIntegration = ({
                   },
                 })}
                 data-testid="kerberos-principal"
+                validated={
+                  form.errors.config?.serverPrincipal?.[0] ? "error" : "default"
+                }
               />
-              {form.errors.config?.serverPrincipal?.[0] && (
-                <div className="error">
-                  {form.errors.config.serverPrincipal[0].message}
-                </div>
-              )}
             </FormGroup>
 
             <FormGroup
@@ -139,6 +145,8 @@ export const LdapSettingsKerberosIntegration = ({
               }
               fieldId="kc-key-tab"
               isRequired
+              validated={form.errors.config?.keyTab?.[0] ? "error" : "default"}
+              helperTextInvalid={form.errors.config?.keyTab?.[0].message}
             >
               <TextInput
                 isRequired
@@ -152,12 +160,10 @@ export const LdapSettingsKerberosIntegration = ({
                   },
                 })}
                 data-testid="kerberos-keytab"
+                validated={
+                  form.errors.config?.keyTab?.[0] ? "error" : "default"
+                }
               />
-              {form.errors.config?.keyTab?.[0] && (
-                <div className="error">
-                  {form.errors.config.keyTab[0].message}
-                </div>
-              )}
             </FormGroup>
 
             <FormGroup
diff --git a/src/user-federation/ldap/LdapSettingsSearching.tsx b/src/user-federation/ldap/LdapSettingsSearching.tsx
index 7a330b2846..9d44ff4998 100644
--- a/src/user-federation/ldap/LdapSettingsSearching.tsx
+++ b/src/user-federation/ldap/LdapSettingsSearching.tsx
@@ -52,6 +52,8 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-edit-mode"
           isRequired
+          validated={form.errors.config?.editMode?.[0] ? "error" : "default"}
+          helperTextInvalid={form.errors.config?.editMode?.[0].message}
         >
           <Controller
             name="config.editMode[0]"
@@ -74,6 +76,9 @@ export const LdapSettingsSearching = ({
                 }}
                 selections={value}
                 variant={SelectVariant.single}
+                validated={
+                  form.errors.config?.editMode?.[0] ? "error" : "default"
+                }
               >
                 <SelectOption value="" isPlaceholder />
                 <SelectOption value="READ_ONLY" />
@@ -82,11 +87,6 @@ export const LdapSettingsSearching = ({
               </Select>
             )}
           />
-          {form.errors.config?.editMode?.[0] && (
-            <div className="error">
-              {form.errors.config.editMode[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("usersDN")}
@@ -98,6 +98,8 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-console-users-dn"
           isRequired
+          validated={form.errors.config?.usersDn?.[0] ? "error" : "default"}
+          helperTextInvalid={form.errors.config?.usersDn?.[0].message}
         >
           <TextInput
             isRequired
@@ -112,10 +114,8 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateUsersDn")}`,
               },
             })}
+            validated={form.errors.config?.usersDn?.[0] ? "error" : "default"}
           />
-          {form.errors.config?.usersDn?.[0] && (
-            <div className="error">{form.errors.config.usersDn[0].message}</div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("usernameLdapAttribute")}
@@ -127,6 +127,12 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-username-ldap-attribute"
           isRequired
+          validated={
+            form.errors.config?.usernameLDAPAttribute?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={
+            form.errors.config?.usernameLDAPAttribute?.[0].message
+          }
         >
           <TextInput
             isRequired
@@ -141,12 +147,12 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateUsernameLDAPAttribute")}`,
               },
             })}
+            validated={
+              form.errors.config?.usernameLDAPAttribute?.[0]
+                ? "error"
+                : "default"
+            }
           />
-          {form.errors.config?.usernameLDAPAttribute?.[0] && (
-            <div className="error">
-              {form.errors.config.usernameLDAPAttribute[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("rdnLdapAttribute")}
@@ -158,6 +164,10 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-rdn-ldap-attribute"
           isRequired
+          validated={
+            form.errors.config?.rdnLDAPAttribute?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.rdnLDAPAttribute?.[0].message}
         >
           <TextInput
             isRequired
@@ -172,12 +182,10 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateRdnLdapAttribute")}`,
               },
             })}
+            validated={
+              form.errors.config?.rdnLDAPAttribute?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.rdnLDAPAttribute?.[0] && (
-            <div className="error">
-              {form.errors.config.rdnLDAPAttribute[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("uuidLdapAttribute")}
@@ -189,6 +197,10 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-uuid-ldap-attribute"
           isRequired
+          validated={
+            form.errors.config?.uuidLDAPAttribute?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.uuidLDAPAttribute?.[0].message}
         >
           <TextInput
             isRequired
@@ -203,12 +215,10 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateUuidLDAPAttribute")}`,
               },
             })}
+            validated={
+              form.errors.config?.uuidLDAPAttribute?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.uuidLDAPAttribute?.[0] && (
-            <div className="error">
-              {form.errors.config.uuidLDAPAttribute[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("userObjectClasses")}
@@ -220,6 +230,10 @@ export const LdapSettingsSearching = ({
           }
           fieldId="kc-user-object-classes"
           isRequired
+          validated={
+            form.errors.config?.userObjectClasses?.[0] ? "error" : "default"
+          }
+          helperTextInvalid={form.errors.config?.userObjectClasses?.[0].message}
         >
           <TextInput
             isRequired
@@ -234,12 +248,10 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateUserObjectClasses")}`,
               },
             })}
+            validated={
+              form.errors.config?.userObjectClasses?.[0] ? "error" : "default"
+            }
           />
-          {form.errors.config?.userObjectClasses?.[0] && (
-            <div className="error">
-              {form.errors.config.userObjectClasses[0].message}
-            </div>
-          )}
         </FormGroup>
         <FormGroup
           label={t("userLdapFilter")}
@@ -250,6 +262,14 @@ export const LdapSettingsSearching = ({
             />
           }
           fieldId="kc-user-ldap-filter"
+          validated={
+            form.errors.config?.customUserSearchFilter?.[0]
+              ? "error"
+              : "default"
+          }
+          helperTextInvalid={
+            form.errors.config?.customUserSearchFilter?.[0].message
+          }
         >
           <TextInput
             type="text"
@@ -261,12 +281,12 @@ export const LdapSettingsSearching = ({
                 message: `${t("validateCustomUserSearchFilter")}`,
               },
             })}
+            validated={
+              form.errors.config?.customUserSearchFilter?.[0]
+                ? "error"
+                : "default"
+            }
           />
-          {form.errors.config?.customUserSearchFilter?.[0] && (
-            <div className="error">
-              {form.errors.config.customUserSearchFilter[0].message}
-            </div>
-          )}
         </FormGroup>
 
         <FormGroup
diff --git a/src/user-federation/user-federation.css b/src/user-federation/user-federation.css
index de9d8896c9..22165b9a80 100644
--- a/src/user-federation/user-federation.css
+++ b/src/user-federation/user-federation.css
@@ -6,10 +6,6 @@
   display: contents;
 }
 
-.error {
-  color: red;
-}
-
 .keycloak__user-federation__assign-role {
   display: flex;
   flex-direction: row;