From 44a6783ca2b47e6f6a28345ed097b0877f224231 Mon Sep 17 00:00:00 2001
From: amercader <amercadero@gmail.com>
Date: Mon, 9 May 2016 11:10:37 +0100
Subject: [PATCH] [#3002] Only allow JSONP callback on the action API for GET
 requests

Updated the tests from the legacy ones.
---
 ckan/controllers/api.py                |  6 ++--
 ckan/tests/controllers/test_api.py     | 50 ++++++++++++++++++++++++++
 ckan/tests/legacy/logic/test_action.py | 16 ---------
 3 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/ckan/controllers/api.py b/ckan/controllers/api.py
index de734483b86..a32ba722ef3 100644
--- a/ckan/controllers/api.py
+++ b/ckan/controllers/api.py
@@ -89,9 +89,9 @@ def _finish(self, status_int, response_data=None,
             else:
                 response_msg = response_data
             # Support "JSONP" callback.
-            if status_int == 200 and 'callback' in request.params and \
-                (request.method == 'GET' or
-                 c.logic_function and request.method == 'POST'):
+            if (status_int == 200 and
+                    'callback' in request.params and
+                    request.method == 'GET'):
                 # escape callback to remove '<', '&', '>' chars
                 callback = cgi.escape(request.params['callback'])
                 response_msg = self._wrap_jsonp(callback, response_msg)
diff --git a/ckan/tests/controllers/test_api.py b/ckan/tests/controllers/test_api.py
index 36b666d21aa..90aaa3dfe34 100644
--- a/ckan/tests/controllers/test_api.py
+++ b/ckan/tests/controllers/test_api.py
@@ -3,6 +3,7 @@
 controller itself.
 '''
 import json
+import re
 
 from routes import url_for
 from nose.tools import assert_equal
@@ -13,6 +14,9 @@
 from ckan import model
 
 
+eq_ = assert_equal
+
+
 class TestApiController(helpers.FunctionalTestBase):
 
     def test_unicode_in_error_message_works_ok(self):
@@ -201,6 +205,52 @@ def test_config_option_list_access_sysadmin_jsonp(self):
             status=403,
         )
 
+    def test_jsonp_works_on_get_requests(self):
+
+        dataset1 = factories.Dataset()
+        dataset2 = factories.Dataset()
+
+        url = url_for(
+            controller='api',
+            action='action',
+            logic_function='package_list',
+            ver='/3')
+        app = self._get_test_app()
+        res = app.get(
+            url=url,
+            params={'callback': 'my_callback'},
+        )
+        assert re.match('my_callback\(.*\);', res.body), res
+        # Unwrap JSONP callback (we want to look at the data).
+        msg = res.body[len('my_callback')+1:-2]
+        res_dict = json.loads(msg)
+        eq_(res_dict['success'], True)
+        eq_(sorted(res_dict['result']),
+            sorted([dataset1['name'], dataset2['name']]))
+
+    def test_jsonp_does_not_work_on_post_requests(self):
+
+        dataset1 = factories.Dataset()
+        dataset2 = factories.Dataset()
+
+        url = url_for(
+            controller='api',
+            action='action',
+            logic_function='package_list',
+            ver='/3',
+            callback='my_callback',
+        )
+        app = self._get_test_app()
+        res = app.post(
+            url=url,
+        )
+        # The callback param is ignored and the normal response is returned
+        assert not res.body.startswith('my_callback')
+        res_dict = json.loads(res.body)
+        eq_(res_dict['success'], True)
+        eq_(sorted(res_dict['result']),
+            sorted([dataset1['name'], dataset2['name']]))
+
 
 class TestRevisionSearch(helpers.FunctionalTestBase):
 
diff --git a/ckan/tests/legacy/logic/test_action.py b/ckan/tests/legacy/logic/test_action.py
index 2c109002bea..334a6b87c52 100644
--- a/ckan/tests/legacy/logic/test_action.py
+++ b/ckan/tests/legacy/logic/test_action.py
@@ -114,22 +114,6 @@ def test_01_package_list_private(self):
         assert 'public_dataset' in res
         assert not 'private_dataset' in res
 
-    def test_01_package_show_with_jsonp(self):
-        anna_id = model.Package.by_name(u'annakarenina').id
-        postparams = '%s=1' % json.dumps({'id': anna_id})
-        res = self.app.post('/api/action/package_show?callback=jsoncallback', params=postparams)
-
-        assert re.match('jsoncallback\(.*\);', res.body), res
-        # Unwrap JSONP callback (we want to look at the data).
-        msg = res.body[len('jsoncallback')+1:-2]
-        res_dict = json.loads(msg)
-        assert_equal(res_dict['success'], True)
-        assert "/api/3/action/help_show?name=package_show" in res_dict['help']
-        pkg = res_dict['result']
-        assert_equal(pkg['name'], 'annakarenina')
-        missing_keys = set(('title', 'groups')) - set(pkg.keys())
-        assert not missing_keys, missing_keys
-
     def test_02_package_autocomplete_match_name(self):
         postparams = '%s=1' % json.dumps({'q':'war', 'limit': 5})
         res = self.app.post('/api/action/package_autocomplete', params=postparams)