diff options
-rw-r--r-- | core/Controller/LoginController.php | 6 | ||||
-rw-r--r-- | core/Controller/TwoFactorChallengeController.php | 6 | ||||
-rw-r--r-- | lib/private/URLGenerator.php | 47 | ||||
-rw-r--r-- | lib/private/legacy/OC_Util.php | 40 | ||||
-rw-r--r-- | lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php | 3 | ||||
-rw-r--r-- | lib/public/IURLGenerator.php | 8 | ||||
-rw-r--r-- | tests/Core/Controller/LoginControllerTest.php | 17 | ||||
-rw-r--r-- | tests/Core/Controller/TwoFactorChallengeControllerTest.php | 3 | ||||
-rw-r--r-- | tests/lib/UrlGeneratorTest.php | 30 | ||||
-rw-r--r-- | tests/lib/UtilTest.php | 30 |
10 files changed, 104 insertions, 86 deletions
diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 8a96db97c9e..5ef40429120 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -38,11 +38,11 @@ use OC\Authentication\WebAuthn\Manager as WebAuthnManager; use OC\Security\Bruteforce\Throttler; use OC\User\Session; use OC_App; -use OC_Util; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; use OCP\IConfig; @@ -150,7 +150,7 @@ class LoginController extends Controller { */ public function showLoginForm(string $user = null, string $redirect_url = null): Http\Response { if ($this->userSession->isLoggedIn()) { - return new RedirectResponse(OC_Util::getDefaultPageUrl()); + return new RedirectToDefaultAppResponse(); } $loginMessages = $this->session->get('loginMessages'); @@ -274,7 +274,7 @@ class LoginController extends Controller { return new RedirectResponse($location); } } - return new RedirectResponse(OC_Util::getDefaultPageUrl()); + return new RedirectToDefaultAppResponse(); } /** diff --git a/core/Controller/TwoFactorChallengeController.php b/core/Controller/TwoFactorChallengeController.php index e08454caea6..c343321a868 100644 --- a/core/Controller/TwoFactorChallengeController.php +++ b/core/Controller/TwoFactorChallengeController.php @@ -27,9 +27,9 @@ namespace OC\Core\Controller; use OC\Authentication\TwoFactorAuth\Manager; use OC_User; -use OC_Util; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\StandaloneTemplateResponse; use OCP\Authentication\TwoFactorAuth\IActivatableAtLogin; use OCP\Authentication\TwoFactorAuth\IProvider; @@ -77,7 +77,7 @@ class TwoFactorChallengeController extends Controller { protected function getLogoutUrl() { return OC_User::getLogoutUrl($this->urlGenerator); } - + /** * @param IProvider[] $providers */ @@ -197,7 +197,7 @@ class TwoFactorChallengeController extends Controller { if (!is_null($redirect_url)) { return new RedirectResponse($this->urlGenerator->getAbsoluteURL(urldecode($redirect_url))); } - return new RedirectResponse(OC_Util::getDefaultPageUrl()); + return new RedirectToDefaultAppResponse(); } } catch (TwoFactorException $e) { /* 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 <blizzz@arthur-schiwon.de> * @author Bart Visscher <bartv@thisnet.nl> * @author Christoph Wurst <christoph@winzerhof-wurst.at> + * @author Daniel Rudolf <github.com@daniel-rudolf.de> * @author Felix Epp <work@felixepp.de> * @author Joas Schilling <coding@schilljs.com> * @author Jörn Friedrich Dreyer <jfd@butonic.de> @@ -45,6 +46,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUserSession; use RuntimeException; /** @@ -268,6 +270,51 @@ class URLGenerator implements IURLGenerator { } /** + * 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 */ public function getBaseUrl(): string { diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php index 87964f98374..1f5f04ff055 100644 --- a/lib/private/legacy/OC_Util.php +++ b/lib/private/legacy/OC_Util.php @@ -1089,46 +1089,8 @@ class OC_Util { * @suppress PhanDeprecatedFunction */ public static function getDefaultPageUrl() { - /** @var IConfig $config */ - $config = \OC::$server->get(IConfig::class); $urlGenerator = \OC::$server->getURLGenerator(); - // 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) { - $location = $urlGenerator->getAbsoluteURL(urldecode($_REQUEST['redirect_url'])); - } else { - $defaultPage = \OC::$server->getConfig()->getAppValue('core', 'defaultpage'); - if ($defaultPage) { - $location = $urlGenerator->getAbsoluteURL($defaultPage); - } else { - $appId = 'files'; - $defaultApps = explode(',', $config->getSystemValue('defaultapp', 'dashboard,files')); - - /** @var IUserSession $userSession */ - $userSession = \OC::$server->get(IUserSession::class); - $user = $userSession->getUser(); - if ($user) { - $userDefaultApps = explode(',', $config->getUserValue($user->getUID(), '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 (static::getAppManager()->isEnabledForUser($defaultApp)) { - $appId = $defaultApp; - break; - } - } - - if ($config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true') { - $location = $urlGenerator->getAbsoluteURL('/apps/' . $appId . '/'); - } else { - $location = $urlGenerator->getAbsoluteURL('/index.php/apps/' . $appId . '/'); - } - } - } - return $location; + return $urlGenerator->linkToDefaultPageUrl(); } /** diff --git a/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php b/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php index 36e629d6342..32604e9e14c 100644 --- a/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php +++ b/lib/public/AppFramework/Http/RedirectToDefaultAppResponse.php @@ -38,6 +38,7 @@ class RedirectToDefaultAppResponse extends RedirectResponse { * @since 16.0.0 */ public function __construct() { - parent::__construct(\OC_Util::getDefaultPageUrl()); + $urlGenerator = \OC::$server->getURLGenerator(); + parent::__construct($urlGenerator->linkToDefaultPageUrl()); } } diff --git a/lib/public/IURLGenerator.php b/lib/public/IURLGenerator.php index 486ca47d252..89ad4332c2d 100644 --- a/lib/public/IURLGenerator.php +++ b/lib/public/IURLGenerator.php @@ -98,6 +98,14 @@ interface IURLGenerator { public function linkToDocs(string $key): string; /** + * Returns the URL of the default page based on the system configuration + * and the apps visible for the current user + * @return string + * @since 23.0.0 + */ + public function linkToDefaultPageUrl(): string; + + /** * @return string base url of the current request * @since 13.0.0 */ diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index e3469621eec..2f964817e7d 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -29,6 +29,7 @@ use OC\Core\Controller\LoginController; use OC\Security\Bruteforce\Throttler; use OC\User\Session; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; use OCP\IConfig; @@ -212,7 +213,7 @@ class LoginControllerTest extends TestCase { ->method('isLoggedIn') ->willReturn(true); - $expectedResponse = new RedirectResponse(\OC_Util::getDefaultPageUrl()); + $expectedResponse = new RedirectToDefaultAppResponse(); $this->assertEquals($expectedResponse, $this->loginController->showLoginForm('', '', '')); } @@ -443,7 +444,7 @@ class LoginControllerTest extends TestCase { 'direct' => 1, ]) ->willReturn($loginPageUrl); - $expected = new \OCP\AppFramework\Http\RedirectResponse($loginPageUrl); + $expected = new RedirectResponse($loginPageUrl); $expected->throttle(['user' => 'MyUserName']); $response = $this->loginController->tryLogin($user, $password, '/apps/files'); @@ -454,7 +455,6 @@ class LoginControllerTest extends TestCase { public function testLoginWithValidCredentials() { $user = 'MyUserName'; $password = 'secret'; - $indexPageUrl = \OC_Util::getDefaultPageUrl(); $this->request ->expects($this->once()) @@ -470,7 +470,7 @@ class LoginControllerTest extends TestCase { ->method('process') ->with($this->equalTo($loginData)) ->willReturn($loginResult); - $expected = new \OCP\AppFramework\Http\RedirectResponse($indexPageUrl); + $expected = new RedirectToDefaultAppResponse(); $response = $this->loginController->tryLogin($user, $password); @@ -499,7 +499,7 @@ class LoginControllerTest extends TestCase { $this->userSession->expects($this->never()) ->method('createRememberMeToken'); - $expected = new \OCP\AppFramework\Http\RedirectResponse(\OC_Util::getDefaultPageUrl()); + $expected = new RedirectToDefaultAppResponse(); $this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl)); } @@ -534,14 +534,13 @@ class LoginControllerTest extends TestCase { ->with('remember_login_cookie_lifetime') ->willReturn(1234); - $expected = new \OCP\AppFramework\Http\RedirectResponse($redirectUrl); + $expected = new RedirectResponse($redirectUrl); $this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl)); } public function testLoginWithValidCredentialsAndRedirectUrl() { $user = 'MyUserName'; $password = 'secret'; - $indexPageUrl = \OC_Util::getDefaultPageUrl(); $redirectUrl = 'https://next.cloud/apps/mail'; $this->request @@ -566,7 +565,7 @@ class LoginControllerTest extends TestCase { ->method('getAbsoluteURL') ->with('/apps/mail') ->willReturn($redirectUrl); - $expected = new \OCP\AppFramework\Http\RedirectResponse($redirectUrl); + $expected = new RedirectResponse($redirectUrl); $response = $this->loginController->tryLogin($user, $password, '/apps/mail'); @@ -601,7 +600,7 @@ class LoginControllerTest extends TestCase { 'direct' => 1, ]) ->willReturn($loginPageUrl); - $expected = new \OCP\AppFramework\Http\RedirectResponse($loginPageUrl); + $expected = new RedirectResponse($loginPageUrl); $expected->throttle(['user' => 'john']); $response = $this->loginController->tryLogin( diff --git a/tests/Core/Controller/TwoFactorChallengeControllerTest.php b/tests/Core/Controller/TwoFactorChallengeControllerTest.php index 1a2a4d8eaa5..f3e4ec5628e 100644 --- a/tests/Core/Controller/TwoFactorChallengeControllerTest.php +++ b/tests/Core/Controller/TwoFactorChallengeControllerTest.php @@ -27,6 +27,7 @@ use OC\Authentication\TwoFactorAuth\ProviderSet; use OC\Core\Controller\TwoFactorChallengeController; use OC_Util; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\StandaloneTemplateResponse; use OCP\Authentication\TwoFactorAuth\IActivatableAtLogin; use OCP\Authentication\TwoFactorAuth\ILoginSetupProvider; @@ -208,7 +209,7 @@ class TwoFactorChallengeControllerTest extends TestCase { ->with('myprovider', $user, 'token') ->willReturn(true); - $expected = new RedirectResponse(OC_Util::getDefaultPageUrl()); + $expected = new RedirectToDefaultAppResponse(); $this->assertEquals($expected, $this->controller->solveChallenge('myprovider', 'token')); } diff --git a/tests/lib/UrlGeneratorTest.php b/tests/lib/UrlGeneratorTest.php index dd2eb2ddc63..732aa2d2a27 100644 --- a/tests/lib/UrlGeneratorTest.php +++ b/tests/lib/UrlGeneratorTest.php @@ -204,4 +204,34 @@ class UrlGeneratorTest extends \Test\TestCase { ['core.WhatsNew.dismiss', 'http://localhost/nextcloud/ocs/v2.php/core/whatsnew'], ]; } + + public function testGetDefaultPageUrlWithRedirectUrlWithoutFrontController() { + putenv('front_controller_active=false'); + \OC::$server->getConfig()->deleteSystemValue('htaccess.IgnoreFrontController'); + + $_REQUEST['redirect_url'] = 'myRedirectUrl.com'; + $this->assertSame('http://localhost'.\OC::$WEBROOT.'/myRedirectUrl.com', $this->urlGenerator->linkToDefaultPageUrl()); + } + + public function testGetDefaultPageUrlWithRedirectUrlRedirectBypassWithoutFrontController() { + putenv('front_controller_active=false'); + \OC::$server->getConfig()->deleteSystemValue('htaccess.IgnoreFrontController'); + + $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; + $this->assertSame('http://localhost'.\OC::$WEBROOT.'/index.php/apps/files/', $this->urlGenerator->linkToDefaultPageUrl()); + } + + public function testGetDefaultPageUrlWithRedirectUrlRedirectBypassWithFrontController() { + putenv('front_controller_active=true'); + $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; + $this->assertSame('http://localhost'.\OC::$WEBROOT.'/apps/files/', $this->urlGenerator->linkToDefaultPageUrl()); + } + + public function testGetDefaultPageUrlWithRedirectUrlWithIgnoreFrontController() { + putenv('front_controller_active=false'); + \OC::$server->getConfig()->setSystemValue('htaccess.IgnoreFrontController', true); + + $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; + $this->assertSame('http://localhost'.\OC::$WEBROOT.'/apps/files/', $this->urlGenerator->linkToDefaultPageUrl()); + } } diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php index e21a5323b1b..55b86f06955 100644 --- a/tests/lib/UtilTest.php +++ b/tests/lib/UtilTest.php @@ -229,36 +229,6 @@ class UtilTest extends \Test\TestCase { ]; } - public function testGetDefaultPageUrlWithRedirectUrlWithoutFrontController() { - putenv('front_controller_active=false'); - \OC::$server->getConfig()->deleteSystemValue('htaccess.IgnoreFrontController'); - - $_REQUEST['redirect_url'] = 'myRedirectUrl.com'; - $this->assertSame('http://localhost'.\OC::$WEBROOT.'/myRedirectUrl.com', OC_Util::getDefaultPageUrl()); - } - - public function testGetDefaultPageUrlWithRedirectUrlRedirectBypassWithoutFrontController() { - putenv('front_controller_active=false'); - \OC::$server->getConfig()->deleteSystemValue('htaccess.IgnoreFrontController'); - - $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; - $this->assertSame('http://localhost'.\OC::$WEBROOT.'/index.php/apps/files/', OC_Util::getDefaultPageUrl()); - } - - public function testGetDefaultPageUrlWithRedirectUrlRedirectBypassWithFrontController() { - putenv('front_controller_active=true'); - $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; - $this->assertSame('http://localhost'.\OC::$WEBROOT.'/apps/files/', OC_Util::getDefaultPageUrl()); - } - - public function testGetDefaultPageUrlWithRedirectUrlWithIgnoreFrontController() { - putenv('front_controller_active=false'); - \OC::$server->getConfig()->setSystemValue('htaccess.IgnoreFrontController', true); - - $_REQUEST['redirect_url'] = 'myRedirectUrl.com@foo.com:a'; - $this->assertSame('http://localhost'.\OC::$WEBROOT.'/apps/files/', OC_Util::getDefaultPageUrl()); - } - /** * Test needUpgrade() when the core version is increased */ |