diff options
author | Andy Scherzinger <info@andy-scherzinger.de> | 2024-07-22 10:10:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-22 10:10:42 +0200 |
commit | c2a571e435bebb08a4b6429eea343c350d3ccaf6 (patch) | |
tree | 9c1c4912147beb66b13d442e57cd8614451fa44e | |
parent | 800dffec31b76a1c6b371d57d41ea9f5085a4a6e (diff) | |
parent | f1d97a318818860d3fff9fccffbab5a1faba752b (diff) | |
download | nextcloud-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
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], + ]; + } +} |