From 990331cd7401c6295552dd1fa7e6211c022af6e6 Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Mon, 25 Aug 2025 09:31:31 +0200
Subject: [PATCH] feat(validation): add ValidIpOrCidr rule for validating IP
 addresses and CIDR notations; update API access settings UI and add
 comprehensive tests

---
 app/Rules/ValidIpOrCidr.php                   |  63 ++++++
 .../livewire/settings/advanced.blade.php      |   4 +-
 tests/Feature/IpAllowlistTest.php             | 183 ++++++++++++++++++
 3 files changed, 248 insertions(+), 2 deletions(-)
 create mode 100644 app/Rules/ValidIpOrCidr.php
 create mode 100644 tests/Feature/IpAllowlistTest.php

diff --git a/app/Rules/ValidIpOrCidr.php b/app/Rules/ValidIpOrCidr.php
new file mode 100644
index 000000000..e172ffd1a
--- /dev/null
+++ b/app/Rules/ValidIpOrCidr.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace App\Rules;
+
+use Closure;
+use Illuminate\Contracts\Validation\ValidationRule;
+
+class ValidIpOrCidr implements ValidationRule
+{
+    public function validate(string $attribute, mixed $value, Closure $fail): void
+    {
+        if (empty($value)) {
+            // Empty is allowed (means allow from anywhere)
+            return;
+        }
+
+        // Special case: 0.0.0.0 is allowed
+        if ($value === '0.0.0.0') {
+            return;
+        }
+
+        $entries = explode(',', $value);
+        $invalidEntries = [];
+
+        foreach ($entries as $entry) {
+            $entry = trim($entry);
+
+            if (empty($entry)) {
+                continue;
+            }
+
+            // Special case: 0.0.0.0 with or without subnet
+            if (str_starts_with($entry, '0.0.0.0')) {
+                continue;
+            }
+
+            // Check if it's CIDR notation
+            if (str_contains($entry, '/')) {
+                $parts = explode('/', $entry);
+                if (count($parts) !== 2) {
+                    $invalidEntries[] = $entry;
+
+                    continue;
+                }
+
+                [$ip, $mask] = $parts;
+
+                if (! filter_var($ip, FILTER_VALIDATE_IP) || ! is_numeric($mask) || $mask < 0 || $mask > 32) {
+                    $invalidEntries[] = $entry;
+                }
+            } else {
+                // Check if it's a valid IP
+                if (! filter_var($entry, FILTER_VALIDATE_IP)) {
+                    $invalidEntries[] = $entry;
+                }
+            }
+        }
+
+        if (! empty($invalidEntries)) {
+            $fail('The following entries are not valid IP addresses or CIDR notations: '.implode(', ', $invalidEntries));
+        }
+    }
+}
diff --git a/resources/views/livewire/settings/advanced.blade.php b/resources/views/livewire/settings/advanced.blade.php
index 6f7e5c867..b68370a09 100644
--- a/resources/views/livewire/settings/advanced.blade.php
+++ b/resources/views/livewire/settings/advanced.blade.php
@@ -32,8 +32,8 @@
                 <x-forms.checkbox instantSave id="is_api_enabled" label="API Access"
                     helper="If enabled, the API will be enabled. If disabled, the API will be disabled." />
                 <x-forms.input id="allowed_ips" label="Allowed IPs for API Access"
-                    helper="Allowed IP lists for the API. A comma separated list of IPs. Empty means you allow from everywhere."
-                    placeholder="1.1.1.1,8.8.8.8" />
+                    helper="Allowed IP addresses or subnets for API access.<br>Supports single IPs (192.168.1.100) and CIDR notation (192.168.1.0/24).<br>Use comma to separate multiple entries.<br>Use 0.0.0.0 or leave empty to allow from anywhere."
+                    placeholder="192.168.1.100,10.0.0.0/8,203.0.113.0/24" />
                 <h4 class="pt-4">Confirmation Settings</h4>
                 <div class="md:w-96 pb-1">
                     <x-forms.checkbox instantSave id="is_sponsorship_popup_enabled" label="Show Sponsorship Popup"
diff --git a/tests/Feature/IpAllowlistTest.php b/tests/Feature/IpAllowlistTest.php
new file mode 100644
index 000000000..3454a9c9d
--- /dev/null
+++ b/tests/Feature/IpAllowlistTest.php
@@ -0,0 +1,183 @@
+<?php
+
+test('IP allowlist with single IPs', function () {
+    $testCases = [
+        ['ip' => '192.168.1.100', 'allowlist' => ['192.168.1.100'], 'expected' => true],
+        ['ip' => '192.168.1.101', 'allowlist' => ['192.168.1.100'], 'expected' => false],
+        ['ip' => '10.0.0.1', 'allowlist' => ['10.0.0.1', '192.168.1.100'], 'expected' => true],
+    ];
+
+    foreach ($testCases as $case) {
+        $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+        expect($result)->toBe($case['expected']);
+    }
+});
+
+test('IP allowlist with CIDR notation', function () {
+    $testCases = [
+        ['ip' => '192.168.1.50', 'allowlist' => ['192.168.1.0/24'], 'expected' => true],
+        ['ip' => '192.168.2.50', 'allowlist' => ['192.168.1.0/24'], 'expected' => false],
+        ['ip' => '10.0.0.5', 'allowlist' => ['10.0.0.0/8'], 'expected' => true],
+        ['ip' => '11.0.0.5', 'allowlist' => ['10.0.0.0/8'], 'expected' => false],
+        ['ip' => '172.16.5.10', 'allowlist' => ['172.16.0.0/12'], 'expected' => true],
+        ['ip' => '172.32.0.1', 'allowlist' => ['172.16.0.0/12'], 'expected' => false],
+    ];
+
+    foreach ($testCases as $case) {
+        $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+        expect($result)->toBe($case['expected']);
+    }
+});
+
+test('IP allowlist with 0.0.0.0 allows all', function () {
+    $testIps = [
+        '1.2.3.4',
+        '192.168.1.1',
+        '10.0.0.1',
+        '255.255.255.255',
+        '127.0.0.1',
+    ];
+
+    // Test 0.0.0.0 without subnet
+    foreach ($testIps as $ip) {
+        $result = check_ip_against_allowlist($ip, ['0.0.0.0']);
+        expect($result)->toBeTrue();
+    }
+
+    // Test 0.0.0.0 with any subnet notation - should still allow all
+    foreach ($testIps as $ip) {
+        expect(check_ip_against_allowlist($ip, ['0.0.0.0/0']))->toBeTrue();
+        expect(check_ip_against_allowlist($ip, ['0.0.0.0/8']))->toBeTrue();
+        expect(check_ip_against_allowlist($ip, ['0.0.0.0/24']))->toBeTrue();
+        expect(check_ip_against_allowlist($ip, ['0.0.0.0/32']))->toBeTrue();
+    }
+});
+
+test('IP allowlist with mixed entries', function () {
+    $allowlist = ['192.168.1.100', '10.0.0.0/8', '172.16.0.0/16'];
+
+    $testCases = [
+        ['ip' => '192.168.1.100', 'expected' => true],  // Exact match
+        ['ip' => '192.168.1.101', 'expected' => false], // No match
+        ['ip' => '10.5.5.5', 'expected' => true],       // Matches 10.0.0.0/8
+        ['ip' => '172.16.255.255', 'expected' => true], // Matches 172.16.0.0/16
+        ['ip' => '172.17.0.1', 'expected' => false],    // Outside 172.16.0.0/16
+        ['ip' => '8.8.8.8', 'expected' => false],       // No match
+    ];
+
+    foreach ($testCases as $case) {
+        $result = check_ip_against_allowlist($case['ip'], $allowlist);
+        expect($result)->toBe($case['expected']);
+    }
+});
+
+test('IP allowlist handles empty and invalid entries', function () {
+    // Empty allowlist blocks all
+    expect(check_ip_against_allowlist('192.168.1.1', []))->toBeFalse();
+    expect(check_ip_against_allowlist('192.168.1.1', ['']))->toBeFalse();
+
+    // Handles spaces
+    expect(check_ip_against_allowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
+    expect(check_ip_against_allowlist('10.0.0.5', [' 10.0.0.0/8 ']))->toBeTrue();
+
+    // Invalid entries are skipped
+    expect(check_ip_against_allowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
+    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
+    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/-1']))->toBeFalse(); // Invalid mask
+});
+
+test('IP allowlist with various subnet sizes', function () {
+    // /32 - single host
+    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
+    expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.1/32']))->toBeFalse();
+
+    // /31 - point-to-point link
+    expect(check_ip_against_allowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
+    expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
+    expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.0/31']))->toBeFalse();
+
+    // /16 - class B
+    expect(check_ip_against_allowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
+    expect(check_ip_against_allowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
+    expect(check_ip_against_allowlist('172.17.0.1', ['172.16.0.0/16']))->toBeFalse();
+
+    // /0 - all addresses
+    expect(check_ip_against_allowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
+    expect(check_ip_against_allowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
+});
+
+test('IP allowlist comma-separated string input', function () {
+    // Test with comma-separated string (as it would come from the settings)
+    $allowlistString = '192.168.1.100,10.0.0.0/8,172.16.0.0/16';
+    $allowlist = explode(',', $allowlistString);
+
+    expect(check_ip_against_allowlist('192.168.1.100', $allowlist))->toBeTrue();
+    expect(check_ip_against_allowlist('10.5.5.5', $allowlist))->toBeTrue();
+    expect(check_ip_against_allowlist('172.16.10.10', $allowlist))->toBeTrue();
+    expect(check_ip_against_allowlist('8.8.8.8', $allowlist))->toBeFalse();
+});
+
+test('ValidIpOrCidr validation rule', function () {
+    $rule = new \App\Rules\ValidIpOrCidr;
+
+    // Helper function to test validation
+    $validate = function ($value) use ($rule) {
+        $errors = [];
+        $fail = function ($message) use (&$errors) {
+            $errors[] = $message;
+        };
+        $rule->validate('allowed_ips', $value, $fail);
+
+        return empty($errors);
+    };
+
+    // Valid cases - should pass
+    expect($validate(''))->toBeTrue(); // Empty is allowed
+    expect($validate('0.0.0.0'))->toBeTrue(); // 0.0.0.0 is allowed
+    expect($validate('192.168.1.1'))->toBeTrue(); // Valid IP
+    expect($validate('192.168.1.0/24'))->toBeTrue(); // Valid CIDR
+    expect($validate('10.0.0.0/8'))->toBeTrue(); // Valid CIDR
+    expect($validate('192.168.1.1,10.0.0.1'))->toBeTrue(); // Multiple valid IPs
+    expect($validate('192.168.1.0/24,10.0.0.0/8'))->toBeTrue(); // Multiple CIDRs
+    expect($validate('0.0.0.0/0'))->toBeTrue(); // 0.0.0.0 with subnet
+    expect($validate('0.0.0.0/24'))->toBeTrue(); // 0.0.0.0 with any subnet
+    expect($validate(' 192.168.1.1 '))->toBeTrue(); // With spaces
+
+    // Invalid cases - should fail
+    expect($validate('1'))->toBeFalse(); // Single digit
+    expect($validate('abc'))->toBeFalse(); // Invalid text
+    expect($validate('192.168.1.256'))->toBeFalse(); // Invalid IP (256)
+    expect($validate('192.168.1.0/33'))->toBeFalse(); // Invalid CIDR mask (>32)
+    expect($validate('192.168.1.0/-1'))->toBeFalse(); // Invalid CIDR mask (<0)
+    expect($validate('192.168.1.1,abc'))->toBeFalse(); // Mix of valid and invalid
+    expect($validate('192.168.1.1,192.168.1.256'))->toBeFalse(); // Mix with invalid IP
+    expect($validate('192.168.1.0/24/32'))->toBeFalse(); // Invalid CIDR format
+    expect($validate('not.an.ip.address'))->toBeFalse(); // Invalid format
+    expect($validate('192.168'))->toBeFalse(); // Incomplete IP
+    expect($validate('192.168.1.1.1'))->toBeFalse(); // Too many octets
+});
+
+test('ValidIpOrCidr validation rule error messages', function () {
+    $rule = new \App\Rules\ValidIpOrCidr;
+
+    // Helper function to get error message
+    $getError = function ($value) use ($rule) {
+        $errors = [];
+        $fail = function ($message) use (&$errors) {
+            $errors[] = $message;
+        };
+        $rule->validate('allowed_ips', $value, $fail);
+
+        return $errors[0] ?? null;
+    };
+
+    // Test error messages
+    $error = $getError('1');
+    expect($error)->toContain('not valid IP addresses or CIDR notations');
+    expect($error)->toContain('1');
+
+    $error = $getError('192.168.1.1,abc,10.0.0.256');
+    expect($error)->toContain('abc');
+    expect($error)->toContain('10.0.0.256');
+    expect($error)->not->toContain('192.168.1.1'); // Valid IP should not be in error
+});