codegouvfr / react-dsfr

🇫🇷 The French Government Design system React toolkit
https://react-dsfr.codegouv.studio
MIT License
416 stars 56 forks source link

Use of modal with strict Content-Security-Policy headers with Nextjs #303

Closed carolineBda closed 1 month ago

carolineBda commented 1 month ago

Hello and thanks for all the work done in react-dsfr :clap:

Modal does not work on our project unless we disable the CSP for script-src (unsafe-inline).

It can be tested here and here is the code.

1) As you can see, I've tried to use nonce, but I did not understood where to get the nonce string so I can add it to my CSP config. Can I hard code like that ?

2) As we aim to pass our website in static rendering, is there an other option ?

garronej commented 1 month ago

Hello @carolineBda,

Thanks for the kind words,

@lsagetlethias maybe you have some insights on this? 🙏🏻

maxgfr commented 1 month ago

C'est dommage que l'approche proposé désactive le contenu statique sur next (car on récupère à travers les headers)...

Peut-être qu'on peut se planifier un point pour voir si on peut contribuer ou refacto pour bypasser cela

garronej commented 1 month ago

@maxgfr l'approche proposé de quoi exactement?

carolineBda commented 1 month ago

Pour mon point 1) le pb d'implémentation du nonce vient de chez nous.

Pour le point 2) et le retour de @maxgfr, si l'on comprends la seule façon d'utiliser react-dsfr sans ajouter un unsafe-inline à notre config CSP est d'utiliser le nonce. Mais comme expliquer dans la doc de Nextjs => "Every time a page is viewed, a fresh nonce should be generated. This means that you must use dynamic rendering to add nonces."

Donc, sauf si l'on a pas bien compris, il n'est pas possible en l'état d'utiliser react-dsfr sur des pages static avec l'App Router de Nextjs ?

maxgfr commented 1 month ago

@garronej Yes, je parlais de l'approche suivante de la documentation :

  const nonce = headers().get("x-nonce") ?? undefined;

Qu'on utilise dans le layout.tsx de base

lsagetlethias commented 1 month ago

Hello !

react-dsfr est utilisable dans une app Nextjs statique, mais sans nonce.

Le nonce est obligatoirement récupéré via les header car il doit être unique à chaque requête comme évoqué précédemment. Du coup, aucun moyen de sortir d'un dynamic rendering.

Une solution non implémentée par Next.js serait de passer sur un nonce avec hash du script embarqué, et du coup qui serait compatible pour un site statique.

lsagetlethias commented 1 month ago

Refs :

maxgfr commented 1 month ago

Hello,

Oui, le but n'étant pas de trouver une solution en bypassant l'approche next du nonce.

La question que je me pose est : techniquement, quelle fut la raison pour laquelle on injecte du JS à la volée. Ça m'intéresserait de comprendre pourquoi avons nous besoin de cela ? Quelle feature utilise cela ?

garronej commented 1 month ago

Merci @lsagetlethias d'avoir pris le temps!

garronej commented 1 month ago

La question que je me pose est : techniquement, quelle est la raison pour laquelle on injecte du JS à la volée ? Ça m'intéresserait de comprendre pourquoi nous avons besoin de cela. Quelle fonctionnalité utilise cette technique ?

Pour éviter les "white flashes", voici un exemple de white flash :

https://github.com/user-attachments/assets/c53ee5ef-4c88-4c1e-a059-eb23ed329e3d

Cela se produit lorsque le serveur rend l'application en mode clair et qu'on doit attendre l'hydratation (l'exécution des callbacks useEffect sur le client) pour que le mode sombre soit appliqué, s'il est activé (si c'est la préférence déflault de l'OS ou si ça a été persité dans le local storage).

On a donc besoin d'un script qui applique le bon attribut HTML le plus tôt possible afin d'éviter les "white flashes".

C'est l'approche canonique qui est implémentée même sur le site officiel de React : https://react.dev/

carolineBda commented 1 month ago

Merci beaucoup pour vos réponses.

Est-ce que les "whites flashes" sont juste du au thème ? Si oui est-ce qu'il serait envisageable d'avoir une version de react-dsfr sans possibilité de changer le thème et donc sans besoin d'ajouter un script à la volée ? Nous sommes prêt à aider pour cette fonctionnalité.

garronej commented 1 month ago

@carolineBda Si par "thème" vous entendez la possibilité d'afficher le site en mode sombre et en mode clair, alors oui.
Quand on utilise Next en mode SSG ou que l'on met en cache les pages pour éviter d'exécuter du JavaScript à chaque requête, on doit générer la page sur le serveur soit en mode sombre, soit en mode clair.
Par défaut, nous générons en mode clair et envoyons cette version à tout le monde.
Cependant, côté client, si l'utilisateur préfère le mode sombre, il faut bien sûr effectuer le changement le plus tôt possible. Si on le faites dans un useEffect, il est déjà trop tard et on a un effet de flash blanc.
C'est pourquoi nous avons un script dans le head.

Si vous me demandez mon avis, je vous conseillerais simplement de désactiver les CSP. Next.js ne propose pas, à ce jour, de solution à ce problème pourtant assez classique.

Si vous tenez absolument à conserver les CSP, la seule chose que je peux vous proposer est d'opter pour une approche où vous dites : "Non, nous ne supportons pas le mode sombre, c'est mode clair uniquement". Dans ce cas, aucun script n'est nécessaire.

À mon avis, c'est dommage, mais si vous le souhaitez, je peux vous faire ca.

carolineBda commented 1 month ago

Pour l'instant nous pensons partir sur cette 2éme approche, sachant que service-public ne propose pas non plus le mode sombre.

Le problème étant que (sauf erreur de ma part) même si on enlève la possibilité de changer le thème, react-dsfr quand il se lance, ajoute quand même un script dans le head. Est-ce qu'il y a possibilité de désactiver complètement ce comportement ?

garronej commented 1 month ago

Le problème étant que (sauf erreur de ma part) même si on enlève la possibilité de changer le thème, react-dsfr quand il se lance, ajoute quand même un script dans le head. Est-ce qu'il y a possibilité de désactiver complètement ce comportement ?

Oui oui je peut le faire

garronej commented 1 month ago

Voilà @carolineBda,

J'ai release une candidate 1.13.4-rc.0.

Si vous mettez defaultColorSheme a light ici:

image

Si vous ne mettez pas de dark mode switch, dans le header tout devrais fonctionner.

Je vous laisse me confirmer

PS: Peut être que vous aurez a clean le browser cache.

carolineBda commented 1 month ago

Merci beaucoup pour cette réactivité ! Je viens d'installer cette version et j'ai encore des erreurs CSP : https://code-du-travail-numerique-carolinebda-unsafe-inline-40qm9ctn.ovh.fabrique.social.gouv.fr/mentions-legales

Est-ce que ça vient de react dsfr ?

garronej commented 1 month ago

Je ne sais pas je n'ai pas le temps d'investiguer.

Dans le doute essayez sur le starter: https://github.com/garronej/react-dsfr-next-appdir-demo £

Je vous laisse me faire un retour

carolineBda commented 1 month ago

Hello, Je vous confirme que la modif fonctionne bien et que le script ne se lance plus.

Il faut juste initialiser defaultColorScheme avec "light". Screenshot from 2024-09-17 12-16-20

Si c'est possible nous aimerions garder cette modif dans les versions futures.

Encore merci pour votre aide !

garronej commented 1 month ago

Ah oui autent pour moi j'ai fait une erreur dans mon message mais évidement je voulais dire "light"

carolineBda commented 1 month ago

Merci pour la release