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

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

Closed
carolineBda opened this issue Sep 10, 2024 · 20 comments
Closed

Comments

@carolineBda
Copy link
Contributor

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

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
Copy link
Collaborator

Hello @carolineBda,

Thanks for the kind words,

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

@maxgfr
Copy link

maxgfr commented Sep 12, 2024

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
Copy link
Collaborator

@maxgfr l'approche proposé de quoi exactement?

@carolineBda
Copy link
Contributor Author

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
Copy link

maxgfr commented Sep 13, 2024

@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
Copy link
Collaborator

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
Copy link
Collaborator

Refs :

@maxgfr
Copy link

maxgfr commented Sep 13, 2024

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
Copy link
Collaborator

Merci @lsagetlethias d'avoir pris le temps!

@garronej
Copy link
Collaborator

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 :

Screen.Recording.2024-09-13.at.16.47.46.mov

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
Copy link
Contributor Author

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
Copy link
Collaborator

@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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Collaborator

garronej commented Sep 16, 2024

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
Copy link
Contributor Author

carolineBda commented Sep 16, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

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

@carolineBda
Copy link
Contributor Author

Merci pour la release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants