Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the login experience to support a new UI flow around ProConnect vs. “classic” credentials login, and adds analytics tracking around login actions.
Changes:
- Adjusts credentials authorization to emit a special error when the user has no credentials password (intended to redirect users to ProConnect).
- Redesigns the login UI into a ProConnect block + DSFR
Tabsfor “Créer un accès” and “Connexion”, with Matomo tracking. - Updates the login CSS module to match the new layout (new
.description, updated.box/.buttonstyles).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/services/auth/config.ts | Changes credentials auth error behavior (introduces "proconnect" error) |
| src/components/Login/LoginForm.tsx | New login page UI with tabs, error handling, and Matomo tracking |
| src/components/Login/LoginForm.module.css | Styles updated for the redesigned login layout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| className='fr-mt-4w' | ||
| title={ | ||
| error === "proconnect" | ||
| ? "Vous essayez de vous connecter avec un compte ProConnect, veuillez utiliser le bouton de connexion ProConnect ci dessus." |
There was a problem hiding this comment.
Spelling/typography in the user-facing message: "ci dessus" should be hyphenated as "ci-dessus".
| ? "Vous essayez de vous connecter avec un compte ProConnect, veuillez utiliser le bouton de connexion ProConnect ci dessus." | |
| ? "Vous essayez de vous connecter avec un compte ProConnect, veuillez utiliser le bouton de connexion ProConnect ci-dessus." |
| const account = user.accounts.find((account) => account.provider === "credentials") | ||
| if (!account || !account.password) { | ||
| throw new Error("Invalid crendentials") | ||
| throw new Error("proconnect") |
There was a problem hiding this comment.
The credentials provider now returns a distinct error ("proconnect") when a user exists but has no credentials password. This makes the login response differ based on whether the email exists / which auth method the account uses, enabling account enumeration. Consider returning the same generic error for all credential failures (including missing credentials account), and handle the UX hint ("try ProConnect") without relying on a server-side distinguishable error code.
| throw new Error("proconnect") | |
| throw new Error("Invalid crendentials") |
src/services/auth/config.ts
Outdated
| throw new Error("Invalid crendentials") | ||
| } |
There was a problem hiding this comment.
Spelling: "Invalid crendentials" is misspelled in this thrown error message; it should be "Invalid credentials".
| tabs={[ | ||
| { | ||
| label: "Créer un accès", | ||
| isDefault: true, |
There was a problem hiding this comment.
The credentials login form is now inside the "Connexion" tab, but the default tab is set to "Créer un accès". This will break the existing Playwright helper loginWithPassword (it expects the Email/Mot de passe fields to be visible immediately on /login). Either make the "Connexion" tab the default or update the e2e tests to select the tab before filling the form.
| isDefault: true, |
| if (result.error === "proconnect") { | ||
| track("Login", "Credentials", "Error ProConnect") | ||
| } else { | ||
| track("Login", "Credentials", "Error") | ||
| } | ||
| setError(result.error) |
There was a problem hiding this comment.
The "proconnect" error code is duplicated as a raw string in multiple places (comparison + backend throw). To avoid drift and make refactors safer, consider centralizing it in a shared constant (e.g., an exported error code enum/module) used by both the auth layer and UI.
desoindx
force-pushed
the
feat/new-login
branch
from
588728f to
7d34999
Compare
desoindx
force-pushed
the
feat/new-login
branch
from
7d34999 to
9c817c5
Compare
No description provided.