From 8c02842ebf60cd481ad1b0ad4f2d32b8a0c44b1b Mon Sep 17 00:00:00 2001 From: amercader Date: Fri, 4 Oct 2019 14:08:40 +0200 Subject: [PATCH] Store current user in SQLAlchemy session so activity hook can get it Up until now, the activity stream session extension used the revision property of the session to get the user performing the action. With the revisions gone we need to find an alternative. The `session.revision.user` property was set after a revision was explicitly created in an action, and its `author` property set (by way of [1]): rev = model.repo.new_revision() rev.author = user So it makes sense to attached the user performing an action when the actions happen. But rather than have to call it explicitly on each action that has activities as above, this change adds a unique function in ckan.logic.get_action that uses the value in the `context` object. These can be provided by users but most of the time they are prepolutated with the following defaults: model, model.Session and c.user. The main drawback of this is that if extensions import and call core action functions directly activites will get registered as a Non logged in user. This is discouraged but it was quite common before chained actions were introduced (and is still used now). To support this situation we should add the call to set the user in the session in all actions that have activities, like it was done before with the revisions. [1] https://github.com/ckan/ckan/blob/42596a014a713f9787d4ee9167f4c51afaaefaa6/ckan/model/__init__.py#L436 --- .../lib/activity_streams_session_extension.py | 13 +++----- ckan/logic/__init__.py | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index d623038fbdd..822e791df3a 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -44,14 +44,11 @@ def before_commit(self, session): except AttributeError: # session had no _object_cache; skipping this commit return - - user = 'DUNNO' # TODO!! - if user: - user_id = user - else: - # If the user is not logged in then revision.user is None and - # revision.author is their IP address. Just log them as 'not logged - # in' rather than logging their IP address. + try: + user_id = session.user.id + except AttributeError: + # If the user is not logged in then session.user is None or not set. + # Just log them as 'Not logged in' rather than logging their IP address. user_id = 'not logged in' # The top-level objects that we will append to the activity table. The diff --git a/ckan/logic/__init__.py b/ckan/logic/__init__.py index a7b77c10844..72854ae2ff9 100644 --- a/ckan/logic/__init__.py +++ b/ckan/logic/__init__.py @@ -6,6 +6,7 @@ import sys from collections import defaultdict +import sqlalchemy import formencode.validators from six import string_types, text_type @@ -238,6 +239,32 @@ def _prepopulate_context(context): return context +def _set_user_in_session(context): + ''' + Attaches the user performing an action to the SQLAlchemy session + + This is needed so the Activity Streams before_commit hook in + ``ckan/lib/activity_streams_session_extension.py`` can record the user + that performed the activity (which used to be done using revisions). + + This assumes model and session properties present in the context, ie + should only be run after ``_prepopulate_context()`` has been executed. + + If present the user object is attached both the Session class and an + instance of it, otherwise the hook won't get the expected attribute. + ''' + user_name = context.get('user') + if user_name: + model = context['model'] + session = context['session'] + user_obj = session.query( + model.User).filter_by(name=user_name).one_or_none() + setattr(session, 'user', user_obj) + if isinstance(session, sqlalchemy.orm.scoping.ScopedSession): + session_instance = session() + setattr(session_instance, 'user', user_obj) + + def check_access(action, context, data_dict=None): '''Calls the authorization function for the provided action @@ -452,6 +479,10 @@ def wrapped(context=None, data_dict=None, **kw): context = _prepopulate_context(context) + # Attach the user (if any) to the SQLAlchemy session so it can + # be used in the activities before_commit hook + _set_user_in_session(context) + # Auth Auditing - checks that the action function did call # check_access (unless there is no accompanying auth function). # We push the action name and id onto the __auth_audit stack