From 7ed583cb8ee64f77696b0e23f79d8d1b4038bcbc Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Thu, 12 Sep 2024 16:17:19 +0200 Subject: chore: Migrate cleanAppId and getAppPath calls to IAppManager from OC_App MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/App/AppManager.php | 9 ++++--- .../TwoFactorAuth/ProviderLoader.php | 30 ++++++++-------------- lib/private/L10N/Factory.php | 2 +- lib/private/Route/Router.php | 26 ++++++++++++------- lib/private/legacy/OC_App.php | 3 +-- 5 files changed, 36 insertions(+), 34 deletions(-) (limited to 'lib/private') diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index fe2f7b74b22..974545cfe92 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -647,11 +647,9 @@ class AppManager implements IAppManager { /** * Get the directory for the given app. * - * @param string $appId - * @return string * @throws AppPathNotFoundException if app folder can't be found */ - public function getAppPath($appId) { + public function getAppPath(string $appId): string { $appPath = \OC_App::getAppPath($appId); if ($appPath === false) { throw new AppPathNotFoundException('Could not find path for ' . $appId); @@ -877,4 +875,9 @@ class AppManager implements IAppManager { return false; } + + public function cleanAppId(string $app): string { + // FIXME should list allowed characters instead + return str_replace(['<', '>', '"', "'", '\0', '/', '\\', '..'], '', $app); + } } diff --git a/lib/private/Authentication/TwoFactorAuth/ProviderLoader.php b/lib/private/Authentication/TwoFactorAuth/ProviderLoader.php index b9a0a97bec4..7e674a01dd8 100644 --- a/lib/private/Authentication/TwoFactorAuth/ProviderLoader.php +++ b/lib/private/Authentication/TwoFactorAuth/ProviderLoader.php @@ -9,8 +9,7 @@ declare(strict_types=1); namespace OC\Authentication\TwoFactorAuth; use Exception; -use OC; -use OC_App; +use OC\AppFramework\Bootstrap\Coordinator; use OCP\App\IAppManager; use OCP\AppFramework\QueryException; use OCP\Authentication\TwoFactorAuth\IProvider; @@ -19,15 +18,10 @@ use OCP\IUser; class ProviderLoader { public const BACKUP_CODES_APP_ID = 'twofactor_backupcodes'; - /** @var IAppManager */ - private $appManager; - - /** @var OC\AppFramework\Bootstrap\Coordinator */ - private $coordinator; - - public function __construct(IAppManager $appManager, OC\AppFramework\Bootstrap\Coordinator $coordinator) { - $this->appManager = $appManager; - $this->coordinator = $coordinator; + public function __construct( + private IAppManager $appManager, + private Coordinator $coordinator, + ) { } /** @@ -58,12 +52,12 @@ class ProviderLoader { } } - $registeredProviders = $this->coordinator->getRegistrationContext()->getTwoFactorProviders(); + $registeredProviders = $this->coordinator->getRegistrationContext()?->getTwoFactorProviders() ?? []; foreach ($registeredProviders as $provider) { try { $this->loadTwoFactorApp($provider->getAppId()); - $provider = \OCP\Server::get($provider->getService()); - $providers[$provider->getId()] = $provider; + $providerInstance = \OCP\Server::get($provider->getService()); + $providers[$providerInstance->getId()] = $providerInstance; } catch (QueryException $exc) { // Provider class can not be resolved throw new Exception('Could not load two-factor auth provider ' . $provider->getService()); @@ -75,12 +69,10 @@ class ProviderLoader { /** * Load an app by ID if it has not been loaded yet - * - * @param string $appId */ - protected function loadTwoFactorApp(string $appId) { - if (!OC_App::isAppLoaded($appId)) { - OC_App::loadApp($appId); + protected function loadTwoFactorApp(string $appId): void { + if (!$this->appManager->isAppLoaded($appId)) { + $this->appManager->loadApp($appId); } } } diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php index 6b6dc5d3b40..fc76a15b07b 100644 --- a/lib/private/L10N/Factory.php +++ b/lib/private/L10N/Factory.php @@ -81,7 +81,7 @@ class Factory implements IFactory { */ public function get($app, $lang = null, $locale = null) { return new LazyL10N(function () use ($app, $lang, $locale) { - $app = \OC_App::cleanAppId($app); + $app = $this->appManager->cleanAppId($app); if ($lang !== null) { $lang = str_replace(['\0', '/', '\\', '..'], '', $lang); } diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index 646d1d4e6ed..ba369eecac0 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -104,7 +104,7 @@ class Router implements IRouter { */ public function loadRoutes($app = null) { if (is_string($app)) { - $app = \OC_App::cleanAppId($app); + $app = $this->appManager->cleanAppId($app); } $requestedApp = $app; @@ -123,11 +123,15 @@ class Router implements IRouter { if (isset($this->loadedApps[$app])) { return; } - $appPath = \OC_App::getAppPath($app); - $file = $appPath . '/appinfo/routes.php'; - if ($appPath !== false && file_exists($file)) { - $routingFiles = [$app => $file]; - } else { + try { + $appPath = $this->appManager->getAppPath($app); + $file = $appPath . '/appinfo/routes.php'; + if (file_exists($file)) { + $routingFiles = [$app => $file]; + } else { + $routingFiles = []; + } + } catch (AppPathNotFoundException) { $routingFiles = []; } @@ -238,14 +242,14 @@ class Router implements IRouter { // empty string / 'apps' / $app / rest of the route [, , $app,] = explode('/', $url, 4); - $app = \OC_App::cleanAppId($app); + $app = $this->appManager->cleanAppId($app); \OC::$REQUESTEDAPP = $app; $this->loadRoutes($app); } elseif (str_starts_with($url, '/ocsapp/apps/')) { // empty string / 'ocsapp' / 'apps' / $app / rest of the route [, , , $app,] = explode('/', $url, 5); - $app = \OC_App::cleanAppId($app); + $app = $this->appManager->cleanAppId($app); \OC::$REQUESTEDAPP = $app; $this->loadRoutes($app); } elseif (str_starts_with($url, '/settings/')) { @@ -433,7 +437,11 @@ class Router implements IRouter { $appControllerPath = __DIR__ . '/../../../core/Controller'; $appNameSpace = 'OC\\Core'; } else { - $appControllerPath = \OC_App::getAppPath($app) . '/lib/Controller'; + try { + $appControllerPath = $this->appManager->getAppPath($app) . '/lib/Controller'; + } catch (AppPathNotFoundException) { + return []; + } $appNameSpace = App::buildAppNamespace($app); } diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php index f48f4fd0b98..d72d5fe8522 100644 --- a/lib/private/legacy/OC_App.php +++ b/lib/private/legacy/OC_App.php @@ -45,8 +45,7 @@ class OC_App { * @psalm-taint-escape html * @psalm-taint-escape has_quotes * - * @param string $app AppId that needs to be cleaned - * @return string + * @deprecated 31.0.0 use IAppManager::cleanAppId */ public static function cleanAppId(string $app): string { return str_replace(['<', '>', '"', "'", '\0', '/', '\\', '..'], '', $app); -- cgit v1.2.3 From 76f2bc0bfc86a9d1ed34d37c574c7e7a327c0fab Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Thu, 12 Sep 2024 16:38:35 +0200 Subject: fix: Replace OC_App::getAllApps with a method in AppManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/App/Enable.php | 2 +- core/Command/App/GetPath.php | 2 +- core/Command/App/Update.php | 2 +- core/Command/Db/Migrations/GenerateCommand.php | 13 ++++----- .../Db/Migrations/GenerateMetadataCommand.php | 2 +- core/Command/L10n/CreateJs.php | 2 +- lib/private/App/AppManager.php | 31 ++++++++++++++++++++++ lib/private/IntegrityCheck/Checker.php | 4 +-- lib/private/IntegrityCheck/Helpers/AppLocator.php | 9 ------- lib/private/Server.php | 2 +- lib/private/legacy/OC_App.php | 28 +++---------------- lib/public/App/IAppManager.php | 8 ++++++ tests/lib/IntegrityCheck/CheckerTest.php | 4 +-- .../lib/IntegrityCheck/Helpers/AppLocatorTest.php | 4 --- 14 files changed, 58 insertions(+), 55 deletions(-) (limited to 'lib/private') diff --git a/core/Command/App/Enable.php b/core/Command/App/Enable.php index b351a14c39e..5366230b841 100644 --- a/core/Command/App/Enable.php +++ b/core/Command/App/Enable.php @@ -145,7 +145,7 @@ class Enable extends Command implements CompletionAwareInterface { */ public function completeArgumentValues($argumentName, CompletionContext $context): array { if ($argumentName === 'app-id') { - $allApps = \OC_App::getAllApps(); + $allApps = $this->appManager->getAllAppsInAppsFolders(); return array_diff($allApps, \OC_App::getEnabledApps(true, true)); } return []; diff --git a/core/Command/App/GetPath.php b/core/Command/App/GetPath.php index 442af2f3570..3ba4ed7781b 100644 --- a/core/Command/App/GetPath.php +++ b/core/Command/App/GetPath.php @@ -63,7 +63,7 @@ class GetPath extends Base { */ public function completeArgumentValues($argumentName, CompletionContext $context): array { if ($argumentName === 'app') { - return \OC_App::getAllApps(); + return $this->appManager->getAllAppsInAppsFolders(); } return []; } diff --git a/core/Command/App/Update.php b/core/Command/App/Update.php index b4018c94b79..b2d02e222de 100644 --- a/core/Command/App/Update.php +++ b/core/Command/App/Update.php @@ -69,7 +69,7 @@ class Update extends Command { return 1; } } elseif ($input->getOption('all') || $input->getOption('showonly')) { - $apps = \OC_App::getAllApps(); + $apps = $this->manager->getAllAppsInAppsFolders(); } else { $output->writeln('Please specify an app to update or "--all" to update all updatable apps"'); return 1; diff --git a/core/Command/Db/Migrations/GenerateCommand.php b/core/Command/Db/Migrations/GenerateCommand.php index 62d5ad201ed..cd92dc5acd6 100644 --- a/core/Command/Db/Migrations/GenerateCommand.php +++ b/core/Command/Db/Migrations/GenerateCommand.php @@ -71,13 +71,10 @@ class {{classname}} extends SimpleMigrationStep { } '; - protected Connection $connection; - protected IAppManager $appManager; - - public function __construct(Connection $connection, IAppManager $appManager) { - $this->connection = $connection; - $this->appManager = $appManager; - + public function __construct( + protected Connection $connection, + protected IAppManager $appManager, + ) { parent::__construct(); } @@ -155,7 +152,7 @@ class {{classname}} extends SimpleMigrationStep { */ public function completeArgumentValues($argumentName, CompletionContext $context) { if ($argumentName === 'app') { - $allApps = \OC_App::getAllApps(); + $allApps = $this->appManager->getAllAppsInAppsFolders(); return array_diff($allApps, \OC_App::getEnabledApps(true, true)); } diff --git a/core/Command/Db/Migrations/GenerateMetadataCommand.php b/core/Command/Db/Migrations/GenerateMetadataCommand.php index 55a2a6aedab..ae83f92b29e 100644 --- a/core/Command/Db/Migrations/GenerateMetadataCommand.php +++ b/core/Command/Db/Migrations/GenerateMetadataCommand.php @@ -64,7 +64,7 @@ class GenerateMetadataCommand extends Command { * @throws \Exception */ private function extractMigrationMetadataFromApps(): array { - $allApps = \OC_App::getAllApps(); + $allApps = $this->appManager->getAllAppsInAppsFolders(); $metadata = []; foreach ($allApps as $appId) { // We need to load app before being able to extract Migrations diff --git a/core/Command/L10n/CreateJs.php b/core/Command/L10n/CreateJs.php index 27d537b093e..7d45fbe91f8 100644 --- a/core/Command/L10n/CreateJs.php +++ b/core/Command/L10n/CreateJs.php @@ -148,7 +148,7 @@ class CreateJs extends Command implements CompletionAwareInterface { */ public function completeArgumentValues($argumentName, CompletionContext $context) { if ($argumentName === 'app') { - return \OC_App::getAllApps(); + return $this->appManager->getAllAppsInAppsFolders(); } elseif ($argumentName === 'lang') { $appName = $context->getWordAtIndex($context->getWordIndex() - 1); try { diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 974545cfe92..4ffddef98c3 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -155,6 +155,37 @@ class AppManager implements IAppManager { return array_keys($this->getInstalledAppsValues()); } + /** + * Get a list of all apps in the apps folder + * + * @return list an array of app names (string IDs) + */ + public function getAllAppsInAppsFolders(): array { + $apps = []; + + foreach (\OC::$APPSROOTS as $apps_dir) { + if (!is_readable($apps_dir['path'])) { + $this->logger->warning('unable to read app folder : ' . $apps_dir['path'], ['app' => 'core']); + continue; + } + $dh = opendir($apps_dir['path']); + + if (is_resource($dh)) { + while (($file = readdir($dh)) !== false) { + if ( + $file[0] != '.' && + is_dir($apps_dir['path'] . '/' . $file) && + is_file($apps_dir['path'] . '/' . $file . '/appinfo/info.xml') + ) { + $apps[] = $file; + } + } + } + } + + return array_values(array_unique($apps)); + } + /** * List all apps enabled for a user * diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php index d38ccf497f4..6443c43d210 100644 --- a/lib/private/IntegrityCheck/Checker.php +++ b/lib/private/IntegrityCheck/Checker.php @@ -46,7 +46,7 @@ class Checker { private ?IConfig $config, private ?IAppConfig $appConfig, ICacheFactory $cacheFactory, - private ?IAppManager $appManager, + private IAppManager $appManager, private IMimeTypeDetector $mimeTypeDetector, ) { $this->cache = $cacheFactory->createDistributed(self::CACHE_KEY); @@ -536,7 +536,7 @@ class Checker { public function runInstanceVerification() { $this->cleanResults(); $this->verifyCoreSignature(); - $appIds = $this->appLocator->getAllApps(); + $appIds = $this->appManager->getAllAppsInAppsFolders(); foreach ($appIds as $appId) { // If an application is shipped a valid signature is required $isShipped = $this->appManager->isShipped($appId); diff --git a/lib/private/IntegrityCheck/Helpers/AppLocator.php b/lib/private/IntegrityCheck/Helpers/AppLocator.php index 3da5cc13227..148a3aeda76 100644 --- a/lib/private/IntegrityCheck/Helpers/AppLocator.php +++ b/lib/private/IntegrityCheck/Helpers/AppLocator.php @@ -30,13 +30,4 @@ class AppLocator { } return $path; } - - /** - * Providers \OC_App::getAllApps() - * - * @return array - */ - public function getAllApps(): array { - return \OC_App::getAllApps(); - } } diff --git a/lib/private/Server.php b/lib/private/Server.php index b0ccb0f4b4d..c514a4b93ff 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -954,10 +954,10 @@ class Server extends ServerContainer implements IServerContainer { if (\OC::$server->get(SystemConfig::class)->getValue('installed', false)) { $config = $c->get(\OCP\IConfig::class); $appConfig = $c->get(\OCP\IAppConfig::class); - $appManager = $c->get(IAppManager::class); } else { $config = $appConfig = $appManager = null; } + $appManager = $c->get(IAppManager::class); return new Checker( new EnvironmentHelper(), diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php index d72d5fe8522..ef84d35d7bc 100644 --- a/lib/private/legacy/OC_App.php +++ b/lib/private/legacy/OC_App.php @@ -468,30 +468,10 @@ class OC_App { * get a list of all apps in the apps folder * * @return string[] an array of app names (string IDs) - * @todo: change the name of this method to getInstalledApps, which is more accurate + * @deprecated 31.0.0 Use IAppManager::getAllAppsInAppsFolders instead */ public static function getAllApps(): array { - $apps = []; - - foreach (OC::$APPSROOTS as $apps_dir) { - if (!is_readable($apps_dir['path'])) { - \OCP\Server::get(LoggerInterface::class)->warning('unable to read app folder : ' . $apps_dir['path'], ['app' => 'core']); - continue; - } - $dh = opendir($apps_dir['path']); - - if (is_resource($dh)) { - while (($file = readdir($dh)) !== false) { - if ($file[0] != '.' and is_dir($apps_dir['path'] . '/' . $file) and is_file($apps_dir['path'] . '/' . $file . '/appinfo/info.xml')) { - $apps[] = $file; - } - } - } - } - - $apps = array_unique($apps); - - return $apps; + return \OCP\Server::get(IAppManager::class)->getAllAppsInAppsFolders(); } /** @@ -512,9 +492,9 @@ class OC_App { * @return array */ public function listAllApps(): array { - $installedApps = OC_App::getAllApps(); - $appManager = \OC::$server->getAppManager(); + + $installedApps = $appManager->getAllAppsInAppsFolders(); //we don't want to show configuration for these $blacklist = $appManager->getAlwaysEnabledApps(); $appList = []; diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php index 68c3cc771f4..580288fbff7 100644 --- a/lib/public/App/IAppManager.php +++ b/lib/public/App/IAppManager.php @@ -292,4 +292,12 @@ interface IAppManager { * @since 31.0.0 */ public function cleanAppId(string $app): string; + + /** + * Get a list of all apps in the apps folder + * + * @return list an array of app names (string IDs) + * @since 31.0.0 + */ + public function getAllAppsInAppsFolders(): array; } diff --git a/tests/lib/IntegrityCheck/CheckerTest.php b/tests/lib/IntegrityCheck/CheckerTest.php index bf4ea16f564..05607f8113c 100644 --- a/tests/lib/IntegrityCheck/CheckerTest.php +++ b/tests/lib/IntegrityCheck/CheckerTest.php @@ -1030,9 +1030,9 @@ class CheckerTest extends TestCase { $this->checker ->expects($this->once()) ->method('verifyCoreSignature'); - $this->appLocator + $this->appManager ->expects($this->once()) - ->method('getAllApps') + ->method('getAllAppsInAppsFolders') ->willReturn([ 'files', 'calendar', diff --git a/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php b/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php index a86507f6bb2..d99a42e0d63 100644 --- a/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php +++ b/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php @@ -30,8 +30,4 @@ class AppLocatorTest extends TestCase { $this->locator->getAppPath('aTotallyNotExistingApp'); } - - public function testGetAllApps() { - $this->assertSame(\OC_App::getAllApps(), $this->locator->getAllApps()); - } } -- cgit v1.2.3