Merge pull request #2254 from noirbizarre/oembed-404

Improve oembed errors handling (no hidden error, CORS)

CHANGELOG.md

@@ -7,6 +7,7 @@
 - JS models load/save/update consistency (`loading` always `true` on query, always handle error, no more silent errors) [#2247](https://github.com/opendatateam/udata/pull/2247)
 - Ensures that date ranges are always positive (ie. `start` < `end`) [#2253](https://github.com/opendatateam/udata/pull/2253)
 - Enable completion on the "`MIME type`" resource form field (needs reindexing) [#2238](https://github.com/opendatateam/udata/pull/2238)
+- Ensure oembed rendering errors are not hidden by default error handlers and have cors headers [#2254](https://github.com/opendatateam/udata/pull/2254)
 
 ## 1.6.13 (2019-07-11)
 

udata/features/oembed/api.py

@@ -3,6 +3,7 @@
 
 from flask import current_app
 from flask_restplus import inputs
+from werkzeug.exceptions import HTTPException
 
 from udata import theme
 from udata.api import api, API
@@ -51,7 +52,7 @@ def get(self):
         """
         args = oembed_parser.parse_args()
         if args['format'] != 'json':
-            api.abort(501, 'Only JSON format is supported')
+            return {'message': 'Only JSON format is supported'}, 501
 
         url = args['url']
 
@@ -61,16 +62,20 @@ def get(self):
 
         with current_app.test_request_context(url) as ctx:
             if not ctx.request.endpoint:
-                api.abort(404, 'Unknown URL')
+                return {'message': 'Unknown URL "{0}"'.format(url)}, 404
             endpoint = ctx.request.endpoint.replace('_redirect', '')
             view_args = ctx.request.view_args
 
         if endpoint not in self.ROUTES:
-            api.abort(404, 'Unknown URL')
+            return {'message': 'The URL "{0}" does not support oembed'.format(url)}, 404
 
         param = self.ROUTES[endpoint]
         item = view_args[param]
         if isinstance(item, Exception):
+            if isinstance(item, HTTPException):
+                return {
+                    'message': 'An error occured on URL "{0}": {1}'.format(url, str(item))
+                }, item.code
             raise item
         width = maxwidth = 1000
         height = maxheight = 200

udata/tests/api/test_oembed_api.py

@@ -18,7 +18,7 @@
 from udata.frontend.markdown import mdstrip
 from udata.settings import Testing
 from udata.utils import faker
-from udata.tests.helpers import assert200, assert400, assert404, assert_status
+from udata.tests.helpers import assert200, assert400, assert404, assert_status, assert_cors
 
 
 class OEmbedAPITest:
@@ -31,6 +31,7 @@ def test_oembed_for_dataset(self, api):
         url = url_for('api.oembed', url=dataset.external_url)
         response = api.get(url)
         assert200(response)
+        assert_cors(response)
         assert 'html' in response.json
         assert 'width' in response.json
         assert 'maxwidth' in response.json
@@ -49,6 +50,7 @@ def test_oembed_for_dataset_with_organization(self, api):
         url = url_for('api.oembed', url=dataset.external_url)
         response = api.get(url)
         assert200(response)
+        assert_cors(response)
 
         card = theme.render('dataset/card.html', dataset=dataset)
         assert card in response.json['html']
@@ -62,6 +64,7 @@ def test_oembed_for_dataset_redirect_link(self, api):
         url = url_for('api.oembed', url=redirect_url)
         response = api.get(url)
         assert200(response)
+        assert_cors(response)
         assert 'html' in response.json
         assert 'width' in response.json
         assert 'maxwidth' in response.json
@@ -79,6 +82,7 @@ def test_oembed_for_unknown_dataset(self, api):
         url = url_for('api.oembed', url=dataset_url)
         response = api.get(url)
         assert404(response)
+        assert_cors(response)
 
     def test_oembed_for_reuse(self, api):
         '''It should fetch a reuse in the oembed format.'''
@@ -87,6 +91,7 @@ def test_oembed_for_reuse(self, api):
         url = url_for('api.oembed', url=reuse.external_url)
         response = api.get(url)
         assert200(response)
+        assert_cors(response)
         assert 'html' in response.json
         assert 'width' in response.json
         assert 'maxwidth' in response.json
@@ -114,6 +119,7 @@ def test_oembed_with_an_unknown_url(self, api):
         url = url_for('api.oembed', url='http://local.test/somewhere')
         response = api.get(url)
         assert404(response)
+        assert_cors(response)
 
     def test_oembed_with_port_in_https_url(self, api):
         '''It should works on HTTPS URLs with explicit port.'''
@@ -130,6 +136,7 @@ def test_oembed_does_not_support_xml(self, api):
         url = url_for('api.oembed', url=dataset.external_url, format='xml')
         response = api.get(url)
         assert_status(response, 501)
+        assert_cors(response)
         assert response.json['message'] == 'Only JSON format is supported'
 
 

udata/tests/helpers.py

@@ -216,3 +216,11 @@ def assert_urls_equal(url1, url2):
     q2 = parse_qs(p2.query)
     assert q1 == q2, 'Query does not match'
     assert p1.fragment == p2.fragment, 'Fragment does not match'
+
+
+def assert_cors(response):
+    '''CORS headers presence assertion'''
+    __tracebackhide__ = True
+    assert 'Access-Control-Allow-Origin' in response.headers
+    assert 'Access-Control-Allow-Methods' in response.headers
+    assert 'Access-Control-Max-Age' in response.headers