diff options
author | Côme Chilliet <91878298+come-nc@users.noreply.github.com> | 2024-09-13 17:44:38 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-09-13 17:44:38 +0200 |
commit | bcb4e781a407798ef9d071dc3a15a1d0ac9e0e8d (patch) | |
tree | e210686e9c6d7fdd33b7e0cc905fbd38143db0a3 | |
parent | 032f17c7956748fd25e33edf5d85d1d25b1ee94b (diff) | |
parent | 7a16d01ea79bf8effedd5e0e9ac7c2e4b4ea41f8 (diff) | |
download | nextcloud-server-bcb4e781a407798ef9d071dc3a15a1d0ac9e0e8d.tar.gz nextcloud-server-bcb4e781a407798ef9d071dc3a15a1d0ac9e0e8d.zip |
Merge pull request #47927 from nextcloud/fix/migrate-away-from-oc_app
Migrate away from OC_App to IAppManager
26 files changed, 186 insertions, 156 deletions
diff --git a/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php index 5061a0ffc8f..bb161f6cb8c 100644 --- a/apps/provisioning_api/lib/Controller/AppConfigController.php +++ b/apps/provisioning_api/lib/Controller/AppConfigController.php @@ -10,6 +10,7 @@ namespace OCA\Provisioning_API\Controller; use OC\AppConfig; use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException; +use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; @@ -35,6 +36,7 @@ class AppConfigController extends OCSController { private IL10N $l10n, private IGroupManager $groupManager, private IManager $settingManager, + private IAppManager $appManager, ) { parent::__construct($appName, $request); } @@ -171,11 +173,10 @@ class AppConfigController extends OCSController { } /** - * @param string $app * @throws \InvalidArgumentException */ - protected function verifyAppId(string $app) { - if (\OC_App::cleanAppId($app) !== $app) { + protected function verifyAppId(string $app): void { + if ($this->appManager->cleanAppId($app) !== $app) { throw new \InvalidArgumentException('Invalid app id given'); } } diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php index 0bdee39cebc..23a1f2ee195 100644 --- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php @@ -7,6 +7,7 @@ namespace OCA\Provisioning_API\Tests\Controller; use OC\AppConfig; use OCA\Provisioning_API\Controller\AppConfigController; +use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\Exceptions\AppConfigUnknownKeyException; @@ -17,6 +18,7 @@ use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; use OCP\Settings\IManager; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; use function json_decode; use function json_encode; @@ -28,16 +30,12 @@ use function json_encode; */ class AppConfigControllerTest extends TestCase { - /** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */ - private $appConfig; - /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ - private $userSession; - /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ - private $l10n; - /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ - private $settingManager; - /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ - private $groupManager; + private IAppConfig&MockObject $appConfig; + private IUserSession&MockObject $userSession; + private IL10N&MockObject $l10n; + private IManager&MockObject $settingManager; + private IGroupManager&MockObject $groupManager; + private IAppManager $appManager; protected function setUp(): void { parent::setUp(); @@ -45,8 +43,9 @@ class AppConfigControllerTest extends TestCase { $this->appConfig = $this->createMock(AppConfig::class); $this->userSession = $this->createMock(IUserSession::class); $this->l10n = $this->createMock(IL10N::class); - $this->groupManager = $this->createMock(IGroupManager::class); $this->settingManager = $this->createMock(IManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->appManager = \OCP\Server::get(IAppManager::class); } /** @@ -64,7 +63,8 @@ class AppConfigControllerTest extends TestCase { $this->userSession, $this->l10n, $this->groupManager, - $this->settingManager + $this->settingManager, + $this->appManager, ); } else { return $this->getMockBuilder(AppConfigController::class) @@ -75,7 +75,8 @@ class AppConfigControllerTest extends TestCase { $this->userSession, $this->l10n, $this->groupManager, - $this->settingManager + $this->settingManager, + $this->appManager, ]) ->setMethods($methods) ->getMock(); diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php index 760584888c0..2c682ac4600 100644 --- a/apps/settings/lib/Controller/AppSettingsController.php +++ b/apps/settings/lib/Controller/AppSettingsController.php @@ -14,7 +14,6 @@ use OC\App\AppStore\Version\VersionParser; use OC\App\DependencyAnalyzer; use OC\App\Platform; use OC\Installer; -use OC_App; use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\AppFramework\Controller; @@ -479,7 +478,7 @@ class AppSettingsController extends Controller { $updateRequired = false; foreach ($appIds as $appId) { - $appId = OC_App::cleanAppId($appId); + $appId = $this->appManager->cleanAppId($appId); // Check if app is already downloaded /** @var Installer $installer */ @@ -537,7 +536,7 @@ class AppSettingsController extends Controller { public function disableApps(array $appIds): JSONResponse { try { foreach ($appIds as $appId) { - $appId = OC_App::cleanAppId($appId); + $appId = $this->appManager->cleanAppId($appId); $this->appManager->disableApp($appId); } return new JSONResponse([]); @@ -553,7 +552,7 @@ class AppSettingsController extends Controller { */ #[PasswordConfirmationRequired] public function uninstallApp(string $appId): JSONResponse { - $appId = OC_App::cleanAppId($appId); + $appId = $this->appManager->cleanAppId($appId); $result = $this->installer->removeApp($appId); if ($result !== false) { $this->appManager->clearAppsCache(); @@ -567,7 +566,7 @@ class AppSettingsController extends Controller { * @return JSONResponse */ public function updateApp(string $appId): JSONResponse { - $appId = OC_App::cleanAppId($appId); + $appId = $this->appManager->cleanAppId($appId); $this->config->setSystemValue('maintenance', true); try { @@ -594,7 +593,7 @@ class AppSettingsController extends Controller { } public function force(string $appId): JSONResponse { - $appId = OC_App::cleanAppId($appId); + $appId = $this->appManager->cleanAppId($appId); $this->appManager->ignoreNextcloudRequirementForApp($appId); return new JSONResponse(); } diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php index 4c9f9f10c97..474daab6030 100644 --- a/apps/theming/tests/UtilTest.php +++ b/apps/theming/tests/UtilTest.php @@ -18,22 +18,17 @@ use Test\TestCase; class UtilTest extends TestCase { - /** @var Util */ - protected $util; - /** @var IConfig|MockObject */ - protected $config; - /** @var IAppData|MockObject */ - protected $appData; - /** @var IAppManager|MockObject */ - protected $appManager; - /** @var ImageManager|MockObject */ - protected $imageManager; + protected Util $util; + protected IConfig&MockObject $config; + protected IAppData&MockObject $appData; + protected IAppManager $appManager; + protected ImageManager&MockObject $imageManager; protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); $this->appData = $this->createMock(IAppData::class); - $this->appManager = $this->createMock(IAppManager::class); + $this->appManager = \OCP\Server::get(IAppManager::class); $this->imageManager = $this->createMock(ImageManager::class); $this->util = new Util($this->config, $this->appManager, $this->appData, $this->imageManager); } @@ -152,19 +147,15 @@ class UtilTest extends TestCase { ->method('getFolder') ->with('global/images') ->willThrowException(new NotFoundException()); - $this->appManager->expects($this->once()) - ->method('getAppPath') - ->with($app) - ->willReturn(\OC_App::getAppPath($app)); $icon = $this->util->getAppIcon($app); $this->assertEquals($expected, $icon); } public function dataGetAppIcon() { return [ - ['user_ldap', \OC_App::getAppPath('user_ldap') . '/img/app.svg'], + ['user_ldap', \OCP\Server::get(IAppManager::class)->getAppPath('user_ldap') . '/img/app.svg'], ['noapplikethis', \OC::$SERVERROOT . '/core/img/logo/logo.svg'], - ['comments', \OC_App::getAppPath('comments') . '/img/comments.svg'], + ['comments', \OCP\Server::get(IAppManager::class)->getAppPath('comments') . '/img/comments.svg'], ]; } @@ -187,12 +178,6 @@ class UtilTest extends TestCase { * @dataProvider dataGetAppImage */ public function testGetAppImage($app, $image, $expected) { - if ($app !== 'core') { - $this->appManager->expects($this->once()) - ->method('getAppPath') - ->with($app) - ->willReturn(\OC_App::getAppPath($app)); - } $this->assertEquals($expected, $this->util->getAppImage($app, $image)); } 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('<error>Please specify an app to update or "--all" to update all updatable apps"</error>'); 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 517016b7e16..7d45fbe91f8 100644 --- a/core/Command/L10n/CreateJs.php +++ b/core/Command/L10n/CreateJs.php @@ -11,6 +11,7 @@ namespace OC\Core\Command\L10n; use DirectoryIterator; +use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface; use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext; @@ -147,10 +148,14 @@ 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); - return $this->getAllLanguages(\OC_App::getAppPath($appName)); + try { + return $this->getAllLanguages($this->appManager->getAppPath($appName)); + } catch(AppPathNotFoundException) { + return []; + } } return []; } diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index fe2f7b74b22..4ffddef98c3 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -156,6 +156,37 @@ class AppManager implements IAppManager { } /** + * Get a list of all apps in the apps folder + * + * @return list<string> 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 * * @param \OCP\IUser $user @@ -647,11 +678,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 +906,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/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/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/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 f48f4fd0b98..ef84d35d7bc 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); @@ -469,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(); } /** @@ -513,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 b735b0d7c64..580288fbff7 100644 --- a/lib/public/App/IAppManager.php +++ b/lib/public/App/IAppManager.php @@ -141,12 +141,10 @@ interface IAppManager { /** * Get the directory for the given app. * - * @param string $appId - * @return string * @since 11.0.0 * @throws AppPathNotFoundException */ - public function getAppPath($appId); + public function getAppPath(string $appId): string; /** * Get the web path for the given app. @@ -282,4 +280,24 @@ interface IAppManager { * @since 30.0.0 */ public function isBackendRequired(string $backend): bool; + + /** + * Clean the appId from forbidden characters + * + * @psalm-taint-escape file + * @psalm-taint-escape include + * @psalm-taint-escape html + * @psalm-taint-escape has_quotes + * + * @since 31.0.0 + */ + public function cleanAppId(string $app): string; + + /** + * Get a list of all apps in the apps folder + * + * @return list<string> an array of app names (string IDs) + * @since 31.0.0 + */ + public function getAllAppsInAppsFolders(): array; } diff --git a/remote.php b/remote.php index 1cdb74c4139..9b56e6c97f9 100644 --- a/remote.php +++ b/remote.php @@ -8,6 +8,7 @@ require_once __DIR__ . '/lib/versioncheck.php'; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; +use OCP\App\IAppManager; use Psr\Log\LoggerInterface; use Sabre\DAV\Exception\ServiceUnavailable; use Sabre\DAV\Server; @@ -20,10 +21,7 @@ use Sabre\DAV\Server; class RemoteException extends \Exception { } -/** - * @param Exception|Error $e - */ -function handleException($e) { +function handleException(Exception|Error $e): void { try { $request = \OC::$server->getRequest(); // in case the request content type is text/xml - we assume it's a WebDAV request @@ -126,20 +124,21 @@ try { // Load all required applications \OC::$REQUESTEDAPP = $app; - OC_App::loadApps(['authentication']); - OC_App::loadApps(['extended_authentication']); - OC_App::loadApps(['filesystem', 'logging']); + $appManager = \OCP\Server::get(IAppManager::class); + $appManager->loadApps(['authentication']); + $appManager->loadApps(['extended_authentication']); + $appManager->loadApps(['filesystem', 'logging']); switch ($app) { case 'core': $file = OC::$SERVERROOT .'/'. $file; break; default: - if (!\OC::$server->getAppManager()->isInstalled($app)) { + if (!$appManager->isInstalled($app)) { throw new RemoteException('App not installed: ' . $app); } - OC_App::loadApp($app); - $file = OC_App::getAppPath($app) .'/'. $parts[1]; + $appManager->loadApp($app); + $file = $appManager->getAppPath($app) .'/'. ($parts[1] ?? ''); break; } $baseuri = OC::$WEBROOT . '/remote.php/'.$service.'/'; diff --git a/tests/apps.php b/tests/apps.php index c01e364c0a8..c2015fecda9 100644 --- a/tests/apps.php +++ b/tests/apps.php @@ -44,10 +44,15 @@ function getSubclasses($parentClassName): array { } $apps = OC_App::getEnabledApps(); +$appManager = \OCP\Server::get(\OCP\App\IAppManager::class); foreach ($apps as $app) { - $dir = OC_App::getAppPath($app); - if (is_dir($dir . '/tests')) { - loadDirectory($dir . '/tests'); + try { + $dir = $appManager->getAppPath($app); + if (is_dir($dir . '/tests')) { + loadDirectory($dir . '/tests'); + } + } catch (\OCP\App\AppPathNotFoundException) { + /* ignore */ } } diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index 82a3f0d2045..ac470c00335 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -341,7 +341,7 @@ class AppManagerTest extends TestCase { $manager->expects($this->once()) ->method('getAppPath') ->with('test') - ->willReturn(null); + ->willReturn(''); $manager->expects($this->once()) ->method('getAppInfo') @@ -402,7 +402,7 @@ class AppManagerTest extends TestCase { $manager->expects($this->once()) ->method('getAppPath') ->with('test') - ->willReturn(null); + ->willReturn(''); $manager->expects($this->once()) ->method('getAppInfo') diff --git a/tests/lib/InfoXmlTest.php b/tests/lib/InfoXmlTest.php index 702eca4c0ce..1527e363bd7 100644 --- a/tests/lib/InfoXmlTest.php +++ b/tests/lib/InfoXmlTest.php @@ -7,6 +7,7 @@ namespace Test; use OCP\App\IAppManager; +use OCP\Server; /** * Class InfoXmlTest @@ -15,6 +16,13 @@ use OCP\App\IAppManager; * @package Test */ class InfoXmlTest extends TestCase { + private IAppManager $appManager; + + protected function setUp(): void { + parent::setUp(); + $this->appManager = Server::get(IAppManager::class); + } + public function dataApps() { return [ ['admin_audit'], @@ -45,8 +53,8 @@ class InfoXmlTest extends TestCase { * @param string $app */ public function testClasses($app) { - $appInfo = \OCP\Server::get(IAppManager::class)->getAppInfo($app); - $appPath = \OC_App::getAppPath($app); + $appInfo = $this->appManager->getAppInfo($app); + $appPath = $this->appManager->getAppPath($app); \OC_App::registerAutoloading($app, $appPath); //Add the appcontainer 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()); - } } diff --git a/tests/lib/Route/RouterTest.php b/tests/lib/Route/RouterTest.php index 713d90d3c20..8647bb0f2f6 100644 --- a/tests/lib/Route/RouterTest.php +++ b/tests/lib/Route/RouterTest.php @@ -13,6 +13,7 @@ use OCP\App\IAppManager; use OCP\Diagnostics\IEventLogger; use OCP\IConfig; use OCP\IRequest; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -26,6 +27,8 @@ use Test\TestCase; */ class RouterTest extends TestCase { private Router $router; + private IAppManager&MockObject $appManager; + protected function setUp(): void { parent::setUp(); /** @var LoggerInterface $logger */ @@ -36,13 +39,16 @@ class RouterTest extends TestCase { $this->fail('Unexpected info log: '.(string)($data['exception'] ?? $message)); } ); + + $this->appManager = $this->createMock(IAppManager::class); + $this->router = new Router( $logger, $this->createMock(IRequest::class), $this->createMock(IConfig::class), $this->createMock(IEventLogger::class), $this->createMock(ContainerInterface::class), - $this->createMock(IAppManager::class), + $this->appManager, ); } @@ -51,6 +57,12 @@ class RouterTest extends TestCase { } public function testGenerateConsecutively(): void { + $this->appManager->expects(self::atLeastOnce()) + ->method('cleanAppId') + ->willReturnArgument(0); + $this->appManager->expects(self::atLeastOnce()) + ->method('getAppPath') + ->willReturnCallback(fn (string $appid): string => \OC::$SERVERROOT . '/apps/' . $appid); $this->assertEquals('/index.php/apps/files/', $this->router->generate('files.view.index')); |