Merge branch 'master'into 1942-secure-cookie

CHANGELOG.rst

@@ -32,13 +32,13 @@ API changes and deprecations
 * Cross-Origin Resource Sharing (CORS) support is no longer enabled by
   default. Previously, Access-Control-Allow-* response headers were added for
   all requests, with Access-Control-Allow-Origin set to the wildcard value
-  ``*``. To re-enable CORS, use the new ``ckan.cors`` settings detailed in the
-  Config File Options documentation (:doc:`/maintaining/configuration`)
+  ``*``. To re-enable CORS, use the new ``ckan.cors`` configuration settings
+  (:ref:`ckan.cors.origin_allow_all` and :ref:`ckan.cors.origin_whitelist`).
 
 * The HttpOnly flag will be set on the authorization cookie by default. For
   enhanced security, we recommend using the HttpOnly flag, but this behaviour
   can be changed in the ``Repoze.who`` settings detailed in the Config File
-  Options documentation (:doc: `/maintaining/configuration`)
+  Options documentation (:ref:`who.httponly`).
 
 Template changes
 ----------------

ckan/lib/auth_tkt.py

@@ -15,51 +15,22 @@ def __init__(self, httponly, *args, **kwargs):
         super(CkanAuthTktCookiePlugin, self).__init__(*args, **kwargs)
         self.httponly = httponly
 
-    def _ensure_httponly_for_cookies(self, cookies):
-        '''
-        Take a list of cookie tuples items and ensure HttpOnly is set
-        correctly.
-        '''
-        # change tuples to lists
-        cookie_list = [list(c) for c in cookies]
-        # ensure httponly is set correctly for each cookie
-        for cookie in cookie_list:
-            cookie[1] = _set_substring(cookie[1], '; HttpOnly', self.httponly)
-        # change lists back to tuples and return
-        return [tuple(c) for c in cookie_list]
-
     def _get_cookies(self, *args, **kwargs):
         '''
         Override method in superclass to ensure HttpOnly is set appropriately.
         '''
         super_cookies = super(CkanAuthTktCookiePlugin, self). \
             _get_cookies(*args, **kwargs)
 
-        cookies = self._ensure_httponly_for_cookies(super_cookies)
+        cookies = []
+        for k, v in super_cookies:
+            replace_with = '; HttpOnly' if self.httponly else ''
+            v = v.replace('; HttpOnly', '') + replace_with
+            cookies.append((k, v))
 
         return cookies
 
 
-def _set_substring(value, substring, presence=True):
-    '''
-    Ensure presence/absence of substring in value.
-
-    If presence is True, ensure value contains substring. Append substring to
-    value if absent.
-
-    If presence is False, ensure value does not contain substring. Remove
-    substring from value is present.
-    '''
-    has_substring = (substring in value)
-    if has_substring and not presence:
-        # remove substring
-        value = value.replace(substring, '')
-    elif not has_substring and presence:
-        # add substring
-        value += substring
-    return value
-
-
 def make_plugin(secret=None,
                 secretfile=None,
                 cookie_name='auth_tkt',

ckan/lib/cli.py

@@ -106,7 +106,7 @@ class CkanCommand(paste.script.command.Command):
     '''
     parser = paste.script.command.Command.standard_parser(verbose=True)
     parser.add_option('-c', '--config', dest='config',
-            default='development.ini', help='Config file to use.')
+        help='Config file to use.')
     parser.add_option('-f', '--file',
         action='store',
         dest='file_path',
@@ -116,12 +116,22 @@ class CkanCommand(paste.script.command.Command):
 
     def _get_config(self):
         from paste.deploy import appconfig
-        if not self.options.config:
-            msg = 'No config file supplied'
-            raise self.BadCommand(msg)
-        self.filename = os.path.abspath(self.options.config)
+
+        if self.options.config:
+            self.filename = os.path.abspath(self.options.config)
+            config_source = '-c parameter'
+        elif os.environ.get('CKAN_INI'):
+            self.filename = os.environ.get('CKAN_INI')
+            config_source = '$CKAN_INI'
+        else:
+            self.filename = 'development.ini'
+            config_source = 'default value'
+
         if not os.path.exists(self.filename):
-            raise AssertionError('Config filename %r does not exist.' % self.filename)
+            msg = 'Config file not found: %s' % self.filename
+            msg += '\n(Given by: %s)' % config_source
+            raise self.BadCommand(msg)
+
         fileConfig(self.filename)
         return appconfig('config:' + self.filename)
 

ckan/new_authz.py

@@ -94,7 +94,6 @@ def _build(self):
 
 def clear_auth_functions_cache():
     _AuthFunctions.clear()
-    CONFIG_PERMISSIONS.clear()
 
 
 def auth_functions_list():
@@ -374,28 +373,41 @@ def get_user_id_for_username(user_name, allow_none=False):
     'roles_that_cascade_to_sub_groups': 'admin',
 }
 
-CONFIG_PERMISSIONS = {}
-
 
 def check_config_permission(permission):
-    ''' Returns the permission configuration, usually True/False '''
-    # set up perms if not already done
-    if not CONFIG_PERMISSIONS:
-        for perm in CONFIG_PERMISSIONS_DEFAULTS:
-            key = 'ckan.auth.' + perm
-            default = CONFIG_PERMISSIONS_DEFAULTS[perm]
-            CONFIG_PERMISSIONS[perm] = config.get(key, default)
-            if perm == 'roles_that_cascade_to_sub_groups':
-                # this permission is a list of strings (space separated)
-                CONFIG_PERMISSIONS[perm] = \
-                    CONFIG_PERMISSIONS[perm].split(' ') \
-                    if CONFIG_PERMISSIONS[perm] else []
-            else:
-                # most permissions are boolean
-                CONFIG_PERMISSIONS[perm] = asbool(CONFIG_PERMISSIONS[perm])
-    if permission in CONFIG_PERMISSIONS:
-        return CONFIG_PERMISSIONS[permission]
-    return False
+    '''Returns the configuration value for the provided permission
+
+    Permission is a string indentifying the auth permission (eg
+    `anon_create_dataset`), optionally prefixed with `ckan.auth.`.
+
+    The possible values for `permission` are the keys of
+    CONFIG_PERMISSIONS_DEFAULTS. These can be overriden in the config file
+    by prefixing them with `ckan.auth.`.
+
+    Returns the permission value, generally True or False, except on
+    `roles_that_cascade_to_sub_groups` which is a list of strings.
+
+    '''
+
+    key = permission.replace('ckan.auth.', '')
+
+    if key not in CONFIG_PERMISSIONS_DEFAULTS:
+        return False
+
+    default_value = CONFIG_PERMISSIONS_DEFAULTS.get(key)
+
+    config_key = 'ckan.auth.' + key
+
+    value = config.get(config_key, default_value)
+
+    if key == 'roles_that_cascade_to_sub_groups':
+        # This permission is set as a list of strings (space separated)
+        value = value.split() if value else []
+    else:
+        value = asbool(value)
+
+    return value
+
 
 @maintain.deprecated('Use auth_is_loggedin_user instead')
 def auth_is_registered_user():

ckan/new_tests/helpers.py

@@ -295,13 +295,11 @@ def decorator(func):
         def wrapper(*args, **kwargs):
             _original_config = config.copy()
             config[key] = value
-            new_authz.clear_auth_functions_cache()
 
             return_value = func(*args, **kwargs)
 
             config.clear()
             config.update(_original_config)
-            new_authz.clear_auth_functions_cache()
 
             return return_value
         return nose.tools.make_decorator(func)(wrapper)

ckan/new_tests/lib/test_auth_tkt.py

@@ -1,123 +1,5 @@
 from ckan.new_tests import helpers
-from ckan.lib.auth_tkt import (CkanAuthTktCookiePlugin,
-                               _set_substring,
-                               make_plugin)
-
-
-class TestSetSubstring(object):
-
-    '''Tests for auth_tkt._set_substring method.'''
-
-    def test_set_substring__value_has_substring_and_substring_should_be_present(self):
-        '''Substring should be retained in value'''
-        value = "I love my kitten, it is sweet."
-        substring = "kitten"
-        presence = True
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == value
-
-    def test_set_substring__value_has_substring_and_substring_should_not_be_present(self):
-        '''Substring should be removed from value.'''
-        value = "I love my kitten, it is sweet."
-        substring = "kitten"
-        presence = False
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == "I love my , it is sweet."
-
-    def test_set_substring__value_doesnot_have_substring_and_substring_should_be_present(self):
-        '''Substring is appended to value.'''
-        value = "I wish I had a "
-        substring = "kitten"
-        presence = True
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == 'I wish I had a kitten'
-
-    def test_set_substring__value_doesnot_have_substring_and_substring_should_not_be_present(self):
-        '''Substring isn't appended to value.'''
-        value = "I don't have one."
-        substring = "kitten"
-        presence = False
-        new_value = _set_substring(value, substring, presence)
-        assert new_value == "I don't have one."
-
-
-class TestEnsureHttpOnlyForCookies(object):
-
-    '''Tests for CkanAuthTktCookiePlugin._ensure_httponly_for_cookies method'''
-
-    def _make_plugin(self, httponly):
-        '''Only httponly needs to be set.'''
-        return CkanAuthTktCookiePlugin(httponly=httponly,
-                                       secret=None,
-                                       cookie_name='auth_tkt',
-                                       secure=False,
-                                       include_ip=False,
-                                       timeout=None,
-                                       reissue_time=None,
-                                       userid_checker=None)
-
-    def test_ensure_httponly_for_cookies__should_have_httponly(self):
-        '''Cookie values should contain HttpOnly.'''
-        plugin = self._make_plugin(httponly=True)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        expected_cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        assert ensured_cookies == expected_cookies
-
-    def test_ensure_httponly_for_cookies__should_have_httponly_already_do(self):
-        '''
-        Cookie values should contain HttpOnly, they already to so nothing
-        should change.
-        '''
-        plugin = self._make_plugin(httponly=True)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        assert ensured_cookies == cookies
-
-    def test_ensure_httponly_for_cookies__should_not_have_httponly_already_absent(self):
-        '''
-        Cookie values should not contain HttpOnly. They don't so nothing
-        should change.
-        '''
-        plugin = self._make_plugin(httponly=False)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        assert ensured_cookies == cookies
-
-    def test_ensure_httponly_for_cookies__should_not_have_httponly(self):
-        '''
-        Cookie values should not contain HttpOnly, they do so it should be
-        removed.
-        '''
-        plugin = self._make_plugin(httponly=False)
-        cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost; HttpOnly'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost; HttpOnly')
-        ]
-        ensured_cookies = plugin._ensure_httponly_for_cookies(cookies)
-        expected_cookies = [
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=localhost'),
-            ('Set-Cookie', 'auth_tkt="HELLO"; Path=/; Domain=.localhost')
-        ]
-        assert ensured_cookies == expected_cookies
+from ckan.lib.auth_tkt import CkanAuthTktCookiePlugin, make_plugin
 
 
 class TestCkanAuthTktCookiePlugin(object):

ckan/new_tests/test_authz.py

@@ -0,0 +1,66 @@
+import nose
+
+from ckan import new_authz as auth
+
+from ckan.new_tests import helpers
+
+
+assert_equals = nose.tools.assert_equals
+
+
+class TestCheckConfigPermission(object):
+
+    @helpers.change_config('ckan.auth.anon_create_dataset', None)
+    def test_get_default_value_if_not_set_in_config(self):
+
+        assert_equals(auth.check_config_permission(
+            'anon_create_dataset'),
+            auth.CONFIG_PERMISSIONS_DEFAULTS['anon_create_dataset'])
+
+    @helpers.change_config('ckan.auth.anon_create_dataset', None)
+    def test_get_default_value_also_works_with_prefix(self):
+
+        assert_equals(auth.check_config_permission(
+            'ckan.auth.anon_create_dataset'),
+            auth.CONFIG_PERMISSIONS_DEFAULTS['anon_create_dataset'])
+
+    @helpers.change_config('ckan.auth.anon_create_dataset', True)
+    def test_config_overrides_default(self):
+
+        assert_equals(auth.check_config_permission(
+            'anon_create_dataset'),
+            True)
+
+    @helpers.change_config('ckan.auth.anon_create_dataset', True)
+    def test_config_override_also_works_with_prefix(self):
+
+        assert_equals(auth.check_config_permission(
+            'ckan.auth.anon_create_dataset'),
+            True)
+
+    @helpers.change_config('ckan.auth.unknown_permission', True)
+    def test_unknown_permission_returns_false(self):
+
+        assert_equals(auth.check_config_permission(
+            'unknown_permission'),
+            False)
+
+    def test_unknown_permission_not_in_config_returns_false(self):
+
+        assert_equals(auth.check_config_permission(
+            'unknown_permission'),
+            False)
+
+    def test_default_roles_that_cascade_to_sub_groups_is_a_list(self):
+
+        assert isinstance(auth.check_config_permission(
+            'roles_that_cascade_to_sub_groups'),
+            list)
+
+    @helpers.change_config('ckan.auth.roles_that_cascade_to_sub_groups',
+                           'admin editor')
+    def test_roles_that_cascade_to_sub_groups_is_a_list(self):
+
+        assert_equals(sorted(auth.check_config_permission(
+            'roles_that_cascade_to_sub_groups')),
+            sorted(['admin', 'editor']))

ckan/templates/package/read_base.html

@@ -11,7 +11,7 @@
   {{ super() }}
   {% set description = h.markdown_extract(pkg.notes, extract_length=200)|forceescape %}
   <meta property="og:title" content="{{ h.dataset_display_name(pkg) }} - {{ g.site_title }}">
-  <meta property="og:description" content="{{ description|forceescape }}">
+  <meta property="og:description" content="{{ description|forceescape|trim }}">
 {% endblock -%}
 
 {% block content_action %}