fix(i18n): les redirections serveur portent la langue de l'utilisateur#261
Merged
Conversation
localePrefix: 'as-needed' puts French on unprefixed URLs — the French prefix is /fr-BE, never the empty string — so a path like /login matches no locale prefix at all. Until #258 next-intl covered for that: the cookie branch of localeDetection read NEXT_LOCALE and 307'd the request to /en/login. #258 turned that detection off, because it was the same gate that let a background prefetch silently rewrite the user's language. The safety net went with it, and every bare redirect() started dropping English users on French pages — session expiry, login, logout, end of onboarding, admin refusal. The helper I planned to write already existed. src/i18n/navigation.ts exposes a redirect from createNavigation(routing) that returns never, throws synchronously like next/navigation's, and whose type FORCES the locale to be passed. No new module, and it removes a real hazard: eslint has no type-aware linting here, so no-floating-promises is off, and an async redirect helper would have let a forgotten await fall through — the guard would not throw, and the visitor would proceed unauthenticated with neither TypeScript nor lint complaining. TypeScript does not propagate never-narrowing from a destructured binding, so the code after each call read as reachable and surfaced 8 possibly-null errors. Each call site now returns the redirect explicitly, which makes the termination visible to both the reader and the compiler. Two things are deliberately NOT localised. Google's OAuth URL is absolute and external — prefixing it would turn it into a broken same-origin path — so it keeps next/navigation's redirect under the explicit alias redirectToExternalUrl. And auth/callback keeps resolving the locale from the cookie: the proxy matcher excludes that route, so the middleware never runs there and the header getLocale() depends on is absent; it would fall back to a path costing an extra auth.getUser() plus a users select on the OAuth hot path, for a value the cookie already carries. Both now say so in a comment. requireUser's redirectTo parameter is removed rather than validated: none of its six callers passes an argument, so localising a caller-supplied path would have invented an open-redirect surface. Measured A/B against production: /en/app used to 307 to /login (French) and now 307s to /en/login, while an unprefixed /app still goes to /login unchanged. The redirecting branches had zero coverage — the neighbouring suite only exercises getOptionalUser and never mocks navigation. The new test mocks a redirect that THROWS, because an inert mock would let requireUser fall through and return undefined as User, keeping the suite green over a guard that no longer guards. Worth stating plainly: the authenticated Playwright specs are skipped in CI, so these unit tests are the only automated net for this regression class, not a complement to an E2E one.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
🧙 Sourcery examine votre pull request ! Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in English🧙 Sourcery is reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Sorry @thierryvm, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
thierryvm
added a commit
that referenced
this pull request
Jul 26, 2026
…SEO (#265) Handoff de fin de session. Documentation uniquement, 1 fichier. **Livré** : #261 (redirections localisées), #262 (registre des destinations de nav), #263 (sélecteur de langue), #264 (SEO) — toutes mergées. **Reste à faire, ordonné** : le lot 2 de la refonte UX en tête, avec les trois limites que le lot 1 s'est explicitement interdit de traiter reprises comme entrées plutôt que laissées à re-découvrir. Puis la reconnexion fantôme, sans diagnostic à ce jour. Puis les dettes tracées. **Frictions d'environnement consignées** parce qu'elles ont coûté du temps deux fois chacune : `playwright.config.ts` ne charge pas `.env.local`, `e2e:auth` sert un build prod où le rate limit échoue en fermé sur l'Upstash factice, la variable est `E2E_BASE_URL`, et le port 3000 appartient à un autre projet. Miroir dans le vault Obsidian, slug vérifié via `_index.md`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ferme le P0 laissé ouvert par #258, que tu avais arbitré.
Le problème
localePrefix: 'as-needed'met le français sur les URLs non préfixées : le préfixe du français est/fr-BE, jamais la chaîne vide. Un chemin comme/loginne matche donc aucun préfixe.Jusqu'à #258, next-intl rattrapait le coup via la branche cookie de
localeDetection. #258 a désactivé cette détection — c'était la même garde qui laissait un prefetch réécrire silencieusement ta langue. Le filet est parti avec, et chaqueredirect()nu renvoyait un utilisateur anglophone sur une page française : session expirée, connexion, déconnexion, fin d'onboarding, refus admin.Le correctif
Le helper que je comptais écrire existait déjà —
plan-reviewerl'a relevé, et c'est meilleur que ma proposition :Ce
redirectde@/i18n/navigationretournenever, lève immédiatement, et son type impose la locale.Le piège évité
J'avais envisagé un helper
async(await redirectLocalised('/login')). Vérifié :no-floating-promisesn'est pas activé (pas de lint typé). Unawaitoublié sur une garde d'auth n'aurait levé ni erreur TypeScript ni erreur de lint — la garde ne lèverait plus et l'utilisateur passerait non authentifié. Le redirect synchrone rend cette classe d'erreur impossible.Terminaison explicite
TypeScript ne propage pas le narrowing
neverdepuis un destructuring, contrairement auredirectdenext/navigation: 8 erreurspossibly nullsont apparues. Corrigé par unreturn redirect(...)à chaque site.Ce qui n'est délibérément PAS localisé
redirectdenext/navigationsous l'alias expliciteredirectToExternalUrl.auth/callbackgarde sa résolution par cookie : la route est exclue du matcher du proxy, doncgetLocale()y retomberait sur un chemin coûtant unauth.getUser()+ un selectuserssur le chemin chaud de l'OAuth.proxy.tsetrouting.tsne sont pas touchés — un 307 par lecture de cookie dans le proxy est l'alternative tentante en une ligne, interdite par la note « do not retry that layer ».Le paramètre
redirectToderequireUserest supprimé : aucun de ses 6 appelants ne le passe, le localiser aurait inventé une surface d'open redirect.Preuve
/en/app— visiteur anglais307 → /login❌ français307 → /en/login✅/app— visiteur français307 → /login307 → /login✅ inchangéCes branches avaient zéro couverture : la suite voisine n'exerce que
getOptionalUser. Le nouveau test mocke unredirectqui lève — un mock inerte laisseraitrequireUserretournerundefined as Useret le test resterait vert au-dessus d'une garde qui ne garde plus.Angle mort assumé : les specs Playwright authentifiées sont skippées en CI. Ces tests unitaires sont le seul filet automatisé pour cette régression, pas un complément.
typecheck0 ·lint0 ·test1652/1652Rapport :
docs/prs/PR-i18n-localised-server-redirects-report.mdSummary by Sourcery
Garantir que les redirections d’authentification et d’onboarding côté serveur préservent la locale de l’utilisateur au lieu de cibler systématiquement les pages en français.
Bug Fixes :
Enhancements :
returnpour satisfaire l’analyse de contrôle de flux de TypeScript.requireUserafin d’éviter d’introduire une surface de redirection ouverte inutile.Documentation :
Tests :
requireUseretrequireUserWithWorkspace, et garantir que ces gardes ne peuvent pas être contournés silencieusement.next-intlau lieu denext/navigation.Original summary in English
Summary by Sourcery
Ensure server-side auth and onboarding redirects preserve the user’s locale instead of always targeting the French pages.
Bug Fixes:
Enhancements:
Documentation:
Tests: