open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: Migrate cleanAppId and getAppPath calls to IAppManager from OC_App

Browse Source 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
pull/47927/head
Côme Chilliet 8 months ago
parent c9e4598360
commit 7ed583cb8e
No known key found for this signature in database
GPG Key ID: A3E2F658B28C760A
15 changed files with 115 additions and 100 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 7
      apps/provisioning_api/lib/Controller/AppConfigController.php
  2. 27
      apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
  3. 11
      apps/settings/lib/Controller/AppSettingsController.php
  4. 31
      apps/theming/tests/UtilTest.php
  5. 7
      core/Command/L10n/CreateJs.php
  6. 9
      lib/private/App/AppManager.php
  7. 30
      lib/private/Authentication/TwoFactorAuth/ProviderLoader.php
  8. 2
      lib/private/L10N/Factory.php
  9. 26
      lib/private/Route/Router.php
  10. 3
      lib/private/legacy/OC_App.php
  11. 16
      lib/public/App/IAppManager.php
  12. 19
      remote.php
  13. 11
      tests/apps.php
  14. 4
      tests/lib/App/AppManagerTest.php
  15. 12
      tests/lib/InfoXmlTest.php

7
apps/provisioning_api/lib/Controller/AppConfigController.php
Unescape View File

@ -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');
}
}

27
apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
Unescape View File

@ -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();

11
apps/settings/lib/Controller/AppSettingsController.php
Unescape View File

@ -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();
}

31
apps/theming/tests/UtilTest.php
Unescape View File

@ -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));
}

7
core/Command/L10n/CreateJs.php
Unescape View File

@ -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;
@ -150,7 +151,11 @@ class CreateJs extends Command implements CompletionAwareInterface {
return \OC_App::getAllApps();
} 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 [];
}

9
lib/private/App/AppManager.php
Unescape View File

@ -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);
}
}

30
lib/private/Authentication/TwoFactorAuth/ProviderLoader.php
Unescape View File

@ -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);
}
}
}

2
lib/private/L10N/Factory.php
Unescape View File

@ -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);
}

26
lib/private/Route/Router.php
Unescape View File

@ -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);
}

3
lib/private/legacy/OC_App.php
Unescape View File

@ -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);

16
lib/public/App/IAppManager.php
Unescape View File

@ -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,16 @@ 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;
}

19
remote.php
Unescape View File

@ -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.'/';

11
tests/apps.php
Unescape View File

@ -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 */
}
}

4
tests/lib/App/AppManagerTest.php
Unescape View File

@ -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')

12
tests/lib/InfoXmlTest.php
Unescape View File

@ -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

Write Preview
Loading…
Cancel
Save