Skip to content

Commit fe1e599

Browse files
authored
fix: handle redirect param after 2FA login
1 parent cde943e commit fe1e599

File tree

8 files changed

+146
-5
lines changed

8 files changed

+146
-5
lines changed

package-lock.json

Lines changed: 77 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
"cypress": "^14.3.2",
102102
"cypress-axe": "^1.6.0",
103103
"cypress-network-idle": "^1.15.0",
104+
"cypress-otp": "^1.0.3",
104105
"eslint": "^9.25.1",
105106
"eslint-plugin-cypress": "^4.3.0",
106107
"eslint-plugin-mocha": "^11.0.0",

src/Auth.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,15 @@ public function login($login_name, $login_password, $noauto = false, $remember_m
10651065
'extauth' => $this->extauth,
10661066
'remember_me' => $remember_me,
10671067
];
1068-
Html::redirect($CFG_GLPI["root_doc"] . '/?mfa=1');
1068+
1069+
$redirect_params = [
1070+
'mfa' => 1,
1071+
];
1072+
if (isset($_POST['redirect'])) {
1073+
$redirect_params['redirect'] = \rawurlencode($_POST['redirect']);
1074+
}
1075+
1076+
Html::redirect($CFG_GLPI["root_doc"] . '/?' . http_build_query($redirect_params));
10691077
} elseif (isset($mfa_params['totp_code']) && !$totp->verifyCodeForUser($mfa_params['totp_code'], $this->user->fields['id'])) {
10701078
$this->addToError(__('Invalid TOTP code'));
10711079
$this->auth_succeded = false;

src/Glpi/Security/TOTPManager.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,9 @@ public function getGracePeriodDaysLeft(): int
472472
*/
473473
public function showTOTPPrompt(int $users_id): void
474474
{
475-
TemplateRenderer::getInstance()->display('pages/2fa/2fa_request.html.twig');
475+
TemplateRenderer::getInstance()->display('pages/2fa/2fa_request.html.twig', [
476+
'redirect' => $_GET['redirect'] ?? '',
477+
]);
476478
}
477479

478480
/**

templates/pages/2fa/2fa_request.html.twig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
{% block content_block %}
4141
<form method="post" action="front/login.php" data-submit-once autocomplete="off" class="m-n4" data-code-type="code">
4242
<input type="hidden" name="_glpi_csrf_token" value="{{ csrf_token() }}" />
43+
<input type="hidden" name="redirect" value="{{ redirect }}"/>
4344
<div>
4445
{{ tfa.tfa_code_input() }}
4546
<input type="text" name="backup_code" class="form-control d-none" disabled="disabled" placeholder="{{ __('Backup code') }}"/>

templates/pages/2fa/macros.html.twig

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@
3434
<div class="d-flex justify-content-center">
3535
{% for i in 1..digits %}
3636
<input type="text" class="form-control text-center {{ i != 1 ? 'ms-2' : '' }} fw-bold" name="totp_code[]"
37-
maxlength="1" pattern="[0-9]" inputmode="numeric" required style="width: 2em; font-size: 1.5em">
37+
maxlength="1" pattern="[0-9]" inputmode="numeric" required style="width: 2em; font-size: 1.5em"
38+
aria-label="{{ __('2FA code digit %s of %s')|format(i, digits) }}">
3839
{% endfor %}
3940
</div>
4041
<script>
@@ -110,7 +111,7 @@
110111
<div class="mx-auto">
111112
<div>
112113
<div class="btn-group d-flex">
113-
<input class="form-control" alt="{{ __('Secret') }}" value="{{ secret }}"/>
114+
<input class="form-control" alt="{{ __('2FA secret') }}" value="{{ secret }}" aria-label="{{ __('2FA secret') }}" />
114115
<button type="button" class="btn btn-icon flex-grow-0 flex-shrink-0" name="copy_secret" data-clipboard-text="{{ secret }}">
115116
<i class="ti ti-clipboard-copy"></i>
116117
</button>

tests/cypress.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ module.exports = defineConfig({
6868
// eslint-disable-next-line no-console
6969
console.table(message);
7070
return null;
71-
}
71+
},
72+
generateOTP: require('cypress-otp')
7273
});
7374
},
7475
},

tests/cypress/e2e/session.cy.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,56 @@ describe("Session", () => {
8989
cy.url().should('contains', "/front/ticket.form.php");
9090
});
9191

92+
it("redirect to requested page after login with 2FA enabled", () => {
93+
// Create a new user
94+
const username = `e2e_tests_2fa${Date.now()}`;
95+
cy.createWithAPI('User', {
96+
'name' : username,
97+
'login' : username,
98+
'password' : 'glpi',
99+
'password2' : 'glpi',
100+
'_profiles_id': 2, // Super-Admin
101+
});
102+
103+
// Login as the new user
104+
cy.login(username, 'glpi');
105+
106+
// Configure 2FA
107+
cy.visit('/front/preference.php');
108+
cy.findByRole('tab', {'name': 'Two-factor authentication (2FA)'}).click();
109+
cy.findByRole('textbox', {'name': '2FA secret'}).invoke('val').then((secret) => {
110+
cy.wrap(secret).as('secret');
111+
cy.task('generateOTP', secret).then((token) => {
112+
cy.findByRole('textbox', {'name': '2FA code digit 1 of 6'}).type(token);
113+
});
114+
});
115+
cy.findByRole('button', {'name': 'Disable 2FA'}).should('exist');
116+
117+
// Logout
118+
cy.findByRole('link', {name: 'User menu'}).click();
119+
cy.findByRole('link', {name: 'Logout'}).click();
120+
121+
cy.visit('/front/ticket.form.php', {
122+
failOnStatusCode: false
123+
});
124+
cy.findByRole('link', {'name': "Log in again"}).click();
125+
126+
// Login as the new user
127+
cy.findByRole('textbox', {'name': "Login"}).type(username);
128+
cy.findByLabelText("Password").type('glpi');
129+
cy.findByRole('button', {name: "Sign in"}).click();
130+
131+
// Fill 2FA code
132+
cy.get('@secret').then((secret) => {
133+
cy.task('generateOTP', secret).then((token) => {
134+
cy.findByRole('textbox', {'name': '2FA code digit 1 of 6'}).type(token);
135+
});
136+
});
137+
138+
// Should be redirected to requested page
139+
cy.url().should('contains', "/front/ticket.form.php");
140+
});
141+
92142
it("can change profile", () => {
93143
// Login and go to any page
94144
cy.login();

0 commit comments

Comments
 (0)