Merge pull request #697 from numerique-gouv/fix-to-many-reauth

feat: login_hint from sp does not trigger login prompt
numerique-gouv · Sep 20, 2024 · 19c7581 · 19c7581
2 parents 436aafa + 73005cb
commit 19c7581
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 22 deletions.
diff --git a/cypress/e2e/signin_from_agentconnect_client/fixtures.sql b/cypress/e2e/signin_from_agentconnect_client/fixtures.sql
@@ -2,7 +2,8 @@ INSERT INTO users
   (id, email, email_verified, email_verified_at, encrypted_password, created_at, updated_at, given_name, family_name,
    phone_number, job)
 VALUES
-  (1, 'unused1@yopmail.com', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Jean', '0123456789', 'Sbire');
+  (1, 'unused1@yopmail.com', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Jean', '0123456789', 'Sbire'),
+  (2, 'unused2@yopmail.com', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Jean', '0123456789', 'Sbire');
 
 
 INSERT INTO organizations
@@ -13,7 +14,8 @@ VALUES
 INSERT INTO users_organizations
   (user_id, organization_id, is_external, verification_type, has_been_greeted)
 VALUES
-  (1, 1, false, 'domain', true);
+  (1, 1, false, 'domain', true),
+  (2, 1, false, 'domain', true);
 
 INSERT INTO oidc_clients
   (client_name, client_id, client_secret, redirect_uris,

diff --git a/cypress/e2e/signin_from_agentconnect_client/index.cy.ts b/cypress/e2e/signin_from_agentconnect_client/index.cy.ts
@@ -1,7 +1,7 @@
 //
 
 describe("sign-in from agentconnect client", () => {
-  it("should sign-in", function () {
+  it("should sign-in", () => {
     cy.visit("http://localhost:4001");
     cy.get("button.moncomptepro-button").click();
 
@@ -14,4 +14,31 @@ describe("sign-in from agentconnect client", () => {
     cy.contains("unused1@yopmail.com");
     cy.contains("21340126800130");
   });
+
+  it("should not prompt for password if a session is already opened", () => {
+    cy.visit("/");
+    cy.login("unused1@yopmail.com");
+
+    cy.visit("http://localhost:4001");
+    cy.get("button.moncomptepro-button").click();
+
+    cy.contains("moncomptepro-agentconnect-client");
+    cy.contains("unused1@yopmail.com");
+  });
+
+  it("login_hint should take precedence over existing session", () => {
+    cy.visit("/");
+    cy.login("unused2@yopmail.com");
+
+    cy.visit("http://localhost:4001");
+    cy.get("button.moncomptepro-button").click();
+
+    cy.get('[name="password"]').type("password123");
+    cy.get('[action="/users/sign-in"]  [type="submit"]')
+      .contains("S’identifier")
+      .click();
+
+    cy.contains("moncomptepro-agentconnect-client");
+    cy.contains("unused1@yopmail.com");
+  });
 });
diff --git a/cypress/e2e/signin_from_standard_client/index.cy.ts b/cypress/e2e/signin_from_standard_client/index.cy.ts
@@ -56,5 +56,14 @@ describe("sign-in from standard client", () => {
     cy.contains("Commune de lamalou-les-bains - Mairie");
   });
 
-  it("should prompt for organization selection", function () {});
+  it("should not prompt for password if a session is already opened", () => {
+    cy.visit("/");
+    cy.login("unused1@yopmail.com");
+
+    cy.visit("http://localhost:4000");
+    cy.get("button.moncomptepro-button").click();
+
+    cy.contains("moncomptepro-standard-client");
+    cy.contains("unused1@yopmail.com");
+  });
 });
diff --git a/src/config/errors.ts b/src/config/errors.ts
@@ -87,7 +87,7 @@ export class WebauthnAuthenticationFailedError extends Error {}
 
 export class UserNotLoggedInError extends Error {}
 
-export class NoEmailFoundInLoggedOutSessionError extends Error {}
+export class NoEmailFoundInUnauthenticatedSessionError extends Error {}
 
 export class InvalidTotpTokenError extends Error {}
 

diff --git a/src/controllers/interaction.ts b/src/controllers/interaction.ts
@@ -4,13 +4,13 @@ import { ENABLE_FIXED_ACR } from "../config/env";
 import {
   getSessionStandardizedAuthenticationMethodsReferences,
   getUserFromAuthenticatedSession,
+  isWithinAuthenticatedSession,
   isWithinTwoFactorAuthenticatedSession,
 } from "../managers/session/authenticated";
-import { setEmailInUnauthenticatedSession } from "../managers/session/unauthenticated";
+import { setLoginHintInUnauthenticatedSession } from "../managers/session/unauthenticated";
 import epochTime from "../services/epoch-time";
 import { mustReturnOneOrganizationInPayload } from "../services/must-return-one-organization-in-payload";
 import { shouldTrigger2fa } from "../services/should-trigger-2fa";
-import { postStartSignInController } from "./user/signin-signup";
 
 export const interactionStartControllerFactory =
   (oidcProvider: any) =>
@@ -29,20 +29,21 @@ export const interactionStartControllerFactory =
         req.session.mustUse2FA = true;
       }
 
-      if (prompt.name === "login" && prompt.reasons.includes("login_prompt")) {
-        if (login_hint) {
-          setEmailInUnauthenticatedSession(req, login_hint);
-          req.body.login = login_hint;
-          return postStartSignInController(req, res, next);
-        }
+      if (login_hint) {
+        setLoginHintInUnauthenticatedSession(req, login_hint);
+      }
 
+      if (prompt.name === "login" && prompt.reasons.includes("login_prompt")) {
         return res.redirect(`/users/start-sign-in`);
       }
 
       if (prompt.name === "login" || prompt.name === "choose_organization") {
-        if (login_hint) {
-          req.body.login = login_hint;
-          return postStartSignInController(req, res, next);
+        if (
+          login_hint &&
+          isWithinAuthenticatedSession(req.session) &&
+          getUserFromAuthenticatedSession(req).email !== login_hint
+        ) {
+          return res.redirect(`/users/start-sign-in`);
         }
 
         return res.redirect(`/interaction/${interactionId}/login`);

diff --git a/src/controllers/user/signin-signup.ts b/src/controllers/user/signin-signup.ts
@@ -11,6 +11,7 @@ import {
 } from "../../config/errors";
 import { createAuthenticatedSession } from "../../managers/session/authenticated";
 import {
+  getAndRemoveLoginHintFromUnauthenticatedSession,
   getEmailFromUnauthenticatedSession,
   setEmailInUnauthenticatedSession,
   setPartialUserFromUnauthenticatedSession,
@@ -33,13 +34,22 @@ export const getStartSignInController = async (
   next: NextFunction,
 ) => {
   try {
+    // Bypass email submission when a login hint is provided in the interaction
+    const hintFromOidcInteraction =
+      getAndRemoveLoginHintFromUnauthenticatedSession(req);
+    if (hintFromOidcInteraction) {
+      setEmailInUnauthenticatedSession(req, hintFromOidcInteraction);
+      req.body.login = hintFromOidcInteraction;
+      return postStartSignInController(req, res, next);
+    }
+
     const schema = z.object({
       did_you_mean: z.string().trim().min(1).optional(),
     });
 
     const { did_you_mean: didYouMean } = await schema.parseAsync(req.query);
 
-    const loginHint = getEmailFromUnauthenticatedSession(req);
+    const hintFromSession = getEmailFromUnauthenticatedSession(req);
 
     const hasEmailError =
       (await getNotificationLabelFromRequest(req)) === "invalid_email";
@@ -49,7 +59,7 @@ export const getStartSignInController = async (
       notifications: !hasEmailError && (await getNotificationsFromRequest(req)),
       hasEmailError,
       didYouMean,
-      loginHint,
+      loginHint: hintFromSession,
       csrfToken: csrfToken(req),
       displayTestEnvWarning: DISPLAY_TEST_ENV_WARNING,
     });

diff --git a/src/managers/session/unauthenticated.ts b/src/managers/session/unauthenticated.ts
@@ -1,8 +1,23 @@
 import type { Request } from "express";
 import { isEmpty } from "lodash-es";
-import { NoEmailFoundInLoggedOutSessionError } from "../../config/errors";
+import { NoEmailFoundInUnauthenticatedSessionError } from "../../config/errors";
 import { findByEmail, update } from "../../repositories/user";
 
+export const getAndRemoveLoginHintFromUnauthenticatedSession = (
+  req: Request,
+) => {
+  const loginHint = req.session.loginHint;
+  delete req.session.loginHint;
+  return loginHint;
+};
+export const setLoginHintInUnauthenticatedSession = (
+  req: Request,
+  loginHint: string,
+) => {
+  req.session.loginHint = loginHint;
+
+  return loginHint;
+};
 export const getEmailFromUnauthenticatedSession = (req: Request) => {
   return req.session.email;
 };
@@ -42,7 +57,7 @@ export const updatePartialUserFromUnauthenticatedSession = async (
   needs_inclusionconnect_welcome_page: boolean;
 }> => {
   if (!req.session.email) {
-    throw new NoEmailFoundInLoggedOutSessionError();
+    throw new NoEmailFoundInUnauthenticatedSessionError();
   }
 
   req.session.needsInclusionconnectWelcomePage =

diff --git a/src/types/express-session.d.ts b/src/types/express-session.d.ts
@@ -1,5 +1,6 @@
-export interface LoggedOutSessionData {
+export interface UnauthenticatedSessionData {
   email?: string;
+  loginHint?: string;
   needsInclusionconnectWelcomePage?: boolean;
   interactionId?: string;
   mustReturnOneOrganizationInPayload?: boolean;
@@ -25,7 +26,7 @@ export interface AuthenticatedSessionData {
 }
 
 declare module "express-session" {
-  export interface SessionData extends LoggedOutSessionData {
+  export interface SessionData extends UnauthenticatedSessionData {
     user?: User;
     temporaryEncryptedTotpKey?: string;
     amr?: AmrValue[];