Merge pull request #1060 from okfn/1060-action-auth-audit

Action Auth Audit

ckan/logic/__init__.py

@@ -1,7 +1,7 @@
 import functools
 import logging
-import types
 import re
+import sys
 
 import formencode.validators
 
@@ -194,8 +194,18 @@ def flatten_to_string_key(dict):
 
 
 def check_access(action, context, data_dict=None):
-    user = context.get('user')
+    action = new_authz.clean_action_name(action)
 
+    # Auth Auditing.  We remove this call from the __auth_audit stack to show
+    # we have called the auth function
+    try:
+        audit = context.get('__auth_audit', [])[-1]
+    except IndexError:
+        audit = ''
+    if audit == action:
+        context['__auth_audit'].pop()
+
+    user = context.get('user')
     log.debug('check access - user %r, action %s' % (user, action))
 
     if action:
@@ -281,13 +291,17 @@ def get_action(action):
             module = getattr(module, part)
         for k, v in module.__dict__.items():
             if not k.startswith('_'):
-                # Only load functions from the action module.
-                if isinstance(v, types.FunctionType):
+                # Only load functions from the action module or already
+                # replaced functions.
+                if (hasattr(v, '__call__')
+                        and (v.__module__ == module_path
+                             or hasattr(v, '__replaced'))):
                     k = new_authz.clean_action_name(k)
                     _actions[k] = v
 
                     # Whitelist all actions defined in logic/action/get.py as
                     # being side-effect free.
+                    # FIXME This looks wrong should it be an 'or' not 'and'
                     v.side_effect_free = getattr(v, 'side_effect_free', True)\
                         and action_module_name == 'get'
 
@@ -306,6 +320,9 @@ def get_action(action):
                 )
             log.debug('Auth function %r was inserted', plugin.name)
             resolved_action_plugins[name] = plugin.name
+            # Extensions are exempted from the auth audit for now
+            # This needs to be resolved later
+            auth_function.auth_audit_exempt = True
             fetched_actions[name] = auth_function
     # Use the updated ones in preference to the originals.
     _actions.update(fetched_actions)
@@ -326,9 +343,34 @@ def wrapped(context=None, data_dict=None, **kw):
                 except TypeError:
                     # c not registered
                     pass
-                return _action(context, data_dict, **kw)
+
+                # Auth Auditing
+                # store this action name in the auth audit so we can see if
+                # check access was called on the function
+                context.setdefault('__auth_audit', [])
+                context['__auth_audit'].append(action_name)
+
+                # check_access(action_name, context, data_dict=None)
+                result = _action(context, data_dict, **kw)
+                try:
+                    if context['__auth_audit'][-1] == action_name:
+                        if action_name not in new_authz.auth_functions_list():
+                            log.debug('No auth function for %s' % action_name)
+                        elif not getattr(_action, 'auth_audit_exempt', False):
+                            raise Exception('Action Auth Audit: %s' % action_name)
+                        # remove from audit stack
+                        context['__auth_audit'].pop()
+                except IndexError:
+                    pass
+                return result
             return wrapped
 
+        # If we have been called multiple times for example during tests then
+        # we need to make sure that we do not rewrap the actions.
+        if hasattr(_action, '__replaced'):
+            _actions[action_name] = _action.__replaced
+            continue
+
         fn = make_wrapped(_action, action_name)
         # we need to mirror the docstring
         fn.__doc__ = _action.__doc__
@@ -337,6 +379,22 @@ def wrapped(context=None, data_dict=None, **kw):
             fn.side_effect_free = True
         _actions[action_name] = fn
 
+
+        def replaced_action(action_name):
+            def warn(context, data_dict):
+                log.critical('Action `%s` is being called directly '
+                             'all action calls should be accessed via '
+                             'logic.get_action' % action_name)
+                return get_action(action_name)(context, data_dict)
+            return warn
+
+        # Store our wrapped function so it is available.  This is to prevent
+        # rewrapping of actions
+        module = sys.modules[_action.__module__]
+        r = replaced_action(action_name)
+        r.__replaced = fn
+        module.__dict__[action_name] = r
+
     return _actions.get(action)
 
 
@@ -400,6 +458,13 @@ def wrapper(context, data_dict):
     return wrapper
 
 
+def auth_audit_exempt(action):
+    ''' Dirty hack to stop auth audit being done '''
+    @functools.wraps(action)
+    def wrapper(context, data_dict):
+        return action(context, data_dict)
+    wrapper.auth_audit_exempt = True
+    return wrapper
 
 class UnknownValidator(Exception):
     pass

ckan/logic/action/create.py

@@ -703,6 +703,7 @@ def organization_create(context, data_dict):
     return _group_or_org_create(context, data_dict, is_org=True)
 
 
+@logic.auth_audit_exempt
 def rating_create(context, data_dict):
     '''Rate a dataset (package).
 

ckan/logic/action/get.py

@@ -826,6 +826,8 @@ def resource_status_show(context, data_dict):
 
     return result_list
 
+
+@logic.auth_audit_exempt
 def revision_show(context, data_dict):
     '''Return the details of a revision.
 
@@ -1008,7 +1010,7 @@ def user_show(context, data_dict):
 
     revisions_list = []
     for revision in revisions_q.limit(20).all():
-        revision_dict = revision_show(context,{'id':revision.id})
+        revision_dict = logic.get_action('revision_show')(context,{'id':revision.id})
         revision_dict['state'] = revision.state
         revisions_list.append(revision_dict)
     user_dict['activity'] = revisions_list
@@ -1020,7 +1022,7 @@ def user_show(context, data_dict):
 
     for dataset in dataset_q:
         try:
-            dataset_dict = package_show(context, {'id': dataset.id})
+            dataset_dict = logic.get_action('package_show')(context, {'id': dataset.id})
         except logic.NotAuthorized:
             continue
         user_dict['datasets'].append(dataset_dict)
@@ -2554,9 +2556,10 @@ def display_name(followee):
 
     # Get the followed objects.
     # TODO: Catch exceptions raised by these *_followee_list() functions?
+    # FIXME should we be changing the context like this it seems dangerous
     followee_dicts = []
     context['skip_validation'] = True
-    context['skip_authorization'] = True
+    context['ignore_auth'] = True
     for followee_list_function, followee_type in (
             (user_followee_list, 'user'),
             (dataset_followee_list, 'dataset'),
@@ -2591,8 +2594,7 @@ def user_followee_list(context, data_dict):
     :rtype: list of dictionaries
 
     '''
-    if not context.get('skip_authorization'):
-        _check_access('user_followee_list', context, data_dict)
+    _check_access('user_followee_list', context, data_dict)
 
     if not context.get('skip_validation'):
         schema = context.get('schema') or (
@@ -2622,8 +2624,7 @@ def dataset_followee_list(context, data_dict):
     :rtype: list of dictionaries
 
     '''
-    if not context.get('skip_authorization'):
-        _check_access('dataset_followee_list', context, data_dict)
+    _check_access('dataset_followee_list', context, data_dict)
 
     if not context.get('skip_validation'):
         schema = context.get('schema') or (
@@ -2654,8 +2655,7 @@ def group_followee_list(context, data_dict):
     :rtype: list of dictionaries
 
     '''
-    if not context.get('skip_authorization'):
-        _check_access('group_followee_list', context, data_dict)
+    _check_access('group_followee_list', context, data_dict)
 
     if not context.get('skip_validation'):
         schema = context.get('schema',

ckan/logic/auth/get.py

@@ -254,14 +254,17 @@ def followee_list(context, data_dict):
     return _followee_list(context, data_dict)
 
 
+@logic.auth_audit_exempt
 def user_followee_list(context, data_dict):
     return _followee_list(context, data_dict)
 
 
+@logic.auth_audit_exempt
 def dataset_followee_list(context, data_dict):
     return _followee_list(context, data_dict)
 
 
+@logic.auth_audit_exempt
 def group_followee_list(context, data_dict):
     return _followee_list(context, data_dict)