Skip to content

[stable29] fix(auth): Keep redirect URL during 2FA setup and challenge #44977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blizzz merged 1 commit into from
Jun 12, 2024
Merged

[stable29] fix(auth): Keep redirect URL during 2FA setup and challenge #44977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions core/Controller/TwoFactorChallengeController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OC\Core\Controller\TwoFactorChallengeController::setupProvider does not have a return type, expecting OCP\AppFramework\Http\RedirectResponse<303, array<never, never>>|OCP\AppFramework\Http\StandaloneTemplateResponse<mixed, mixed>
$user = $this->userSession->getUser();
$providers = $this->twoFactorManager->getLoginSetupProviders($user);

Expand All @@ -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');
Expand All @@ -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) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OC\Core\Controller\TwoFactorChallengeController::confirmProviderSetup does not have a return type, expecting OCP\AppFramework\Http\RedirectResponse<303, array<never, never>>
return new RedirectResponse($this->urlGenerator->linkToRoute(
'core.TwoFactorChallenge.showChallenge',
[
'challengeProviderId' => $providerId,
'redirect_url' => $redirect_url,
]
));
}
Expand Down
6 changes: 4 additions & 2 deletions core/Middleware/TwoFactorMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {

Check notice

Code scanning / Psalm

NoInterfaceProperties

Interfaces cannot have properties
$params['redirect_url'] = $this->request->server['REQUEST_URI'];
}
return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge', $params));
Expand Down
1 change: 1 addition & 0 deletions core/templates/twofactorsetupselection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.setupProvider',
[
'providerId' => $provider->getId(),
'redirect_url' => $_['redirect_url'],
]
)) ?>">
<?php
Expand Down
9 changes: 6 additions & 3 deletions tests/Core/Controller/TwoFactorChallengeControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -357,6 +357,7 @@ public function testSetUpProviders() {
$provider,
],
'logout_url' => 'logoutAttribute',
'redirect_url' => null,
],
'guest'
);
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -426,6 +427,7 @@ public function testSetUpProvider() {
'provider' => $provider,
'logout_url' => 'logoutAttribute',
'template' => 'tmpl',
'redirect_url' => null,
],
'guest'
);
Expand All @@ -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');
Expand Down