[#1663] Add authorization and validation to activity_create() logic a…

…ction function

activity_create() logic action function now checks authorization and
does validation. Has an ignore_auth argument (default: False) which if
True will skip the authorization.

Other logic functions that call activity_create() as a side-effect (and
after doing their own authorization, if any) pass ignore_auth=True to
activity_create().

Also made those other logic functions explicitly construct an
activity_create_context dict instead of just passing in their own
context dicts. This way unwanted stuff such as the calling function's
schema does not get passed into activity_create(), and the logic
functions also put 'defer_commit' into activity_create_context because
they want to do their own commit later.

The user_create() logic action function now flushes the session before
calling activity_create() so that the new user's id is initialised. This
bug wasn't showing itself before when activity_create() didn't do any
validation.

Added activity_create() logic auth function, just authorizes only
sysadmins to create activities.

Add default_create_activity_schema() to validate activity dicts, and add
user_id_exists(), group_id_exists(), activity_type_exists() and
object_id_validator() validator functions.

ckan/logic/action/create.py

@@ -26,7 +26,10 @@
                                                 activity_dictize)
 
 
-from ckan.logic.schema import default_create_package_schema, default_resource_schema, default_create_relationship_schema
+from ckan.logic.schema import (default_create_package_schema,
+                               default_resource_schema,
+                               default_create_relationship_schema,
+                               default_create_activity_schema)
 
 from ckan.logic.schema import default_group_schema, default_user_schema
 from ckan.lib.navl.dictization_functions import validate 
@@ -156,14 +159,15 @@ def package_relationship_create(context, data_dict):
 def group_create(context, data_dict):
     model = context['model']
     user = context['user']
+    session = context['session']
     schema = context.get('schema') or default_group_schema()
 
     check_access('group_create',context,data_dict)
 
     data, errors = validate(data_dict, schema, context)
 
     if errors:
-        model.Session.rollback()
+        session.rollback()
         raise ValidationError(errors, group_error_summary(errors))
 
     rev = model.repo.new_revision()
@@ -182,7 +186,7 @@ def group_create(context, data_dict):
         admins = []
     model.setup_default_user_roles(group, admins)
     # Needed to let extensions know the group id
-    model.Session.flush()
+    session.flush()
     for item in PluginImplementations(IGroupController):
         item.create(group)
 
@@ -194,7 +198,13 @@ def group_create(context, data_dict):
     activity_dict['data'] = {
             'group': ckan.lib.dictization.table_dictize(group, context)
             }
-    activity_create(context, activity_dict)
+    activity_create_context = {
+        'model': model,
+        'user': user,
+        'defer_commit':True,
+        'session': session
+    }
+    activity_create(activity_create_context, activity_dict, ignore_auth=True)
 
     if not context.get('defer_commit'):
         model.repo.commit()        
@@ -242,26 +252,37 @@ def user_create(context, data_dict):
 
     model = context['model']
     schema = context.get('schema') or default_user_schema()
+    session = context['session']
 
     check_access('user_create', context, data_dict)
 
     data, errors = validate(data_dict, schema, context)
 
     if errors:
-        model.Session.rollback()
+        session.rollback()
         raise ValidationError(errors, group_error_summary(errors))
 
     user = user_dict_save(data, context)
 
-    if not context.get('defer_commit'):
-        model.repo.commit()        
+    # Flush the session to cause user.id to be initialised, because
+    # activity_create() (below) needs it.
+    session.flush()
 
+    activity_create_context = {
+        'model': model,
+        'user': context['user'],
+        'defer_commit': True,
+        'session': session
+    }
     activity_dict = {
             'user_id': user.id,
             'object_id': user.id,
             'activity_type': 'new user',
             }
-    activity_create(context, activity_dict)
+    activity_create(activity_create_context, activity_dict, ignore_auth=True)
+
+    if not context.get('defer_commit'):
+        model.repo.commit()
 
     context['user'] = user
     context['id'] = user.id
@@ -310,7 +331,7 @@ def group_create_rest(context, data_dict):
 
     return group_dict
 
-def activity_create(context, activity_dict):
+def activity_create(context, activity_dict, ignore_auth=False):
     '''Create an activity stream event and add it to the model. Return a
     dictionary representation of the new event.
 
@@ -328,13 +349,23 @@ def activity_create(context, activity_dict):
 
     '''
     model = context['model']
+    user = context['user']
 
-    assert not activity_dict.has_key('revision_id')
-    if getattr(model.Session, "revision", None):
+    # Any revision_id that the caller attempts to pass in the activity_dict is
+    # ignored and overwritten here.
+    if getattr(model.Session, 'revision', None):
         activity_dict['revision_id'] = model.Session.revision.id
     else:
         activity_dict['revision_id'] = None
 
+    if not ignore_auth:
+        check_access('activity_create', context, activity_dict)
+
+    schema = context.get('schema') or default_create_activity_schema()
+    data, errors = validate(activity_dict, schema, context)
+    if errors:
+        raise ValidationError(errors)
+
     activity = activity_dict_save(activity_dict, context)
 
     if not context.get('defer_commit'):

ckan/logic/action/update.py

@@ -320,9 +320,9 @@ def package_relationship_update(context, data_dict):
     return _update_package_relationship(entity, comment, context)
 
 def group_update(context, data_dict):
-
     model = context['model']
     user = context['user']
+    session = context['session']
     schema = context.get('schema') or default_update_group_schema()
     id = data_dict['id']
 
@@ -335,7 +335,7 @@ def group_update(context, data_dict):
 
     data, errors = validate(data_dict, schema, context)
     if errors:
-        model.Session.rollback()
+        session.rollback()
         raise ValidationError(errors, group_error_summary(errors))
 
     rev = model.repo.new_revision()
@@ -361,7 +361,7 @@ def group_update(context, data_dict):
     # a 'changed' group activity. We detect this and change it to a 'deleted'
     # activity.
     if group.state == u'deleted':
-        if ckan.model.Session.query(ckan.model.Activity).filter_by(
+        if session.query(ckan.model.Activity).filter_by(
                 object_id=group.id, activity_type='deleted').all():
             # A 'deleted group' activity for this group has already been
             # emitted.
@@ -375,14 +375,19 @@ def group_update(context, data_dict):
                 'group': ckan.lib.dictization.table_dictize(group, context)
                 }
         from ckan.logic.action.create import activity_create
-        activity_create(context, activity_dict)
+        activity_create_context = {
+            'model': model,
+            'user': user,
+            'defer_commit':True,
+            'session': session
+        }
+        activity_create(activity_create_context, activity_dict,
+                ignore_auth=True)
         # TODO: Also create an activity detail recording what exactly changed
         # in the group.
 
     if not context.get('defer_commit'):
         model.repo.commit()        
-    if errors:
-        raise ValidationError(errors)
 
     return group_dictize(group, context)
 
@@ -391,6 +396,7 @@ def user_update(context, data_dict):
 
     model = context['model']
     user = context['user']
+    session = context['session']
     schema = context.get('schema') or default_update_user_schema() 
     id = data_dict['id']
 
@@ -403,7 +409,7 @@ def user_update(context, data_dict):
 
     data, errors = validate(data_dict, schema, context)
     if errors:
-        model.Session.rollback()
+        session.rollback()
         raise ValidationError(errors, group_error_summary(errors))
 
     user = user_dict_save(data, context)
@@ -413,9 +419,14 @@ def user_update(context, data_dict):
             'object_id': user.id,
             'activity_type': 'changed user',
             }
-
+    activity_create_context = {
+        'model': model,
+        'user': user,
+        'defer_commit':True,
+        'session': session
+    }
     from ckan.logic.action.create import activity_create
-    activity_create(context, activity_dict)
+    activity_create(activity_create_context, activity_dict, ignore_auth=True)
     # TODO: Also create an activity detail recording what exactly changed in
     # the user.
 

ckan/logic/auth/create.py

@@ -129,3 +129,7 @@ def group_create_rest(context, data_dict):
         return {'success': False, 'msg': _('Valid API key needed to create a group')}
 
     return group_create(context, data_dict)
+
+def activity_create(context, data_dict):
+    user = context['user']
+    return {'success': Authorizer.is_sysadmin(user)}

ckan/logic/schema.py

@@ -30,11 +30,13 @@
                                    user_password_not_empty,
                                    isodate,
                                    int_validator,
-                                   user_about_validator)
+                                   user_about_validator,
+                                   user_id_exists,
+                                   object_id_validator,
+                                   activity_type_exists)
 from formencode.validators import OneOf
 import ckan.model
 
-
 def default_resource_schema():
 
     schema = {
@@ -291,3 +293,18 @@ def default_task_status_schema():
         'error': [ignore_missing]
     }
     return schema
+
+def default_create_activity_schema():
+    schema = {
+        'id': [ignore],
+        'timestamp': [ignore],
+        'user_id': [not_missing, not_empty, unicode, user_id_exists],
+        'object_id': [not_missing, not_empty, unicode, object_id_validator],
+        # We don't bother to validate revision ID, since it's always created
+        # internally by the activity_create() logic action function.
+        'revision_id': [],
+        'activity_type': [not_missing, not_empty, unicode,
+            activity_type_exists],
+        'data': [ignore_empty, ignore_missing, unicode],
+    }
+    return schema

ckan/logic/validators.py

@@ -84,6 +84,78 @@ def package_id_or_name_exists(value, context):
 
     return result.id
 
+def user_id_exists(user_id, context):
+    """Raises Invalid if the given user_id does not exist in the model given
+    in the context, otherwise returns the given user_id.
+
+    """
+    model = context['model']
+    session = context['session']
+
+    result = session.query(model.User).get(user_id)
+    if not result:
+        raise Invalid(_("That user ID does not exist."))
+    return user_id
+
+def group_id_exists(group_id, context):
+    """Raises Invalid if the given group_id does not exist in the model given
+    in the context, otherwise returns the given group_id.
+
+    """
+    model = context['model']
+    session = context['session']
+
+    result = session.query(model.Group).get(group_id)
+    if not result:
+        raise Invalid(_("That group ID does not exist."))
+    return group_id
+
+def activity_type_exists(activity_type):
+    """Raises Invalid if there is no registered activity renderer for the
+    given activity_type. Otherwise returns the given activity_type.
+
+    """
+    from ckan.logic.action.get import activity_renderers
+    if activity_renderers.has_key(activity_type):
+        return activity_type
+    else:
+        raise Invalid(_("That activity type does not exist."))
+
+# A dictionary mapping activity_type values from activity dicts to functions
+# for validating the object_id values from those same activity dicts.
+object_id_validators = {
+    'new package' : package_id_exists,
+    'changed package' : package_id_exists,
+    'deleted package' : package_id_exists,
+    'new user' : user_id_exists,
+    'changed user' : user_id_exists,
+    'new group' : group_id_exists,
+    'changed group' : group_id_exists,
+    'deleted group' : group_id_exists,
+    }
+
+def object_id_validator(key, activity_dict, errors, context):
+    """Validate the 'object_id' value of an activity_dict.
+
+    Uses the object_id_validators dict (above) to find and call an 'object_id'
+    validator function for the given activity_dict's 'activity_type' value.
+
+    Raises Invalid if the model given in context contains no object of the
+    correct type (according to the 'activity_type' value of the activity_dict)
+    with the given ID.
+
+    Raises NotImplemented if there is no object_id_validator for the 
+    activity_dict's 'activity_type' value.
+
+    """
+    activity_type = activity_dict[('activity_type',)]
+    if object_id_validators.has_key(activity_type):
+        object_id = activity_dict[('object_id',)]
+        return object_id_validators[activity_type](object_id, context)
+    else:
+        raise NotImplementedError, ("There is no object_id validator for "
+            "activity type '%s'" % str(activity_type))
+
 def extras_unicode_convert(extras, context):
     for extra in extras:
         extras[extra] = unicode(extras[extra])

ckan/tests/functional/test_group.py

@@ -183,7 +183,6 @@ def test_2_edit(self):
         pkg = model.Package.by_name(self.packagename)
         form['packages__2__name'] = pkg.name
 
-
         res = form.submit('save', status=302, extra_environ={'REMOTE_USER': 'russianfan'})
         # should be read page
         # assert 'Groups - %s' % self.groupname in res, res