feat(security): Add public API to allow validating IP Ranges and checking for "in range"

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

3 files changed, 21 insertions, 18 deletions

diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index 1aa7ad456a8..fb457579fac 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -18,7 +18,6 @@ use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Middleware\Security\SecurityMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
-use OC\Security\RemoteIpAddress;
 use OC\Settings\AuthorizedGroupMapper;
 use OC\User\Session;
 use OCP\App\IAppManager;
@@ -33,6 +32,7 @@ use OCP\IRequestId;
 use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\IUserSession;
+use OCP\Security\Ip\IRemoteAddress;
 use Psr\Log\LoggerInterface;
 use Test\AppFramework\Middleware\Security\Mock\NormalController;
 use Test\AppFramework\Middleware\Security\Mock\OCSController;
@@ -91,7 +91,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 		$this->appManager->expects($this->any())
 			->method('isEnabledForUser')
 			->willReturn($isAppEnabledForUser);
-		$remoteIpAddress = $this->createMock(RemoteIpAddress::class);
+		$remoteIpAddress = $this->createMock(IRemoteAddress::class);
 		$remoteIpAddress->method('allowsAdminActions')->willReturn(true);
 
 		return new SecurityMiddleware(
diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index e738f424065..4f42e10537d 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -9,7 +9,6 @@
 namespace Test\Group;
 
 use OC\Group\Database;
-use OC\Security\RemoteIpAddress;
 use OC\User\Manager;
 use OC\User\User;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -17,6 +16,7 @@ use OCP\Group\Backend\ISearchableGroupBackend;
 use OCP\GroupInterface;
 use OCP\ICacheFactory;
 use OCP\IUser;
+use OCP\Security\Ip\IRemoteAddress;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -33,7 +33,7 @@ class ManagerTest extends TestCase {
 	protected $logger;
 	/** @var ICacheFactory|MockObject */
 	private $cache;
-	/** @var RemoteIpAddress|MockObject */
+	/** @var IRemoteAddress|MockObject */
 	private $remoteIpAddress;
 
 	protected function setUp(): void {
@@ -44,7 +44,7 @@ class ManagerTest extends TestCase {
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->cache = $this->createMock(ICacheFactory::class);
 
-		$this->remoteIpAddress = $this->createMock(RemoteIpAddress::class);
+		$this->remoteIpAddress = $this->createMock(IRemoteAddress::class);
 		$this->remoteIpAddress->method('allowsAdminActions')->willReturn(true);
 	}
 
diff --git a/tests/lib/Security/RemoteIpAddressTest.php b/tests/lib/Security/Ip/RemoteAddressTest.php
index 9cf9458156a..22f38f62356 100644
--- a/tests/lib/Security/RemoteIpAddressTest.php
+++ b/tests/lib/Security/Ip/RemoteAddressTest.php
@@ -7,33 +7,27 @@ declare(strict_types=1);
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-namespace Test\Security;
+namespace Test\Security\Ip;
 
-use OC\Security\RemoteIpAddress;
+use OC\Security\Ip\RemoteAddress;
 use OCP\IConfig;
 use OCP\IRequest;
-use Psr\Log\LoggerInterface;
 
-class RemoteIpAddressTest extends \Test\TestCase {
+class RemoteAddressTest extends \Test\TestCase {
 	private IConfig $config;
 	private IRequest $request;
-	private LoggerInterface $logger;
-
-	private RemoteIpAddress $remoteIpAddress;
 
 	protected function setUp(): void {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
 		$this->request = $this->createMock(IRequest::class);
-		$this->logger = $this->createMock(LoggerInterface::class);
-		$this->remoteIpAddress = new RemoteIpAddress($this->config, $this->request, $this->logger);
 	}
 
 	/**
 	 * @param mixed $allowedRanges
 	 * @dataProvider dataProvider
 	 */
-	public function testEmptyConfig(string $remoteIp, $allowedRanges, bool $expected): void {
+	public function testAllowedIps(string $remoteIp, $allowedRanges, bool $expected): void {
 		$this->request
 			->method('getRemoteAddress')
 			->willReturn($remoteIp);
@@ -42,7 +36,9 @@ class RemoteIpAddressTest extends \Test\TestCase {
 		   ->with('allowed_admin_ranges', false)
 		   ->willReturn($allowedRanges);
 
-		$this->assertEquals($expected, $this->remoteIpAddress->allowsAdminActions());
+		$remoteAddress = new RemoteAddress($this->config, $this->request);
+
+		$this->assertEquals($expected, $remoteAddress->allowsAdminActions());
 	}
 
 	/**
@@ -50,6 +46,9 @@ class RemoteIpAddressTest extends \Test\TestCase {
 	 */
 	public function dataProvider(): array {
 		return [
+			// No IP (ie. CLI)
+			['', ['192.168.1.2/24'], true],
+			['', ['fe80/8'], true],
 			// No configuration
 			['1.2.3.4', false, true],
 			['1234:4567:8910::', false, true],
@@ -59,6 +58,10 @@ class RemoteIpAddressTest extends \Test\TestCase {
 			// Invalid configuration
 			['1.2.3.4', 'hello', true],
 			['1234:4567:8910::', 'world', true],
+			// Mixed configuration
+			['192.168.1.5', ['1.2.3.*', '1234::/8'], false],
+			['::1', ['127.0.0.1', '1234::/8'], false],
+			['192.168.1.5', ['192.168.1.0/24', '1234::/8'], true],
 			// Allowed IP
 			['1.2.3.4', ['1.2.3.*'], true],
 			['fc00:1:2:3::1', ['fc00::/7'], true],
@@ -66,9 +69,9 @@ class RemoteIpAddressTest extends \Test\TestCase {
 			['1234:4567:8910::1', ['fe80::/8','1234:4567::/16'], true],
 			// Blocked IP
 			['192.168.1.5', ['1.2.3.*'], false],
-			['9234:4567:8910::', ['1234:4567:*'], false],
+			['9234:4567:8910::', ['1234:4567::1'], false],
 			['192.168.2.1', ['192.168.1.2/24', '1.2.3.0/24'], false],
-			['9234:4567:8910::', ['fe80/8','1234:4567/16'], false],
+			['9234:4567:8910::', ['fe80::/8','1234:4567::/16'], false],
 		];
 	}
 }