refs 123281: imprv form validation

apps/app/src/components/Admin/UserGroup/ExternalGroup/LDAPGroupSyncSettingsForm.tsx

@@ -31,7 +31,8 @@ export const LDAPGroupSyncSettingsForm: FC = () => {
     }
   }, [ldapGroupSyncSettings, setFormValues]);
 
-  const submitHandler = useCallback(async() => {
+  const submitHandler = useCallback(async(e) => {
+    e.preventDefault();
     try {
       await apiv3Put('/external-user-groups/ldap/sync-settings', formValues);
       toastSuccess(t('external_group.ldap.updated_group_sync_settings'));
@@ -43,185 +44,190 @@ export const LDAPGroupSyncSettingsForm: FC = () => {
 
   return <>
     <h3 className="border-bottom">{t('external_group.ldap.group_sync_settings')}</h3>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupsDN" className="text-left text-md-right col-md-3 col-form-label">{t('external_group.ldap.group_search_base_DN')}</label>
-      <div className="col-md-6">
-        <input
-          className="form-control"
-          type="text"
-          name="ldapGroupsDN"
-          id="ldapGroupsDN"
-          value={formValues.ldapGroupsDN}
-          onChange={e => setFormValues({ ...formValues, ldapGroupsDN: e.target.value })}
-        />
-        <p className="form-text text-muted">
-          <small>{t('external_group.ldap.group_search_base_dn_detail')}</small>
-        </p>
+    <form onSubmit={submitHandler}>
+      <div className="row form-group">
+        <label htmlFor="ldapGroupsDN" className="text-left text-md-right col-md-3 col-form-label">{t('external_group.ldap.group_search_base_DN')}</label>
+        <div className="col-md-6">
+          <input
+            className="form-control"
+            required
+            type="text"
+            name="ldapGroupsDN"
+            id="ldapGroupsDN"
+            value={formValues.ldapGroupsDN}
+            onChange={e => setFormValues({ ...formValues, ldapGroupsDN: e.target.value })}
+          />
+          <p className="form-text text-muted">
+            <small>{t('external_group.ldap.group_search_base_dn_detail')}</small>
+          </p>
+        </div>
       </div>
-    </div>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupMembershipAttribute" className="text-left text-md-right col-md-3 col-form-label">
-        {t('external_group.ldap.membership_attribute')}
-      </label>
-      <div className="col-md-6">
-        <input
-          className="form-control"
-          type="text"
-          name="ldapGroupMembershipAttribute"
-          id="ldapGroupMembershipAttribute"
-          value={formValues.ldapGroupMembershipAttribute}
-          onChange={e => setFormValues({ ...formValues, ldapGroupMembershipAttribute: e.target.value })}
-        />
-        <p className="form-text text-muted">
-          <small>
-            {t('external_group.ldap.membership_attribute_detail')} <br />
+      <div className="row form-group">
+        <label htmlFor="ldapGroupMembershipAttribute" className="text-left text-md-right col-md-3 col-form-label">
+          {t('external_group.ldap.membership_attribute')}
+        </label>
+        <div className="col-md-6">
+          <input
+            className="form-control"
+            required
+            type="text"
+            name="ldapGroupMembershipAttribute"
+            id="ldapGroupMembershipAttribute"
+            value={formValues.ldapGroupMembershipAttribute}
+            onChange={e => setFormValues({ ...formValues, ldapGroupMembershipAttribute: e.target.value })}
+          />
+          <p className="form-text text-muted">
+            <small>
+              {t('external_group.ldap.membership_attribute_detail')} <br />
             e.g.) <code>member</code>, <code>memberUid</code>
-          </small>
-        </p>
+            </small>
+          </p>
+        </div>
       </div>
-    </div>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupMembershipAttributeType" className="text-left text-md-right col-md-3 col-form-label">
-        {t('external_group.ldap.membership_attribute_type')}
-      </label>
-      <div className="col-md-6">
-        <select
-          className="form-control"
-          name="ldapGroupMembershipAttributeType"
-          id="ldapGroupMembershipAttributeType"
-          value={formValues.ldapGroupMembershipAttributeType}
-          onChange={e => setFormValues({ ...formValues, ldapGroupMembershipAttributeType: e.target.value })}>
-          <option value="DN">DN</option>
-          <option value="UID">UID</option>
-        </select>
-        <p className="form-text text-muted">
-          <small>
-            {t('external_group.ldap.membership_attribute_type_detail')}
-          </small>
-        </p>
+      <div className="row form-group">
+        <label htmlFor="ldapGroupMembershipAttributeType" className="text-left text-md-right col-md-3 col-form-label">
+          {t('external_group.ldap.membership_attribute_type')}
+        </label>
+        <div className="col-md-6">
+          <select
+            className="form-control"
+            required
+            name="ldapGroupMembershipAttributeType"
+            id="ldapGroupMembershipAttributeType"
+            value={formValues.ldapGroupMembershipAttributeType}
+            onChange={e => setFormValues({ ...formValues, ldapGroupMembershipAttributeType: e.target.value })}>
+            <option value="DN">DN</option>
+            <option value="UID">UID</option>
+          </select>
+          <p className="form-text text-muted">
+            <small>
+              {t('external_group.ldap.membership_attribute_type_detail')}
+            </small>
+          </p>
+        </div>
       </div>
-    </div>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupChildGroupAttribute" className="text-left text-md-right col-md-3 col-form-label">
-        {t('external_group.ldap.child_group_attribute')}
-      </label>
-      <div className="col-md-6">
-        <input
-          className="form-control"
-          type="text"
-          name="ldapGroupChildGroupAttribute"
-          id="ldapGroupChildGroupAttribute"
-          value={formValues.ldapGroupChildGroupAttribute}
-          onChange={e => setFormValues({ ...formValues, ldapGroupChildGroupAttribute: e.target.value })}/>
-        <p className="form-text text-muted">
-          <small>
-            {t('external_group.ldap.child_group_attribute_detail')}<br />
+      <div className="row form-group">
+        <label htmlFor="ldapGroupChildGroupAttribute" className="text-left text-md-right col-md-3 col-form-label">
+          {t('external_group.ldap.child_group_attribute')}
+        </label>
+        <div className="col-md-6">
+          <input
+            className="form-control"
+            required
+            type="text"
+            name="ldapGroupChildGroupAttribute"
+            id="ldapGroupChildGroupAttribute"
+            value={formValues.ldapGroupChildGroupAttribute}
+            onChange={e => setFormValues({ ...formValues, ldapGroupChildGroupAttribute: e.target.value })}/>
+          <p className="form-text text-muted">
+            <small>
+              {t('external_group.ldap.child_group_attribute_detail')}<br />
             e.g.) <code>member</code>
-          </small>
-        </p>
+            </small>
+          </p>
+        </div>
+      </div>
+      <div className="row form-group">
+        <label
+          className="text-left text-md-right col-md-3 col-form-label"
+        >
+          {/* {t('external_group.ldap.auto_generate_user_on_sync')} */}
+        </label>
+        <div className="col-md-6">
+          <div className="custom-control custom-checkbox custom-checkbox-info">
+            <input
+              type="checkbox"
+              className="custom-control-input"
+              name="autoGenerateUserOnLDAPGroupSync"
+              id="autoGenerateUserOnLDAPGroupSync"
+              checked={formValues.autoGenerateUserOnLDAPGroupSync}
+              onChange={() => setFormValues({ ...formValues, autoGenerateUserOnLDAPGroupSync: !formValues.autoGenerateUserOnLDAPGroupSync })}
+            />
+            <label
+              className="custom-control-label"
+              htmlFor="autoGenerateUserOnLDAPGroupSync"
+            >
+              {t('external_group.ldap.auto_generate_user_on_sync')}
+            </label>
+          </div>
+        </div>
+      </div>
+      <div className="row form-group">
+        <label
+          className="text-left text-md-right col-md-3 col-form-label"
+        >
+          {/* {t('external_group.ldap.preserve_deleted_ldap_groups')} */}
+        </label>
+        <div className="col-md-6">
+          <div className="custom-control custom-checkbox custom-checkbox-info">
+            <input
+              type="checkbox"
+              className="custom-control-input"
+              name="preserveDeletedLDAPGroups"
+              id="preserveDeletedLDAPGroups"
+              checked={formValues.preserveDeletedLDAPGroups}
+              onChange={() => setFormValues({ ...formValues, preserveDeletedLDAPGroups: !formValues.preserveDeletedLDAPGroups })}
+            />
+            <label
+              className="custom-control-label"
+              htmlFor="preserveDeletedLDAPGroups"
+            >
+              {t('external_group.ldap.preserve_deleted_ldap_groups')}
+            </label>
+          </div>
+        </div>
       </div>
-    </div>
-    <div className="row form-group">
-      <label
-        className="text-left text-md-right col-md-3 col-form-label"
-      >
-        {/* {t('external_group.ldap.auto_generate_user_on_sync')} */}
-      </label>
-      <div className="col-md-6">
-        <div className="custom-control custom-checkbox custom-checkbox-info">
+      <h3 className="border-bottom">Attribute Mapping ({t('optional')})</h3>
+      <div className="row form-group">
+        <label htmlFor="ldapGroupNameAttribute" className="text-left text-md-right col-md-3 col-form-label">{t('Name')}</label>
+        <div className="col-md-6">
           <input
-            type="checkbox"
-            className="custom-control-input"
-            name="autoGenerateUserOnLDAPGroupSync"
-            id="autoGenerateUserOnLDAPGroupSync"
-            checked={formValues.autoGenerateUserOnLDAPGroupSync}
-            onChange={() => setFormValues({ ...formValues, autoGenerateUserOnLDAPGroupSync: !formValues.autoGenerateUserOnLDAPGroupSync })}
+            className="form-control"
+            type="text"
+            name="ldapGroupNameAttribute"
+            id="ldapGroupNameAttribute"
+            value={formValues.ldapGroupNameAttribute}
+            onChange={e => setFormValues({ ...formValues, ldapGroupNameAttribute: e.target.value })}
+            placeholder="Default: cn"
           />
-          <label
-            className="custom-control-label"
-            htmlFor="autoGenerateUserOnLDAPGroupSync"
-          >
-            {t('external_group.ldap.auto_generate_user_on_sync')}
-          </label>
+          <p className="form-text text-muted">
+            <small>
+              {t('external_group.ldap.name_mapper_detail')}
+            </small>
+          </p>
         </div>
       </div>
-    </div>
-    <div className="row form-group">
-      <label
-        className="text-left text-md-right col-md-3 col-form-label"
-      >
-        {/* {t('external_group.ldap.preserve_deleted_ldap_groups')} */}
-      </label>
-      <div className="col-md-6">
-        <div className="custom-control custom-checkbox custom-checkbox-info">
+      <div className="row form-group">
+        <label htmlFor="ldapGroupDescriptionAttribute" className="text-left text-md-right col-md-3 col-form-label">
+          {t('Description')}
+        </label>
+        <div className="col-md-6">
           <input
-            type="checkbox"
-            className="custom-control-input"
-            name="preserveDeletedLDAPGroups"
-            id="preserveDeletedLDAPGroups"
-            checked={formValues.preserveDeletedLDAPGroups}
-            onChange={() => setFormValues({ ...formValues, preserveDeletedLDAPGroups: !formValues.preserveDeletedLDAPGroups })}
+            className="form-control"
+            type="text"
+            name="ldapGroupDescriptionAttribute"
+            id="ldapGroupDescriptionAttribute"
+            value={formValues.ldapGroupDescriptionAttribute}
+            onChange={e => setFormValues({ ...formValues, ldapGroupDescriptionAttribute: e.target.value })}
           />
-          <label
-            className="custom-control-label"
-            htmlFor="preserveDeletedLDAPGroups"
-          >
-            {t('external_group.ldap.preserve_deleted_ldap_groups')}
-          </label>
+          <p className="form-text text-muted">
+            <small>
+              {t('external_group.ldap.description_mapper_detail')}
+            </small>
+          </p>
         </div>
       </div>
-    </div>
-    <h3 className="border-bottom">Attribute Mapping ({t('optional')})</h3>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupNameAttribute" className="text-left text-md-right col-md-3 col-form-label">{t('Name')}</label>
-      <div className="col-md-6">
-        <input
-          className="form-control"
-          type="text"
-          name="ldapGroupNameAttribute"
-          id="ldapGroupNameAttribute"
-          value={formValues.ldapGroupNameAttribute}
-          onChange={e => setFormValues({ ...formValues, ldapGroupNameAttribute: e.target.value })}
-          placeholder="Default: cn"
-        />
-        <p className="form-text text-muted">
-          <small>
-            {t('external_group.ldap.name_mapper_detail')}
-          </small>
-        </p>
-      </div>
-    </div>
-    <div className="row form-group">
-      <label htmlFor="ldapGroupDescriptionAttribute" className="text-left text-md-right col-md-3 col-form-label">
-        {t('Description')}
-      </label>
-      <div className="col-md-6">
-        <input
-          className="form-control"
-          type="text"
-          name="ldapGroupDescriptionAttribute"
-          id="ldapGroupDescriptionAttribute"
-          value={formValues.ldapGroupDescriptionAttribute}
-          onChange={e => setFormValues({ ...formValues, ldapGroupDescriptionAttribute: e.target.value })}
-        />
-        <p className="form-text text-muted">
-          <small>
-            {t('external_group.ldap.description_mapper_detail')}
-          </small>
-        </p>
-      </div>
-    </div>
 
-    <div className="row my-3">
-      <div className="offset-3 col-5">
-        <button
-          type="button"
-          className="btn btn-primary"
-          onClick={submitHandler}
-        >
-          {t('Update')}
-        </button>
+      <div className="row my-3">
+        <div className="offset-3 col-5">
+          <button
+            type="submit"
+            className="btn btn-primary"
+          >
+            {t('Update')}
+          </button>
+        </div>
       </div>
-    </div>
+    </form>
   </>;
 };

apps/app/src/server/routes/apiv3/external-user-group.ts

@@ -19,14 +19,14 @@ module.exports = (crowi: Crowi): Router => {
 
   const validators = {
     ldapSyncSettings: [
-      body('ldapGroupsDN').exists().isString(),
-      body('ldapGroupMembershipAttribute').exists().isString(),
-      body('ldapGroupMembershipAttributeType').exists().isString(),
-      body('ldapGroupChildGroupAttribute').exists().isString(),
+      body('ldapGroupsDN').exists({ checkFalsy: true }).isString(),
+      body('ldapGroupMembershipAttribute').exists({ checkFalsy: true }).isString(),
+      body('ldapGroupMembershipAttributeType').exists({ checkFalsy: true }).isString(),
+      body('ldapGroupChildGroupAttribute').exists({ checkFalsy: true }).isString(),
       body('autoGenerateUserOnLDAPGroupSync').exists().isBoolean(),
       body('preserveDeletedLDAPGroups').exists().isBoolean(),
-      body('ldapGroupNameAttribute').exists().isString(),
-      body('ldapGroupDescriptionAttribute').isString(),
+      body('ldapGroupNameAttribute').optional({ nullable: true }).isString(),
+      body('ldapGroupDescriptionAttribute').optional({ nullable: true }).isString(),
     ],
   };
 
@@ -46,7 +46,12 @@ module.exports = (crowi: Crowi): Router => {
   });
 
   router.put('/ldap/sync-settings', loginRequiredStrictly, adminRequired, validators.ldapSyncSettings, async(req: AuthorizedRequest, res: ApiV3Response) => {
-    const requestParams = {
+    const errors = validationResult(req);
+    if (!errors.isEmpty()) {
+      return res.status(400).json({ error: 'invalid body params' });
+    }
+
+    const params = {
       'external-user-group:ldap:groupsDN': req.body.ldapGroupsDN,
       'external-user-group:ldap:groupMembershipAttribute': req.body.ldapGroupMembershipAttribute,
       'external-user-group:ldap:groupMembershipAttributeType': req.body.ldapGroupMembershipAttributeType,
@@ -57,14 +62,14 @@ module.exports = (crowi: Crowi): Router => {
       'external-user-group:ldap:groupDescriptionAttribute': req.body.ldapGroupDescriptionAttribute,
     };
 
-    const errors = validationResult(req);
-    if (!errors.isEmpty()) {
-      return res.apiv3({}, 204);
+    if (params['external-user-group:ldap:groupNameAttribute'] == null || params['external-user-group:ldap:groupNameAttribute'] === '') {
+      // default is cn
+      params['external-user-group:ldap:groupNameAttribute'] = 'cn';
     }
 
     try {
-      await crowi.configManager?.updateConfigsInTheSameNamespace('crowi', requestParams, true);
-      return res.status(204).json();
+      await crowi.configManager?.updateConfigsInTheSameNamespace('crowi', params, true);
+      return res.apiv3({}, 204);
     }
     catch (err) {
       logger.error(err);