Repository: datagouv/udata-front (language: python)

Issue:
Use url_for instead of request.url in security next param
Fix https://github.com/etalab/data.gouv.fr/issues/509.

Uses `url_for(request.endpoint, **request.view_args)` instead of `request.url`.

Some security pages in `udata` also use `next=request.url`. Not sure if the same issue could happen there ?


Discussion / hints:
> Do we know why the loop caused a crash BTW?

Nope, I couldn't find the reason, and couldn't reproduce it locally. It didn't look like a loop in the log, I'll give it another look.

Maybe 20 or 30 clicks on staging

<img width="1532" alt="Capture d’écran 2021-09-08 à 17 38 48" src="https://user-images.githubusercontent.com/119625/132540643-650052db-73a8-475b-8757-e89137941ec5.png">

nginx

```
151.80.35.135 - - [08/Sep/2021:17:38:41 +0200] "GET /fr/login?next=https%3A%2F%2Fstaging.data.gouv.fr%2Ffr%2Flogin%3Fnext%3Dhttps%253A%252F%252Fstaging.data.gouv.fr%252Ffr%252Flogin%253Fnext%253Dhttps%25253A%25252F%25252Fstaging.data.gouv.fr%25252Ffr%25252Flogin%25253Fnext%25253Dhttps%2525253A%2525252F%2525252Fstaging.data.gouv.fr%2525252Ffr%2525252Flogin%2525253Fnext%2525253Dhttps%252525253A%252525252F%252525252Fstaging.data.gouv.fr%252525252Ffr%252525252Flogin%252525253Fnext%252525253Dhttps%25252525253A%25252525252F%25252525252Fstaging.data.gouv.fr%25252525252Ffr%25252525252Flogin%25252525253Fnext%25252525253Dhttps%2525252525253A%2525252525252F%2525252525252Fstaging.data.gouv.fr%2525252525252Ffr%2525252525252Flogin%2525252525253Fnext%2525252525253Dhttps%252525252525253A%252525252525252F%252525252525252Fstaging.data.gouv.fr%252525252525252Ffr%252525252525252Flogin%252525252525253Fnext%252525252525253Dhttps%25252525252525253A%25252525252525252F%25252525252525252Fstaging.data.gouv.fr%25252525252525252Ffr%25252525252525252F HTTP/1.1" 502 568 "https://staging.data.gouv.fr/fr/login?next=https%3A%2F%2Fstaging.data.gouv.fr%2Ffr%2Flogin%3Fnext%3Dhttps%253A%252F%252Fstaging.data.gouv.fr%252Ffr%252Flogin%253Fnext%253Dhttps%25253A%25252F%25252Fstaging.data.gouv.fr%25252Ffr%25252Flogin%25253Fnext%25253Dhttps%2525253A%2525252F%2525252Fstaging.data.gouv.fr%2525252Ffr%2525252Flogin%2525253Fnext%2525253Dhttps%252525253A%252525252F%252525252Fstaging.data.gouv.fr%252525252Ffr%252525252Flogin%252525253Fnext%252525253Dhttps%25252525253A%25252525252F%25252525252Fstaging.data.gouv.fr%25252525252Ffr%25252525252Flogin%25252525253Fnext%25252525253Dhttps%2525252525253A%2525252525252F%2525252525252Fstaging.data.gouv.fr%2525252525252Ffr%2525252525252Flogin%2525252525253Fnext%2525252525253Dhttps%252525252525253A%252525252525252F%252525252525252Fstaging.data.gouv.fr%252525252525252Ffr%252525252525252F" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.159 Safari/537.36"
```

```
2021/09/08 17:38:41 [error] 12906#12906: *2508994 recv() failed (104: Connection reset by peer) while reading response header from upstream, client: 151.80.35.135, server: staging.data.gouv.fr, request: "GET /fr/login?next=https%3A%2F%2Fstaging.data.gouv.fr%2Ffr%2Flogin%3Fnext%3Dhttps%253A%252F%252Fstaging.data.gouv.fr%252Ffr%252Flogin%253Fnext%253Dhttps%25253A%25252F%25252Fstaging.data.gouv.fr%25252Ffr%25252Flogin%25253Fnext%25253Dhttps%2525253A%2525252F%2525252Fstaging.data.gouv.fr%2525252Ffr%2525252Flogin%2525253Fnext%2525253Dhttps%252525253A%252525252F%252525252Fstaging.data.gouv.fr%252525252Ffr%252525252Flogin%252525253Fnext%252525253Dhttps%25252525253A%25252525252F%25252525252Fstaging.data.gouv.fr%25252525252Ffr%25252525252Flogin%25252525253Fnext%25252525253Dhttps%2525252525253A%2525252525252F%2525252525252Fstaging.data.gouv.fr%2525252525252Ffr%2525252525252Flogin%2525252525253Fnext%2525252525253Dhttps%252525252525253A%252525252525252F%252525252525252Fstaging.data.gouv.fr%252525252525252Ffr%252525252525252Flogin%252525252525253Fnext%252525252525253Dhttps%25252525252525253A%25252525252525252F%25252525252525252Fstaging.data.gouv.fr%25252525252525252Ffr%25252525252525252F HTTP/1.1", upstream: "uwsgi://unix:///tmp/staging-front.sock:", host: "staging.data.gouv.fr", referrer: "https://staging.data.gouv.fr/fr/login?next=https%3A%2F%2Fstaging.data.gouv.fr%2Ffr%2Flogin%3Fnext%3Dhttps%253A%252F%252Fstaging.data.gouv.fr%252Ffr%252Flogin%253Fnext%253Dhttps%25253A%25252F%25252Fstaging.data.gouv.fr%25252Ffr%25252Flogin%25253Fnext%25253Dhttps%2525253A%2525252F%2525252Fstaging.data.gouv.fr%2525252Ffr%2525252Flogin%2525253Fnext%2525253Dhttps%252525253A%252525252F%252525252Fstaging.data.gouv.fr%252525252Ffr%252525252Flogin%252525253Fnext%252525253Dhttps%25252525253A%25252525252F%25252525252Fstaging.data.gouv.fr%25252525252Ffr%25252525252Flogin%25252525253Fnext%25252525253Dhttps%2525252525253A%2525252525252F%2525252525252Fstaging.data.gouv.fr%2525252525252Ffr%2525252525252Flogin%2525252525253Fnext%2525252525253Dhttps%252525
```

uwsgi 

```
Wed Sep  8 17:43:10 2021 - invalid request block size: 4818 (max 4096)...skip
```

This does not seem to kill the uwsgi worker though 🤔 
> Maybe add a test?

Is this what you had in mind: https://github.com/etalab/udata-front/pull/4/commits/a2a63ef7b88b6252909dec1e8d3d3ea3d3393a97 ?
 
> Do we know why the loop caused a crash BTW?

Not exactly. When opening the url that failed with a 502 in a new tab, it doesn't crash.


Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4cb4d5613..ddec64f4f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,7 +6,7 @@
   - udata-gouvfr is now udata-front
 - Update feedparser following setuptools 58.0.2 release that drops support for `use_2to3` [#6](https://github.com/etalab/udata-front/pull/6)
 - Show correct number of latest reuses on homepage [#3](https://github.com/etalab/udata-front/pull/3)
-- Fix next value on login to prevent infinite loop [#4](https://github.com/etalab/udata-front/pull/4)
+- Fix next value on login to prevent infinite loop [#4](https://github.com/etalab/udata-front/pull/4) [#8](https://github.com/etalab/udata-front/pull/8)
 
 ## Previous udata-gouvfr changelog
 


Test patch:
diff --git a/udata_front/tests/views/test_security.py b/udata_front/tests/views/test_security.py
index acea46bb2..bccf2a9d0 100644
--- a/udata_front/tests/views/test_security.py
+++ b/udata_front/tests/views/test_security.py
@@ -10,21 +10,16 @@ class SecurityViewsTest(GouvfrFrontTestCase):
 
     def test_security_login_next_home(self):
         '''Login should redirect to the correct next endpoint: homepage'''
-        with self.app.test_request_context(''):
-            response = self.get(url_for('site.home'))
-            assert b'<a href="/en/login?next=%2Fen%2F"' in response.data
+        response = self.get(url_for('site.home'))
+        assert b'<a href="/en/login?next=%2Fen%2F"' in response.data
 
     def test_security_login_next_datasets(self):
         '''Login should redirect to the correct next endpoint: dataset'''
-        with self.app.test_request_context(''):
-            self.app.preprocess_request()
-            dataset = DatasetFactory(slug="dataset-slug")
-            response = self.get(url_for('datasets.show', dataset=dataset))
-            assert b'<a href="/en/login?next=%2Fen%2Fdatasets%2Fdataset-slug%2F"' in response.data
+        dataset = DatasetFactory(slug="dataset-slug")
+        response = self.get(url_for('datasets.show', dataset=dataset))
+        assert b'<a href="/en/login?next=%2Fen%2Fdatasets%2Fdataset-slug%2F"' in response.data
 
     def test_security_login_next_exception_site_home(self):
         '''Login should redirect to the homepage if current request has an exception'''
-        with self.app.test_request_context(''):
-            self.app.preprocess_request()
-            response = self.get(url_for('datasets.show', dataset='dataset-not-found'))
-            assert b'<a href="/en/login?next=%2Fen%2F"' in response.data
+        response = self.get(url_for('datasets.show', dataset='dataset-not-found'))
+        assert b'<a href="/en/login?next=%2Fen%2F"' in response.data