From 601048f0241a7f2a52c22cc6228da6cfeef40110 Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Tue, 8 Jun 2021 17:47:38 +0200 Subject: [PATCH 1/6] [TASK] Implemented allowedDomains logic --- Classes/Client.php | 47 ++++++++++++++++- Tests/Unit/ClientTest.php | 104 ++++++++++++++++++++++++++++++++++++++ ext_conf_template.txt | 5 +- 3 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 Tests/Unit/ClientTest.php diff --git a/Classes/Client.php b/Classes/Client.php index 83ceeeb..45ca780 100644 --- a/Classes/Client.php +++ b/Classes/Client.php @@ -149,14 +149,59 @@ protected function checkAccess(ServerRequestInterface $request): string if (!isset($settings['allowedIps']) || empty($settings['allowedIps'])) { return 'No allowed ips defined'; } + if (!isset($settings['allowedDomains']) || empty($settings['allowedDomains'])) { + return 'No allowed domains defined'; + } + + $allowedIps = $this->getAllowedIps($settings['allowedIps'], $settings['allowedDomains']); + $remoteIp = GeneralUtility::getIndpEnv('REMOTE_ADDR'); - if (!GeneralUtility::cmpIP($remoteIp, $settings['allowedIps'])) { + if (!GeneralUtility::cmpIP($remoteIp, $allowedIps)) { return sprintf('IP comparison failed, remote IP: %s!', $remoteIp); } return ''; } + /** + * Parses the allowed domains and IPs from the extension settings into a single allowed IPs string + * @return string The allowed ips, comma separated. + */ + public function getAllowedIps(string $allowedIps, string $allowedDomains): string + { + $allowedIps = trim($allowedIps); + $allowedDomains = trim($allowedDomains); + + /* Return * when anything is allowed in both fields */ + if($allowedIps === '*' && $allowedDomains === '*') { + return '*'; + } + + $allowedIpsResult = ''; + if($allowedIps !== '*' && !empty($allowedIps)) { + $allowedIpsResult .= $allowedIps; + } + + /* Convert the domain names to IP addresses */ + if($allowedDomains !== '*' && !empty($allowedDomains)) { + $allowedDomainsArray = explode(',', $allowedDomains); + $allowedIpsResult .= ', '; + foreach ($allowedDomainsArray as $allowedDomain) { + $resultIp4 = dns_get_record($allowedDomain, DNS_A); + if(isset($resultIp4[0]['ip'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIp4[0]['ip']); + } + $resultIp6 = dns_get_record($allowedDomain, DNS_AAAA); + if(isset($resultIp6[0]['ipv6'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIp6[0]['ipv6']); + } + } + } + + /* Return concatenated IPs and domains IPs */ + return trim($allowedIpsResult, ' ,'); + } + protected function getSettings(): array { $configuration = []; diff --git a/Tests/Unit/ClientTest.php b/Tests/Unit/ClientTest.php new file mode 100644 index 0000000..ec90320 --- /dev/null +++ b/Tests/Unit/ClientTest.php @@ -0,0 +1,104 @@ +client = GeneralUtility::makeInstance(Client::class); + } + + /** + * @test + * @throws RequiredArgumentMissingException + */ + public function testInputIpsAndDomainsStarsReturnOneStarOnly() + { + $allowedIps = '*'; + $allowedDomains = '*'; + $this->assertEquals('*', $this->client->getAllowedIps($allowedIps, $allowedDomains)); + } + + /** + * @test + */ + public function testInputDomainsOnlyReturnsIPv4AndIPv6() + { + $allowedIps = ''; + $allowedDomains = 'www.google.com'; + + /* Expect 2 IPs to be returned; one IPv4 and one IPv6 */ + $this->assertGreaterThanOrEqual( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + } + + /** + * @test + */ + public function testInputIpsAndDomainsReturnsBothConcatenated() + { + $allowedIps = '78.47.171.202, 142.250.184.206'; + $allowedDomains = 'www.google.com, www.beech.it'; + + /* Expect 4 or more IPs (depending on IPv6 implementation) to be returned */ + $this->assertGreaterThanOrEqual( + 4, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + } + + /** + * @test + */ + public function testExpectTypeErrorOnNull() + { + $allowedIps = null; + $allowedDomains = null; + + $this->expectException(TypeError::class); + $this->client->getAllowedIps($allowedIps, $allowedDomains); + } + + /** + * @test + */ + public function testExpectZeroIpsWithEmptyParameters() + { + $allowedIps = ''; + $allowedDomains = ''; + + $this->assertEquals( + '', + $this->client->getAllowedIps($allowedIps, $allowedDomains) + ); + } + + /** + * @test + */ + public function testExpectBogusDomainNamesToNotBeProcessed() + { + $allowedIps = '78.47.171.202, 142.250.184.206'; + $allowedDomains = 'com.google.www, it.beech.www'; + + $this->assertCount( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + } + +} diff --git a/ext_conf_template.txt b/ext_conf_template.txt index 7c34928..f96061c 100644 --- a/ext_conf_template.txt +++ b/ext_conf_template.txt @@ -6,4 +6,7 @@ secret = allowedIps = * # cat=features/enable/3; type=boolean; label=Enable debugging: Instead of silently stopping the endpoint call, the error is outputted. Just enable this to debug if a connection is not possible! -enableDebugForErrors = 0 \ No newline at end of file +enableDebugForErrors = 0 + +# cat=features/enable/2; type=string; label=Allowed domains:Restrict the endpoint to specific domains. Comma separated lists are possible +allowedDomains = * From ec8d495f15df98091552270b617361155fda82fa Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Tue, 8 Jun 2021 17:49:27 +0200 Subject: [PATCH 2/6] [TASK] Update README.md --- README.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.rst b/README.rst index 6dfa6a4..d6e302a 100644 --- a/README.rst +++ b/README.rst @@ -29,6 +29,10 @@ allowedIps """""""""" Define a comma separated list of IPs which are allowed to fetch the client data. +allowedDomains +"""""""""" +Define a comma separated list of domains which are allowed to fetch the client data. + enableDebugForErrors """""""""""""""""""" If set, the errors are outputted if you call `http://yourdomain.tld/?eID=t3monitoring&secret=`. This can help to identify problems. From c35a40765a3f00f364c4ea295cc534ca840ce47a Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Wed, 9 Jun 2021 13:19:23 +0200 Subject: [PATCH 3/6] [TASK] Implemented logic for values > * > empty --- Classes/Client.php | 33 +++++++++++++----------- Tests/Unit/ClientTest.php | 54 +++++++++++++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/Classes/Client.php b/Classes/Client.php index 45ca780..d3053ea 100644 --- a/Classes/Client.php +++ b/Classes/Client.php @@ -146,11 +146,9 @@ protected function checkAccess(ServerRequestInterface $request): string return 'No secret or too small secret defined'; } - if (!isset($settings['allowedIps']) || empty($settings['allowedIps'])) { - return 'No allowed ips defined'; - } - if (!isset($settings['allowedDomains']) || empty($settings['allowedDomains'])) { - return 'No allowed domains defined'; + /* Only returns an error when both are empty */ + if (empty($settings['allowedIps']) && empty($settings['allowedDomains'])) { + return 'No allowed ips or domains defined'; } $allowedIps = $this->getAllowedIps($settings['allowedIps'], $settings['allowedDomains']); @@ -172,28 +170,33 @@ public function getAllowedIps(string $allowedIps, string $allowedDomains): strin $allowedIps = trim($allowedIps); $allowedDomains = trim($allowedDomains); - /* Return * when anything is allowed in both fields */ - if($allowedIps === '*' && $allowedDomains === '*') { + /* Return * when anything is allowed in one field, and the other is empty */ + if (($allowedIps === '*' && $allowedDomains === '*') || + ($allowedIps === '*' && empty($allowedDomains)) || + (empty($allowedIps) && $allowedDomains === '*')) { return '*'; } $allowedIpsResult = ''; if($allowedIps !== '*' && !empty($allowedIps)) { - $allowedIpsResult .= $allowedIps; + $allowedIpsResult = sprintf('%s, ', $allowedIps); } /* Convert the domain names to IP addresses */ if($allowedDomains !== '*' && !empty($allowedDomains)) { $allowedDomainsArray = explode(',', $allowedDomains); - $allowedIpsResult .= ', '; foreach ($allowedDomainsArray as $allowedDomain) { - $resultIp4 = dns_get_record($allowedDomain, DNS_A); - if(isset($resultIp4[0]['ip'])) { - $allowedIpsResult .= sprintf('%s, ', $resultIp4[0]['ip']); + $resultsIpv4 = dns_get_record($allowedDomain, DNS_A); + foreach ($resultsIpv4 as $resultIpv4) { + if(isset($resultIpv4['ip'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIpv4['ip']); + } } - $resultIp6 = dns_get_record($allowedDomain, DNS_AAAA); - if(isset($resultIp6[0]['ipv6'])) { - $allowedIpsResult .= sprintf('%s, ', $resultIp6[0]['ipv6']); + $resultsIpv6 = dns_get_record($allowedDomain, DNS_AAAA); + foreach ($resultsIpv6 as $resultIpv6) { + if(isset($resultIpv6['ipv6'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIpv6['ipv6']); + } } } } diff --git a/Tests/Unit/ClientTest.php b/Tests/Unit/ClientTest.php index ec90320..a284d19 100644 --- a/Tests/Unit/ClientTest.php +++ b/Tests/Unit/ClientTest.php @@ -3,7 +3,6 @@ use PHPUnit\Framework\TestCase; use T3Monitor\T3monitoringClient\Client; use TYPO3\CMS\Core\Utility\GeneralUtility; -use TYPO3\CMS\Extbase\Mvc\Controller\Exception\RequiredArgumentMissingException; class ClientTest extends TestCase { @@ -22,13 +21,58 @@ protected function setUp(): void /** * @test - * @throws RequiredArgumentMissingException */ public function testInputIpsAndDomainsStarsReturnOneStarOnly() { $allowedIps = '*'; $allowedDomains = '*'; $this->assertEquals('*', $this->client->getAllowedIps($allowedIps, $allowedDomains)); + + $allowedIps = '*'; + $allowedDomains = ''; + $this->assertEquals('*', $this->client->getAllowedIps($allowedIps, $allowedDomains)); + + $allowedIps = ''; + $allowedDomains = '*'; + $this->assertEquals('*', $this->client->getAllowedIps($allowedIps, $allowedDomains)); + } + + /** + * @test + */ + public function testValuesHasPriorityOverStarOverEmpty() + { + $allowedIps = '78.47.171.202, 142.250.184.206'; + $allowedDomains = '*'; + /* Expect the IPs to be returned */ + $this->assertCount( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + + $allowedIps = '78.47.171.202, 142.250.184.206'; + $allowedDomains = ''; + /* Expect the IPs to be returned */ + $this->assertCount( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + + $allowedIps = '*'; + $allowedDomains = 'www.google.com, www.beech.it'; + /* Expect the IPs from the domains to be returned */ + $this->assertGreaterThanOrEqual( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); + + $allowedIps = ''; + $allowedDomains = 'www.google.com, www.beech.it'; + /* Expect the IPs from the domains to be returned */ + $this->assertGreaterThanOrEqual( + 2, + explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) + ); } /** @@ -39,9 +83,9 @@ public function testInputDomainsOnlyReturnsIPv4AndIPv6() $allowedIps = ''; $allowedDomains = 'www.google.com'; - /* Expect 2 IPs to be returned; one IPv4 and one IPv6 */ + /* Expect the IPs to be returned */ $this->assertGreaterThanOrEqual( - 2, + 1, explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) ); } @@ -54,7 +98,7 @@ public function testInputIpsAndDomainsReturnsBothConcatenated() $allowedIps = '78.47.171.202, 142.250.184.206'; $allowedDomains = 'www.google.com, www.beech.it'; - /* Expect 4 or more IPs (depending on IPv6 implementation) to be returned */ + /* Expect 4 or more IPs (depending on IPv6 implementation and ) to be returned */ $this->assertGreaterThanOrEqual( 4, explode(',', $this->client->getAllowedIps($allowedIps, $allowedDomains)) From fcda22c87836ea0d850bb9dc478064abb188a491 Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Fri, 11 Jun 2021 12:14:54 +0200 Subject: [PATCH 4/6] [TASK] Refined catching non-existing and incomplete domain names --- Classes/Client.php | 10 ++++++++-- Tests/Unit/ClientTest.php | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Classes/Client.php b/Classes/Client.php index d3053ea..f58eaa5 100644 --- a/Classes/Client.php +++ b/Classes/Client.php @@ -186,13 +186,19 @@ public function getAllowedIps(string $allowedIps, string $allowedDomains): strin if($allowedDomains !== '*' && !empty($allowedDomains)) { $allowedDomainsArray = explode(',', $allowedDomains); foreach ($allowedDomainsArray as $allowedDomain) { - $resultsIpv4 = dns_get_record($allowedDomain, DNS_A); + $resultsIpv4 = @dns_get_record($allowedDomain, DNS_A); + if(!$resultsIpv4) { + continue; + } foreach ($resultsIpv4 as $resultIpv4) { if(isset($resultIpv4['ip'])) { $allowedIpsResult .= sprintf('%s, ', $resultIpv4['ip']); } } - $resultsIpv6 = dns_get_record($allowedDomain, DNS_AAAA); + $resultsIpv6 = @dns_get_record($allowedDomain, DNS_AAAA); + if(!$resultsIpv6) { + continue; + } foreach ($resultsIpv6 as $resultIpv6) { if(isset($resultIpv6['ipv6'])) { $allowedIpsResult .= sprintf('%s, ', $resultIpv6['ipv6']); diff --git a/Tests/Unit/ClientTest.php b/Tests/Unit/ClientTest.php index a284d19..590f521 100644 --- a/Tests/Unit/ClientTest.php +++ b/Tests/Unit/ClientTest.php @@ -137,7 +137,7 @@ public function testExpectZeroIpsWithEmptyParameters() public function testExpectBogusDomainNamesToNotBeProcessed() { $allowedIps = '78.47.171.202, 142.250.184.206'; - $allowedDomains = 'com.google.www, it.beech.www'; + $allowedDomains = 'com.google.www, it.beech.www, beech@beech.com, www,beech,it, www/beech/it, jksdafjsdnidmfhsnbsdfkjnjuhasghdsn.nl'; $this->assertCount( 2, From e2aa291b2e647acadd5c6605077242be6de077af Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Mon, 14 Jun 2021 17:03:14 +0200 Subject: [PATCH 5/6] [TASK] Removed references to company --- Tests/Unit/ClientTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/Unit/ClientTest.php b/Tests/Unit/ClientTest.php index 590f521..0c25243 100644 --- a/Tests/Unit/ClientTest.php +++ b/Tests/Unit/ClientTest.php @@ -59,7 +59,7 @@ public function testValuesHasPriorityOverStarOverEmpty() ); $allowedIps = '*'; - $allowedDomains = 'www.google.com, www.beech.it'; + $allowedDomains = 'www.google.com, www.test.io'; /* Expect the IPs from the domains to be returned */ $this->assertGreaterThanOrEqual( 2, @@ -67,7 +67,7 @@ public function testValuesHasPriorityOverStarOverEmpty() ); $allowedIps = ''; - $allowedDomains = 'www.google.com, www.beech.it'; + $allowedDomains = 'www.google.com, www.test.io'; /* Expect the IPs from the domains to be returned */ $this->assertGreaterThanOrEqual( 2, @@ -96,7 +96,7 @@ public function testInputDomainsOnlyReturnsIPv4AndIPv6() public function testInputIpsAndDomainsReturnsBothConcatenated() { $allowedIps = '78.47.171.202, 142.250.184.206'; - $allowedDomains = 'www.google.com, www.beech.it'; + $allowedDomains = 'www.google.com, www.test.io'; /* Expect 4 or more IPs (depending on IPv6 implementation and ) to be returned */ $this->assertGreaterThanOrEqual( @@ -137,7 +137,7 @@ public function testExpectZeroIpsWithEmptyParameters() public function testExpectBogusDomainNamesToNotBeProcessed() { $allowedIps = '78.47.171.202, 142.250.184.206'; - $allowedDomains = 'com.google.www, it.beech.www, beech@beech.com, www,beech,it, www/beech/it, jksdafjsdnidmfhsnbsdfkjnjuhasghdsn.nl'; + $allowedDomains = 'com.google.www, io.test.www, test@test.com, www,test,io, www/test/io, jksdafjsdnidmfhsnbsdfkjnjuhasghdsn.nl'; $this->assertCount( 2, From e1424b49190ce69515ce377b0cd8a25bbb2997be Mon Sep 17 00:00:00 2001 From: Peter Verstappen Date: Fri, 25 Jun 2021 09:39:01 +0200 Subject: [PATCH 6/6] [TASK] Fixed allowedDomains foreach continue if no ipv4 was found --- Classes/Client.php | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/Classes/Client.php b/Classes/Client.php index f58eaa5..d55ebca 100644 --- a/Classes/Client.php +++ b/Classes/Client.php @@ -187,21 +187,19 @@ public function getAllowedIps(string $allowedIps, string $allowedDomains): strin $allowedDomainsArray = explode(',', $allowedDomains); foreach ($allowedDomainsArray as $allowedDomain) { $resultsIpv4 = @dns_get_record($allowedDomain, DNS_A); - if(!$resultsIpv4) { - continue; - } - foreach ($resultsIpv4 as $resultIpv4) { - if(isset($resultIpv4['ip'])) { - $allowedIpsResult .= sprintf('%s, ', $resultIpv4['ip']); + if($resultsIpv4) { + foreach ($resultsIpv4 as $resultIpv4) { + if(isset($resultIpv4['ip'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIpv4['ip']); + } } } $resultsIpv6 = @dns_get_record($allowedDomain, DNS_AAAA); - if(!$resultsIpv6) { - continue; - } - foreach ($resultsIpv6 as $resultIpv6) { - if(isset($resultIpv6['ipv6'])) { - $allowedIpsResult .= sprintf('%s, ', $resultIpv6['ipv6']); + if($resultsIpv6) { + foreach ($resultsIpv6 as $resultIpv6) { + if(isset($resultIpv6['ipv6'])) { + $allowedIpsResult .= sprintf('%s, ', $resultIpv6['ipv6']); + } } } }