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

[FEATURE] Faire en sorte que le cookie de langue de pix.org soit lisible sur app.pix.org (PIX-6904) #466

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

lego-technix
Copy link
Contributor

@lego-technix lego-technix commented Jan 13, 2023

🎄 Problème

On ne peut pas accéder au cookie locale de https://pix.org/ sur https://app.pix.org/ et donc on ne peut pas réutiliser cette information dans un contexte plus large, notamment :

  • pour que l'utilisateur puisse utiliser Pix App immédiatement dans la locale qu'il a choisi lorsqu'il s'est rendu sur https://pix.org/
  • pour que Pix puisse par la suite effectuer des statistiques sur les locales utilisées par les utilisateurs

🎁 Proposition

Positionner le cookie locale spécifiquement sur le domaine et ne pas laisser le cookie être positionné par défaut sur le host, cf. https://developer.mozilla.org/en-US/docs/Web/API/Document/cookie

🌟 Remarques

Consentement

On n'a pas à demander de consentement des utilisateurs pour stocker et utiliser ce cookie locale sur aucun site web de Pix, que ce soit https://pix.org/, https://app.pix.org, etc. En effet le choix de la langue fait partie des exceptions (explicitement listée même) définies par la CNIL :

Les traceurs qui sont exemptés de consentement … les traceurs de personnalisation de l'interface utilisateur (par exemple, pour le choix de la langue ou de la présentation d’un service), lorsqu’une telle personnalisation constitue un élément intrinsèque et attendu du service

cf. https://www.cnil.fr/en/node/120386#:~:text=Qu%27entend%2Don%20par%20les%20termes%20%22cookies%22%20ou%20%22%20traceurs%20%22%3F

Domaine

Le terme domain dans la Web API des cookies est polysémique et peut donc apporter de la confusion.

Quand le cookie est positionné sur le domaine alors la propriété domain du cookie contient un . devant le nom du domaine. Ainsi cette PR crée des cookies avec la propriété domain ayant la valeur .pix.org, alors qu'actuellement le comportement sur https://pix.org/ est que les cookies ont par défaut une propriété domaine ayant pour valeur pix.org (sans point devant).

Portée

L'impact de cette PR est que la préférence de locale de l'utilisateur est aussi partagée entre tous les sites vitrines du même domaine (https://pix.org/ et https://pro.pix.org/ ).

À noter également que toutes toutes toutes les requêtes effectuées sur le domaine .pix.org vont faire transiter ce cookie locale.

Sécurité

Sur la sécurisation du cookie locale :

  • On ne peut pas configurer la propriété HTTPOnly car la page locale-choice de pix-site positionne le cookie locale par le biais de JavaScript côté client justement
  • On pourrait mettre la propriété Secure pour éviter certaines classes d'attaques menées par le biais du cookie. Mais
    pour l'instant nous renonçons à commiter une solution avec la propriété Secure car nous n'avons pas trouvé de condition adéquate pour décider quand positionner ce Secure. Une solution pourrait être de se baser sur une variable d'environnement (par exemple if (process.env.UNSECURE) etc.) mais cela créerait une nouvelle divergence de fonctionnement qui se rajouterait aux autres divergences de fonctionnement déjà présentes. Or c'est ce que nous cherchons à limiter dans ce projet. Donc à rediscuter plus tard.

🎅 Pour tester

  1. Aller sur la RA de pix.org, choisir sa locale et constater que le cookie est positionné sur .pix.org
  2. Aller sur https://app.pix.org et constater que ce cookie nouvellement créé est bien disponible dans ce contexte

@lego-technix lego-technix self-assigned this Jan 13, 2023
@lego-technix lego-technix marked this pull request as draft January 13, 2023 11:14
@pix-service
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-pr466/environment

@lego-technix lego-technix force-pushed the feat-set-cookie-on-domain branch 3 times, most recently from 7225893 to 5d14848 Compare January 23, 2023 10:44
@lego-technix lego-technix changed the title feat: set cookie on domain [FEATURE] Faire en sorte que le cookie de langue de pix.org soit lisible sur app.pix.org (PIX-6904) Jan 25, 2023
@lego-technix lego-technix force-pushed the feat-set-cookie-on-domain branch 2 times, most recently from b641d6b to 6a653d2 Compare January 26, 2023 08:38
@lego-technix lego-technix marked this pull request as ready for review January 26, 2023 08:38
Copy link
Member

@VincentHardouin VincentHardouin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Top c'est marrant ça marche déjà entre pix.org et pro.pix.org du coup, on est directement redirigé 🎉

@VincentHardouin
Copy link
Member

Est-ce qu'on peut noter que l'impact de cette PR est que notre préférence est partagé entre tous les sites vitrines

@lego-technix
Copy link
Contributor Author

Est-ce qu'on peut noter que l'impact de cette PR est que notre préférence est partagé entre tous les sites vitrines

@VincentHardouin oui et même que toutes les requêtes vont faire transiter ce cookie. Je l'ajoute tout de suite.

@yannbertrand
Copy link
Member

Je vois 2 petits impacts fonctionnel :

  • Cette modification ne concerne que les nouveaux dépots de cookie, donc les utilisateurs qui ont déjà fait leur choix dans les semaines passées n'auront pas cette fonctionnalité tant qu'ils n'ont pas changé de locale préférée (via le locale switcher).
  • En l'état, le cookie est partagé entre Pix Site et Pix Pro. Comme on a pas les mêmes locales disponibles sur les deux sites, ça peut poser problème si un utilisateur fr-BE est arrivé de pro.pix.org et doit donc choisir fr (qui sera transmis dans pix.org). Je pense que ce cas peut être considéré comme suffisamment à la marge pour ne pas s'en inquiéter.

Pour éviter le 1er point, je pense qu'une transmission de la locale via query param pourrait solutionner le soucis mais peut être que ce n'est pas si grave non plus.

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.

7 participants