Skip to content

Commit

Permalink
feat(ProjectHistoryLogs): create ph logs for permissions changes TASK…
Browse files Browse the repository at this point in the history
…-944 (#5297)



### 📣 Summary
Create project history logs when users are assigned permissions or have
permissions revoked for a project.

### 👀 Preview steps
Note: for this test, all PH logs should have
`metadata['log_subtype']='permission'` and `metadata['asset_uid']=<the
uid of the project you're updating>`

1. ℹ️ have 3 accounts (at least 2 non-superuser). For this test we'll
call them user1, user2, and user3 (user2 and user3 are not super)
2.  ℹ️  have a project owned by user1 with at least 1 submission
3. Log in as user1
4. Go to Project > Settings > Sharing
5. Toggle 'Allow submissions to this form without a username and
password'
6. Go to `/api/v2/audit-logs/?q=log_type:project-history AND
metadata__asset_uid:<Project1.uid>`
7. 🟢 There should be a new PH log with
action='allow-anonymous-submissions' (usual metadata)
8. Toggle 'Allow submissions...' off again
9. 🟢 Reload the endpoint. There should be a new PH log with
action='disallow-anonymous-submissions'
10. Check "Anyone can view this form"
11. 🟢 Reload the endpoint. There should be a new PH log with
action='share-form-publicly'
12. Check "Anyone can view submissions made to this form"
13. 🟢 Reload the endpoint. There should be a new PH log with
action='share-data-publicly'
14. Uncheck "Anyone can view submissions made to this form"
15. 🟢 Reload the endpoint. There should be a new PH log with
action='unshare-data-publicly'
16. Uncheck "Anyone can view this form"
17. 🟢 Reload the endpoint. There should be a new PH log with
action='unshare-form-publicly'
18. Click 'Add User'. Grant user2 permission to Edit Form
19. 🟢 Reload the endpoint. There should be a new PH log with
action='modify-user-permissions' and
```
metadata['permissions'] =
{
"username": "user2",
"added": ["change_asset", "view_asset"],  # order doesn't matter
"removed":[]
}
```
20. Uncheck "Edit form" and check "Edit submissions only from specific
users" -> "user2"
21. Click "Update permissions"
22. 🟢 Reload the endpoint. There should be a new PH log with
action='modify-user-permissions' and
```
metadata['permissions'] =
{
"username": "user2",
"added": [             # order doesn't matter
    "partial_submissions",
    "add_submissions", 
    {
         "code": "view_submissions",
          "filters": [
                {"_submitted_by": "user2"}
          ],
    },
    {
         "code": "add_submissions",
          "filters": [
                {"_submitted_by": "user2"}
          ],
    },
    {
         "code": "change_submissions",
          "filters": [
                {"_submitted_by": "user2"}
          ],
    },
], 
"removed":["change_asset"]
}
```
23. Click the trashcan next to "user2" to remove all permissions
24. 🟢 Reload the endpoint. Deletion will actually be 2 delete, requests,
so you'll end up with 2 new ph logs with
```
metadata['permissions'] =
{
"username": "user2",
"added": [],  
"removed": ["partial_submissions", "view_asset"]    # order doesn't matter
}
```
and
```
metadata['permissions'] =
{
"username": "user2",
"added": [],  
"removed":["add_submissions"]
}
```
25. Copy the following into a `tmp.json` file (unless you really want to
type the whole json into the command line):
```
[
    {
	"user":"http://localhost/api/v2/users/user2/",
	"permission": "http://localhost/api/v2/permissions/add_submissions/"
    },
    {
	"user": "http://localhost/api/v2/users/user3/",
	"permission": "http://localhost/api/v2/permissions/view_asset/"
    }
 
]
```
26. In a terminal, run `curl -v -X POST -H "Content-Type:
application/json" -H "Authorization: Token <your token>"
http://localhost/api/v2/assets/<asset_uid>/permission-assignments/bulk/?format=json
-d @/tmp.json`
27. 🟢 Reload the endpoint. You should see 2 new PH logs, one with the
new permissions for user2 and one for user3.

### 💭 Notes
This PR does a lot:
1. Update assign_perm to call post_assign_perm right after we create the
ObjectPermission object. This way it is called when we assign implied
permissions as well. There's only one other thing listening to the
signal and it only cares about when we grant anonymous users the
'add_submission' permission.
2. Update remove_perm to call post_remove_perm iff we actually delete
anything
3. Add new signals for adding/removing partial permissions objects
4. Listen for permission signals. Add permissions added/removed to the
request in a dictionary keyed by username. We get the request from
get_current_request because passing the request around wherever
permissions might change is too complicated and would require at least
one major refactor.
5. At the end of the request, assuming it succeeded, use the
dictionaries stored on the request object to create PH logs for each
user whose permissions were updated

Additional notes:
Partial permissions are handled a bit differently than normal ones since
they are not additive. Every time we set partial permissions, we're
wiping away any old partial permissions.

I tried to be thorough in the unit tests but there are a LOT of
different ways permissions can change, especially with a bulk request.

This PR purposely leaves out changes that result from collection
actions. Those will be dealt with later. Similarly, the PR does not
handle the v1 endpoint or requests to transfer ownership.
  • Loading branch information
rgraber authored Dec 2, 2024
1 parent 522eb19 commit 4c3da9b
Show file tree
Hide file tree
Showing 10 changed files with 790 additions and 54 deletions.
7 changes: 7 additions & 0 deletions kobo/apps/audit_log/audit_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

class AuditAction(models.TextChoices):
ADD_MEDIA = 'add-media'
ALLOW_ANONYMOUS_SUBMISSIONS = 'allow-anonymous-submissions'
ARCHIVE = 'archive'
AUTH = 'auth'
CONNECT_PROJECT = 'connect-project'
Expand All @@ -12,19 +13,25 @@ class AuditAction(models.TextChoices):
DELETE_SERVICE = 'delete-service'
DEPLOY = 'deploy'
DISABLE_SHARING = 'disable-sharing'
DISALLOW_ANONYMOUS_SUBMISSIONS = 'disallow-anonymous-submissions'
DISCONNECT_PROJECT = 'disconnect-project'
ENABLE_SHARING = 'enable-sharing'
EXPORT = 'export'
IN_TRASH = 'in-trash'
MODIFY_IMPORTED_FIELDS = 'modify-imported-fields'
MODIFY_SERVICE = 'modify-service'
MODIFY_SHARING = 'modify_sharing'
MODIFY_USER_PERMISSIONS = 'modify-user-permissions'
PUT_BACK = 'put-back'
REDEPLOY = 'redeploy'
REGISTER_SERVICE = 'register-service'
REMOVE = 'remove'
REPLACE_FORM = 'replace-form'
SHARE_FORM_PUBLICLY = 'share-form-publicly'
SHARE_DATA_PUBLICLY = 'share-data-publicly'
UNARCHIVE = 'unarchive'
UNSHARE_FORM_PUBLICLY = 'unshare-form-publicly'
UNSHARE_DATA_PUBLICLY = 'unshare-data-publicly'
UPDATE = 'update'
UPDATE_CONTENT = 'update-content'
UPDATE_NAME = 'update-name'
Expand Down
143 changes: 138 additions & 5 deletions kobo/apps/audit_log/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import copy

import jsonschema
from django.conf import settings
from django.db import models
Expand All @@ -19,6 +21,10 @@
ACCESS_LOG_SUBMISSION_AUTH_TYPE,
ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE,
ACCESS_LOG_UNKNOWN_AUTH_TYPE,
PERM_ADD_SUBMISSIONS,
PERM_VIEW_ASSET,
PERM_VIEW_SUBMISSIONS,
PROJECT_HISTORY_LOG_PERMISSION_SUBTYPE,
PROJECT_HISTORY_LOG_PROJECT_SUBTYPE,
)
from kpi.fields.kpi_uid import UUID_LENGTH
Expand All @@ -30,6 +36,18 @@
ADDED = 'added'
REMOVED = 'removed'

ANONYMOUS_USER_PERMISSION_ACTIONS = {
# key: (permission, granting?), value: ph log action
# True means the permission is being granted,
# False means it's being revoked
(PERM_VIEW_ASSET, True): AuditAction.SHARE_FORM_PUBLICLY,
(PERM_VIEW_SUBMISSIONS, True): AuditAction.SHARE_DATA_PUBLICLY,
(PERM_ADD_SUBMISSIONS, True): AuditAction.ALLOW_ANONYMOUS_SUBMISSIONS,
(PERM_VIEW_ASSET, False): AuditAction.UNSHARE_FORM_PUBLICLY,
(PERM_VIEW_SUBMISSIONS, False): AuditAction.UNSHARE_DATA_PUBLICLY,
(PERM_ADD_SUBMISSIONS, False): AuditAction.DISALLOW_ANONYMOUS_SUBMISSIONS,
}


class AuditType(models.TextChoices):
ACCESS = 'access'
Expand Down Expand Up @@ -247,10 +265,7 @@ def create_from_request(
elif is_loginas:
# third option: loginas
auth_type = ACCESS_LOG_LOGINAS_AUTH_TYPE
elif (
hasattr(logged_in_user, 'backend')
and logged_in_user.backend is not None
):
elif hasattr(logged_in_user, 'backend') and logged_in_user.backend is not None:
# fourth option: the backend that authenticated the user
auth_type = logged_in_user.backend
else:
Expand Down Expand Up @@ -336,6 +351,9 @@ def create_from_request(cls, request):
'asset-export-list': cls.create_from_export_request,
'submissionexporttask-list': cls.create_from_v1_export,
'asset-bulk': cls.create_from_bulk_request,
'asset-permission-assignment-bulk-assignments': cls.create_from_permissions_request, # noqa
'asset-permission-assignment-detail': cls.create_from_permissions_request,
'asset-permission-assignment-list': cls.create_from_permissions_request,
}
url_name = request.resolver_match.url_name
method = url_name_to_action.get(url_name, None)
Expand Down Expand Up @@ -456,7 +474,7 @@ def create_from_detail_request(cls, request):
'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE,
'ip_address': get_client_ip(request),
'source': get_human_readable_client_user_agent(request),
'latest_version_uid': updated_data['latest_version.uid']
'latest_version_uid': updated_data['latest_version.uid'],
}

changed_field_to_action_map = {
Expand Down Expand Up @@ -668,3 +686,118 @@ def create_from_v1_export(cls, request):
'source': get_human_readable_client_user_agent(request),
},
)

@classmethod
def create_from_permissions_request(cls, request):
logs = []
initial_data = getattr(request, 'initial_data', None)
updated_data = getattr(request, 'updated_data', None)
asset_uid = request.resolver_match.kwargs['parent_lookup_asset']
source_data = updated_data if updated_data else initial_data
if source_data is None:
# there was an error on the request, ignore
return
asset_id = source_data['asset.id']
# these will be dicts of username: [permissions] (as set or list)
permissions_added = getattr(request, 'permissions_added', {})
permissions_removed = getattr(request, 'permissions_removed', {})
partial_permissions_added = getattr(request, 'partial_permissions_added', {})
# basic metadata for all PH logs
base_metadata = {
'ip_address': get_client_ip(request),
'source': get_human_readable_client_user_agent(request),
'asset_uid': asset_uid,
'log_subtype': PROJECT_HISTORY_LOG_PERMISSION_SUBTYPE,
}
# we'll be bulk creating logs instead of using .create, so we have to set
# all fields manually
log_base = {
'user': request.user,
'object_id': asset_id,
'app_label': Asset._meta.app_label,
'model_name': Asset._meta.model_name,
'log_type': AuditType.PROJECT_HISTORY,
'user_uid': request.user.extra_details.uid,
}
# get all users whose permissions changed
for username in {
*permissions_added,
*permissions_removed,
*partial_permissions_added,
}:
user_permissions_added = permissions_added.get(username, {})
user_permissions_removed = permissions_removed.get(username, {})
user_partial_permissions_added = partial_permissions_added.get(username, [])
if username == 'AnonymousUser':
cls.handle_anonymous_user_permissions(
user_permissions_added,
user_permissions_removed,
user_partial_permissions_added,
base_metadata,
log_base,
logs,
)
continue
metadata = copy.deepcopy(base_metadata)
metadata['permissions'] = {
'username': username,
REMOVED: list(user_permissions_removed),
ADDED: list(user_permissions_added) + user_partial_permissions_added,
}
logs.append(
ProjectHistoryLog(
**log_base,
action=AuditAction.MODIFY_USER_PERMISSIONS,
metadata=metadata,
)
)
ProjectHistoryLog.objects.bulk_create(logs)

@classmethod
def handle_anonymous_user_permissions(
cls,
perms_added,
perms_removed,
partial_perms_added,
base_metadata,
log_base,
logs,
):
# go through all the usual anonymous user permissions and create
# logs if they were changed
# remove each permission as it is logged so we can see if there
# are any unusual ones left over
for combination, action in ANONYMOUS_USER_PERMISSION_ACTIONS.items():
# ANONYMOUS_USER_PERMISSION_ACTIONS has tuples as keys
permission, was_added = combination
list_to_update = perms_added if was_added else perms_removed
if permission in list_to_update:
list_to_update.discard(permission)
logs.append(
ProjectHistoryLog(
**log_base,
action=action,
metadata=base_metadata,
)
)

# this shouldn't happen, but if anonymous users are granted other permissions,
# we want to know
if (
len(perms_removed) > 0
or len(perms_added) > 0
or len(partial_perms_added) > 0
):
metadata = copy.deepcopy(base_metadata)
metadata['permissions'] = {
'username': 'AnonymousUser',
ADDED: list(perms_added) + partial_perms_added,
REMOVED: list(perms_removed),
}
logs.append(
ProjectHistoryLog(
**log_base,
metadata=metadata,
action=AuditAction.MODIFY_USER_PERMISSIONS,
)
)
71 changes: 69 additions & 2 deletions kobo/apps/audit_log/signals.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
from collections import defaultdict

from celery.signals import task_success
from django.contrib.auth.signals import user_logged_in
from django.dispatch import receiver
from django_userforeignkey.request import get_current_request

from kpi.models import ImportTask
from kpi.constants import ASSET_TYPE_SURVEY, PERM_PARTIAL_SUBMISSIONS
from kpi.models import Asset, ImportTask
from kpi.tasks import import_in_background
from kpi.utils.log import logging

from kpi.utils.object_permission import (
post_assign_partial_perm,
post_assign_perm,
post_remove_partial_perms,
post_remove_perm,
)
from .models import AccessLog, ProjectHistoryLog


Expand All @@ -24,3 +33,61 @@ def create_access_log(sender, user, **kwargs):
def create_ph_log_for_import(sender, result, **kwargs):
task = ImportTask.objects.get(uid=result)
ProjectHistoryLog.create_from_import_task(task)


def initialize_permission_lists_if_necessary(request):
if getattr(request, 'permissions_added', None) is None:
request.permissions_added = defaultdict(set)
if getattr(request, 'permissions_removed', None) is None:
request.permissions_removed = defaultdict(set)
if getattr(request, 'partial_permissions_added', None) is None:
request.partial_permissions_added = defaultdict(list)


def initialize_request():
request = get_current_request()
if request is None:
return None
initialize_permission_lists_if_necessary(request)
return request


@receiver(post_assign_perm, sender=Asset)
def add_assigned_perms(sender, instance, user, codename, deny, **kwargs):
request = initialize_request()
if not request or instance.asset_type != ASSET_TYPE_SURVEY or deny:
return
request.permissions_added[user.username].add(codename)
request.permissions_removed[user.username].discard(codename)


@receiver(post_remove_perm, sender=Asset)
def add_removed_perms(sender, instance, user, codename, **kwargs):
request = initialize_request()
if not request or instance.asset_type != ASSET_TYPE_SURVEY:
return
request.permissions_removed[user.username].add(codename)
request.permissions_added[user.username].discard(codename)


@receiver(post_assign_partial_perm, sender=Asset)
def add_assigned_partial_perms(sender, instance, user, perms, **kwargs):
request = initialize_request()
if not request or instance.asset_type != ASSET_TYPE_SURVEY:
return
perms_as_list_of_dicts = [{'code': k, 'filters': v} for k, v in perms.items()]
# partial permissions are replaced rather than added
request.partial_permissions_added[user.username] = perms_as_list_of_dicts


@receiver(post_remove_partial_perms, sender=Asset)
def remove_partial_perms(sender, instance, user, **kwargs):
request = initialize_request()
if not request or instance.asset_type != ASSET_TYPE_SURVEY:
return
request.partial_permissions_added[user.username] = []
# in case we somehow deleted partial permissions
# without deleting 'partial_submissions', take care of that now since
# we can't have one without the other
request.permissions_added[user.username].discard(PERM_PARTIAL_SUBMISSIONS)
request.permissions_removed[user.username].add(PERM_PARTIAL_SUBMISSIONS)
33 changes: 33 additions & 0 deletions kobo/apps/audit_log/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from kobo.apps.audit_log.models import (
ACCESS_LOG_LOGINAS_AUTH_TYPE,
ACCESS_LOG_UNKNOWN_AUTH_TYPE,
ADDED,
REMOVED,
AccessLog,
AuditLog,
AuditType,
Expand Down Expand Up @@ -644,3 +646,34 @@ def test_create_from_import_task_with_name_change(self):
'name': {'old': old_name, 'new': 'new_name'},
},
)

def test_create_from_unexpected_anonymous_permissions(self):
# Normal anonymous permissions tested elsewhere
# This test is for if somehow other permissions are assigned
factory = RequestFactory()
request = factory.post('/')
request.user = User.objects.get(username='someuser')
request.resolver_match = Mock()
request.resolver_match.kwargs = {'parent_lookup_asset': 'a12345'}
request.updated_data = {
'asset.id': 1,
}
request.permissions_added = {
# these permissions are not allowed for anonymous users,
# pretend something went wrong/changed and they were assigned anyway
'AnonymousUser': {'discover_asset', 'validate_submissions'}
}
ProjectHistoryLog.create_from_permissions_request(
request,
)
self.assertEqual(ProjectHistoryLog.objects.count(), 1)
log = ProjectHistoryLog.objects.first()
self.assertEqual(log.object_id, 1)
# should create a regular 'MODIFY_USER_PERMISSIONS' log
self.assertEqual(log.action, AuditAction.MODIFY_USER_PERMISSIONS)
permissions = log.metadata['permissions']
self.assertEqual(permissions['username'], 'AnonymousUser')
self.assertListEqual(permissions[REMOVED], [])
self.assertListEqual(
sorted(permissions[ADDED]), ['discover_asset', 'validate_submissions']
)
Loading

0 comments on commit 4c3da9b

Please sign in to comment.