Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Corriger le positionnement du cookie locale en local en mode dev #511

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

lego-technix
Copy link
Contributor

@lego-technix lego-technix commented Feb 20, 2023

🦄 Problème

Actuellement en local en dev mode, le choix de locale de l'utilisateur n'est pas enregistré.

Dans la console web l'erreur est la suivante :

Le cookie « locale » a été rejeté car le domaine est invalide.

Par ailleurs il y a également une autre erreur dans la console web :

Le cookie « locale » n’a pas de valeur d’attribut « SameSite » appropriée. Bientôt, les cookies dont l’attribut « SameSite » est manquant ou défini avec une valeur invalide seront traités comme « Lax ». Cela signifie que le cookie ne sera plus envoyé dans des contextes tiers. Si votre application dépend de la disponibilité de ce cookie dans de tels contextes, veuillez lui ajouter l’attribut « SameSite=None ». Pour en savoir plus sur l’attribut « SameSite », consultez https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite

C'est une régression introduite par #466.

🤖 Proposition

Correction de la régression

Positionnement de la propriété domain dans le cookie uniquement lorsque pas isDev.

Propriété SameSite

En plus de la correction de la régression, ajout supplémentaire d'une propriété SameSite=Strict pour corriger l'autre warning dans la console web.

Pour la propriété SameSite c'est le choix SameSite=Strict qui est fait pour les raisons suivantes :

  • SameSite=None enverrait le cookie locale tout le temps, dans tous les contextes, pour toutes les requêtes, ce n'est pas ce que nous souhaitons
  • SameSite=Lax fait que : Cookies are not sent on normal cross-site subrequests (for example to load images or frames into a third party site), but are sent when a user is navigating to the origin site (i.e., when following a link). Nous n'avons pas ce besoin.
  • SameSite=Strict fait que le cookie locale n'est envoyé que lorsqu'il est dans un contexte où il est un first-party cookie https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#third-party_cookies, ce qui convient exactement aux cas où on veut pouvoir lire/écrire la valeur du cookie locale sur les sites web *.pix.org.

🌈 Remarques

RAS

💯 Pour tester

Vérifier que le problème est corrigé

  1. Exécuter npm run dev:site:org
  2. Aller sur http://localhost:8000/ et supprimer tout cookie locale qui pourrait exister
  3. Aller sur la page racine du site web et choisir une locale
  4. Vérifier que ce choix de locale est bien enregistré dans le cookie locale

Vérifier qu'il n'y a pas de régression sur le fonctionnement de production (c'est à dire pas isDev)

  1. Aller sur la RA https://site-pr511.review.pix.org/ et supprimer tout cookie locale qui pourrait exister
  2. Aller sur la page racine du site web et choisir une locale
  3. Vérifier que ce choix de locale est bien enregistré dans le cookie locale
  4. Aller sur https://app.pix.org et constater que ce cookie nouvellement créé est bien disponible dans ce contexte et avec la propriété SameSite=Strict

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles sur scalingo https://dashboard.scalingo.com/apps/osc-fr1/pix-site-review-pr511/environment

@lego-technix lego-technix force-pushed the fix-dev-set-cookie-on-domain branch from a6f98ef to 1445864 Compare February 21, 2023 08:50
@lego-technix lego-technix force-pushed the fix-dev-set-cookie-on-domain branch 2 times, most recently from ed98ace to bdce2c7 Compare February 21, 2023 09:54
@lego-technix lego-technix marked this pull request as ready for review February 21, 2023 09:54
@lego-technix lego-technix force-pushed the fix-dev-set-cookie-on-domain branch from bdce2c7 to 06ee1a3 Compare February 21, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants