From 2b6c1edb36c54114110c0490d0b1ed5287499565 Mon Sep 17 00:00:00 2001
From: amercader <amercadero@gmail.com>
Date: Thu, 6 Nov 2014 16:50:43 +0000
Subject: [PATCH] [#2035] Refactor new_authz.check_config_permission to not
 cache things

Otherwise the values can not be overriden from tests
---
 ckan/new_authz.py            | 54 +++++++++++++++++------------
 ckan/new_tests/test_authz.py | 66 ++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 21 deletions(-)
 create mode 100644 ckan/new_tests/test_authz.py

diff --git a/ckan/new_authz.py b/ckan/new_authz.py
index 7d4cdc3716d..365f50371ee 100644
--- a/ckan/new_authz.py
+++ b/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():
diff --git a/ckan/new_tests/test_authz.py b/ckan/new_tests/test_authz.py
new file mode 100644
index 00000000000..e963821afbc
--- /dev/null
+++ b/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']))