Skip to content

Commit

Permalink
Merge pull request #771 from numerique-gouv/add-totp-error-message
Browse files Browse the repository at this point in the history
feat: display errored input when totp code is invalid
  • Loading branch information
rdubigny authored Dec 20, 2024
2 parents 747edb7 + 84f8592 commit 70952dd
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 11 deletions.
6 changes: 4 additions & 2 deletions cypress/e2e/activate_totp/fixtures.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
INSERT INTO users
(id, email, email_verified, email_verified_at, encrypted_password, created_at, updated_at, given_name, family_name, phone_number, job, force_2fa)
VALUES
(1, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Lion', 'El''Jonson', 'I', 'Primarque', false);
(1, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Lion', 'El''Jonson', 'I', 'Primarque', false),
(2, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Raphapha', 'Dubibi', '0123456789', 'Sbire', false);

INSERT INTO organizations
(id, siret, created_at, updated_at)
Expand All @@ -11,4 +12,5 @@ VALUES
INSERT INTO users_organizations
(user_id, organization_id, is_external, verification_type, has_been_greeted)
VALUES
(1, 1, false, 'verified_email_domain', true);
(1, 1, false, 'verified_email_domain', true),
(2, 1, false, 'verified_email_domain', true);
19 changes: 19 additions & 0 deletions cypress/e2e/activate_totp/index.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ describe("add 2fa authentication", () => {
.contains("Configurer un code à usage unique")
.click();

cy.contains("Configurer une application d’authentification");

// Extract the code from the front to generate the TOTP key
cy.get("#humanReadableTotpKey")
.invoke("text")
Expand All @@ -36,4 +38,21 @@ describe("add 2fa authentication", () => {
},
);
});

it("should see an help link on third failed attempt", function () {
cy.visit("/connection-and-account");

cy.login("[email protected]");

cy.get('[href="/authenticator-app-configuration"]')
.contains("Configurer un code à usage unique")
.click();

cy.get("[name=totpToken]").type("123456");
cy.get(
'[action="/authenticator-app-configuration"] [type="submit"]',
).click();

cy.contains("Code invalide.");
});
});
10 changes: 9 additions & 1 deletion cypress/e2e/signin_with_totp/fixtures.sql
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ VALUES
'Jean', 'Jean', '0123456789', 'Sbire',
'kuOSXGk68H2B3pYnph0uyXAHrmpbWaWyX/iX49xVaUc=.VMPBZSO+eAng7mjS.cI2kRY9rwhXchcKiiaMZIg==',
CURRENT_TIMESTAMP, true
),
(4, '[email protected]', true, CURRENT_TIMESTAMP,
'$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP,
'Jean', 'Jean', '0123456789', 'Sbire',
'kuOSXGk68H2B3pYnph0uyXAHrmpbWaWyX/iX49xVaUc=.VMPBZSO+eAng7mjS.cI2kRY9rwhXchcKiiaMZIg==',
CURRENT_TIMESTAMP, true
);

INSERT INTO organizations
Expand All @@ -30,7 +36,9 @@ INSERT INTO users_organizations
(user_id, organization_id, is_external, verification_type, has_been_greeted)
VALUES
(1, 1, false, 'domain', true),
(2, 1, false, 'domain', true);
(2, 1, false, 'domain', true),
(3, 1, false, 'domain', true),
(4, 1, false, 'domain', true);

INSERT INTO oidc_clients
(client_name, client_id, client_secret, redirect_uris,
Expand Down
15 changes: 13 additions & 2 deletions cypress/e2e/signin_with_totp/index.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,28 @@ describe("sign-in with TOTP on untrusted browser", () => {
cy.contains('"amr": [\n "pwd",\n "totp",\n "mfa"\n ],');
});

it("should trigger totp rate limiting", function () {
it("should display error message", function () {
cy.visit("/users/start-sign-in");

cy.login("[email protected]");

cy.get("[name=totpToken]").type("123456");
cy.get(
'[action="/users/2fa-sign-in-with-authenticator-app"] [type="submit"]',
).click();
cy.contains("Code invalide.");
});

it("should trigger totp rate limiting", function () {
cy.visit("/users/start-sign-in");

cy.login("[email protected]");

for (let i = 0; i < 5; i++) {
cy.get("[name=totpToken]").type("123456");
cy.get(
'[action="/users/2fa-sign-in-with-authenticator-app"] [type="submit"]',
).click();
cy.contains("le code que vous avez utilisé est invalide.");
}

cy.get("[name=totpToken]").type("123456");
Expand Down
8 changes: 7 additions & 1 deletion src/controllers/totp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import {
} from "../managers/user";
import { csrfToken } from "../middlewares/csrf-protection";
import { codeSchema } from "../services/custom-zod-schemas";
import getNotificationsFromRequest from "../services/get-notifications-from-request";
import getNotificationsFromRequest, {
getNotificationLabelFromRequest,
} from "../services/get-notifications-from-request";

export const getAuthenticatorAppConfigurationController = async (
req: Request,
Expand All @@ -45,9 +47,13 @@ export const getAuthenticatorAppConfigurationController = async (

setTemporaryTotpKey(req, totpKey);

const notificationLabel = await getNotificationLabelFromRequest(req);
const hasCodeError = notificationLabel === "invalid_totp_token";

return res.render("authenticator-app-configuration", {
pageTitle: "Configuration TOTP",
notifications: await getNotificationsFromRequest(req),
hasCodeError,
csrfToken: csrfToken(req),
isAuthenticatorAlreadyConfigured:
await isAuthenticatorAppConfiguredForUser(user_id),
Expand Down
15 changes: 13 additions & 2 deletions src/controllers/user/2fa-sign-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
import { isAuthenticatorAppConfiguredForUser } from "../../managers/totp";
import { isWebauthnConfiguredForUser } from "../../managers/webauthn";
import { csrfToken } from "../../middlewares/csrf-protection";
import getNotificationsFromRequest from "../../services/get-notifications-from-request";
import getNotificationsFromRequest, {
getNotificationLabelFromRequest,
} from "../../services/get-notifications-from-request";

export const get2faSignInController = async (
req: Request,
Expand All @@ -17,6 +19,14 @@ export const get2faSignInController = async (
const { id, email } = getUserFromAuthenticatedSession(req);

const showsTotpSection = await isAuthenticatorAppConfiguredForUser(id);
let hasCodeError = false;
if (showsTotpSection) {
const notificationLabel = await getNotificationLabelFromRequest(req);
hasCodeError = notificationLabel === "invalid_totp_token";
}
const notifications = hasCodeError
? []
: await getNotificationsFromRequest(req);

// If a passkey has already been used for authentication in this session,
// we cannot use another passkey, or even the same one, for a second factor.
Expand All @@ -28,7 +38,8 @@ export const get2faSignInController = async (

return res.render("user/2fa-sign-in", {
pageTitle: "Se connecter en deux étapes",
notifications: await getNotificationsFromRequest(req),
notifications,
hasCodeError,
csrfToken: csrfToken(req),
email,
showsTotpSection,
Expand Down
2 changes: 1 addition & 1 deletion src/managers/totp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const confirmAuthenticatorAppRegistration = async (
throw new UserNotFoundError();
}

if (!temporaryTotpKey || !validateToken(temporaryTotpKey, totpToken)) {
if (!temporaryTotpKey || !validateToken(temporaryTotpKey, totpToken, 2)) {
throw new InvalidTotpTokenError();
}

Expand Down
18 changes: 17 additions & 1 deletion src/views/authenticator-app-configuration.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
<form action="/authenticator-app-configuration" method="post" class="fr-mb-6w">
<input type="hidden" name="_csrf" value="<%= csrfToken; %>">

<div class="fr-input-group">
<div class="fr-input-group<% if (locals.hasCodeError) { %> fr-input-group--error<% } %>">
<label class="fr-label" for="totpToken">
Saisissez le code à six chiffres affiché dans l’application
</label>
Expand All @@ -60,7 +60,23 @@
pattern="^(\s*\d){6}$"
title="code composé de 6 chiffres"
autocomplete="off"
<% if (locals.hasCodeError) { %>
autofocus
aria-describedby="email-error"
<% } %>
>
<% if (locals.hasCodeError) { %>
<p class="fr-error-text" id="email-error">
Code invalide. 
<a href="https://agentconnect.crisp.help/fr/article/quest-ce-que-la-double-authentification-1m5mpmj/#2-comment-faire-si-le-code-a-usage-unique-ne-fonctionne-pas"
target="_blank"
rel="noopener noreferrer"
aria-label="Page d'aide"
>
Consultez notre page d’aide.
</a>
</p>
<% } %>
</div>


Expand Down
17 changes: 16 additions & 1 deletion src/views/user/2fa-sign-in.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<form action="/users/2fa-sign-in-with-authenticator-app" method="post" class="fr-mb-5w">
<input type="hidden" name="_csrf" value="<%= csrfToken; %>" />
<div class="fr-input-group">
<div class="fr-input-group<% if (locals.hasCodeError) { %> fr-input-group--error<% } %>">
<label class="fr-label" for="totpToken">
Obtenir un code à usage unique depuis votre application mobile.
</label>
Expand All @@ -42,7 +42,22 @@
title="code composé de 6 chiffres"
autocomplete="off"
autofocus
<% if (locals.hasCodeError) { %>
aria-describedby="email-error"
<% } %>
>
<% if (locals.hasCodeError) { %>
<p class="fr-error-text" id="email-error">
Code invalide. 
<a href="https://agentconnect.crisp.help/fr/article/quest-ce-que-la-double-authentification-1m5mpmj/#2-comment-faire-si-le-code-a-usage-unique-ne-fonctionne-pas"
target="_blank"
rel="noopener noreferrer"
aria-label="Page d'aide"
>
Consultez notre page d’aide.
</a>
</p>
<% } %>
</div>
<% if (showsPasskeySection) { %>
<button class="fr-btn btn--fullwidth" type="submit">
Expand Down

0 comments on commit 70952dd

Please sign in to comment.