Hypertopic / AAAforREST

An HTTP reverse proxy to bring authentication, authorization and accounting to RESTful applications
GNU Affero General Public License v3.0
6 stars 5 forks source link

FIX : Safari should not loose the session #165 #49

Closed jejroux closed 3 years ago

jejroux commented 3 years ago

This pull request adds the environment variable 'trust proxy' and some cookie parameters to be able to do HTTPS requests.

benel commented 3 years ago

Merci pour cette contribution @jejroux !

Vous aviez testé les tests de AAAforREST sur votre poste ?

jejroux commented 3 years ago

Il me semblait avoir fait les tests pourtant, je vais revoir tout ça et je corrigerai le commit.

benel commented 3 years ago

Il me semblait avoir fait les tests pourtant, je vais revoir tout ça et je corrigerai le commit.

Les tests de Porphyry ou de AAAforREST ?

jejroux commented 3 years ago

Les tests de Porphyry ou de AAAforREST ?

Les deux, mais j'avais peut-être oublié de retester AAAforREST après les modifications... Puisque je viens de lancer les tests sur mon poste et en effet j'ai le même échec.

jejroux commented 3 years ago

Je ne suis pas sûr de moi, mais je pense que c'est le problème que j'avais rencontré la semaine dernière. Lorsque j'avais essayé de m'authentifier sans les modifications et sans ngrok, ça "fonctionnait". En tout cas j'étais authentifié et je le restais. Ensuite, en utilisant ngrok pour passer AAAforREST en HTTPS et en ayant fait les modifications nécessaire sur les cookies et la variable d'environnement 'trust proxy', ça fonctionnait aussi. Mais mon erreur a été d'oublier de lancer les tests automatisés à ce moment là. J'ai ensuite essayé de m'authentifier en gardant les modifications mais sans utiliser ngrok. Là l'authentification n'était pas fonctionnelle (comme les tests automatisés l'indiquent). Finalement le résultat des tests est le même que ce soit avec ou sans ngrok, alors que l'authentification est bien fonctionnelle avec ngrok. Je ne sais pas s'il est possible d'ajouter l'utilisation de ngrok dans ces tests ou si nous pouvons trouver une façon de contourner ce problème...

jejroux commented 3 years ago

J'ai fait quelques recherches pour voir si on peut inclure ngrok dans les tests automatisés. Je ne sais pas si c'est une bonne solution mais c'est ma seule idée pour le moment. Cela consisterait à ajouter quelques lignes dans le fichier .travis.yml faire un npm install de ngrok, lancer la commande qui permet de passer le port voulu en HTTPS. Mais il faudrait aussi remplacer les URI codés en durs : http://localhost:1337 par l'URI donnée par ngrok mais pour ça je ne sais pas encore comment on pourrait faire.

benel commented 3 years ago

il faudrait aussi remplacer les URI codés en durs : http://localhost:1337 par l'URI donnée par ngrok mais pour ça je ne sais pas encore comment on pourrait faire.

Ça doit être faisable en "shell" : récupérer l'URI et la passer en paramètres à la commande qui lance les tests. Le problème ensuite c'est qu'il faut passer ce paramètre aux tests Jest.

Une autre solution serait d'installer un petit proxy avec un certificat autosigné. Aviez-vous testé cette solution en local ?

jejroux commented 3 years ago

Non je n'avais pas essayé mais je vais voir ça !

jejroux commented 3 years ago

J'ai trouvé ce site (sur lequel j'étais déjà tombé il y a quelques semaines) mais j'ai dû mal à l'appliquer au fichier index.js de AAAforREST. Je vais essayer plusieurs façons de faire.

benel commented 3 years ago

Heu, non, ne modifiez pas AAAforREST. Ce n'est pas une nouvelle fonctionnalité de AAAforREST qu'il vous faut. Il faut juste simuler le proxy https (par exemple celui de l'UTT) pour que les tests passent. L'approche serait par exemple de configurer un petit nginx pour faire le passage à https.

benel commented 3 years ago

Il y a quand même un truc bizarre, @jejroux ...

Quand on fait :

curl -X POST -H 'Content-Type: application/x-www-form-urlencoded' -H 'Origin: https://localhost:3000/' -i http://localhost:1337/_session --data 'name=alice&password=whiterabbit'

Avec la nouvelle configuration (sameSite:none et secure:true), on a :

HTTP/1.1 200 OK
X-Powered-By: Express
Access-Control-Allow-Origin: https://localhost:3000
Vary: Origin
Access-Control-Allow-Credentials: true
Content-Type: application/json; charset=utf-8
Content-Length: 26
ETag: W/"1a-VxXIIVs2Z+NIgNis5T1vHZb/P+4"
Date: Wed, 03 Feb 2021 13:10:58 GMT
Connection: keep-alive
Keep-Alive: timeout=5

Donc, même si tout se passe bien, aucune session n'est créée. Alors qu'avec l'ancienne configuration, on avait :

HTTP/1.1 200 OK
X-Powered-By: Express
Access-Control-Allow-Origin: https://localhost:3000
Vary: Origin
Access-Control-Allow-Credentials: true
Content-Type: application/json; charset=utf-8
Content-Length: 26
ETag: W/"1a-VxXIIVs2Z+NIgNis5T1vHZb/P+4"
Set-Cookie: connect.sid=s%3AsJS4dCaDuatL16JYVrs-r_IhbOAzVbkJ.%2BvD85xOdyNEV%2FAqJf7q5BS09VY3%2FlaIIjNaXkh02c9Q; Path=/; HttpOnly
Set-Cookie: connect.sid=s%3AsJS4dCaDuatL16JYVrs-r_IhbOAzVbkJ.%2BvD85xOdyNEV%2FAqJf7q5BS09VY3%2FlaIIjNaXkh02c9Q; Path=/; HttpOnly
Date: Wed, 03 Feb 2021 13:13:09 GMT
Connection: keep-alive
Keep-Alive: timeout=5

Je croyais que ce blocage de la session n'arrivait que quand le enable proxy n'était pas en place... Qu'avaient donné vos tests ? Vous souvenez-vous des différents cas ?

jejroux commented 3 years ago

Quand vous dîtes avec l'ancienne config, vous voulez dire également avec les "anciens" navigateurs ?

jejroux commented 3 years ago

Je n'ai plus vraiment en tête tous les résultats que j'ai pu avoir, il faut que je revois tout ça...

benel commented 3 years ago

Quand vous dîtes avec l'ancienne config, vous voulez dire également avec les "anciens" navigateurs ?

Non, seulement, en commentant les lignes que vous avez ajoutées dans la configuration.

Autrement dit c'est comme si le plugin "session" trouvait que la situation était trop dangereuse (alors que vous avez indiqué "trust proxy").

benel commented 3 years ago

En fait, la documentation mentionne un mode pour secure qui justement permet de combiner des tests en http et une mise en prod en https. C'est le mode auto.

benel commented 3 years ago

@jejroux Si vous pouvez me déclarer comme collaborateur de votre branche, je la mettrai en jour...