diff --git a/packages/manager/.changeset/pr-9336-fixed-1687982995866.md b/packages/manager/.changeset/pr-9336-fixed-1687982995866.md new file mode 100644 index 00000000000..af3816b78e1 --- /dev/null +++ b/packages/manager/.changeset/pr-9336-fixed-1687982995866.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Fixed +--- + +Firewall custom ports validation ([#9336](https://github.com/linode/manager/pull/9336)) diff --git a/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx b/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx index 124d37fddb7..2715d6b6de8 100644 --- a/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx +++ b/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx @@ -121,7 +121,8 @@ describe('utilities', () => { label: 'Firewalllabel', addresses: 'All IPv4', }; - expect(validateForm({ protocol: 'TCP', ports: '1', ...rest })).toEqual( + // SUCCESS CASES + expect(validateForm({ protocol: 'TCP', ports: '1234', ...rest })).toEqual( {} ); expect( @@ -134,28 +135,46 @@ describe('utilities', () => { {} ); expect( - validateForm({ protocol: 'TCP', ports: 'abc', ...rest }) - ).toHaveProperty( - 'ports', - 'Ports must be an integer, range of integers, or a comma-separated list of integers.' - ); - expect( - validateForm({ protocol: 'TCP', ports: '1--20', ...rest }) - ).toHaveProperty( - 'ports', - 'Ports must be an integer, range of integers, or a comma-separated list of integers.' - ); + validateForm({ + protocol: 'TCP', + ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15', + ...rest, + }) + ).toEqual({}); expect( validateForm({ protocol: 'TCP', ports: '1-2,3-4', ...rest }) + ).toEqual({}); + expect( + validateForm({ protocol: 'TCP', ports: '1,5-12', ...rest }) + ).toEqual({}); + // FAILURE CASES + expect( + validateForm({ protocol: 'TCP', ports: '1,21-12', ...rest }) ).toHaveProperty( 'ports', - 'Ports must be an integer, range of integers, or a comma-separated list of integers.' + 'Range must start with a smaller number and end with a larger number' ); + expect( + validateForm({ protocol: 'TCP', ports: '1-21-45', ...rest }) + ).toHaveProperty('ports', 'Ranges must have 2 values'); + expect( + validateForm({ protocol: 'TCP', ports: 'abc', ...rest }) + ).toHaveProperty('ports', 'Must be 1-65535'); + expect( + validateForm({ protocol: 'TCP', ports: '1--20', ...rest }) + ).toHaveProperty('ports', 'Must be 1-65535'); expect( validateForm({ protocol: 'TCP', ports: '-20', ...rest }) + ).toHaveProperty('ports', 'Must be 1-65535'); + expect( + validateForm({ + protocol: 'TCP', + ports: '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16', + ...rest, + }) ).toHaveProperty( 'ports', - 'Ports must be an integer, range of integers, or a comma-separated list of integers.' + 'Number of ports or port range endpoints exceeded. Max allowed is 15' ); }); it('validates label', () => { diff --git a/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.utils.ts b/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.utils.ts index fad2eb3658b..e19971e62a5 100644 --- a/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.utils.ts +++ b/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.utils.ts @@ -12,8 +12,8 @@ import { predefinedFirewallFromRule, } from 'src/features/Firewalls/shared'; import { - CUSTOM_PORTS_VALIDATION_REGEX, CUSTOM_PORTS_ERROR_MESSAGE, + isCustomPortsValid, } from '@linode/validation'; import type { FirewallRuleProtocol, @@ -332,7 +332,7 @@ export const validateForm = ({ if ((protocol === 'ICMP' || protocol === 'IPENCAP') && ports) { errors.ports = `Ports are not allowed for ${protocol} protocols.`; - } else if (ports && !ports.match(CUSTOM_PORTS_VALIDATION_REGEX)) { + } else if (ports && !isCustomPortsValid(ports)) { errors.ports = CUSTOM_PORTS_ERROR_MESSAGE; } diff --git a/packages/validation/.changeset/pr-9336-fixed-1687983041491.md b/packages/validation/.changeset/pr-9336-fixed-1687983041491.md new file mode 100644 index 00000000000..69266a068bb --- /dev/null +++ b/packages/validation/.changeset/pr-9336-fixed-1687983041491.md @@ -0,0 +1,5 @@ +--- +"@linode/validation": Fixed +--- + +Firewall custom port validation ([#9336](https://github.com/linode/manager/pull/9336)) diff --git a/packages/validation/src/firewalls.schema.ts b/packages/validation/src/firewalls.schema.ts index b9aa0d36946..e1a2eeb6e4f 100644 --- a/packages/validation/src/firewalls.schema.ts +++ b/packages/validation/src/firewalls.schema.ts @@ -5,9 +5,6 @@ import { array, mixed, number, object, string } from 'yup'; export const IP_ERROR_MESSAGE = 'Must be a valid IPv4 or IPv6 address or range.'; -export const CUSTOM_PORTS_ERROR_MESSAGE = - 'Ports must be an integer, range of integers, or a comma-separated list of integers.'; -export const CUSTOM_PORTS_VALIDATION_REGEX = /^(?:\d+|\d+-\d+|(?:\d+,\s*)*\d+)$/; export const validateIP = (ipAddress?: string | null): boolean => { if (!ipAddress) { @@ -42,10 +39,104 @@ export const ipAddress = string().test({ test: validateIP, }); -export const validateFirewallPorts = string().matches( - CUSTOM_PORTS_VALIDATION_REGEX, - CUSTOM_PORTS_ERROR_MESSAGE -); +export let CUSTOM_PORTS_ERROR_MESSAGE = ''; + +/** + * @param port + * @returns boolean + * @description Validates a single port or port range and sets the error message + */ +const validatePort = (port: string): boolean => { + CUSTOM_PORTS_ERROR_MESSAGE = + 'Ports must be an integer, range of integers, or a comma-separated list of integers.'; + + if (!port) { + CUSTOM_PORTS_ERROR_MESSAGE = 'Must be 1-65535'; + return false; + } + + const convertedPort = parseInt(port, 10); + if (!(1 <= convertedPort && convertedPort <= 65535)) { + CUSTOM_PORTS_ERROR_MESSAGE = 'Must be 1-65535'; + return false; + } + + if (port.startsWith('0')) { + CUSTOM_PORTS_ERROR_MESSAGE = 'Port must not have leading zeroes'; + return false; + } + + if (String(convertedPort) !== port) { + return false; + } + + return true; +}; + +/** + * @param ports + * @returns boolean + * @description Validates a comma-separated list of ports and port ranges and sets the error message + */ +export const isCustomPortsValid = (ports: string): boolean => { + const portList = ports?.split(',') || []; + let portLimitCount = 0; + + for (const port of portList) { + const cleanedPort = port.trim(); + + if (cleanedPort.includes('-')) { + const portRange = cleanedPort.split('-'); + + if (!validatePort(portRange[0]) || !validatePort(portRange[1])) { + return false; + } + + if (portRange.length !== 2) { + CUSTOM_PORTS_ERROR_MESSAGE = 'Ranges must have 2 values'; + return false; + } + + if (parseInt(portRange[0], 10) >= parseInt(portRange[1], 10)) { + CUSTOM_PORTS_ERROR_MESSAGE = + 'Range must start with a smaller number and end with a larger number'; + return false; + } + + portLimitCount += 2; + } else { + if (!validatePort(cleanedPort)) { + return false; + } + portLimitCount++; + } + } + + if (portLimitCount > 15) { + CUSTOM_PORTS_ERROR_MESSAGE = + 'Number of ports or port range endpoints exceeded. Max allowed is 15'; + return false; + } + + return true; +}; + +const validateFirewallPorts = string().test({ + name: 'firewall-ports', + message: CUSTOM_PORTS_ERROR_MESSAGE, + test: (value) => { + if (!value) { + return false; + } + + try { + isCustomPortsValid(value); + } catch (err) { + return false; + } + return true; + }, +}); const validFirewallRuleProtocol = ['ALL', 'TCP', 'UDP', 'ICMP', 'IPENCAP']; export const FirewallRuleTypeSchema = object().shape({