feat(security): restrict admin actions to IP ranges

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

8 files changed, 139 insertions, 67 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index fbc61bcb1a6..9519dccd109 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -896,6 +896,7 @@ return array(
     'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
     'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
     'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
+    'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1814,6 +1815,7 @@ return array(
     'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
     'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php',
     'OC\\Security\\RemoteHostValidator' => $baseDir . '/lib/private/Security/RemoteHostValidator.php',
+    'OC\\Security\\RemoteIpAddress' => $baseDir . '/lib/private/Security/RemoteIpAddress.php',
     'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php',
     'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php',
     'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index dd25d62f7e3..1b886b0483e 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -929,6 +929,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
         'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
         'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
+        'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1847,6 +1848,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
         'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php',
         'OC\\Security\\RemoteHostValidator' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteHostValidator.php',
+        'OC\\Security\\RemoteIpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteIpAddress.php',
         'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php',
         'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php',
         'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php',
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index c25b6994b4f..3c5c6ad433c 100644
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -21,6 +21,7 @@ use OC\AppFramework\Utility\SimpleContainer;
 use OC\Core\Middleware\TwoFactorMiddleware;
 use OC\Diagnostics\EventLogger;
 use OC\Log\PsrLoggerAdapter;
+use OC\Security\RemoteIpAddress;
 use OC\ServerContainer;
 use OC\Settings\AuthorizedGroupMapper;
 use OCA\WorkflowEngine\Manager;
@@ -232,7 +233,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 				$server->getAppManager(),
 				$server->getL10N('lib'),
 				$c->get(AuthorizedGroupMapper::class),
-				$server->get(IUserSession::class)
+				$server->get(IUserSession::class),
+				$c->get(RemoteIpAddress::class),
 			);
 			$dispatcher->registerMiddleware($securityMiddleware);
 			$dispatcher->registerMiddleware(
diff --git a/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php b/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php
new file mode 100644
index 00000000000..36eb8f18928
--- /dev/null
+++ b/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+namespace OC\AppFramework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class AdminIpNotAllowed is thrown when a resource has been requested by a
+ * an admin user connecting from an unauthorized IP address
+ * See configuration `allowed_admin_ranges`
+ *
+ * @package OC\AppFramework\Middleware\Security\Exceptions
+ */
+class AdminIpNotAllowedException extends SecurityException {
+	public function __construct(string $message) {
+		parent::__construct($message, Http::STATUS_FORBIDDEN);
+	}
+}
diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index d7479f674d9..df20c131e03 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -8,6 +8,7 @@ declare(strict_types=1);
  */
 namespace OC\AppFramework\Middleware\Security;
 
+use OC\AppFramework\Middleware\Security\Exceptions\AdminIpNotAllowedException;
 use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException;
 use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
 use OC\AppFramework\Middleware\Security\Exceptions\ExAppRequiredException;
@@ -16,6 +17,7 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Security\RemoteIpAddress;
 use OC\Settings\AuthorizedGroupMapper;
 use OC\User\Session;
 use OCP\App\AppPathNotFoundException;
@@ -51,60 +53,22 @@ use ReflectionMethod;
  * check fails
  */
 class SecurityMiddleware extends Middleware {
-	/** @var INavigationManager */
-	private $navigationManager;
-	/** @var IRequest */
-	private $request;
-	/** @var ControllerMethodReflector */
-	private $reflector;
-	/** @var string */
-	private $appName;
-	/** @var IURLGenerator */
-	private $urlGenerator;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var bool */
-	private $isLoggedIn;
-	/** @var bool */
-	private $isAdminUser;
-	/** @var bool */
-	private $isSubAdmin;
-	/** @var IAppManager */
-	private $appManager;
-	/** @var IL10N */
-	private $l10n;
-	/** @var AuthorizedGroupMapper */
-	private $groupAuthorizationMapper;
-	/** @var IUserSession */
-	private $userSession;
-
-	public function __construct(IRequest $request,
-		ControllerMethodReflector $reflector,
-		INavigationManager $navigationManager,
-		IURLGenerator $urlGenerator,
-		LoggerInterface $logger,
-		string $appName,
-		bool $isLoggedIn,
-		bool $isAdminUser,
-		bool $isSubAdmin,
-		IAppManager $appManager,
-		IL10N $l10n,
-		AuthorizedGroupMapper $mapper,
-		IUserSession $userSession
+	public function __construct(
+		private IRequest $request,
+		private ControllerMethodReflector $reflector,
+		private INavigationManager $navigationManager,
+		private IURLGenerator $urlGenerator,
+		private LoggerInterface $logger,
+		private string $appName,
+		private bool $isLoggedIn,
+		private bool $isAdminUser,
+		private bool $isSubAdmin,
+		private IAppManager $appManager,
+		private IL10N $l10n,
+		private AuthorizedGroupMapper $groupAuthorizationMapper,
+		private IUserSession $userSession,
+		private RemoteIpAddress $remoteIpAddress,
 	) {
-		$this->navigationManager = $navigationManager;
-		$this->request = $request;
-		$this->reflector = $reflector;
-		$this->appName = $appName;
-		$this->urlGenerator = $urlGenerator;
-		$this->logger = $logger;
-		$this->isLoggedIn = $isLoggedIn;
-		$this->isAdminUser = $isAdminUser;
-		$this->isSubAdmin = $isSubAdmin;
-		$this->appManager = $appManager;
-		$this->l10n = $l10n;
-		$this->groupAuthorizationMapper = $mapper;
-		$this->userSession = $userSession;
 	}
 
 	/**
@@ -170,6 +134,9 @@ class SecurityMiddleware extends Middleware {
 				if (!$authorized) {
 					throw new NotAdminException($this->l10n->t('Logged in account must be an admin, a sub admin or gotten special right to access this setting'));
 				}
+				if (!$this->remoteIpAddress->allowsAdminActions()) {
+					throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+				}
 			}
 			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
 				&& !$this->isSubAdmin
@@ -183,6 +150,16 @@ class SecurityMiddleware extends Middleware {
 				&& !$authorized) {
 				throw new NotAdminException($this->l10n->t('Logged in account must be an admin'));
 			}
+			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
+				&& !$this->remoteIpAddress->allowsAdminActions()) {
+				throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+			}
+			if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
+				&& !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class)
+				&& !$this->remoteIpAddress->allowsAdminActions()) {
+				throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+			}
+
 		}
 
 		// Check for strict cookie requirement
diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index 0ab64907c8b..67d6f8cbe9c 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -8,6 +8,7 @@
 namespace OC\Group;
 
 use OC\Hooks\PublicEmitter;
+use OC\Security\RemoteIpAddress;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Group\Backend\IBatchMethodsBackend;
 use OCP\Group\Backend\ICreateNamedGroupBackend;
@@ -41,11 +42,6 @@ class Manager extends PublicEmitter implements IGroupManager {
 	/** @var GroupInterface[] */
 	private $backends = [];
 
-	/** @var \OC\User\Manager */
-	private $userManager;
-	private IEventDispatcher $dispatcher;
-	private LoggerInterface $logger;
-
 	/** @var array<string, IGroup> */
 	private $cachedGroups = [];
 
@@ -59,13 +55,13 @@ class Manager extends PublicEmitter implements IGroupManager {
 
 	private const MAX_GROUP_LENGTH = 255;
 
-	public function __construct(\OC\User\Manager $userManager,
-		IEventDispatcher $dispatcher,
-		LoggerInterface $logger,
-		ICacheFactory $cacheFactory) {
-		$this->userManager = $userManager;
-		$this->dispatcher = $dispatcher;
-		$this->logger = $logger;
+	public function __construct(
+		private \OC\User\Manager $userManager,
+		private IEventDispatcher $dispatcher,
+		private LoggerInterface $logger,
+		ICacheFactory $cacheFactory,
+		private RemoteIpAddress $remoteIpAddress,
+	) {
 		$this->displayNameCache = new DisplayNameCache($cacheFactory, $this);
 
 		$this->listen('\OC\Group', 'postDelete', function (IGroup $group): void {
@@ -325,6 +321,10 @@ class Manager extends PublicEmitter implements IGroupManager {
 	 * @return bool if admin
 	 */
 	public function isAdmin($userId) {
+		if (!$this->remoteIpAddress->allowsAdminActions()) {
+			return false;
+		}
+
 		foreach ($this->backends as $backend) {
 			if (is_string($userId) && $backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) {
 				return true;
diff --git a/lib/private/Security/RemoteIpAddress.php b/lib/private/Security/RemoteIpAddress.php
new file mode 100644
index 00000000000..ef9debf4a65
--- /dev/null
+++ b/lib/private/Security/RemoteIpAddress.php
@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security;
+
+use IPLib\Factory;
+use OCP\IConfig;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
+
+class RemoteIpAddress {
+	public const SETTING_NAME = 'allowed_admin_ranges';
+
+	public function __construct(
+		private IConfig $config,
+		private IRequest $request,
+		private LoggerInterface $logger,
+	) {
+	}
+
+	public function allowsAdminActions(): bool {
+		$allowedAdminRanges = $this->config->getSystemValue(self::SETTING_NAME, false);
+		if ($allowedAdminRanges === false) {
+			// No restriction applied
+			return true;
+		}
+
+		if (!is_array($allowedAdminRanges)) {
+			return true;
+		}
+
+		if (empty($allowedAdminRanges)) {
+			return true;
+		}
+
+		$ipAddress = Factory::parseAddressString($this->request->getRemoteAddress());
+		if ($ipAddress === null) {
+			$this->logger->warning(
+				'Unable to parse remote IP "{ip}"',
+				['ip' => $ipAddress,]
+			);
+
+			return false;
+		}
+
+		foreach ($allowedAdminRanges as $rangeString) {
+			$range = Factory::parseRangeString($rangeString);
+			if ($range === null) {
+				continue;
+			}
+			if ($range->contains($ipAddress)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 4d549918a8f..4e29b4c6ec0 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -101,6 +101,7 @@ use OC\Security\CSRF\CsrfTokenManager;
 use OC\Security\CSRF\TokenStorage\SessionStorage;
 use OC\Security\Hasher;
 use OC\Security\RateLimiting\Limiter;
+use OC\Security\RemoteIpAddress;
 use OC\Security\SecureRandom;
 use OC\Security\TrustedDomainHelper;
 use OC\Security\VerificationToken\VerificationToken;
@@ -464,7 +465,8 @@ class Server extends ServerContainer implements IServerContainer {
 				$this->get(IUserManager::class),
 				$this->get(IEventDispatcher::class),
 				$this->get(LoggerInterface::class),
-				$this->get(ICacheFactory::class)
+				$this->get(ICacheFactory::class),
+				$this->get(RemoteIpAddress::class),
 			);
 			return $groupManager;
 		});