Merge branch...

Merge branch '227442-add-frontend-validation-to-restrict-access-by-ip-address-field-in-group-settings-general' into 'master'

Add frontend validation to "Restrict access by IP address" field in Group -> Settings -> General -> Permissions, LFS, 2FA

See merge request gitlab-org/gitlab!39061

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/index.js

@@ -2,7 +2,7 @@ import Vue from 'vue';
 import { __, sprintf } from '~/locale';
 import CommaSeparatedListTokenSelector from '../components/comma_separated_list_token_selector.vue';
 
-export default (el, props = {}, qaSelector) => {
+export default (el, props = {}, qaSelector, customValidator) => {
   // eslint-disable-next-line no-new
   new Vue({
     el,
@@ -20,8 +20,14 @@ export default (el, props = {}, qaSelector) => {
         regexValidator,
         ...(regexValidator ? { regexValidator: new RegExp(regexValidator) } : {}),
         ...(disallowedValues ? { disallowedValues: JSON.parse(disallowedValues) } : {}),
+        customErrorMessage: '',
       };
     },
+    methods: {
+      handleTextInput(value) {
+        this.customErrorMessage = customValidator(value);
+      },
+    },
     render(createElement) {
       return createElement('comma-separated-list-token-selector', {
         attrs: {
@@ -32,8 +38,10 @@ export default (el, props = {}, qaSelector) => {
           ariaLabelledby: this.labelId,
           regexValidator: this.regexValidator,
           disallowedValues: this.disallowedValues,
+          customErrorMessage: this.customErrorMessage,
           ...props,
         },
+        on: customValidator ? { 'text-input': this.handleTextInput } : {},
         scopedSlots: {
           'user-defined-token-content': ({ inputText: value }) => {
             return sprintf(__('Add "%{value}"'), { value });

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

+import validateIpAddress from 'ee/validators/ip_address';
+import { __, sprintf } from '~/locale';
+
+export default address => {
+  if (!validateIpAddress(address)) {
+    return sprintf(__('%{address} is an invalid IP address range'), { address });
+  }
+
+  return '';
+};

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

@@ -30,7 +30,7 @@ export default {
       required: false,
       default: () => [],
     },
-    errorMessage: {
+    regexErrorMessage: {
       type: String,
       required: false,
       default: '',
@@ -40,6 +40,11 @@ export default {
       required: false,
       default: '',
     },
+    customErrorMessage: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   data() {
     return {
@@ -54,14 +59,14 @@ export default {
     },
     computedErrorMessage() {
       if (this.regexValidator !== null && this.textInputValue.match(this.regexValidator) === null) {
-        return this.errorMessage;
+        return this.regexErrorMessage;
       }
 
       if (!isEmpty(this.disallowedValues) && this.disallowedValues.includes(this.textInputValue)) {
         return this.disallowedValueErrorMessage;
       }
 
-      return '';
+      return this.customErrorMessage;
     },
   },
   watch: {
@@ -111,6 +116,7 @@ export default {
     handleTextInput(value) {
       this.hideErrorMessage = true;
       this.textInputValue = value;
+      this.$emit('text-input', value);
     },
     handleBlur() {
       this.hideErrorMessage = true;

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', () => {
   initAccessRestrictionField('.js-allowed-email-domains', {
     placeholder: __('Enter domain'),
-    errorMessage: __('The domain you entered is misformatted.'),
+    regexErrorMessage: __('The domain you entered is misformatted.'),
     disallowedValueErrorMessage: __('The domain you entered is not allowed.'),
   });
   initAccessRestrictionField(
     '.js-ip-restriction',
     { placeholder: __('Enter IP address range') },
     'ip_restriction_field',
+    validateRestrictedIpAddress,
   );
 });

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

+import ipaddr from 'ipaddr.js';
+
+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)) {
+    return false;
+  }
+
+  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 ipaddr from 'ipaddr.js';
+import validateRestrictedIpAddress from 'ee/groups/settings/access_restriction_field/validate_ip_address';
+
+describe('validateRestrictedIpAddress', () => {
+  describe('when IP address is not valid', () => {
+    it('returns an error message', () => {
+      ipaddr.isValid = jest.fn(() => false);
+
+      const result = validateRestrictedIpAddress('foo bar');
+
+      expect(ipaddr.isValid).toHaveBeenCalledWith('foo bar');
+      expect(result).toBe(`foo bar is an invalid IP address range`);
+    });
+  });
+
+  describe('when IP address is valid', () => {
+    it('returns an empty string', () => {
+      ipaddr.isValid = jest.fn(() => true);
+
+      const result = validateRestrictedIpAddress('192.168.0.0');
+
+      expect(ipaddr.isValid).toHaveBeenCalledWith('192.168.0.0');
+      expect(result).toBe('');
+    });
+  });
+});

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

@@ -12,13 +12,16 @@ describe('CommaSeparatedListTokenSelector', () => {
   const defaultProps = {
     hiddenInputId: 'comma-separated-list',
     ariaLabelledby: 'comma-separated-list-label',
-    errorMessage: 'The value entered is invalid',
+    regexErrorMessage: 'The value entered is invalid',
     disallowedValueErrorMessage: 'The value entered is not allowed',
   };
 
   const createComponent = options => {
     wrapper = mount(CommaSeparatedListTokenSelector, {
       attachTo: div,
+      scopedSlots: {
+        'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
+      },
       ...options,
       propsData: {
         ...defaultProps,
@@ -132,6 +135,16 @@ describe('CommaSeparatedListTokenSelector', () => {
     });
   });
 
+  describe('when text input is typed in', () => {
+    it('emits `text-input` event', async () => {
+      createComponent();
+
+      await setTokenSelectorInputValue('foo bar');
+
+      expect(wrapper.emitted('text-input')[0]).toEqual(['foo bar']);
+    });
+  });
+
   describe('when enter key is pressed', () => {
     it('does not submit the form if token selector text input has a value', async () => {
       createComponent();
@@ -145,7 +158,7 @@ describe('CommaSeparatedListTokenSelector', () => {
     });
 
     describe('when `regexValidator` prop is set', () => {
-      it('displays `errorMessage` if regex fails', async () => {
+      it('displays `regexErrorMessage` if regex fails', async () => {
         createComponent({
           propsData: {
             regexValidator: /baz/,
@@ -176,8 +189,22 @@ describe('CommaSeparatedListTokenSelector', () => {
       });
     });
 
-    describe('when `regexValidator` and `disallowedValues` props are set', () => {
-      it('displays `errorMessage` if regex fails', async () => {
+    describe('when `customErrorMessage` prop is set', () => {
+      it('displays `customErrorMessage`', () => {
+        createComponent({
+          propsData: {
+            customErrorMessage: 'Value is invalid',
+          },
+        });
+
+        tokenSelectorTriggerEnter();
+
+        expect(findErrorMessageText()).toBe('Value is invalid');
+      });
+    });
+
+    describe('when `regexValidator`, `disallowedValues` and `customErrorMessage` props are set', () => {
+      it('displays `regexErrorMessage` if regex fails', async () => {
         createComponent({
           propsData: {
             regexValidator: /baz/,
@@ -206,6 +233,22 @@ describe('CommaSeparatedListTokenSelector', () => {
 
         expect(findErrorMessageText()).toBe('The value entered is not allowed');
       });
+
+      it('displays `customErrorMessage` if regex passes and value is not in the disallowed list', async () => {
+        createComponent({
+          propsData: {
+            regexValidator: /foo bar/,
+            disallowedValues: ['foo', 'bar', 'baz'],
+            customErrorMessage: 'Value is invalid',
+          },
+        });
+
+        await setTokenSelectorInputValue('foo bar');
+
+        tokenSelectorTriggerEnter();
+
+        expect(findErrorMessageText()).toBe('Value is invalid');
+      });
     });
   });
 
@@ -216,38 +259,21 @@ describe('CommaSeparatedListTokenSelector', () => {
           regexValidator: /foo/,
           disallowedValues: ['bar', 'baz'],
         },
-        scopedSlots: {
-          'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
-        },
       });
 
       await setTokenSelectorInputValue('foo');
 
-      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', () => {
+  describe('when `regexValidator`, `disallowedValues` and `customErrorMessage` props are not set', () => {
     it('allows any value to be added', async () => {
-      createComponent({
-        scopedSlots: {
-          'user-defined-token-content': '<span>Add "{{props.inputText}}"</span>',
-        },
-      });
+      createComponent();
 
       await setTokenSelectorInputValue('foo');
 
-      expect(
-        findTokenSelector()
-          .find('[role="menuitem"]')
-          .text(),
-      ).toBe('Add "foo"');
+      expect(findTokenSelectorDropdown().text()).toBe('Add "foo"');
     });
   });
 

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 only integers', () => {
+    it.each`
+      address
+      ${1}
+      ${19}
+      ${192}
+    `('$address - returns false', ({ address }) => {
+      expect(validateIpAddress(address)).toBe(false);
+    });
+  });
+
+  describe('when IP address is in valid CIDR format', () => {
+    it('returns true', () => {
+      ipaddr.parseCIDR = jest.fn(() => [
+        {
+          octets: [192, 168, 0, 0],
+        },
+        24,
+      ]);
+
+      const result = validateIpAddress('192.168.0.0/24');
+
+      expect(ipaddr.parseCIDR).toHaveBeenCalledWith('192.168.0.0/24');
+      expect(result).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 ""