From a7671ed379f42ccbe64e3d80d6db671276d5fc35 Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Tue, 9 Sep 2025 09:00:35 +0200
Subject: [PATCH] refactor(dns-validation): rename DNS validation functions for
consistency and clarity, and remove unused code
---
app/Http/Middleware/ApiAllowed.php | 2 +-
app/Livewire/Project/Application/General.php | 4 +-
app/Livewire/Project/Application/Previews.php | 2 +-
app/Livewire/Settings/Index.php | 2 +-
bootstrap/helpers/shared.php | 80 +------------------
tests/Feature/IpAllowlistTest.php | 58 +++++++-------
6 files changed, 38 insertions(+), 110 deletions(-)
diff --git a/app/Http/Middleware/ApiAllowed.php b/app/Http/Middleware/ApiAllowed.php
index dd85c3521..21441a117 100644
--- a/app/Http/Middleware/ApiAllowed.php
+++ b/app/Http/Middleware/ApiAllowed.php
@@ -28,7 +28,7 @@ class ApiAllowed
$allowedIps = array_map('trim', $allowedIps);
$allowedIps = array_filter($allowedIps); // Remove empty entries
- if (! empty($allowedIps) && ! check_ip_against_allowlist($request->ip(), $allowedIps)) {
+ if (! empty($allowedIps) && ! checkIPAgainstAllowlist($request->ip(), $allowedIps)) {
return response()->json(['success' => true, 'message' => 'You are not allowed to access the API.'], 403);
}
}
diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php
index aa72b7c5f..76aa909c8 100644
--- a/app/Livewire/Project/Application/General.php
+++ b/app/Livewire/Project/Application/General.php
@@ -487,7 +487,7 @@ class General extends Component
$domains = str($this->application->fqdn)->trim()->explode(',');
if ($this->application->additional_servers->count() === 0) {
foreach ($domains as $domain) {
- if (! validate_dns_entry($domain, $this->application->destination->server)) {
+ if (! validateDNSEntry($domain, $this->application->destination->server)) {
$showToaster && $this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.
$domain->{$this->application->destination->server->ip}
Check this documentation for further help.");
}
}
@@ -615,7 +615,7 @@ class General extends Component
foreach ($this->parsedServiceDomains as $service) {
$domain = data_get($service, 'domain');
if ($domain) {
- if (! validate_dns_entry($domain, $this->application->destination->server)) {
+ if (! validateDNSEntry($domain, $this->application->destination->server)) {
$showToaster && $this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.
$domain->{$this->application->destination->server->ip}
Check this documentation for further help.");
}
}
diff --git a/app/Livewire/Project/Application/Previews.php b/app/Livewire/Project/Application/Previews.php
index ebfd84489..e0f517428 100644
--- a/app/Livewire/Project/Application/Previews.php
+++ b/app/Livewire/Project/Application/Previews.php
@@ -77,7 +77,7 @@ class Previews extends Component
$preview->fqdn = str($preview->fqdn)->replaceEnd(',', '')->trim();
$preview->fqdn = str($preview->fqdn)->replaceStart(',', '')->trim();
$preview->fqdn = str($preview->fqdn)->trim()->lower();
- if (! validate_dns_entry($preview->fqdn, $this->application->destination->server)) {
+ if (! validateDNSEntry($preview->fqdn, $this->application->destination->server)) {
$this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.
$preview->fqdn->{$this->application->destination->server->ip}
Check this documentation for further help.");
$success = false;
}
diff --git a/app/Livewire/Settings/Index.php b/app/Livewire/Settings/Index.php
index d05433082..13d690352 100644
--- a/app/Livewire/Settings/Index.php
+++ b/app/Livewire/Settings/Index.php
@@ -115,7 +115,7 @@ class Index extends Component
$this->validate();
if ($this->settings->is_dns_validation_enabled && $this->fqdn) {
- if (! validate_dns_entry($this->fqdn, $this->server)) {
+ if (! validateDNSEntry($this->fqdn, $this->server)) {
$this->dispatch('error', "Validating DNS failed.
Make sure you have added the DNS records correctly.
{$this->fqdn}->{$this->server->ip}
Check this documentation for further help.");
$error_show = true;
}
diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php
index 9c30282b4..be509d546 100644
--- a/bootstrap/helpers/shared.php
+++ b/bootstrap/helpers/shared.php
@@ -961,7 +961,7 @@ function getRealtime()
}
}
-function validate_dns_entry(string $fqdn, Server $server)
+function validateDNSEntry(string $fqdn, Server $server)
{
// https://www.cloudflare.com/ips-v4/#
$cloudflare_ips = collect(['173.245.48.0/20', '103.21.244.0/22', '103.22.200.0/22', '103.31.4.0/22', '141.101.64.0/18', '108.162.192.0/18', '190.93.240.0/20', '188.114.96.0/20', '197.234.240.0/22', '198.41.128.0/17', '162.158.0.0/15', '104.16.0.0/13', '172.64.0.0/13', '131.0.72.0/22']);
@@ -994,7 +994,7 @@ function validate_dns_entry(string $fqdn, Server $server)
} else {
foreach ($results as $result) {
if ($result->getType() == $type) {
- if (ip_match($result->getData(), $cloudflare_ips->toArray(), $match)) {
+ if (ipMatch($result->getData(), $cloudflare_ips->toArray(), $match)) {
$found_matching_ip = true;
break;
}
@@ -1012,7 +1012,7 @@ function validate_dns_entry(string $fqdn, Server $server)
return $found_matching_ip;
}
-function ip_match($ip, $cidrs, &$match = null)
+function ipMatch($ip, $cidrs, &$match = null)
{
foreach ((array) $cidrs as $cidr) {
[$subnet, $mask] = explode('/', $cidr);
@@ -1026,7 +1026,7 @@ function ip_match($ip, $cidrs, &$match = null)
return false;
}
-function check_ip_against_allowlist($ip, $allowlist)
+function checkIPAgainstAllowlist($ip, $allowlist)
{
if (empty($allowlist)) {
return false;
@@ -1084,78 +1084,6 @@ function check_ip_against_allowlist($ip, $allowlist)
return false;
}
-function parseCommandsByLineForSudo(Collection $commands, Server $server): array
-{
- $commands = $commands->map(function ($line) {
- if (
- ! str(trim($line))->startsWith([
- 'cd',
- 'command',
- 'echo',
- 'true',
- 'if',
- 'fi',
- ])
- ) {
- return "sudo $line";
- }
-
- if (str(trim($line))->startsWith('if')) {
- return str_replace('if', 'if sudo', $line);
- }
-
- return $line;
- });
-
- $commands = $commands->map(function ($line) use ($server) {
- if (Str::startsWith($line, 'sudo mkdir -p')) {
- return "$line && sudo chown -R $server->user:$server->user ".Str::after($line, 'sudo mkdir -p').' && sudo chmod -R o-rwx '.Str::after($line, 'sudo mkdir -p');
- }
-
- return $line;
- });
-
- $commands = $commands->map(function ($line) {
- $line = str($line);
- if (str($line)->contains('$(')) {
- $line = $line->replace('$(', '$(sudo ');
- }
- if (str($line)->contains('||')) {
- $line = $line->replace('||', '|| sudo');
- }
- if (str($line)->contains('&&')) {
- $line = $line->replace('&&', '&& sudo');
- }
- if (str($line)->contains(' | ')) {
- $line = $line->replace(' | ', ' | sudo ');
- }
-
- return $line->value();
- });
-
- return $commands->toArray();
-}
-function parseLineForSudo(string $command, Server $server): string
-{
- if (! str($command)->startSwith('cd') && ! str($command)->startSwith('command')) {
- $command = "sudo $command";
- }
- if (Str::startsWith($command, 'sudo mkdir -p')) {
- $command = "$command && sudo chown -R $server->user:$server->user ".Str::after($command, 'sudo mkdir -p').' && sudo chmod -R o-rwx '.Str::after($command, 'sudo mkdir -p');
- }
- if (str($command)->contains('$(') || str($command)->contains('`')) {
- $command = str($command)->replace('$(', '$(sudo ')->replace('`', '`sudo ')->value();
- }
- if (str($command)->contains('||')) {
- $command = str($command)->replace('||', '|| sudo ')->value();
- }
- if (str($command)->contains('&&')) {
- $command = str($command)->replace('&&', '&& sudo ')->value();
- }
-
- return $command;
-}
-
function get_public_ips()
{
try {
diff --git a/tests/Feature/IpAllowlistTest.php b/tests/Feature/IpAllowlistTest.php
index 3454a9c9d..959dc757d 100644
--- a/tests/Feature/IpAllowlistTest.php
+++ b/tests/Feature/IpAllowlistTest.php
@@ -8,7 +8,7 @@ test('IP allowlist with single IPs', function () {
];
foreach ($testCases as $case) {
- $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+ $result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
expect($result)->toBe($case['expected']);
}
});
@@ -24,7 +24,7 @@ test('IP allowlist with CIDR notation', function () {
];
foreach ($testCases as $case) {
- $result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
+ $result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
expect($result)->toBe($case['expected']);
}
});
@@ -40,16 +40,16 @@ test('IP allowlist with 0.0.0.0 allows all', function () {
// Test 0.0.0.0 without subnet
foreach ($testIps as $ip) {
- $result = check_ip_against_allowlist($ip, ['0.0.0.0']);
+ $result = checkIPAgainstAllowlist($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();
+ expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/0']))->toBeTrue();
+ expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/8']))->toBeTrue();
+ expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/24']))->toBeTrue();
+ expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/32']))->toBeTrue();
}
});
@@ -66,44 +66,44 @@ test('IP allowlist with mixed entries', function () {
];
foreach ($testCases as $case) {
- $result = check_ip_against_allowlist($case['ip'], $allowlist);
+ $result = checkIPAgainstAllowlist($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();
+ expect(checkIPAgainstAllowlist('192.168.1.1', []))->toBeFalse();
+ expect(checkIPAgainstAllowlist('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();
+ expect(checkIPAgainstAllowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('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
+ expect(checkIPAgainstAllowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
+ expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
+ expect(checkIPAgainstAllowlist('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();
+ expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('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();
+ expect(checkIPAgainstAllowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('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();
+ expect(checkIPAgainstAllowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('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();
+ expect(checkIPAgainstAllowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
+ expect(checkIPAgainstAllowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
});
test('IP allowlist comma-separated string input', function () {
@@ -111,10 +111,10 @@ test('IP allowlist comma-separated string input', function () {
$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();
+ expect(checkIPAgainstAllowlist('192.168.1.100', $allowlist))->toBeTrue();
+ expect(checkIPAgainstAllowlist('10.5.5.5', $allowlist))->toBeTrue();
+ expect(checkIPAgainstAllowlist('172.16.10.10', $allowlist))->toBeTrue();
+ expect(checkIPAgainstAllowlist('8.8.8.8', $allowlist))->toBeFalse();
});
test('ValidIpOrCidr validation rule', function () {