aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndy Scherzinger <info@andy-scherzinger.de>2024-07-22 10:10:42 +0200
committerGitHub <noreply@github.com>2024-07-22 10:10:42 +0200
commitc2a571e435bebb08a4b6429eea343c350d3ccaf6 (patch)
tree9c1c4912147beb66b13d442e57cd8614451fa44e
parent800dffec31b76a1c6b371d57d41ea9f5085a4a6e (diff)
parentf1d97a318818860d3fff9fccffbab5a1faba752b (diff)
downloadnextcloud-server-c2a571e435bebb08a4b6429eea343c350d3ccaf6.tar.gz
nextcloud-server-c2a571e435bebb08a4b6429eea343c350d3ccaf6.zip
Merge pull request #46473 from nextcloud/feat/restrict_admin_to_ips
feat(security): restrict admin actions to IP ranges
-rw-r--r--apps/settings/composer/composer/autoload_classmap.php1
-rw-r--r--apps/settings/composer/composer/autoload_static.php1
-rw-r--r--apps/settings/composer/composer/installed.php4
-rw-r--r--apps/settings/lib/AppInfo/Application.php2
-rw-r--r--apps/settings/lib/SetupChecks/AllowedAdminRanges.php63
-rw-r--r--config/config.sample.php10
-rw-r--r--lib/composer/composer/autoload_classmap.php9
-rw-r--r--lib/composer/composer/autoload_static.php9
-rw-r--r--lib/private/AppFramework/DependencyInjection/DIContainer.php4
-rw-r--r--lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php23
-rw-r--r--lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php83
-rw-r--r--lib/private/Group/Manager.php24
-rw-r--r--lib/private/Security/Ip/Address.php48
-rw-r--r--lib/private/Security/Ip/Factory.php23
-rw-r--r--lib/private/Security/Ip/Range.php39
-rw-r--r--lib/private/Security/Ip/RemoteAddress.php71
-rw-r--r--lib/private/Server.php9
-rw-r--r--lib/public/Security/Ip/IAddress.php35
-rw-r--r--lib/public/Security/Ip/IFactory.php30
-rw-r--r--lib/public/Security/Ip/IRange.php37
-rw-r--r--lib/public/Security/Ip/IRemoteAddress.php22
-rw-r--r--tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php6
-rw-r--r--tests/lib/Group/ManagerTest.php67
-rw-r--r--tests/lib/Security/Ip/RemoteAddressTest.php77
24 files changed, 596 insertions, 101 deletions
diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php
index 488aaee264d..27c1496008e 100644
--- a/apps/settings/composer/composer/autoload_classmap.php
+++ b/apps/settings/composer/composer/autoload_classmap.php
@@ -78,6 +78,7 @@ return array(
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php',
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => $baseDir . '/../lib/Settings/Personal/Security/WebAuthn.php',
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/../lib/Settings/Personal/ServerDevNotice.php',
+ 'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => $baseDir . '/../lib/SetupChecks/AllowedAdminRanges.php',
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => $baseDir . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php',
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => $baseDir . '/../lib/SetupChecks/CheckServerResponseTrait.php',
diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php
index ac2e4645239..14e4c362887 100644
--- a/apps/settings/composer/composer/autoload_static.php
+++ b/apps/settings/composer/composer/autoload_static.php
@@ -93,6 +93,7 @@ class ComposerStaticInitSettings
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php',
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/WebAuthn.php',
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/..' . '/../lib/Settings/Personal/ServerDevNotice.php',
+ 'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => __DIR__ . '/..' . '/../lib/SetupChecks/AllowedAdminRanges.php',
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => __DIR__ . '/..' . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php',
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckServerResponseTrait.php',
diff --git a/apps/settings/composer/composer/installed.php b/apps/settings/composer/composer/installed.php
index d2b87e1bdfd..651d70adcf8 100644
--- a/apps/settings/composer/composer/installed.php
+++ b/apps/settings/composer/composer/installed.php
@@ -3,7 +3,7 @@
'name' => '__root__',
'pretty_version' => 'dev-master',
'version' => 'dev-master',
- 'reference' => '4ff660ca2e0baa02440ba07296ed7e75fa544c0e',
+ 'reference' => '071fe73d0a28f44c6e24cc87fbd00e54a3b92b57',
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
@@ -13,7 +13,7 @@
'__root__' => array(
'pretty_version' => 'dev-master',
'version' => 'dev-master',
- 'reference' => '4ff660ca2e0baa02440ba07296ed7e75fa544c0e',
+ 'reference' => '071fe73d0a28f44c6e24cc87fbd00e54a3b92b57',
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php
index f62555f0cdb..80420cb3335 100644
--- a/apps/settings/lib/AppInfo/Application.php
+++ b/apps/settings/lib/AppInfo/Application.php
@@ -22,6 +22,7 @@ use OCA\Settings\Middleware\SubadminMiddleware;
use OCA\Settings\Search\AppSearch;
use OCA\Settings\Search\SectionSearch;
use OCA\Settings\Search\UserSearch;
+use OCA\Settings\SetupChecks\AllowedAdminRanges;
use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner;
use OCA\Settings\SetupChecks\BruteForceThrottler;
use OCA\Settings\SetupChecks\CheckUserCertificates;
@@ -154,6 +155,7 @@ class Application extends App implements IBootstrap {
Util::getDefaultEmailAddress('no-reply')
);
});
+ $context->registerSetupCheck(AllowedAdminRanges::class);
$context->registerSetupCheck(AppDirsWithDifferentOwner::class);
$context->registerSetupCheck(BruteForceThrottler::class);
$context->registerSetupCheck(CheckUserCertificates::class);
diff --git a/apps/settings/lib/SetupChecks/AllowedAdminRanges.php b/apps/settings/lib/SetupChecks/AllowedAdminRanges.php
new file mode 100644
index 00000000000..87e11b06be7
--- /dev/null
+++ b/apps/settings/lib/SetupChecks/AllowedAdminRanges.php
@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Settings\SetupChecks;
+
+use OC\Security\Ip\Range;
+use OC\Security\Ip\RemoteAddress;
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\SetupCheck\ISetupCheck;
+use OCP\SetupCheck\SetupResult;
+
+class AllowedAdminRanges implements ISetupCheck {
+ public function __construct(
+ private IConfig $config,
+ private IL10N $l10n,
+ ) {
+ }
+
+ public function getCategory(): string {
+ return 'system';
+ }
+
+ public function getName(): string {
+ return $this->l10n->t('Allowed admin IP ranges');
+ }
+
+ public function run(): SetupResult {
+ $allowedAdminRanges = $this->config->getSystemValue(RemoteAddress::SETTING_NAME, false);
+ if (
+ $allowedAdminRanges === false
+ || (is_array($allowedAdminRanges) && empty($allowedAdminRanges))
+ ) {
+ return SetupResult::success($this->l10n->t('Admin IP filtering isn’t applied.'));
+ }
+
+ if (!is_array($allowedAdminRanges)) {
+ return SetupResult::error(
+ $this->l10n->t(
+ 'Configuration key "%1$s" expects an array (%2$s found). Admin IP range validation will not be applied.',
+ [RemoteAddress::SETTING_NAME, gettype($allowedAdminRanges)],
+ )
+ );
+ }
+
+ $invalidRanges = array_filter($allowedAdminRanges, static fn (mixed $range): bool => !is_string($range) || !Range::isValid($range));
+ if (!empty($invalidRanges)) {
+ return SetupResult::warning(
+ $this->l10n->t(
+ 'Configuration key "%1$s" contains invalid IP range(s): "%2$s"',
+ [RemoteAddress::SETTING_NAME, implode('", "', $invalidRanges)],
+ ),
+ );
+ }
+
+ return SetupResult::success($this->l10n->t('Admin IP filtering is correctly configured.'));
+ }
+}
diff --git a/config/config.sample.php b/config/config.sample.php
index 67110a1844a..9840fcfc97c 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -2208,6 +2208,16 @@ $CONFIG = [
'forwarded_for_headers' => ['HTTP_X_FORWARDED', 'HTTP_FORWARDED_FOR'],
/**
+ * List of trusted IP ranges for admin actions
+ *
+ * If this list is non-empty, all admin actions must be triggered from
+ * IP addresses inside theses ranges.
+ *
+ * Defaults to an empty array.
+ */
+'allowed_admin_ranges' => ['192.0.2.42/32', '233.252.0.0/24', '2001:db8::13:37/64'],
+
+/**
* max file size for animating gifs on public-sharing-site.
* If the gif is bigger, it'll show a static preview
*
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index fbc61bcb1a6..aa6e50eab20 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -644,6 +644,10 @@ return array(
'OCP\\Security\\IRemoteHostValidator' => $baseDir . '/lib/public/Security/IRemoteHostValidator.php',
'OCP\\Security\\ISecureRandom' => $baseDir . '/lib/public/Security/ISecureRandom.php',
'OCP\\Security\\ITrustedDomainHelper' => $baseDir . '/lib/public/Security/ITrustedDomainHelper.php',
+ 'OCP\\Security\\Ip\\IAddress' => $baseDir . '/lib/public/Security/Ip/IAddress.php',
+ 'OCP\\Security\\Ip\\IFactory' => $baseDir . '/lib/public/Security/Ip/IFactory.php',
+ 'OCP\\Security\\Ip\\IRange' => $baseDir . '/lib/public/Security/Ip/IRange.php',
+ 'OCP\\Security\\Ip\\IRemoteAddress' => $baseDir . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\RateLimiting\\ILimiter' => $baseDir . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => $baseDir . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php',
@@ -896,6 +900,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',
@@ -1807,6 +1812,10 @@ return array(
'OC\\Security\\IdentityProof\\Key' => $baseDir . '/lib/private/Security/IdentityProof/Key.php',
'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php',
'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php',
+ 'OC\\Security\\Ip\\Address' => $baseDir . '/lib/private/Security/Ip/Address.php',
+ 'OC\\Security\\Ip\\Factory' => $baseDir . '/lib/private/Security/Ip/Factory.php',
+ 'OC\\Security\\Ip\\Range' => $baseDir . '/lib/private/Security/Ip/Range.php',
+ 'OC\\Security\\Ip\\RemoteAddress' => $baseDir . '/lib/private/Security/Ip/RemoteAddress.php',
'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php',
'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index dd25d62f7e3..5c0535b4362 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -677,6 +677,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Security\\IRemoteHostValidator' => __DIR__ . '/../../..' . '/lib/public/Security/IRemoteHostValidator.php',
'OCP\\Security\\ISecureRandom' => __DIR__ . '/../../..' . '/lib/public/Security/ISecureRandom.php',
'OCP\\Security\\ITrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/public/Security/ITrustedDomainHelper.php',
+ 'OCP\\Security\\Ip\\IAddress' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IAddress.php',
+ 'OCP\\Security\\Ip\\IFactory' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IFactory.php',
+ 'OCP\\Security\\Ip\\IRange' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRange.php',
+ 'OCP\\Security\\Ip\\IRemoteAddress' => __DIR__ . '/../../..' . '/lib/public/Security/Ip/IRemoteAddress.php',
'OCP\\Security\\RateLimiting\\ILimiter' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/ILimiter.php',
'OCP\\Security\\RateLimiting\\IRateLimitExceededException' => __DIR__ . '/../../..' . '/lib/public/Security/RateLimiting/IRateLimitExceededException.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php',
@@ -929,6 +933,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',
@@ -1840,6 +1845,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Security\\IdentityProof\\Key' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Key.php',
'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php',
'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php',
+ 'OC\\Security\\Ip\\Address' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Address.php',
+ 'OC\\Security\\Ip\\Factory' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Factory.php',
+ 'OC\\Security\\Ip\\Range' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Range.php',
+ 'OC\\Security\\Ip\\RemoteAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/RemoteAddress.php',
'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php',
'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index c25b6994b4f..81365900d9b 100644
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -46,6 +46,7 @@ use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\Ip\IRemoteAddress;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
@@ -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(IRemoteAddress::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..b8de09072ce 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;
@@ -40,6 +41,7 @@ use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUserSession;
+use OCP\Security\Ip\IRemoteAddress;
use OCP\Util;
use Psr\Log\LoggerInterface;
use ReflectionMethod;
@@ -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 IRemoteAddress $remoteAddress,
) {
- $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->remoteAddress->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->remoteAddress->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->remoteAddress->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..092c56fe00a 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -19,6 +19,7 @@ use OCP\ICacheFactory;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
+use OCP\Security\Ip\IRemoteAddress;
use Psr\Log\LoggerInterface;
use function is_string;
@@ -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 IRemoteAddress $remoteAddress,
+ ) {
$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->remoteAddress->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/Ip/Address.php b/lib/private/Security/Ip/Address.php
new file mode 100644
index 00000000000..b4f12a7e295
--- /dev/null
+++ b/lib/private/Security/Ip/Address.php
@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security\Ip;
+
+use InvalidArgumentException;
+use IPLib\Address\AddressInterface;
+use IPLib\Factory;
+use OCP\Security\Ip\IAddress;
+use OCP\Security\Ip\IRange;
+
+/**
+ * @since 30.0.0
+ */
+class Address implements IAddress {
+ private readonly AddressInterface $ip;
+
+ public function __construct(string $ip) {
+ $ip = Factory::parseAddressString($ip);
+ if ($ip === null) {
+ throw new InvalidArgumentException('Given IP address can’t be parsed');
+ }
+ $this->ip = $ip;
+ }
+
+ public static function isValid(string $ip): bool {
+ return Factory::parseAddressString($ip) !== null;
+ }
+
+ public function matches(IRange... $ranges): bool {
+ foreach($ranges as $range) {
+ if ($range->contains($this)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ public function __toString(): string {
+ return $this->ip->toString();
+ }
+}
diff --git a/lib/private/Security/Ip/Factory.php b/lib/private/Security/Ip/Factory.php
new file mode 100644
index 00000000000..1eedcf27a09
--- /dev/null
+++ b/lib/private/Security/Ip/Factory.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security\Ip;
+
+use OCP\Security\Ip\IAddress;
+use OCP\Security\Ip\IFactory;
+use OCP\Security\Ip\IRange;
+
+class Factory implements IFactory {
+ public function rangeFromString(string $range): IRange {
+ return new Range($range);
+ }
+
+ public function addressFromString(string $ip): IAddress {
+ return new Address($ip);
+ }
+}
diff --git a/lib/private/Security/Ip/Range.php b/lib/private/Security/Ip/Range.php
new file mode 100644
index 00000000000..4f960166d6a
--- /dev/null
+++ b/lib/private/Security/Ip/Range.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security\Ip;
+
+use InvalidArgumentException;
+use IPLib\Factory;
+use IPLib\Range\RangeInterface;
+use OCP\Security\Ip\IAddress;
+use OCP\Security\Ip\IRange;
+
+class Range implements IRange {
+ private readonly RangeInterface $range;
+
+ public function __construct(string $range) {
+ $range = Factory::parseRangeString($range);
+ if ($range === null) {
+ throw new InvalidArgumentException('Given range can’t be parsed');
+ }
+ $this->range = $range;
+ }
+
+ public static function isValid(string $range): bool {
+ return Factory::parseRangeString($range) !== null;
+ }
+
+ public function contains(IAddress $address): bool {
+ return $this->range->contains(Factory::parseAddressString((string) $address));
+ }
+
+ public function __toString(): string {
+ return $this->range->toString();
+ }
+}
diff --git a/lib/private/Security/Ip/RemoteAddress.php b/lib/private/Security/Ip/RemoteAddress.php
new file mode 100644
index 00000000000..54cdb96132a
--- /dev/null
+++ b/lib/private/Security/Ip/RemoteAddress.php
@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security\Ip;
+
+use OCP\IConfig;
+use OCP\IRequest;
+use OCP\Security\Ip\IAddress;
+use OCP\Security\Ip\IRange;
+use OCP\Security\Ip\IRemoteAddress;
+
+class RemoteAddress implements IRemoteAddress, IAddress {
+ public const SETTING_NAME = 'allowed_admin_ranges';
+
+ private readonly ?IAddress $ip;
+
+ public function __construct(
+ private IConfig $config,
+ IRequest $request,
+ ) {
+ $remoteAddress = $request->getRemoteAddress();
+ $this->ip = $remoteAddress === ''
+ ? null
+ : new Address($remoteAddress);
+ }
+
+ public static function isValid(string $ip): bool {
+ return Address::isValid($ip);
+ }
+
+ public function matches(IRange... $ranges): bool {
+ return $this->ip === null
+ ? true
+ : $this->ip->matches(... $ranges);
+ }
+
+ public function allowsAdminActions(): bool {
+ if ($this->ip === null) {
+ return true;
+ }
+
+ $allowedAdminRanges = $this->config->getSystemValue(self::SETTING_NAME, false);
+
+ // Don't apply restrictions on empty or invalid configuration
+ if (
+ $allowedAdminRanges === false
+ || !is_array($allowedAdminRanges)
+ || empty($allowedAdminRanges)
+ ) {
+ return true;
+ }
+
+ foreach ($allowedAdminRanges as $allowedAdminRange) {
+ if ((new Range($allowedAdminRange))->contains($this->ip)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ public function __toString(): string {
+ return (string) $this->ip;
+ }
+}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 4d549918a8f..cfbb6dc317f 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -100,6 +100,7 @@ use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager;
use OC\Security\CSRF\TokenStorage\SessionStorage;
use OC\Security\Hasher;
+use OC\Security\Ip\RemoteAddress;
use OC\Security\RateLimiting\Limiter;
use OC\Security\SecureRandom;
use OC\Security\TrustedDomainHelper;
@@ -213,6 +214,7 @@ use OCP\Security\IContentSecurityPolicyManager;
use OCP\Security\ICredentialsManager;
use OCP\Security\ICrypto;
use OCP\Security\IHasher;
+use OCP\Security\Ip\IRemoteAddress;
use OCP\Security\ISecureRandom;
use OCP\Security\ITrustedDomainHelper;
use OCP\Security\RateLimiting\ILimiter;
@@ -464,7 +466,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(IRemoteAddress::class),
);
return $groupManager;
});
@@ -1403,6 +1406,10 @@ class Server extends ServerContainer implements IServerContainer {
$this->registerAlias(\OCP\TaskProcessing\IManager::class, \OC\TaskProcessing\Manager::class);
+ $this->registerAlias(IRemoteAddress::class, RemoteAddress::class);
+
+ $this->registerAlias(\OCP\Security\Ip\IFactory::class, \OC\Security\Ip\Factory::class);
+
$this->connectDispatcher();
}
diff --git a/lib/public/Security/Ip/IAddress.php b/lib/public/Security/Ip/IAddress.php
new file mode 100644
index 00000000000..242418962fc
--- /dev/null
+++ b/lib/public/Security/Ip/IAddress.php
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCP\Security\Ip;
+
+/**
+ * @since 30.0.0
+ */
+interface IAddress {
+ /**
+ * Check if a given IP address is valid
+ *
+ * @since 30.0.0
+ */
+ public static function isValid(string $ip): bool;
+
+ /**
+ * Check if current address is contained by given ranges
+ *
+ * @since 30.0.0
+ */
+ public function matches(IRange... $ranges): bool;
+
+ /**
+ * Normalized IP address
+ *
+ * @since 30.0.0
+ */
+ public function __toString(): string;
+}
diff --git a/lib/public/Security/Ip/IFactory.php b/lib/public/Security/Ip/IFactory.php
new file mode 100644
index 00000000000..3b88aa8c756
--- /dev/null
+++ b/lib/public/Security/Ip/IFactory.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCP\Security\Ip;
+
+/**
+ * @since 30.0.0
+ */
+interface IFactory {
+ /**
+ * Creates a range from string
+ *
+ * @since 30.0.0
+ * @throws \InvalidArgumentException on invalid range
+ */
+ public function rangeFromString(string $range): IRange;
+
+ /**
+ * Creates a address from string
+ *
+ * @since 30.0.0
+ * @throws \InvalidArgumentException on invalid IP
+ */
+ public function addressFromString(string $ip): IAddress;
+}
diff --git a/lib/public/Security/Ip/IRange.php b/lib/public/Security/Ip/IRange.php
new file mode 100644
index 00000000000..70e1815c75e
--- /dev/null
+++ b/lib/public/Security/Ip/IRange.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCP\Security\Ip;
+
+/**
+ * IP Range (IPv4 or IPv6)
+ *
+ * @since 30.0.0
+ */
+interface IRange {
+ /**
+ * Check if a given range is valid
+ *
+ * @since 30.0.0
+ */
+ public static function isValid(string $range): bool;
+
+ /**
+ * Check if an address is in the current range
+ *
+ * @since 30.0.0
+ */
+ public function contains(IAddress $address): bool;
+
+ /**
+ * Normalized IP range
+ *
+ * @since 30.0.0
+ */
+ public function __toString(): string;
+}
diff --git a/lib/public/Security/Ip/IRemoteAddress.php b/lib/public/Security/Ip/IRemoteAddress.php
new file mode 100644
index 00000000000..19a1dab9734
--- /dev/null
+++ b/lib/public/Security/Ip/IRemoteAddress.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCP\Security\Ip;
+
+/**
+ * IP address of the connected client
+ *
+ * @since 30.0.0
+ */
+interface IRemoteAddress {
+ /**
+ * Check if the current remote address is allowed to perform admin actions
+ * @since 30.0.0
+ */
+ public function allowsAdminActions(): bool;
+}
diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index bda71c4e8ed..fb457579fac 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -32,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;
@@ -90,6 +91,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->appManager->expects($this->any())
->method('isEnabledForUser')
->willReturn($isAppEnabledForUser);
+ $remoteIpAddress = $this->createMock(IRemoteAddress::class);
+ $remoteIpAddress->method('allowsAdminActions')->willReturn(true);
return new SecurityMiddleware(
$this->request,
@@ -104,7 +107,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->appManager,
$this->l10n,
$this->authorizedGroupMapper,
- $this->userSession
+ $this->userSession,
+ $remoteIpAddress
);
}
diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 81fd77cc78a..4f42e10537d 100644
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -16,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;
@@ -32,6 +33,8 @@ class ManagerTest extends TestCase {
protected $logger;
/** @var ICacheFactory|MockObject */
private $cache;
+ /** @var IRemoteAddress|MockObject */
+ private $remoteIpAddress;
protected function setUp(): void {
parent::setUp();
@@ -40,6 +43,9 @@ class ManagerTest extends TestCase {
$this->dispatcher = $this->createMock(IEventDispatcher::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->cache = $this->createMock(ICacheFactory::class);
+
+ $this->remoteIpAddress = $this->createMock(IRemoteAddress::class);
+ $this->remoteIpAddress->method('allowsAdminActions')->willReturn(true);
}
private function getTestUser($userId) {
@@ -103,7 +109,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$group = $manager->get('group1');
@@ -112,7 +118,7 @@ class ManagerTest extends TestCase {
}
public function testGetNoBackend() {
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$this->assertNull($manager->get('group1'));
}
@@ -127,7 +133,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(false);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$this->assertNull($manager->get('group1'));
@@ -137,7 +143,7 @@ class ManagerTest extends TestCase {
$backend = new \Test\Util\Group\Dummy();
$backend->createGroup('group1');
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$group = $manager->get('group1');
@@ -164,7 +170,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend1);
$manager->addBackend($backend2);
@@ -190,7 +196,7 @@ class ManagerTest extends TestCase {
return true;
});
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$group = $manager->createGroup('group1');
@@ -219,7 +225,7 @@ class ManagerTest extends TestCase {
->method('getGroupDetails')
->willReturn([]);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$group = $manager->createGroup('group1');
@@ -243,7 +249,7 @@ class ManagerTest extends TestCase {
->with($groupName)
->willReturn(false);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$this->expectException(\Exception::class);
@@ -260,7 +266,7 @@ class ManagerTest extends TestCase {
$backend->expects($this->never())
->method('createGroup');
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$group = $manager->createGroup('group1');
@@ -281,7 +287,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$groups = $manager->search('1');
@@ -315,7 +321,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend1);
$manager->addBackend($backend2);
@@ -352,7 +358,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend1);
$manager->addBackend($backend2);
@@ -381,7 +387,7 @@ class ManagerTest extends TestCase {
/** @var \OC\User\Manager $userManager */
$userManager = $this->createMock(Manager::class);
- $manager = new \OC\Group\Manager($userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$groups = $manager->search('1');
@@ -402,7 +408,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$groups = $manager->getUserGroups($this->getTestUser('user1'));
@@ -420,7 +426,7 @@ class ManagerTest extends TestCase {
->with('myUID')
->willReturn(['123', 'abc']);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
/** @var \OC\User\User|\PHPUnit\Framework\MockObject\MockObject $user */
@@ -450,7 +456,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(false);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
/** @var \OC\User\User|\PHPUnit\Framework\MockObject\MockObject $user */
@@ -476,7 +482,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$this->assertTrue($manager->isInGroup('user1', 'group1'));
@@ -495,7 +501,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$this->assertTrue($manager->isAdmin('user1'));
@@ -514,7 +520,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$this->assertFalse($manager->isAdmin('user1'));
@@ -545,7 +551,7 @@ class ManagerTest extends TestCase {
->method('groupExists')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend1);
$manager->addBackend($backend2);
@@ -604,7 +610,7 @@ class ManagerTest extends TestCase {
}
});
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3');
@@ -664,7 +670,7 @@ class ManagerTest extends TestCase {
}
});
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1);
@@ -728,7 +734,7 @@ class ManagerTest extends TestCase {
}
});
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1);
@@ -757,7 +763,7 @@ class ManagerTest extends TestCase {
$this->userManager->expects($this->never())->method('get');
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '');
@@ -785,7 +791,7 @@ class ManagerTest extends TestCase {
$this->userManager->expects($this->never())->method('get');
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1);
@@ -813,7 +819,7 @@ class ManagerTest extends TestCase {
$this->userManager->expects($this->never())->method('get');
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1, 1);
@@ -841,7 +847,7 @@ class ManagerTest extends TestCase {
->with('group1')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
// prime cache
@@ -884,7 +890,7 @@ class ManagerTest extends TestCase {
->method('removeFromGroup')
->willReturn(true);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
// prime cache
@@ -914,7 +920,7 @@ class ManagerTest extends TestCase {
->with('user1')
->willReturn(null);
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
$groups = $manager->getUserIdGroups('user1');
@@ -939,8 +945,7 @@ class ManagerTest extends TestCase {
['group1', ['gid' => 'group1', 'displayName' => 'Group One']],
['group2', ['gid' => 'group2']],
]);
-
- $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
+ $manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
$manager->addBackend($backend);
// group with display name
diff --git a/tests/lib/Security/Ip/RemoteAddressTest.php b/tests/lib/Security/Ip/RemoteAddressTest.php
new file mode 100644
index 00000000000..22f38f62356
--- /dev/null
+++ b/tests/lib/Security/Ip/RemoteAddressTest.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test\Security\Ip;
+
+use OC\Security\Ip\RemoteAddress;
+use OCP\IConfig;
+use OCP\IRequest;
+
+class RemoteAddressTest extends \Test\TestCase {
+ private IConfig $config;
+ private IRequest $request;
+
+ protected function setUp(): void {
+ parent::setUp();
+ $this->config = $this->createMock(IConfig::class);
+ $this->request = $this->createMock(IRequest::class);
+ }
+
+ /**
+ * @param mixed $allowedRanges
+ * @dataProvider dataProvider
+ */
+ public function testAllowedIps(string $remoteIp, $allowedRanges, bool $expected): void {
+ $this->request
+ ->method('getRemoteAddress')
+ ->willReturn($remoteIp);
+ $this->config
+ ->method('getSystemValue')
+ ->with('allowed_admin_ranges', false)
+ ->willReturn($allowedRanges);
+
+ $remoteAddress = new RemoteAddress($this->config, $this->request);
+
+ $this->assertEquals($expected, $remoteAddress->allowsAdminActions());
+ }
+
+ /**
+ * @return array<string, mixed, bool>
+ */
+ 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],
+ // Empty configuration
+ ['1.2.3.4', [], true],
+ ['1234:4567:8910::', [], true],
+ // 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],
+ ['1.2.3.4', ['192.168.1.2/24', '1.2.3.0/24'], true],
+ ['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::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],
+ ];
+ }
+}