From 12059eb65b5f72974aab05f81ded890ebd73daab Mon Sep 17 00:00:00 2001 From: Daniel Rudolf Date: Wed, 30 Jun 2021 16:16:05 +0200 Subject: Add IUrlGenerator::linkToDefaultPageUrl() Replaces the deprecated \OC_Util::getDefaultPageUrl() and makes this API public. Signed-off-by: Daniel Rudolf --- lib/private/URLGenerator.php | 47 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) (limited to 'lib/private/URLGenerator.php') diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 34bb65cd0e6..1d8cb041611 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -8,6 +8,7 @@ declare(strict_types=1); * @author Arthur Schiwon * @author Bart Visscher * @author Christoph Wurst + * @author Daniel Rudolf * @author Felix Epp * @author Joas Schilling * @author Jörn Friedrich Dreyer @@ -45,6 +46,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUserSession; use RuntimeException; /** @@ -267,6 +269,51 @@ class URLGenerator implements IURLGenerator { return $theme->buildDocLinkToKey($key); } + /** + * Returns the URL of the default page based on the system configuration + * and the apps visible for the current user + * @return string + */ + public function linkToDefaultPageUrl(): string { + // Deny the redirect if the URL contains a @ + // This prevents unvalidated redirects like ?redirect_url=:user@domain.com + if (isset($_REQUEST['redirect_url']) && strpos($_REQUEST['redirect_url'], '@') === false) { + return $this->getAbsoluteURL(urldecode($_REQUEST['redirect_url'])); + } + + $defaultPage = \OC::$server->getConfig()->getAppValue('core', 'defaultpage'); + if ($defaultPage) { + return $this->getAbsoluteURL($defaultPage); + } + + $appId = 'files'; + $defaultApps = explode(',', $this->config->getSystemValue('defaultapp', 'dashboard,files')); + + /** @var IUserSession $userSession */ + $userSession = \OC::$server->get(IUserSession::class); + $userId = $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : null; + if ($userId !== null) { + $userDefaultApps = explode(',', $this->config->getUserValue($userId, 'core', 'defaultapp')); + $defaultApps = array_filter(array_merge($userDefaultApps, $defaultApps)); + } + + // find the first app that is enabled for the current user + foreach ($defaultApps as $defaultApp) { + $defaultApp = \OC_App::cleanAppId(strip_tags($defaultApp)); + if (\OC::$server->getAppManager()->isEnabledForUser($defaultApp)) { + $appId = $defaultApp; + break; + } + } + + if ($this->config->getSystemValue('htaccess.IgnoreFrontController', false) === true + || getenv('front_controller_active') === 'true') { + return $this->getAbsoluteURL('/apps/' . $appId . '/'); + } + + return $this->getAbsoluteURL('/index.php/apps/' . $appId . '/'); + } + /** * @return string base url of the current request */ -- cgit v1.2.3 From 15a445f743ccf3bcdff69bf09e55367568411c36 Mon Sep 17 00:00:00 2001 From: Daniel Rudolf Date: Thu, 1 Jul 2021 16:26:55 +0200 Subject: Move UtilTest::testDefaultApps() to UrlGeneratorTest Signed-off-by: Daniel Rudolf --- lib/private/URLGenerator.php | 15 ++++++--- tests/lib/UrlGeneratorTest.php | 73 ++++++++++++++++++++++++++++++++++++++++++ tests/lib/UtilTest.php | 62 ----------------------------------- 3 files changed, 84 insertions(+), 66 deletions(-) (limited to 'lib/private/URLGenerator.php') diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 1d8cb041611..7c97a76e257 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -42,6 +42,7 @@ namespace OC; use OC\Route\Router; use OCA\Theming\ThemingDefaults; +use OCP\App\IAppManager; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; @@ -55,6 +56,10 @@ use RuntimeException; class URLGenerator implements IURLGenerator { /** @var IConfig */ private $config; + /** @var IUserSession */ + public $userSession; + /** @var IAppManager */ + public $appManager; /** @var ICacheFactory */ private $cacheFactory; /** @var IRequest */ @@ -65,10 +70,14 @@ class URLGenerator implements IURLGenerator { private $baseUrl = null; public function __construct(IConfig $config, + IUserSession $userSession, + IAppManager $appManager, ICacheFactory $cacheFactory, IRequest $request, Router $router) { $this->config = $config; + $this->userSession = $userSession; + $this->appManager = $appManager; $this->cacheFactory = $cacheFactory; $this->request = $request; $this->router = $router; @@ -289,9 +298,7 @@ class URLGenerator implements IURLGenerator { $appId = 'files'; $defaultApps = explode(',', $this->config->getSystemValue('defaultapp', 'dashboard,files')); - /** @var IUserSession $userSession */ - $userSession = \OC::$server->get(IUserSession::class); - $userId = $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : null; + $userId = $this->userSession->isLoggedIn() ? $this->userSession->getUser()->getUID() : null; if ($userId !== null) { $userDefaultApps = explode(',', $this->config->getUserValue($userId, 'core', 'defaultapp')); $defaultApps = array_filter(array_merge($userDefaultApps, $defaultApps)); @@ -300,7 +307,7 @@ class URLGenerator implements IURLGenerator { // find the first app that is enabled for the current user foreach ($defaultApps as $defaultApp) { $defaultApp = \OC_App::cleanAppId(strip_tags($defaultApp)); - if (\OC::$server->getAppManager()->isEnabledForUser($defaultApp)) { + if ($this->appManager->isEnabledForUser($defaultApp)) { $appId = $defaultApp; break; } diff --git a/tests/lib/UrlGeneratorTest.php b/tests/lib/UrlGeneratorTest.php index 732aa2d2a27..5ba2e166829 100644 --- a/tests/lib/UrlGeneratorTest.php +++ b/tests/lib/UrlGeneratorTest.php @@ -9,10 +9,13 @@ namespace Test; use OC\Route\Router; +use OCP\App\IAppManager; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; /** * Class UrlGeneratorTest @@ -23,6 +26,10 @@ class UrlGeneratorTest extends \Test\TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|IConfig */ private $config; + /** @var \PHPUnit\Framework\MockObject\MockObject|IUserSession */ + private $userSession; + /** @var \PHPUnit\Framework\MockObject\MockObject|IAppManager */ + private $appManager; /** @var \PHPUnit\Framework\MockObject\MockObject|ICacheFactory */ private $cacheFactory; /** @var \PHPUnit\Framework\MockObject\MockObject|IRequest */ @@ -37,11 +44,15 @@ class UrlGeneratorTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->appManager = $this->createMock(IAppManager::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->request = $this->createMock(IRequest::class); $this->router = $this->createMock(Router::class); $this->urlGenerator = new \OC\URLGenerator( $this->config, + $this->userSession, + $this->appManager, $this->cacheFactory, $this->request, $this->router @@ -234,4 +245,66 @@ class UrlGeneratorTest extends \Test\TestCase { $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; $this->assertSame('http://localhost'.\OC::$WEBROOT.'/apps/files/', $this->urlGenerator->linkToDefaultPageUrl()); } + + /** + * @dataProvider provideDefaultApps + */ + public function testGetDefaultPageUrlWithDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) { + $oldDefaultApps = \OC::$server->getConfig()->getSystemValue('defaultapp', ''); + + /** @var \PHPUnit\Framework\MockObject\MockObject|IUser $userMock */ + $userMock = $this->createMock(IUser::class); + $userMock->expects($this->once()) + ->method('getUID') + ->willReturn($this->getUniqueID()); + + $this->userSession->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($userMock); + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->willReturnCallback(function ($appId) use ($enabledApps) { + return in_array($appId, $enabledApps); + }); + + try { + \OC::$server->getConfig()->setSystemValue('defaultapp', $defaultAppConfig); + $this->assertEquals('http://localhost' . \OC::$WEBROOT . $expectedPath, $this->urlGenerator->linkToDefaultPageUrl()); + } finally { + // restore old state + \OC::$server->getConfig()->setSystemValue('defaultapp', $oldDefaultApps); + } + } + + public function provideDefaultApps() { + return [ + // none specified, default to files + [ + '', + 'apps/files/', + ['files'], + ], + // unexisting or inaccessible app specified, default to files + [ + 'unexist', + 'apps/files/', + ['files'], + ], + // non-standard app + [ + 'calendar', + 'apps/calendar/', + ['files', 'calendar'], + ], + // non-standard app with fallback + [ + 'contacts,calendar', + 'apps/calendar/', + ['files', 'calendar'], + ], + ]; + } } diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php index 9a82f3c5ac6..04d4858fb26 100644 --- a/tests/lib/UtilTest.php +++ b/tests/lib/UtilTest.php @@ -9,7 +9,6 @@ namespace Test; use OC_Util; -use OCP\App\IAppManager; /** * Class UtilTest @@ -169,67 +168,6 @@ class UtilTest extends \Test\TestCase { ]; } - /** - * Test default apps - * - * @dataProvider defaultAppsProvider - * @group DB - */ - public function testDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) { - $oldDefaultApps = \OC::$server->getConfig()->getSystemValue('defaultapp', ''); - // CLI is doing messy stuff with the webroot, so need to work it around - $oldWebRoot = \OC::$WEBROOT; - \OC::$WEBROOT = ''; - - $appManager = $this->createMock(IAppManager::class); - $appManager->expects($this->any()) - ->method('isEnabledForUser') - ->willReturnCallback(function ($appId) use ($enabledApps) { - return in_array($appId, $enabledApps); - }); - $this->overwriteService(IAppManager::class, $appManager); - - // need to set a user id to make sure enabled apps are read from cache - \OC_User::setUserId($this->getUniqueID()); - \OC::$server->getConfig()->setSystemValue('defaultapp', $defaultAppConfig); - $this->assertEquals('http://localhost/' . $expectedPath, OC_Util::getDefaultPageUrl()); - - // restore old state - \OC::$WEBROOT = $oldWebRoot; - $this->restoreService(IAppManager::class); - \OC::$server->getConfig()->setSystemValue('defaultapp', $oldDefaultApps); - \OC_User::setUserId(null); - } - - public function defaultAppsProvider() { - return [ - // none specified, default to files - [ - '', - 'index.php/apps/files/', - ['files'], - ], - // unexisting or inaccessible app specified, default to files - [ - 'unexist', - 'index.php/apps/files/', - ['files'], - ], - // non-standard app - [ - 'calendar', - 'index.php/apps/calendar/', - ['files', 'calendar'], - ], - // non-standard app with fallback - [ - 'contacts,calendar', - 'index.php/apps/calendar/', - ['files', 'calendar'], - ], - ]; - } - /** * Test needUpgrade() when the core version is increased */ -- cgit v1.2.3 From a1d6189dfa11e006a130bdd1c35200c1ef21c501 Mon Sep 17 00:00:00 2001 From: Daniel Rudolf Date: Thu, 5 Aug 2021 11:02:21 +0200 Subject: Fix UrlGeneratorTest And again... :unamused: Signed-off-by: Daniel Rudolf --- lib/private/URLGenerator.php | 9 ++------- tests/lib/UrlGeneratorTest.php | 28 +++++++++------------------- 2 files changed, 11 insertions(+), 26 deletions(-) (limited to 'lib/private/URLGenerator.php') diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index 7c97a76e257..1bb5610d95f 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -42,7 +42,6 @@ namespace OC; use OC\Route\Router; use OCA\Theming\ThemingDefaults; -use OCP\App\IAppManager; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; @@ -58,8 +57,6 @@ class URLGenerator implements IURLGenerator { private $config; /** @var IUserSession */ public $userSession; - /** @var IAppManager */ - public $appManager; /** @var ICacheFactory */ private $cacheFactory; /** @var IRequest */ @@ -71,13 +68,11 @@ class URLGenerator implements IURLGenerator { public function __construct(IConfig $config, IUserSession $userSession, - IAppManager $appManager, ICacheFactory $cacheFactory, IRequest $request, Router $router) { $this->config = $config; $this->userSession = $userSession; - $this->appManager = $appManager; $this->cacheFactory = $cacheFactory; $this->request = $request; $this->router = $router; @@ -290,7 +285,7 @@ class URLGenerator implements IURLGenerator { return $this->getAbsoluteURL(urldecode($_REQUEST['redirect_url'])); } - $defaultPage = \OC::$server->getConfig()->getAppValue('core', 'defaultpage'); + $defaultPage = $this->config->getAppValue('core', 'defaultpage'); if ($defaultPage) { return $this->getAbsoluteURL($defaultPage); } @@ -307,7 +302,7 @@ class URLGenerator implements IURLGenerator { // find the first app that is enabled for the current user foreach ($defaultApps as $defaultApp) { $defaultApp = \OC_App::cleanAppId(strip_tags($defaultApp)); - if ($this->appManager->isEnabledForUser($defaultApp)) { + if (\OC::$server->getAppManager()->isEnabledForUser($defaultApp)) { $appId = $defaultApp; break; } diff --git a/tests/lib/UrlGeneratorTest.php b/tests/lib/UrlGeneratorTest.php index 27f08c42f36..259cf81164d 100644 --- a/tests/lib/UrlGeneratorTest.php +++ b/tests/lib/UrlGeneratorTest.php @@ -9,7 +9,6 @@ namespace Test; use OC\Route\Router; -use OCP\App\IAppManager; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; @@ -28,8 +27,6 @@ class UrlGeneratorTest extends \Test\TestCase { private $config; /** @var \PHPUnit\Framework\MockObject\MockObject|IUserSession */ private $userSession; - /** @var \PHPUnit\Framework\MockObject\MockObject|IAppManager */ - private $appManager; /** @var \PHPUnit\Framework\MockObject\MockObject|ICacheFactory */ private $cacheFactory; /** @var \PHPUnit\Framework\MockObject\MockObject|IRequest */ @@ -45,14 +42,12 @@ class UrlGeneratorTest extends \Test\TestCase { parent::setUp(); $this->config = $this->createMock(IConfig::class); $this->userSession = $this->createMock(IUserSession::class); - $this->appManager = $this->createMock(IAppManager::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->request = $this->createMock(IRequest::class); $this->router = $this->createMock(Router::class); $this->urlGenerator = new \OC\URLGenerator( $this->config, $this->userSession, - $this->appManager, $this->cacheFactory, $this->request, $this->router @@ -227,6 +222,10 @@ class UrlGeneratorTest extends \Test\TestCase { $defaultAppConfig, $ignoreFrontControllerConfig )); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'defaultpage') + ->willReturn(''); } public function testLinkToDefaultPageUrlWithRedirectUrlWithoutFrontController() { @@ -266,7 +265,7 @@ class UrlGeneratorTest extends \Test\TestCase { /** * @dataProvider provideDefaultApps */ - public function testLinkToDefaultPageUrlWithDefaultApps($defaultAppConfig, $expectedPath, $enabledApps) { + public function testLinkToDefaultPageUrlWithDefaultApps($defaultAppConfig, $expectedPath) { $userId = $this->getUniqueID(); /** @var \PHPUnit\Framework\MockObject\MockObject|IUser $userMock */ @@ -288,11 +287,6 @@ class UrlGeneratorTest extends \Test\TestCase { $this->userSession->expects($this->once()) ->method('getUser') ->willReturn($userMock); - $this->appManager->expects($this->any()) - ->method('isEnabledForUser') - ->willReturnCallback(function ($appId) use ($enabledApps) { - return in_array($appId, $enabledApps); - }); $this->assertEquals('http://localhost' . \OC::$WEBROOT . $expectedPath, $this->urlGenerator->linkToDefaultPageUrl()); } @@ -303,25 +297,21 @@ class UrlGeneratorTest extends \Test\TestCase { [ '', '/index.php/apps/files/', - ['files'], ], // unexisting or inaccessible app specified, default to files [ 'unexist', '/index.php/apps/files/', - ['files'], ], // non-standard app [ - 'calendar', - '/index.php/apps/calendar/', - ['files', 'calendar'], + 'settings', + '/index.php/apps/settings/', ], // non-standard app with fallback [ - 'contacts,calendar', - '/index.php/apps/calendar/', - ['files', 'calendar'], + 'unexist,settings', + '/index.php/apps/settings/', ], ]; } -- cgit v1.2.3