search

datagouv / data.gouv.fr

Ce dépôt rassemble les tickets techniques qui portent sur data.gouv.fr.
https://www.data.gouv.fr
76 stars 14 forks source link

OPTIONS: ne pas retourner de code redirect (3xx) ou erreur #1359

Closed abulte closed 5 months ago

abulte commented 7 months ago

Conditions de reproduction :

Dans ce cas :

Un code de redirect n'est pas acceptable pour une requête preflight (voir Spécifications, étape 7 : ok status). Chrome affiche une erreur Redirect is not allowed for a preflight request..

Il faudrait donc retourner une réponse de type 2xx (probablement 204) pour OPTIONS, même si la requête GET subséquente retournera bien un 3xx. Cf la conf simplifiée ici qui retourne toujours un 204.

Ci-dessous un exemple pour https://demo.data.gouv.fr/api/2/topics/plantabilite-des-arbres/ qui redirige vers https://demo.data.gouv.fr/api/2/topics/calque-de-plantabilite/.

Requête :

OPTIONS /api/2/topics/plantabilite-des-arbres/ HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate, br, zstd
Accept-Language: fr-FR,fr;q=0.9,en-US;q=0.8,en;q=0.7
Access-Control-Request-Headers: authorization
Access-Control-Request-Method: GET
Connection: keep-alive
Host: demo.data.gouv.fr
Origin: https://demo.ecologie.data.gouv.fr
Referer: https://demo.ecologie.data.gouv.fr/
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36

Réponse :

HTTP/1.1 308 PERMANENT REDIRECT
server: nginx
date: Thu, 02 May 2024 10:22:47 GMT
content-type: text/html; charset=utf-8
content-length: 261
location: /api/2/topics/calque-de-plantabilite/
access-control-allow-origin: https://demo.ecologie.data.gouv.fr
access-control-allow-headers: authorization
access-control-allow-methods: DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT
vary: Origin
pragma: public
cache-control: public
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-frame-options: SAMEORIGIN
maudetes commented 7 months ago

Merci pour le ticket déjà bien détaillé ! Vous avez une solution de contournement ou c'est vraiment très bloquant chez vous ?

abulte commented 7 months ago

Je vois deux contournements :

  1. faire un truc compliqué et moche pour faire une première requête non authentifiée et détecter s'il y a un redirect
  2. utiliser des ids au lieu des slugs dans les URLs

1/ je ne pense vraiment pas me lancer là-dedans 👹 et 2/ c'est triste 😢 surtout qu'on travaille sur le SEO en ce moment et que les slugs c'est bien mieux à ce niveau là (et pour les usagers au passage).

Donc ça dépend de ce que tu appelles très bloquant :-) Bloquant mais pas très ?

abulte commented 6 months ago

Idem 404

Access to XMLHttpRequest at 'https://demo.data.gouv.fr/api/2/topics/xxx/' from origin 'http://localhost:5173' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: It does not have HTTP ok status.

abulte commented 6 months ago

@maudetes on a identifié un autre cas où ça nous bloque (https://github.com/ecolabdata/ecospheres/issues/221) donc je change ma priorisation à très bloquant 👹 😉 🙏

ThibaudDauce commented 6 months ago

A priori le problème vient de la spécification de CORS plutôt que de Nginx ou Flask… Il y a quelques propositions ici sur la page de Firefox https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#preflighted_requests_and_redirects mais la spécification de CORS est en train de changer pour autoriser les redirections de ce que j'en comprends… Peut-être que la solution la plus simple dans ton cas c'est de ne pas envoyer le header Authorization pour ces requêtes là (qui ne le nécessitent pas ?)

ThibaudDauce commented 6 months ago

Précision, la section sur le site de Firefox parle d'une redirection après la requête pre-flight ce qui n'est peut-être pas la même chose qu'une redirection en retour de la requête pre-flight, mais je ne vois vraiment pas commet ça pourrait fonctionner côté Python où on a forcément la redirection… Dans tous les cas, ne pas envoyer le header Authorization devrait fonctionner je pense…

ThibaudDauce commented 6 months ago

J'ai push une PR https://github.com/opendatateam/udata/pull/3046 qui corrige le problème mais je ne suis pas du tout sûr de la validité de faire ça et si ça posera pas de problème autre part…

abulte commented 6 months ago

Effectivement ça marcherait de ne pas envoyer le header Authorization mais ce n'est pas trivial car il faudrait indiquer pour chaque appel à l'API si on a besoin de l'Authorization ou non. Discriminer sur le verbe ne suffirait pas (eg GET /me a besoin de Authorization). Possible aussi qu'on ait des cas où on a à la fois besoin d'être authentifié et de faire une requête sur une page qui atterrit en redirect. Par exemple sur un dataset supprimé (410) visible seulement pour un admin ou le owner.

Cool pour https://github.com/opendatateam/udata/pull/3046 mais ça ne traite que le cas des redirections, on en aurait aussi besoin pour les 4xx, voire même les 5xx parce qu'on peut vouloir réagir de manière différente dans ces cas là (d'ailleurs dans ce cas, impossible de savoir à l'avance si on a besoin d'être authentifié ou non).

Je pense que le plus simple pour vous est de le gérer au niveau Nginx. Moins de code à maintenir et réglé une fois pour toute. Vous pourriez même désactiver Flask-Cors quand pas en mode DEBUG.

NB : quelques issues pas très intéressantes sur Flask-Cors

ThibaudDauce commented 5 months ago

@jordanguedj Tu as un avis sur ce problème ? On le fait côté Nginx ?

ThibaudDauce commented 4 months ago

Salut, j'ai déployé sur demo une nouvelle version avec les CORS corrigés. Est-ce que vous avez des pages spécifiques qui plantaient sur ecologie.data.gouv.fr que l'on pourrait tester sur demo.ecologie.data.gouv.fr et vérifier que ça fonctionne mieux ?

abulte commented 4 months ago

Hello ! Plus de page qui plante depuis https://github.com/opendatateam/udata-front-kit/pull/464, on contourne la plupart des problèmes en n'envoyant pas le header d'authorization quand pas besoin.

C'est ok en démo sur les redirections

❯ curl -X OPTIONS -i https://demo.data.gouv.fr/api/2/topics/premier-slug/
HTTP/1.1 308 PERMANENT REDIRECT
server: nginx
date: Wed, 10 Jul 2024 07:39:34 GMT

Ok 401 sur /api/me

❯ curl -X OPTIONS -i https://demo.data.gouv.fr/api/1/me/
HTTP/1.1 200 OK
server: nginx
date: Wed, 10 Jul 2024 07:43:45 GMT

Mais pas sur les autres codes, comme 404 (logique, vu la PR).

❯ curl -X OPTIONS -i https://demo.data.gouv.fr/api/2/topics/not-a-topic/
HTTP/1.1 404 NOT FOUND
server: nginx
date: Wed, 10 Jul 2024 07:39:53 GMT

Je n'ai pas de endpoint pour tester les 5xx, mais ce serait bien qu'on puisse les gérer proprement dans la SPA et donc récupérer la "vraie" requête. Ce serait intéressant d'avoir un endpoint qui retourne arbitrairement une 500 d'ailleurs.

ThibaudDauce commented 4 months ago

Les requêtes preflight doivent contenir un header Access-Control-Request-Method et un Origin sinon elles ne sont pas valides (donc les tests avec uniquement -X OPTIONS n'exerce pas le nouveau code)

Mais tant que ça fonctionne pour vous, ça me va. Si vous rencontrez d'autres soucis n'hésitez pas à me les envoyer. (normalement le code fonctionne pour tous les codes d'erreurs 3xx, 4xx, 5xx…)

https://demo.data.gouv.fr/api/2/topics/premier-slug/

➜  udata git:(fix_cors) curl -X OPTIONS -H "Access-Control-Request-Method: GET" -H "Origin: demo.ecologie.data.gouv.fr" -i https://demo.data.gouv.fr/api/2/topics/premier-slug/
HTTP/1.1 204 NO CONTENT
server: nginx
date: Wed, 10 Jul 2024 07:59:29 GMT
content-type: text/html; charset=utf-8
access-control-allow-origin: demo.ecologie.data.gouv.fr
vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers, Access-Control-Request-Method, Origin
access-control-allow-credentials: true
access-control-allow-methods: GET
access-control-allow-headers: 
pragma: public
cache-control: public
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-frame-options: SAMEORIGIN

https://demo.data.gouv.fr/api/1/me/

➜  udata git:(fix_cors) curl -X OPTIONS -H "Access-Control-Request-Method: GET" -H "Origin: demo.ecologie.data.gouv.fr" -i https://demo.data.gouv.fr/api/1/me/
HTTP/1.1 204 NO CONTENT
server: nginx
date: Wed, 10 Jul 2024 07:59:03 GMT
content-type: text/html; charset=utf-8
access-control-allow-origin: demo.ecologie.data.gouv.fr
vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers, Access-Control-Request-Method, Origin
access-control-allow-credentials: true
access-control-allow-methods: GET
access-control-allow-headers: 
pragma: public
cache-control: public
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-frame-options: SAMEORIGIN

https://demo.data.gouv.fr/api/2/topics/not-a-topic/ 

➜  udata git:(fix_cors) curl -X OPTIONS -H "Access-Control-Request-Method: GET" -H "Origin: demo.ecologie.data.gouv.fr" -i https://demo.data.gouv.fr/api/2/topics/not-a-topic/ 
HTTP/1.1 204 NO CONTENT
server: nginx
date: Wed, 10 Jul 2024 08:00:15 GMT
content-type: text/html; charset=utf-8
access-control-allow-origin: demo.ecologie.data.gouv.fr
vary: Origin, Access-Control-Request-Method, Access-Control-Request-Headers, Access-Control-Request-Method, Origin
access-control-allow-credentials: true
access-control-allow-methods: GET
access-control-allow-headers: 
pragma: public
cache-control: public
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-frame-options: SAMEORIGIN
abulte commented 4 months ago

Ah ok, c'est https://github.com/opendatateam/udata/pull/3074 que tu as mis sur démo ? Looks nice, je vais faire quelques tests depuis notre SPA et je te dis.

abulte commented 4 months ago

C'est KO même avec les contournements actuels sur démo.

Tu peux tester sur demo.ecologie.data.gouv.fr et essayer de te logguer. Au retour oauth, la requête vers https://demo.data.gouv.fr/oauth/token plante. A priori la requête preflight est bien passée puisque c'est le POST qui pose problème.

Access to XMLHttpRequest at 'https://demo.data.gouv.fr/oauth/token' from origin 'https://demo.ecologie.data.gouv.fr' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

Request:

POST /oauth/token HTTP/1.1
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate, br, zstd
Accept-Language: fr-FR,fr;q=0.9,en-US;q=0.8,en;q=0.7
Connection: keep-alive
Content-Length: 872
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryC3qW654q1rZ54elU
Host: demo.data.gouv.fr
Origin: https://demo.ecologie.data.gouv.fr
Referer: https://demo.ecologie.data.gouv.fr/
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36
sec-ch-ua: "Google Chrome";v="125", "Chromium";v="125", "Not.A/Brand";v="24"
sec-ch-ua-mobile: ?0

Response:

HTTP/1.1 200 OK
server: nginx
date: Thu, 11 Jul 2024 07:01:29 GMT
content-type: application/json
content-length: 178
cache-control: no-store
pragma: no-cache
pragma: public
cache-control: public
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
x-frame-options: SAMEORIGIN
ThibaudDauce commented 4 months ago

Peut-être que le problème ne vient pas de là mais j'ai activé CORS de façon open uniquement sur les endpoints API, j'avais oublié les endpoints OAuth, je viens de l'activer je redéploie et je reteste et je te dis

ThibaudDauce commented 4 months ago

C'est déployé et le flow OAuth fonctionne :-) N'hésite pas à me dire si tu vois d'autres problèmes

abulte commented 4 months ago

C'est toujours KO sur demo.ecologie.data.gouv.fr pour moi :-/

ThibaudDauce commented 4 months ago

Tu peux essayer en navigation privée pour voir si c'est pas un problème de cache ?

abulte commented 4 months ago

Oui j'ai fait ça, mais rien n'y fait. Testé sous Chrome et FF.

abulte commented 4 months ago

Je ne sais pas si tu as changé qqchose mais le login marche maintenant :-)

ThibaudDauce commented 4 months ago

Oui en fait j'avais déployé mon fix sur dev à la place de demo… :-(

abulte commented 4 months ago

Ca marche bien sans les contournements sur notre front (donc en envoyant toujours le header d'Authorization) : 308 et 404 ok 👌