Merge branch 'poc-flask-views' into poc-flask-views.common-url_for-ta…

…ke-2

Conflicts:
	ckan/lib/base.py

ckan/config/middleware/flask_app.py

@@ -27,7 +27,7 @@
 from ckan.common import c
 from ckan.plugins import PluginImplementations
 from ckan.plugins.interfaces import IBlueprint
-from ckan.views import identify_user
+from ckan.views import identify_user, set_cors_headers_for_response
 
 
 import logging
@@ -116,6 +116,11 @@ def save_session(self, app, session, response):
     def ckan_before_request():
         identify_user()
 
+    @app.after_request
+    def ckan_after_request(response):
+        set_cors_headers_for_response(response)
+        return response
+
     # Template context processors
     @app.context_processor
     def helper_functions():

ckan/lib/base.py

@@ -7,7 +7,6 @@
 import logging
 import time
 
-from paste.deploy.converters import asbool
 from pylons import cache, config
 from pylons.controllers import WSGIController
 from pylons.controllers.util import abort as _abort
@@ -27,6 +26,7 @@
 import ckan.plugins as p
 import ckan.model as model
 import ckan.lib.maintain as maintain
+from ckan.views import identify_user, set_cors_headers_for_response
 
 # These imports are for legacy usages and will be removed soon these should
 # be imported directly from ckan.common for internal ckan code and via the
@@ -233,7 +233,6 @@ def _identify_user(self):
           c.user = None
           c.userobj = None
           c.author = user's IP address (unicode)'''
-        from ckan.views import identify_user
         identify_user()
 
     def __call__(self, environ, start_response):
@@ -278,33 +277,11 @@ def __after__(self, action, **params):
         # Do we have CORS settings in config?
         if config.get('ckan.cors.origin_allow_all') \
                 and request.headers.get('Origin'):
-            self._set_cors()
+            set_cors_headers_for_response(response)
         r_time = time.time() - c.__timer
         url = request.environ['CKAN_CURRENT_URL'].split('?')[0]
         log.info(' %s render time %.3f seconds' % (url, r_time))
 
-    def _set_cors(self):
-        '''
-        Set up Access Control Allow headers if either origin_allow_all is
-        True, or the request Origin is in the origin_whitelist.
-        '''
-        cors_origin_allowed = None
-        if asbool(config.get('ckan.cors.origin_allow_all')):
-            cors_origin_allowed = "*"
-        elif config.get('ckan.cors.origin_whitelist') and \
-                request.headers.get('Origin') \
-                in config['ckan.cors.origin_whitelist'].split(" "):
-            # set var to the origin to allow it.
-            cors_origin_allowed = request.headers.get('Origin')
-
-        if cors_origin_allowed is not None:
-            response.headers['Access-Control-Allow-Origin'] = \
-                cors_origin_allowed
-            response.headers['Access-Control-Allow-Methods'] = \
-                "POST, PUT, GET, DELETE, OPTIONS"
-            response.headers['Access-Control-Allow-Headers'] = \
-                "X-CKAN-API-KEY, Authorization, Content-Type"
-
     def _get_page_number(self, params, key='page', default=1):
         """
         Returns the page number from the provided params after

ckan/tests/config/test_middleware.py

@@ -5,6 +5,7 @@
 from nose.tools import assert_not_equals, eq_
 from routes import url_for
 from flask import Blueprint
+import flask
 
 import ckan.model as model
 import ckan.plugins as p
@@ -413,62 +414,72 @@ class TestFlaskUserIdentifiedInRequest(helpers.FunctionalTestBase):
 
     '''Flask identifies user during each request.
 
-    API route has been migrated to Flask, so using as an example.
+    Flask route provided by test.helpers.SimpleFlaskPlugin.
     '''
 
+    @classmethod
+    def setup_class(cls):
+        super(TestFlaskUserIdentifiedInRequest, cls).setup_class()
+        cls.app = cls._get_test_app()
+        cls.flask_app = helpers.find_flask_app(cls.app)
+
+        if not p.plugin_loaded('test_simple_flask_plugin'):
+            p.load('test_simple_flask_plugin')
+            plugin = p.get_plugin('test_simple_flask_plugin')
+            cls.flask_app.register_blueprint(plugin.get_blueprint(),
+                                             prioritise_rules=True)
+
+    @classmethod
+    def teardown_class(cls):
+        super(TestFlaskUserIdentifiedInRequest, cls).teardown_class()
+        p.unload('test_simple_flask_plugin')
+
     def test_user_objects_in_g_normal_user(self):
         '''
         A normal logged in user request will have expected user objects added
         to request.
         '''
-        self.app = helpers._get_test_app()
-        flask_app = helpers.find_flask_app(self.app)
         user = factories.User()
         test_user_obj = model.User.by_name(user['name'])
 
-        with flask_app.test_request_context('/api/action/status_show'):
+        with self.flask_app.app_context():
             self.app.get(
-                '/api/action/status_show',
+                '/simple_flask',
                 extra_environ={'REMOTE_USER': user['name'].encode('ascii')},)
-            eq_(c.user, user['name'])
-            eq_(c.userobj, test_user_obj)
-            eq_(c.author, user['name'])
-            eq_(c.remote_addr, 'Unknown IP Address')
+            eq_(flask.g.user, user['name'])
+            eq_(flask.g.userobj, test_user_obj)
+            eq_(flask.g.author, user['name'])
+            eq_(flask.g.remote_addr, 'Unknown IP Address')
 
     def test_user_objects_in_g_anon_user(self):
         '''
         An anon user request will have expected user objects added to request.
         '''
-        self.app = helpers._get_test_app()
-        flask_app = helpers.find_flask_app(self.app)
-
-        with flask_app.test_request_context('/api/action/status_show'):
+        with self.flask_app.app_context():
             self.app.get(
-                '/api/action/status_show',
+                '/simple_flask',
                 extra_environ={'REMOTE_USER': ''},)
-            eq_(c.user, '')
-            eq_(c.userobj, None)
-            eq_(c.author, 'Unknown IP Address')
-            eq_(c.remote_addr, 'Unknown IP Address')
+            eq_(flask.g.user, '')
+            eq_(flask.g.userobj, None)
+            eq_(flask.g.author, 'Unknown IP Address')
+            eq_(flask.g.remote_addr, 'Unknown IP Address')
 
     def test_user_objects_in_g_sysadmin(self):
         '''
         A sysadmin user request will have expected user objects added to
         request.
         '''
-        self.app = helpers._get_test_app()
-        flask_app = helpers.find_flask_app(self.app)
         user = factories.Sysadmin()
         test_user_obj = model.User.by_name(user['name'])
 
-        with flask_app.test_request_context('/api/action/status_show'):
+        with self.flask_app.app_context():
             self.app.get(
-                '/api/action/status_show',
+                '/simple_flask',
                 extra_environ={'REMOTE_USER': user['name'].encode('ascii')},)
-            eq_(c.user, user['name'])
-            eq_(c.userobj, test_user_obj)
-            eq_(c.author, user['name'])
-            eq_(c.remote_addr, 'Unknown IP Address')
+            eq_(flask.g.user, user['name'])
+            eq_(flask.g.userobj, test_user_obj)
+            eq_(flask.g.author, user['name'])
+            eq_(flask.g.remote_addr, 'Unknown IP Address')
 
 
 class TestPylonsUserIdentifiedInRequest(helpers.FunctionalTestBase):

ckan/tests/helpers.py

@@ -24,11 +24,13 @@
 from pylons import config
 import nose.tools
 import mock
+from flask import Blueprint
 
 import ckan.lib.search as search
 import ckan.config.middleware
 import ckan.model as model
 import ckan.logic as logic
+import ckan.plugins as p
 
 
 try:
@@ -468,3 +470,26 @@ def find_flask_app(test_app):
               'a reference to the app they wrap?')
     else:
         return find_flask_app(app)
+
+
+class SimpleFlaskPlugin(p.SingletonPlugin):
+
+    '''
+    A simple extension that implements the Flask IBlueprint interface.
+
+    This is useful to test a route that we know will be served by Flask.
+    '''
+
+    p.implements(p.IBlueprint)
+
+    def flask_plugin_view(self):
+        return 'Hello World, this is served from a simple Flask extension.'
+
+    def get_blueprint(self):
+        # Create Blueprint for plugin
+        blueprint = Blueprint(self.name, self.__module__)
+        # Add plugin url rule to Blueprint object
+        blueprint.add_url_rule('/simple_flask', 'flask_plugin_view',
+                               self.flask_plugin_view)
+
+        return blueprint

ckan/tests/lib/test_base.py

@@ -3,6 +3,7 @@
 from nose import tools as nose_tools
 
 import ckan.tests.helpers as helpers
+import ckan.plugins as p
 
 
 class TestRenderSnippet(helpers.FunctionalTestBase):
@@ -160,3 +161,153 @@ def test_cors_config_origin_allow_all_false_with_whitelist_not_containing_origin
         assert 'Access-Control-Allow-Origin' not in response_headers
         assert 'Access-Control-Allow-Methods' not in response_headers
         assert 'Access-Control-Allow-Headers' not in response_headers
+
+
+class TestCORSFlask(helpers.FunctionalTestBase):
+
+    @classmethod
+    def setup_class(cls):
+        super(TestCORSFlask, cls).setup_class()
+        cls.app = cls._get_test_app()
+        flask_app = helpers.find_flask_app(cls.app)
+
+        if not p.plugin_loaded('test_simple_flask_plugin'):
+            p.load('test_simple_flask_plugin')
+            plugin = p.get_plugin('test_simple_flask_plugin')
+            flask_app.register_blueprint(plugin.get_blueprint(),
+                                         prioritise_rules=True)
+
+    @classmethod
+    def teardown_class(cls):
+        super(TestCORSFlask, cls).teardown_class()
+        p.unload('test_simple_flask_plugin')
+
+    def test_options(self):
+        response = self.app.options(url='/simple_flask', status=200)
+        assert len(str(response.body)) == 0, 'OPTIONS must return no content'
+
+    def test_cors_config_no_cors(self):
+        '''
+        No ckan.cors settings in config, so no Access-Control-Allow headers in
+        response.
+        '''
+        response = self.app.get('/simple_flask')
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' not in response_headers
+        assert 'Access-Control-Allow-Methods' not in response_headers
+        assert 'Access-Control-Allow-Headers' not in response_headers
+
+    def test_cors_config_no_cors_with_origin(self):
+        '''
+        No ckan.cors settings in config, so no Access-Control-Allow headers in
+        response, even with origin header in request.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' not in response_headers
+        assert 'Access-Control-Allow-Methods' not in response_headers
+        assert 'Access-Control-Allow-Headers' not in response_headers
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'true')
+    def test_cors_config_origin_allow_all_true_no_origin(self):
+        '''
+        With origin_allow_all set to true, but no origin in the request
+        header, no Access-Control-Allow headers should be in the response.
+        '''
+        response = self.app.get('/simple_flask')
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' not in response_headers
+        assert 'Access-Control-Allow-Methods' not in response_headers
+        assert 'Access-Control-Allow-Headers' not in response_headers
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'true')
+    @helpers.change_config('ckan.site_url', 'http://test.ckan.org')
+    def test_cors_config_origin_allow_all_true_with_origin(self):
+        '''
+        With origin_allow_all set to true, and an origin in the request
+        header, the appropriate Access-Control-Allow headers should be in the
+        response.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' in response_headers
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Origin'], '*')
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Methods'], "POST, PUT, GET, DELETE, OPTIONS")
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Headers'], "X-CKAN-API-KEY, Authorization, Content-Type")
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'false')
+    @helpers.change_config('ckan.site_url', 'http://test.ckan.org')
+    def test_cors_config_origin_allow_all_false_with_origin_without_whitelist(self):
+        '''
+        With origin_allow_all set to false, with an origin in the request
+        header, but no whitelist defined, there should be no Access-Control-
+        Allow headers in the response.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' not in response_headers
+        assert 'Access-Control-Allow-Methods' not in response_headers
+        assert 'Access-Control-Allow-Headers' not in response_headers
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'false')
+    @helpers.change_config('ckan.cors.origin_whitelist', 'http://thirdpartyrequests.org')
+    @helpers.change_config('ckan.site_url', 'http://test.ckan.org')
+    def test_cors_config_origin_allow_all_false_with_whitelisted_origin(self):
+        '''
+        With origin_allow_all set to false, with an origin in the request
+        header, and a whitelist defined (containing the origin), the
+        appropriate Access-Control-Allow headers should be in the response.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' in response_headers
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Origin'], 'http://thirdpartyrequests.org')
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Methods'], "POST, PUT, GET, DELETE, OPTIONS")
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Headers'], "X-CKAN-API-KEY, Authorization, Content-Type")
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'false')
+    @helpers.change_config('ckan.cors.origin_whitelist', 'http://google.com http://thirdpartyrequests.org http://yahoo.co.uk')
+    @helpers.change_config('ckan.site_url', 'http://test.ckan.org')
+    def test_cors_config_origin_allow_all_false_with_multiple_whitelisted_origins(self):
+        '''
+        With origin_allow_all set to false, with an origin in the request
+        header, and a whitelist defining multiple allowed origins (containing
+        the origin), the appropriate Access-Control-Allow headers should be in
+        the response.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' in response_headers
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Origin'], 'http://thirdpartyrequests.org')
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Methods'], "POST, PUT, GET, DELETE, OPTIONS")
+        nose_tools.assert_equal(response_headers['Access-Control-Allow-Headers'], "X-CKAN-API-KEY, Authorization, Content-Type")
+
+    @helpers.change_config('ckan.cors.origin_allow_all', 'false')
+    @helpers.change_config('ckan.cors.origin_whitelist', 'http://google.com http://yahoo.co.uk')
+    @helpers.change_config('ckan.site_url', 'http://test.ckan.org')
+    def test_cors_config_origin_allow_all_false_with_whitelist_not_containing_origin(self):
+        '''
+        With origin_allow_all set to false, with an origin in the request
+        header, and a whitelist defining multiple allowed origins (but not
+        containing the requesting origin), there should be no Access-Control-
+        Allow headers in the response.
+        '''
+        request_headers = {'Origin': 'http://thirdpartyrequests.org'}
+        response = self.app.get('/simple_flask', headers=request_headers)
+        response_headers = dict(response.headers)
+
+        assert 'Access-Control-Allow-Origin' not in response_headers
+        assert 'Access-Control-Allow-Methods' not in response_headers
+        assert 'Access-Control-Allow-Headers' not in response_headers