Add frontend validation for group IP restriction field

Add frontend validation that matches the backend validation to improve
UX

ee/app/assets/javascripts/geo_settings/validations.js

-import ipaddr from 'ipaddr.js';
+import validateIpAddress from 'ee/validators/ip_address';
 import { s__ } from '~/locale';
 
-const validateAddress = address => {
-  try {
-    // Checks if Valid IPv4/IPv6 (CIDR) - Throws if not
-    return Boolean(ipaddr.parseCIDR(address));
-  } catch (e) {
-    // Checks if Valid IPv4/IPv6 (Non-CIDR) - Does not Throw
-    return ipaddr.isValid(address);
-  }
-};
-
 const validateIP = data => {
   let addresses = data.replace(/\s/g, '').split(',');
 
-  addresses = addresses.map(address => validateAddress(address));
+  addresses = addresses.map(address => validateIpAddress(address));
 
   return !addresses.some(a => !a);
 };

ee/app/assets/javascripts/groups/settings/access_restriction_field/validate_ip_address.js

+import validateIpAddress from 'ee/validators/ip_address';
+import { escape } from 'lodash';
+import { __, sprintf } from '~/locale';
+
+export default address => {
+  // Reject IP addresses that are only integers to match Ruby IPAddr
+  // https://github.com/whitequark/ipaddr.js/issues/7#issuecomment-158545695
+  if (/^\d+$/.exec(address) || !validateIpAddress(address)) {
+    return sprintf(
+      __('%{address} is an invalid IP address range'),
+      { address: escape(address) },
+      false,
+    );
+  }
+
+  return '';
+};

ee/app/assets/javascripts/groups/settings/components/comma_separated_list_token_selector.vue

@@ -30,6 +30,11 @@ export default {
       required: false,
       default: () => [],
     },
+    customValidator: {
+      type: Function,
+      required: false,
+      default: () => '',
+    },
     errorMessage: {
       type: String,
       required: false,
@@ -61,7 +66,7 @@ export default {
         return this.disallowedValueErrorMessage;
       }
 
-      return '';
+      return this.customValidator(this.textInputValue);
     },
   },
   watch: {

ee/app/assets/javascripts/pages/groups/edit/index.js

 import '~/pages/groups/edit';
 import initAccessRestrictionField from 'ee/groups/settings/access_restriction_field';
+import validateRestrictedIpAddress from 'ee/groups/settings/access_restriction_field/validate_ip_address';
 import { __ } from '~/locale';
 
 document.addEventListener('DOMContentLoaded', () => {
@@ -10,7 +11,7 @@ document.addEventListener('DOMContentLoaded', () => {
   });
   initAccessRestrictionField(
     '.js-ip-restriction',
-    { placeholder: __('Enter IP address range') },
+    { placeholder: __('Enter IP address range'), customValidator: validateRestrictedIpAddress },
     'ip_restriction_field',
   );
 });

ee/app/assets/javascripts/validators/ip_address.js

+import ipaddr from 'ipaddr.js';
+
+export default address => {
+  try {
+    // Checks if Valid IPv4/IPv6 (CIDR) - Throws if not
+    return Boolean(ipaddr.parseCIDR(address));
+  } catch (e) {
+    // Checks if Valid IPv4/IPv6 (Non-CIDR) - Does not Throw
+    return ipaddr.isValid(address);
+  }
+};

ee/changelogs/unreleased/227442-add-frontend-validation-to-restrict-access-by-ip-address-field-.yml

+---
+title: Add frontend validation to "Restrict access by IP address" field
+merge_request: 39061
+author:
+type: changed

ee/spec/frontend/groups/settings/access_restriction_field/validate_ip_address_spec.js

+import * as validateIpAddress from 'ee/validators/ip_address';
+import validateRestrictedIpAddress from 'ee/groups/settings/access_restriction_field/validate_ip_address';
+
+describe('validateRestrictedIpAddress', () => {
+  describe('when IP address is only integers', () => {
+    it.each`
+      address
+      ${1}
+      ${19}
+      ${192}
+    `('$address - returns an error message', ({ address }) => {
+      expect(validateRestrictedIpAddress(address)).toBe(
+        `${address} is an invalid IP address range`,
+      );
+    });
+  });
+
+  describe('when `validateIpAddress` returns false', () => {
+    it('returns an error message', () => {
+      validateIpAddress.default = jest.fn(() => false);
+
+      expect(validateRestrictedIpAddress('foo bar')).toBe(`foo bar is an invalid IP address range`);
+    });
+  });
+
+  describe('when IP address is valid', () => {
+    it('returns an empty string', () => {
+      validateIpAddress.default = jest.fn(() => true);
+
+      expect(validateRestrictedIpAddress('192.168.0.0/24')).toBe('');
+    });
+  });
+});

ee/spec/frontend/groups/settings/components/comma_separated_list_token_selector_spec.js

@@ -225,29 +225,40 @@ describe('CommaSeparatedListTokenSelector', () => {
 
       tokenSelectorTriggerEnter();
 
-      expect(
-        findTokenSelector()
-          .find('[role="menuitem"]')
-          .text(),
-      ).toBe('Add "foo"');
+      expect(findTokenSelectorDropdown().text()).toBe('Add "foo"');
     });
   });
 
   describe('when `regexValidator` and `disallowedValues` props are not set', () => {
-    it('allows any value to be added', async () => {
-      createComponent({
-        scopedSlots: {
-          'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
-        },
+    describe('when `customValidator` prop is not set', () => {
+      it('allows any value to be added', async () => {
+        createComponent({
+          scopedSlots: {
+            'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
+          },
+        });
+
+        await setTokenSelectorInputValue('foo');
+
+        expect(findTokenSelectorDropdown().text()).toBe('Add "foo"');
       });
+    });
 
-      await setTokenSelectorInputValue('foo');
+    describe('when `customValidator` prop is set', () => {
+      it('displays error message that is returned by `customValidator`', () => {
+        createComponent({
+          propsData: {
+            customValidator: () => 'Value is invalid',
+          },
+          scopedSlots: {
+            'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
+          },
+        });
 
-      expect(
-        findTokenSelector()
-          .find('[role="menuitem"]')
-          .text(),
-      ).toBe('Add "foo"');
+        tokenSelectorTriggerEnter();
+
+        expect(findErrorMessageText()).toBe('Value is invalid');
+      });
     });
   });
 

ee/spec/frontend/validators/ip_address_spec.js

+import ipaddr from 'ipaddr.js';
+import validateIpAddress from 'ee/validators/ip_address';
+
+describe('validateIpAddress', () => {
+  describe('when IP address is in valid CIDR format', () => {
+    it('returns true', () => {
+      ipaddr.parseCIDR = jest.fn(() => [
+        {
+          octets: [192, 168, 0, 0],
+        },
+        24,
+      ]);
+
+      expect(validateIpAddress('192.168.0.0/24')).toBe(true);
+    });
+  });
+
+  describe('when IP address is not in valid CIDR format', () => {
+    it('calls `ipaddr.isValid`', () => {
+      ipaddr.parseCIDR = jest.fn(() => {
+        throw new Error();
+      });
+
+      ipaddr.isValid = jest.fn();
+
+      validateIpAddress('192.168.0.0');
+
+      expect(ipaddr.isValid).toHaveBeenCalledWith('192.168.0.0');
+    });
+  });
+});

locale/gitlab.pot

@@ -297,6 +297,9 @@ msgstr[1] ""
 msgid "%{actionText} & %{openOrClose} %{noteable}"
 msgstr ""
 
+msgid "%{address} is an invalid IP address range"
+msgstr ""
+
 msgid "%{author_link} wrote:"
 msgstr ""