fix(auth): Keep redirect URL during 2FA setup and challenge

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
nextcloud · Apr 19, 2024 · 22dc278 · 22dc278
1 parent fc560d8
commit 22dc278
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 8 deletions.
diff --git a/core/Controller/TwoFactorChallengeController.php b/core/Controller/TwoFactorChallengeController.php
@@ -213,13 +213,14 @@ public function solveChallenge($challengeProviderId, $challenge, $redirect_url =
 	 * @NoCSRFRequired
 	 */
 	#[FrontpageRoute(verb: 'GET', url: 'login/setupchallenge')]
-	public function setupProviders(): StandaloneTemplateResponse {
+	public function setupProviders(?string $redirect_url = null): StandaloneTemplateResponse {
 		$user = $this->userSession->getUser();
 		$setupProviders = $this->twoFactorManager->getLoginSetupProviders($user);
 
 		$data = [
 			'providers' => $setupProviders,
 			'logout_url' => $this->getLogoutUrl(),
+			'redirect_url' => $redirect_url,
 		];
 
 		return new StandaloneTemplateResponse($this->appName, 'twofactorsetupselection', $data, 'guest');
@@ -230,7 +231,7 @@ public function setupProviders(): StandaloneTemplateResponse {
 	 * @NoCSRFRequired
 	 */
 	#[FrontpageRoute(verb: 'GET', url: 'login/setupchallenge/{providerId}')]
-	public function setupProvider(string $providerId) {
+	public function setupProvider(string $providerId, ?string $redirect_url = null) {
 		$user = $this->userSession->getUser();
 		$providers = $this->twoFactorManager->getLoginSetupProviders($user);
 
@@ -251,6 +252,7 @@ public function setupProvider(string $providerId) {
 		$data = [
 			'provider' => $provider,
 			'logout_url' => $this->getLogoutUrl(),
+			'redirect_url' => $redirect_url,
 			'template' => $tmpl->fetchPage(),
 		];
 		$response = new StandaloneTemplateResponse($this->appName, 'twofactorsetupchallenge', $data, 'guest');
@@ -264,11 +266,12 @@ public function setupProvider(string $providerId) {
 	 * @todo handle the extreme edge case of an invalid provider ID and redirect to the provider selection page
 	 */
 	#[FrontpageRoute(verb: 'POST', url: 'login/setupchallenge/{providerId}')]
-	public function confirmProviderSetup(string $providerId) {
+	public function confirmProviderSetup(string $providerId, ?string $redirect_url = null) {
 		return new RedirectResponse($this->urlGenerator->linkToRoute(
 			'core.TwoFactorChallenge.showChallenge',
 			[
 				'challengeProviderId' => $providerId,
+				'redirect_url' => $redirect_url,
 			]
 		));
 	}

diff --git a/core/Middleware/TwoFactorMiddleware.php b/core/Middleware/TwoFactorMiddleware.php
@@ -134,8 +134,10 @@ private function checkTwoFactor(Controller $controller, $methodName, IUser $user
 
 	public function afterException($controller, $methodName, Exception $exception) {
 		if ($exception instanceof TwoFactorAuthRequiredException) {
-			$params = [];
-			if (isset($this->request->server['REQUEST_URI'])) {
+			$params = [
+				'redirect_url' => $this->request->getParam('redirect_url'),
+			];
+			if (!isset($params['redirect_url']) && isset($this->request->server['REQUEST_URI'])) {
 				$params['redirect_url'] = $this->request->server['REQUEST_URI'];
 			}
 			return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', $params));

diff --git a/core/templates/twofactorsetupselection.php b/core/templates/twofactorsetupselection.php
@@ -33,6 +33,7 @@
 			   href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.setupProvider',
 			   	[
 			   		'providerId' => $provider->getId(),
+			   		'redirect_url' => $_['redirect_url'],
 			   	]
 			   )) ?>">
 				<?php

diff --git a/tests/Core/Controller/TwoFactorChallengeControllerTest.php b/tests/Core/Controller/TwoFactorChallengeControllerTest.php
@@ -337,7 +337,7 @@ public function testSolveChallengeTwoFactorException() {
 		$this->assertEquals($expected, $this->controller->solveChallenge('myprovider', 'token', '/url'));
 	}
 
-	public function testSetUpProviders() {
+	public function testSetUpProviders(): void {
 		$user = $this->createMock(IUser::class);
 		$this->userSession->expects($this->once())
 			->method('getUser')
@@ -357,6 +357,7 @@ public function testSetUpProviders() {
 					$provider,
 				],
 				'logout_url' => 'logoutAttribute',
+				'redirect_url' => null,
 			],
 			'guest'
 		);
@@ -392,7 +393,7 @@ public function testSetUpInvalidProvider() {
 		$this->assertEquals($expected, $response);
 	}
 
-	public function testSetUpProvider() {
+	public function testSetUpProvider(): void {
 		$user = $this->createMock(IUser::class);
 		$this->userSession->expects($this->once())
 			->method('getUser')
@@ -426,6 +427,7 @@ public function testSetUpProvider() {
 				'provider' => $provider,
 				'logout_url' => 'logoutAttribute',
 				'template' => 'tmpl',
+				'redirect_url' => null,
 			],
 			'guest'
 		);
@@ -435,13 +437,14 @@ public function testSetUpProvider() {
 		$this->assertEquals($expected, $response);
 	}
 
-	public function testConfirmProviderSetup() {
+	public function testConfirmProviderSetup(): void {
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->with(
 				'core.TwoFactorChallenge.showChallenge',
 				[
 					'challengeProviderId' => 'totp',
+					'redirect_url' => null,
 				])
 			->willReturn('2fa/select/page');
 		$expected = new RedirectResponse('2fa/select/page');