From eacc5262b627d7092d04e5cb5a5bbe5d12331ed2 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 5 Sep 2024 11:12:29 -0400 Subject: [PATCH 01/50] Use same utility to calculate hash --- kobo/apps/openrosa/apps/api/tools.py | 37 ++-- .../openrosa/apps/logger/models/attachment.py | 11 +- .../apps/openrosa/apps/logger/models/xform.py | 6 +- .../openrosa/apps/main/models/meta_data.py | 19 +- .../openrosa/apps/main/tests/test_process.py | 4 +- .../libs/serializers/xform_serializer.py | 2 +- kobo/apps/openrosa/libs/utils/gravatar.py | 5 +- kobo/apps/openrosa/libs/utils/hash.py | 191 ------------------ kobo/apps/openrosa/libs/utils/logger_tools.py | 4 +- kobo/settings/base.py | 8 - kobo/settings/dev.py | 8 - 11 files changed, 52 insertions(+), 243 deletions(-) delete mode 100644 kobo/apps/openrosa/libs/utils/hash.py diff --git a/kobo/apps/openrosa/apps/api/tools.py b/kobo/apps/openrosa/apps/api/tools.py index 34fc551cf7..1c5357791d 100644 --- a/kobo/apps/openrosa/apps/api/tools.py +++ b/kobo/apps/openrosa/apps/api/tools.py @@ -1,8 +1,6 @@ -# coding: utf-8 import inspect import os import re -import time from datetime import datetime import requests @@ -14,6 +12,7 @@ HttpResponseNotFound, HttpResponseRedirect, ) +from django.urls import resolve, Resolver404 from django.utils.translation import gettext as t from rest_framework import exceptions from rest_framework.request import Request @@ -35,6 +34,11 @@ from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) +from kpi.views.v2.paired_data import ( + PairedDataViewset, + SubmissionXMLRenderer, + XMLExternalDataPermission, +) DECIMAL_PRECISION = 2 @@ -156,18 +160,23 @@ def get_media_file_response( return HttpResponseRedirect(metadata.data_value) # When `request.user` is authenticated, their authentication is lost with - # an HTTP redirection. We use KoBoCAT to proxy the response from KPI - # Send the request internally to avoid extra traffic on the public interface - internal_url = metadata.data_value.replace( - settings.KOBOFORM_URL, settings.KOBOFORM_INTERNAL_URL - ) - response = requests.get(internal_url) - - return HttpResponse( - content=response.content, - status=response.status_code, - content_type=response.headers['content-type'], - ) + # an HTTP redirection. We need to call KPI viewset directly + internal_url = metadata.data_value.replace(settings.KOBOFORM_URL, '') + try: + resolver_match = resolve(internal_url) + except Resolver404: + return HttpResponseNotFound() + + args = resolver_match.args + kwargs = resolver_match.kwargs + + paired_data_viewset = PairedDataViewset.as_view({'get': 'external'}) + django_http_request = request._request + paired_data_viewset.cls.permission_classes = [XMLExternalDataPermission] + paired_data_viewset.cls.renderer_classes = [SubmissionXMLRenderer] + paired_data_viewset.cls.filter_backends = [] + return paired_data_viewset(request=django_http_request, *args, **kwargs) + def get_view_name(view_obj): diff --git a/kobo/apps/openrosa/apps/logger/models/attachment.py b/kobo/apps/openrosa/apps/logger/models/attachment.py index 5fa0457e96..1310a1657f 100644 --- a/kobo/apps/openrosa/apps/logger/models/attachment.py +++ b/kobo/apps/openrosa/apps/logger/models/attachment.py @@ -13,12 +13,12 @@ get_optimized_image_path, resize, ) -from kobo.apps.openrosa.libs.utils.hash import get_hash from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, KobocatFileSystemStorage, ) from kpi.mixins.audio_transcoding import AudioTranscodingMixin +from kpi.utils.hash import calculate_hash from .instance import Instance @@ -36,10 +36,6 @@ def upload_to(attachment, filename): return generate_attachment_filename(attachment.instance, filename) -def hash_attachment_contents(contents): - return get_hash(contents) - - class AttachmentDefaultManager(models.Manager): def get_queryset(self): @@ -101,9 +97,12 @@ def absolute_path(self): @property def file_hash(self): if self.media_file.storage.exists(self.media_file.name): + # TODO optimize calculation of hash when using cloud storage. + # Instead of reading the whole file, we could pass the url of the + # file to build the hash based on headers (e.g.: Etag). media_file_position = self.media_file.tell() self.media_file.seek(0) - media_file_hash = hash_attachment_contents(self.media_file.read()) + media_file_hash = calculate_hash(self.media_file.read()) self.media_file.seek(media_file_position) return media_file_hash return '' diff --git a/kobo/apps/openrosa/apps/logger/models/xform.py b/kobo/apps/openrosa/apps/logger/models/xform.py index 5ca359dbc5..a48f534b53 100644 --- a/kobo/apps/openrosa/apps/logger/models/xform.py +++ b/kobo/apps/openrosa/apps/logger/models/xform.py @@ -24,12 +24,12 @@ CAN_DELETE_DATA_XFORM, CAN_TRANSFER_OWNERSHIP, ) -from kobo.apps.openrosa.libs.utils.hash import get_hash from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) from kpi.fields.file import ExtendedFileField from kpi.models.abstract_models import AbstractTimeStampedModel +from kpi.utils.hash import calculate_hash from kpi.utils.xml import XMLFormWithDisclaimer XFORM_TITLE_LENGTH = 255 @@ -259,11 +259,11 @@ def time_of_last_submission_update(self): @property def md5_hash(self): - return get_hash(self.xml) + return calculate_hash(self.xml) @property def md5_hash_with_disclaimer(self): - return get_hash(self.xml_with_disclaimer) + return calculate_hash(self.xml_with_disclaimer) @property def can_be_replaced(self): diff --git a/kobo/apps/openrosa/apps/main/models/meta_data.py b/kobo/apps/openrosa/apps/main/models/meta_data.py index c08a537949..3dd390ea4c 100644 --- a/kobo/apps/openrosa/apps/main/models/meta_data.py +++ b/kobo/apps/openrosa/apps/main/models/meta_data.py @@ -1,4 +1,3 @@ -# coding: utf-8 import mimetypes import os import requests @@ -15,12 +14,13 @@ from requests.exceptions import RequestException from kobo.apps.openrosa.apps.logger.models import XForm -from kobo.apps.openrosa.libs.utils.hash import get_hash from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) from kpi.fields.file import ExtendedFileField from kpi.models.abstract_models import AbstractTimeStampedModel +from kpi.utils.hash import calculate_hash + CHUNK_SIZE = 1024 @@ -230,12 +230,14 @@ def _set_hash(self) -> str: string, if the file is a reference to a remote URL) when synchronizing form media. """ - if self.file_hash: - return self.file_hash if self.data_file: + + if self.file_hash: + return self.file_hash + try: - self.file_hash = get_hash(self.data_file, prefix=True) + self.file_hash = calculate_hash(self.data_file, prefix=True) except (IOError, FileNotFoundError) as e: return '' else: @@ -245,9 +247,14 @@ def _set_hash(self) -> str: return '' # Object should be a URL at this point `POST`ed by KPI. + + # Verify first whether it is dynamic external XML. + if self.is_paired_data: + return self.file_hash + # We have to set the hash try: - self.file_hash = get_hash(self.data_value, prefix=True, fast=True) + self.file_hash = calculate_hash(self.data_value, prefix=True) except RequestException as e: return '' else: diff --git a/kobo/apps/openrosa/apps/main/tests/test_process.py b/kobo/apps/openrosa/apps/main/tests/test_process.py index c86efb9a9c..69ffb583c8 100644 --- a/kobo/apps/openrosa/apps/main/tests/test_process.py +++ b/kobo/apps/openrosa/apps/main/tests/test_process.py @@ -19,7 +19,7 @@ from kobo.apps.openrosa.apps.logger.xform_instance_parser import clean_and_parse_xml from kobo.apps.openrosa.apps.viewer.models.data_dictionary import DataDictionary from kobo.apps.openrosa.libs.utils.common_tags import UUID, SUBMISSION_TIME -from kobo.apps.openrosa.libs.utils.hash import get_hash +from kpi.utils.hash import calculate_hash from .test_base import TestBase @@ -133,7 +133,7 @@ def _check_formList(self): self.manifest_url = \ 'http://testserver/%s/xformsManifest/%s'\ % (self.user.username, self.xform.pk) - md5_hash = get_hash(self.xform.xml) + md5_hash = calculate_hash(self.xform.xml) expected_content = """ transportation_2011_07_25transportation_2011_07_25md5:%(hash)stransportation_2011_07_25%(download_url)s%(manifest_url)s""" # noqa expected_content = expected_content % { diff --git a/kobo/apps/openrosa/libs/serializers/xform_serializer.py b/kobo/apps/openrosa/libs/serializers/xform_serializer.py index f9ad5c0685..4703675087 100644 --- a/kobo/apps/openrosa/libs/serializers/xform_serializer.py +++ b/kobo/apps/openrosa/libs/serializers/xform_serializer.py @@ -165,7 +165,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def get_filename(self, obj): - # If file has been synchronized from KPI and it is a remote URL, + # If file has been synchronized from KPI, and it is a remote URL, # manifest.xml should return only the name, not the full URL. # See https://github.com/kobotoolbox/kobocat/issues/344 if obj.from_kpi: diff --git a/kobo/apps/openrosa/libs/utils/gravatar.py b/kobo/apps/openrosa/libs/utils/gravatar.py index 1aad58510c..4e1815390f 100644 --- a/kobo/apps/openrosa/libs/utils/gravatar.py +++ b/kobo/apps/openrosa/libs/utils/gravatar.py @@ -2,7 +2,8 @@ from urllib.request import urlopen from django.utils.http import urlencode -from kobo.apps.openrosa.libs.utils.hash import get_hash + +from kpi.utils.hash import calculate_hash DEFAULT_GRAVATAR = "https://formhub.org/static/images/formhub_avatar.png" GRAVATAR_ENDPOINT = "https://secure.gravatar.com/avatar/" @@ -11,7 +12,7 @@ def get_gravatar_img_link(user): url = GRAVATAR_ENDPOINT +\ - get_hash(user.email.lower()) + "?" + urlencode({ + calculate_hash(user.email.lower()) + "?" + urlencode({ 'd': DEFAULT_GRAVATAR, 's': GRAVATAR_SIZE }) return url diff --git a/kobo/apps/openrosa/libs/utils/hash.py b/kobo/apps/openrosa/libs/utils/hash.py deleted file mode 100644 index 7fca6de6ec..0000000000 --- a/kobo/apps/openrosa/libs/utils/hash.py +++ /dev/null @@ -1,191 +0,0 @@ -# coding: utf-8 -import hashlib -import os -from typing import Union, BinaryIO - -import requests -from django.conf import settings -from rest_framework import status - - -def get_hash(source: Union[str, bytes, BinaryIO], - algorithm: str = 'md5', - prefix: bool = False, - fast: bool = False) -> str: - """ - Calculates the hash for an object. - - :param source: string, bytes or FileObject. Files must be opened in binary mode # noqa - :param algorithm: Can be 'md5' or 'sha1'. Default: 'md5'. - :param prefix: Prefix the return value with the algorithm, e.g.: 'md5:34523' - :param fast: If True, only calculate on 3 small pieces of the object. - Useful with big (remote) files. - """ - - supported_algorithm = ['md5', 'sha1'] - - if algorithm not in supported_algorithm: - raise NotImplementedError('Only `{algorithms}` are supported'.format( - algorithms=', '.join(supported_algorithm) - )) - - if algorithm == 'md5': - hashlib_def = hashlib.md5 - else: - hashlib_def = hashlib.sha1 - - def _prefix_hash(hex_digest: str) -> str: - if prefix: - return f'{algorithm}:{hex_digest}' - return hex_digest - - # If `source` is a string, it can be a URL or real string - if isinstance(source, str): - if not source.startswith('http'): - hashable = source.encode() - else: - # Try to get hash from the content of a URL. - # If the any requests fail, it returns the hash of the URL itself. - # We do not want to raise a `RequestException` while calculating - # the hash. - # ToDo: Evaluate whether it could be better to return ̀`None` when - # a request exception occurs - - # Ensure we do not receive a gzip response. - headers = {'Accept-Encoding': None} - url_hash = hashlib_def(source.encode()).hexdigest() - # Get remote file size - # `requests.get(url, stream=True)` only gets headers. Body is - # only retrieved when `response.content` is called. - # It avoids making a second request to get the body - # (i.e.: vs `requests.head()`). - try: - response = requests.get(source, stream=True, headers=headers) - response.raise_for_status() - except requests.exceptions.RequestException: - # With `stream=True`, the connection is kept alive until it is - # closed or all the data is consumed. Because, we do not consume - # the data, we have to close the connexion manually. - try: - response.close() - except UnboundLocalError: - pass - return _prefix_hash(hex_digest=url_hash) - - try: - file_size = int(response.headers['Content-Length']) - except KeyError: - file_size = 0 - fast = True - - # If the remote file is smaller than the threshold or smaller than - # 3 times the chunk, we retrieve the whole file to avoid extra - # requests. - if ( - not fast - or file_size < settings.HASH_BIG_FILE_SIZE_THRESHOLD - or file_size < 3 * settings.HASH_BIG_FILE_CHUNK - ): - hashable = response.content - else: - # With `stream=True`, the connection is kept alive until it is - # closed or all the data is consumed. Because, we do not consume - # the data, we have to close the connexion manually. - response.close() - - # If file_size is empty, do not go further as we will probably - # not be able to get file by chunk - if not file_size: - return _prefix_hash(hex_digest=url_hash) - - # We fetch 3 parts of the file. - # - One chunk at the beginning - # - One chunk in the middle - # - One chunk at the end - # It should be accurate enough to detect whether big files - # over the network have changed without locking uWSGI workers - # for a long time. - # Drawback on this solution, it relies on the remote server to - # support `Accept-Ranges`. If it does not, it returns the whole - # file. In that case, we calculate on the URL itself, not its - # content - - # 1) First part - range_ = f'0-{settings.HASH_BIG_FILE_CHUNK - 1}' - headers['Range'] = f'bytes={range_}' - try: - response = requests.get(source, stream=True, headers=headers) - response.raise_for_status() - except requests.exceptions.RequestException: - try: - response.close() - except UnboundLocalError: - pass - return _prefix_hash(hex_digest=url_hash) - else: - if response.status_code != status.HTTP_206_PARTIAL_CONTENT: - # Remove server does not support ranges. The file may be - # huge. Do not try to download it - response.close() - return _prefix_hash(hex_digest=url_hash) - hashable = response.content - - # 2) Second part - range_lower_bound = file_size // 2 - range_upper_bound = (range_lower_bound - + settings.HASH_BIG_FILE_CHUNK - 1) - range_ = f'{range_lower_bound}-{range_upper_bound}' - headers['Range'] = f'bytes={range_}' - try: - response = requests.get(source, headers=headers) - response.raise_for_status() - except requests.exceptions.RequestException: - return _prefix_hash(hex_digest=url_hash) - hashable += response.content - - # 3) Last part - range_lower_bound = file_size - settings.HASH_BIG_FILE_CHUNK - range_upper_bound = file_size - 1 - range_ = f'{range_lower_bound}-{range_upper_bound}' - headers['Range'] = f'bytes={range_}' - try: - response = requests.get(source, headers=headers) - response.raise_for_status() - except requests.exceptions.RequestException: - return _prefix_hash(hex_digest=url_hash) - hashable += response.content - - return _prefix_hash(hashlib_def(hashable).hexdigest()) - - try: - source.read(settings.HASH_BIG_FILE_CHUNK) - except AttributeError: - # Source is `bytes`, just return its hash - return _prefix_hash(hashlib_def(source).hexdigest()) - - # Get local file size - source.seek(0, os.SEEK_END) - file_size = source.tell() - source.seek(0, os.SEEK_SET) - - # If the local file is smaller than the threshold or smaller than - # 3 times the chunk, we retrieve the whole file to avoid error when seeking - # out of bounds. - if ( - not fast - or file_size < settings.HASH_BIG_FILE_SIZE_THRESHOLD - or file_size < 3 * settings.HASH_BIG_FILE_CHUNK - ): - hashable = hashlib_def() - while chunk := source.read(settings.HASH_BIG_FILE_CHUNK): - hashable.update(chunk) - - return _prefix_hash(hashable.hexdigest()) - - hashable = source.read(settings.HASH_BIG_FILE_CHUNK) - source.seek(file_size // 2, os.SEEK_SET) - hashable += source.read(settings.HASH_BIG_FILE_CHUNK) - source.seek(-settings.HASH_BIG_FILE_CHUNK, os.SEEK_END) - hashable += source.read(settings.HASH_BIG_FILE_CHUNK) - - return _prefix_hash(hashlib_def(hashable).hexdigest()) diff --git a/kobo/apps/openrosa/libs/utils/logger_tools.py b/kobo/apps/openrosa/libs/utils/logger_tools.py index 81decfc462..4fb6bf9d58 100644 --- a/kobo/apps/openrosa/libs/utils/logger_tools.py +++ b/kobo/apps/openrosa/libs/utils/logger_tools.py @@ -47,7 +47,6 @@ from kobo.apps.openrosa.apps.logger.models import Attachment, Instance, XForm from kobo.apps.openrosa.apps.logger.models.attachment import ( generate_attachment_filename, - hash_attachment_contents, ) from kobo.apps.openrosa.apps.logger.models.instance import ( InstanceHistory, @@ -83,6 +82,7 @@ default_kobocat_storage as default_storage, ) from kpi.utils.object_permission import get_database_user +from kpi.utils.hash import calculate_hash OPEN_ROSA_VERSION_HEADER = 'X-OpenRosa-Version' @@ -621,7 +621,7 @@ def save_attachments( mimetype=f.content_type, ).first() if existing_attachment and ( - existing_attachment.file_hash == hash_attachment_contents(f.read()) + existing_attachment.file_hash == calculate_hash(f.read()) ): # We already have this attachment! continue diff --git a/kobo/settings/base.py b/kobo/settings/base.py index e207737429..fb126908c7 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -1576,14 +1576,6 @@ def dj_stripe_request_callback_method(): # Should match KoBoCAT setting PAIRED_DATA_EXPIRATION = 300 # seconds -# Minimum size (in bytes) of files to allow fast calculation of hashes -# Should match KoBoCAT setting -HASH_BIG_FILE_SIZE_THRESHOLD = 0.5 * 1024 * 1024 # 512 kB - -# Chunk size in bytes to read per iteration when hash of a file is calculated -# Should match KoBoCAT setting -HASH_BIG_FILE_CHUNK = 16 * 1024 # 16 kB - # add some mimetype add_type('application/wkt', '.wkt') add_type('application/geo+json', '.geojson') diff --git a/kobo/settings/dev.py b/kobo/settings/dev.py index 958ad50d43..f05de243ad 100644 --- a/kobo/settings/dev.py +++ b/kobo/settings/dev.py @@ -23,14 +23,6 @@ def show_toolbar(request): # Does not need to match KoBoCAT setting PAIRED_DATA_EXPIRATION = 5 -# Minimum size (in bytes) of files to allow fast calculation of hashes -# Should match KoBoCAT setting -HASH_BIG_FILE_SIZE_THRESHOLD = 200 * 1024 # 200 kB - -# Chunk size in bytes to read per iteration when hash of a file is calculated -# Should match KoBoCAT setting -HASH_BIG_FILE_CHUNK = 5 * 1024 # 5 kB - # To avoid buffer to be truncated when running `runserver_plus` or `shell_plus` # with option `--print-sql` SHELL_PLUS_PRINT_SQL_TRUNCATE = None From 80d5994ed431f88f3c1b43f99229b1f42310302a Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 5 Sep 2024 13:20:02 -0400 Subject: [PATCH 02/50] Use random hash for URL --- kpi/utils/hash.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/kpi/utils/hash.py b/kpi/utils/hash.py index d652c8b616..d18317e8d9 100644 --- a/kpi/utils/hash.py +++ b/kpi/utils/hash.py @@ -1,4 +1,4 @@ -# coding: utf-8 +import time import hashlib from typing import Union, BinaryIO, Optional @@ -40,9 +40,18 @@ def _finalize_hash( Return final string with/without the algorithm as prefix and specified suffix if any """ + if isinstance(hashable_, str): hashable_ = hashable_.encode() + if suffix == 'url': + # When entering this condition, `hashable_` is a URL which never + # changes. If remote server does not provide required headers to + # build a hash (e.g.: ETag, Last-Modified), we want the hash to + # change each time (useful to force Enketo or Collect to fetch data). + # Too bad for the remote server, it will receive more hits. + hashable_ += f'-{int(time.time())}'.encode() + hash_ = hashlib_def(hashable_).hexdigest() if prefix: @@ -102,5 +111,3 @@ def _finalize_hash( return _finalize_hash(f'{content_type}:{content_length}', 'length') return _finalize_hash(source, 'url') - - From c88f1e44a0e14a7d3e55e6aad533ea2a19501770 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 5 Sep 2024 14:33:14 -0400 Subject: [PATCH 03/50] Cache hash of remote media files when no headers are provided --- kobo/settings/base.py | 6 ++---- kobo/settings/dev.py | 3 ++- kpi/utils/hash.py | 18 ++++++++++++++++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/kobo/settings/base.py b/kobo/settings/base.py index fb126908c7..76e7ba0ea7 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -1573,9 +1573,10 @@ def dj_stripe_request_callback_method(): OPENROSA_DEFAULT_CONTENT_LENGTH = 10000000 # Expiration time in sec. after which paired data xml file must be regenerated -# Should match KoBoCAT setting PAIRED_DATA_EXPIRATION = 300 # seconds +CALCULATED_HASH_CACHE_EXPIRATION = 300 # seconds + # add some mimetype add_type('application/wkt', '.wkt') add_type('application/geo+json', '.geojson') @@ -1745,6 +1746,3 @@ def dj_stripe_request_callback_method(): SILENCED_SYSTEM_CHECKS = ['guardian.W001'] DIGEST_LOGIN_FACTORY = 'django_digest.NoEmailLoginFactory' - - -DIGEST_LOGIN_FACTORY = 'django_digest.NoEmailLoginFactory' diff --git a/kobo/settings/dev.py b/kobo/settings/dev.py index f05de243ad..2bdf474604 100644 --- a/kobo/settings/dev.py +++ b/kobo/settings/dev.py @@ -20,9 +20,10 @@ def show_toolbar(request): ENV = 'dev' # Expiration time in sec. after which paired data xml file must be regenerated -# Does not need to match KoBoCAT setting PAIRED_DATA_EXPIRATION = 5 +CALCULATED_HASH_CACHE_EXPIRATION = 5 + # To avoid buffer to be truncated when running `runserver_plus` or `shell_plus` # with option `--print-sql` SHELL_PLUS_PRINT_SQL_TRUNCATE = None diff --git a/kpi/utils/hash.py b/kpi/utils/hash.py index d18317e8d9..f4a413da5e 100644 --- a/kpi/utils/hash.py +++ b/kpi/utils/hash.py @@ -3,6 +3,8 @@ from typing import Union, BinaryIO, Optional import requests +from django.conf import settings +from django.core.cache import cache def calculate_hash( @@ -49,8 +51,20 @@ def _finalize_hash( # changes. If remote server does not provide required headers to # build a hash (e.g.: ETag, Last-Modified), we want the hash to # change each time (useful to force Enketo or Collect to fetch data). - # Too bad for the remote server, it will receive more hits. - hashable_ += f'-{int(time.time())}'.encode() + # Too bad for the remote server, it will receive more hits. BUT + # we still want to cache it for a specific amount of time to avoid + # Enketo/Collect to warn about a new version each time the project + # is (re)loaded. + cache_key = 'cached_hash::' + hashlib_def(source.encode()).hexdigest() + if not (cached_hashable := cache.get(cache_key)): + hashable_ += f'-{int(time.time())}'.encode() + cache.set( + cache_key, + hashable_, + settings.CALCULATED_HASH_CACHE_EXPIRATION, + ) + else: + hashable_ = cached_hashable hash_ = hashlib_def(hashable_).hexdigest() From dbdc624d760aa2034e33e0d5bb1ddb14ba2ef573 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 11 Sep 2024 16:10:24 -0400 Subject: [PATCH 04/50] feat: endpoint to log user out of all sessions --- kobo/settings/base.py | 2 ++ kpi/tests/api/v2/test_api_logout_all.py | 48 +++++++++++++++++++++++++ kpi/urls/__init__.py | 2 ++ kpi/urls/router_api_v2.py | 1 + kpi/views/v2/logout.py | 32 +++++++++++++++++ 5 files changed, 85 insertions(+) create mode 100644 kpi/tests/api/v2/test_api_logout_all.py create mode 100644 kpi/views/v2/logout.py diff --git a/kobo/settings/base.py b/kobo/settings/base.py index e207737429..2b942448af 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -105,6 +105,7 @@ 'allauth.socialaccount', 'allauth.socialaccount.providers.microsoft', 'allauth.socialaccount.providers.openid_connect', + 'allauth.usersessions', 'hub.HubAppConfig', 'loginas', 'webpack_loader', @@ -154,6 +155,7 @@ 'django.contrib.sessions.middleware.SessionMiddleware', 'hub.middleware.LocaleMiddleware', 'allauth.account.middleware.AccountMiddleware', + 'allauth.usersessions.middleware.UserSessionsMiddleware', 'django.middleware.common.CommonMiddleware', # Still needed really? 'kobo.apps.openrosa.libs.utils.middleware.LocaleMiddlewareWithTweaks', diff --git a/kpi/tests/api/v2/test_api_logout_all.py b/kpi/tests/api/v2/test_api_logout_all.py new file mode 100644 index 0000000000..e1fcc5c389 --- /dev/null +++ b/kpi/tests/api/v2/test_api_logout_all.py @@ -0,0 +1,48 @@ +from allauth.usersessions.models import UserSession +from django.urls import reverse + +from kobo.apps.kobo_auth.shortcuts import User +from kpi.tests.base_test_case import BaseTestCase + + +class TestLogoutAll(BaseTestCase): + + fixtures = ['test_data'] + + def test_logout_all_sessions(self): + # create 2 user sessions + user = User.objects.get(username='someuser') + UserSession.objects.create(user=user, session_key='12345', ip='1.2.3.4') + UserSession.objects.create(user=user, session_key='56789', ip='5.6.7.8') + count = UserSession.objects.filter(user=user).count() + self.assertEqual(count, 2) + self.client.force_login(user) + url = self._get_endpoint('logout_all') + self.client.post(reverse(url)) + + # ensure both sessions have been deleted + count = UserSession.objects.filter(user=user).count() + self.assertEqual(count, 0) + + def test_logout_all_sessions_does_not_affect_other_users(self): + # create 2 user sessions + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + # create sessions for user1 + UserSession.objects.create( + user=user1, session_key='12345', ip='1.2.3.4' + ) + UserSession.objects.create( + user=user1, session_key='56789', ip='5.6.7.8' + ) + count = UserSession.objects.count() + self.assertEqual(count, 2) + + # login user2 + self.client.force_login(user2) + url = self._get_endpoint('logout_all') + self.client.post(reverse(url)) + + # ensure no sessions have been deleted + count = UserSession.objects.filter().count() + self.assertEqual(count, 2) diff --git a/kpi/urls/__init__.py b/kpi/urls/__init__.py index 786df65e38..9cf2ce732b 100644 --- a/kpi/urls/__init__.py +++ b/kpi/urls/__init__.py @@ -13,6 +13,7 @@ from .router_api_v1 import router_api_v1 from .router_api_v2 import router_api_v2, URL_NAMESPACE +from ..views.v2.logout import logout_from_all_devices # TODO: Give other apps their own `urls.py` files instead of importing their # views directly! See @@ -50,6 +51,7 @@ re_path(r'^private-media/', include(private_storage.urls)), # Statistics for superusers re_path(r'^superuser_stats/', include(('kobo.apps.superuser_stats.urls', 'superuser_stats'))), + path('logout-all/', logout_from_all_devices, name='logout_all') ] diff --git a/kpi/urls/router_api_v2.py b/kpi/urls/router_api_v2.py index 7c7dbd2575..033fc1566c 100644 --- a/kpi/urls/router_api_v2.py +++ b/kpi/urls/router_api_v2.py @@ -21,6 +21,7 @@ from kpi.views.v2.data import DataViewSet from kpi.views.v2.export_task import ExportTaskViewSet from kpi.views.v2.import_task import ImportTaskViewSet +from kpi.views.v2.logout import logout_from_all_devices from kpi.views.v2.paired_data import PairedDataViewset from kpi.views.v2.permission import PermissionViewSet from kpi.views.v2.service_usage import ServiceUsageViewSet diff --git a/kpi/views/v2/logout.py b/kpi/views/v2/logout.py new file mode 100644 index 0000000000..959f6a8330 --- /dev/null +++ b/kpi/views/v2/logout.py @@ -0,0 +1,32 @@ +from allauth.usersessions.adapter import get_adapter +from allauth.usersessions.models import UserSession +from rest_framework.decorators import api_view, permission_classes +from rest_framework.response import Response + +from kpi.permissions import IsAuthenticated + + +@api_view(['POST']) +@permission_classes((IsAuthenticated,)) +def logout_from_all_devices(request): + """ + Log calling user out from all devices + + 
+    POST /logout-all/
+
+ + > Example + > + > curl -H 'Authorization Token 12345' -X POST https://[kpi-url]/logout-all + + > Response 200 + + > { "Logged out of all sessions" } + + """ + user = request.user + all_user_sessions = UserSession.objects.purge_and_list(user) + adapter = get_adapter() + adapter.end_sessions(all_user_sessions) + return Response('Logged out of all sessions') From f5e0ed3fb2d71b5613a06b8d6e3f0709407a16b0 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 12 Sep 2024 12:26:07 -0400 Subject: [PATCH 05/50] fixup!: rm bad comment --- kpi/tests/api/v2/test_api_logout_all.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kpi/tests/api/v2/test_api_logout_all.py b/kpi/tests/api/v2/test_api_logout_all.py index e1fcc5c389..c4c04ee2ea 100644 --- a/kpi/tests/api/v2/test_api_logout_all.py +++ b/kpi/tests/api/v2/test_api_logout_all.py @@ -25,7 +25,6 @@ def test_logout_all_sessions(self): self.assertEqual(count, 0) def test_logout_all_sessions_does_not_affect_other_users(self): - # create 2 user sessions user1 = User.objects.get(username='someuser') user2 = User.objects.get(username='anotheruser') # create sessions for user1 From a98ae4bc6c39fc888ad177f39da08f7481dbb605 Mon Sep 17 00:00:00 2001 From: Leszek Date: Fri, 13 Sep 2024 11:20:44 +0200 Subject: [PATCH 06/50] Add Access Log section to security route, add access log query and use it together with UniversalTable --- .../accessLog/accessLogSection.component.tsx | 27 +++++++++++ .../security/securityRoute.component.tsx | 2 + jsapp/js/api.endpoints.ts | 1 + jsapp/js/query/queries/accessLog.query.ts | 48 +++++++++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 jsapp/js/account/security/accessLog/accessLogSection.component.tsx create mode 100644 jsapp/js/query/queries/accessLog.query.ts diff --git a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx new file mode 100644 index 0000000000..8576f58558 --- /dev/null +++ b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx @@ -0,0 +1,27 @@ +import React from 'react'; +import useAccessLogQuery, {type AccessLog} from 'js/query/queries/accessLog.query'; +import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component'; +import {formatTime} from 'js/utils'; + +export default function AccessLogSection() { + return ( + <> +

{t('Recent account activity')}

+ + + queryHook={useAccessLogQuery} + columns={[ + // The `key`s of these columns are matching the `AccessLog` interface + // properties (from `accessLog.query.ts` file) using dot notation. + {key: 'metadata.source', label: t('Source')}, + { + key: 'date_created', + label: t('Last activity'), + cellFormatter: (date: string) => formatTime(date), + }, + {key: 'metadata.ip_address', label: t('IP Address')}, + ]} + /> + + ); +} diff --git a/jsapp/js/account/security/securityRoute.component.tsx b/jsapp/js/account/security/securityRoute.component.tsx index ff35aa7fcb..83386920c7 100644 --- a/jsapp/js/account/security/securityRoute.component.tsx +++ b/jsapp/js/account/security/securityRoute.component.tsx @@ -5,6 +5,7 @@ import EmailSection from './email/emailSection.component'; import ApiTokenSection from './apiToken/apiTokenSection.component'; import SsoSection from './sso/ssoSection.component'; import style from './securityRoute.module.scss'; +import AccessLogSection from './accessLog/accessLogSection.component'; export default function securityRoute() { return ( @@ -15,6 +16,7 @@ export default function securityRoute() { + ); } diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts index 9d0ade0300..c1b6a86d64 100644 --- a/jsapp/js/api.endpoints.ts +++ b/jsapp/js/api.endpoints.ts @@ -10,4 +10,5 @@ export const endpoints = { PORTAL_URL: '/api/v2/stripe/customer-portal', /** Expected parameters: price_id and subscription_id **/ CHANGE_PLAN_URL: '/api/v2/stripe/change-plan', + ACCESS_LOG_URL: '/api/v2/access-logs/', }; diff --git a/jsapp/js/query/queries/accessLog.query.ts b/jsapp/js/query/queries/accessLog.query.ts new file mode 100644 index 0000000000..6da583bc35 --- /dev/null +++ b/jsapp/js/query/queries/accessLog.query.ts @@ -0,0 +1,48 @@ +import {keepPreviousData, useQuery} from '@tanstack/react-query'; +import {endpoints} from 'js/api.endpoints'; +import type {PaginatedResponse} from 'js/dataInterface'; +import {fetchGet} from 'js/api'; + +export interface AccessLog { + app_label: 'kobo_auth' | string; + model_name: 'User' | string; + object_id: number; + /** User URL */ + user: string; + user_uid: string; + username: string; + action: 'auth' | string; + metadata: { + /** E.g. "Firefox (Ubuntu)" */ + source: string; + auth_type: 'Digest' | string; + ip_address: string; + }; + /** Date string */ + date_created: string; + log_type: 'access' | string; +} + +async function getAccessLog(limit: number, offset: number) { + const params = new URLSearchParams({ + limit: limit.toString(), + offset: offset.toString(), + }); + return fetchGet>( + endpoints.ACCESS_LOG_URL + '?' + params, + { + errorMessageDisplay: t('There was an error getting the list.'), + } + ); +} + +export default function useAccessLogQuery( + itemLimit: number, + pageOffset: number +) { + return useQuery({ + queryKey: ['accessLog', itemLimit, pageOffset], + queryFn: () => getAccessLog(itemLimit, pageOffset), + placeholderData: keepPreviousData, + }); +} From a3bb916a19b8b6893987bf6066fe964e24ac8a9f Mon Sep 17 00:00:00 2001 From: Leszek Date: Mon, 16 Sep 2024 18:31:06 +0200 Subject: [PATCH 07/50] update styles of all security route sections --- .../accessLog/accessLogSection.component.tsx | 31 +++++++- .../apiToken/apiTokenSection.component.tsx | 53 ++++++++------ .../apiToken/apiTokenSection.module.scss | 60 +++------------- .../security/email/emailSection.component.tsx | 58 +++++++++------ .../security/email/emailSection.module.scss | 43 ++--------- .../security/mfa/mfaSection.component.tsx | 21 ++++-- jsapp/js/account/security/mfa/mfaSection.scss | 8 +-- .../password/passwordSection.component.tsx | 38 ++++++---- .../password/passwordSection.module.scss | 72 ++++--------------- .../security/securityRoute.component.tsx | 21 +++++- .../security/securityRoute.module.scss | 69 ++++++++++++++++-- .../security/sso/ssoSection.component.tsx | 57 ++++++++------- .../security/sso/ssoSection.module.scss | 36 +--------- 13 files changed, 283 insertions(+), 284 deletions(-) diff --git a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx index 8576f58558..6aaabe2cde 100644 --- a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx +++ b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx @@ -1,12 +1,39 @@ +// Libraries import React from 'react'; -import useAccessLogQuery, {type AccessLog} from 'js/query/queries/accessLog.query'; + +// Partial components +import Button from 'js/components/common/button'; import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component'; + +// Utilities +import useAccessLogQuery, {type AccessLog} from 'js/query/queries/accessLog.query'; import {formatTime} from 'js/utils'; +// Styles +import securityStyles from 'js/account/security/securityRoute.module.scss'; + export default function AccessLogSection() { + function logOutAllSessions() { + console.log('log\'em all'); + } + return ( <> -

{t('Recent account activity')}

+
+

+ {t('Recent account activity')} +

+ +
+
+
queryHook={useAccessLogQuery} diff --git a/jsapp/js/account/security/apiToken/apiTokenSection.component.tsx b/jsapp/js/account/security/apiToken/apiTokenSection.component.tsx index ea435a56e1..49c5f36b2e 100644 --- a/jsapp/js/account/security/apiToken/apiTokenSection.component.tsx +++ b/jsapp/js/account/security/apiToken/apiTokenSection.component.tsx @@ -1,12 +1,21 @@ +// Libraries import React, { useState, useEffect, } from 'react'; -import {dataInterface} from 'js/dataInterface'; +import cx from 'classnames'; + +// Partial components import TextBox from 'js/components/common/textBox'; import Button from 'js/components/common/button'; + +// Utils +import {dataInterface} from 'js/dataInterface'; import {notify} from 'js/utils'; + +// Styles import styles from './apiTokenSection.module.scss'; +import securityStyles from 'js/account/security/securityRoute.module.scss'; const HIDDEN_TOKEN_VALUE = '*'.repeat(40); @@ -42,28 +51,28 @@ export default function ApiTokenDisplay() { }, [isVisible]); return ( -
-
-

{t('API Key')}

-
- -
- -
+
+
+

{t('API Key')}

+
-
-
-
+
+ +
+
+
+ ); } diff --git a/jsapp/js/account/security/apiToken/apiTokenSection.module.scss b/jsapp/js/account/security/apiToken/apiTokenSection.module.scss index 130237141b..5a2525338f 100644 --- a/jsapp/js/account/security/apiToken/apiTokenSection.module.scss +++ b/jsapp/js/account/security/apiToken/apiTokenSection.module.scss @@ -2,59 +2,19 @@ @use 'scss/sizes'; @use 'scss/libs/_mdl'; -.root { - margin-top: sizes.$x20; - margin-bottom: sizes.$x60; - padding-top: sizes.$x14; - display: flex; - align-items: baseline; - column-gap: sizes.$x16; - border-top: sizes.$x1 solid; - border-color: colors.$kobo-gray-300; -} - -.titleSection { - flex: 2; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; -} - -.title { - margin: 0; - color: colors.$kobo-gray-700; - font-weight: 600; - line-height: 1.6; +.body { + flex: 5; } -.bodySection { - flex: 6; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; - padding-left: sizes.$x4; - - label { - display: inline-block; - vertical-align: top; - width: 100%; - - input { - // API token display - font-family: mdl.$font_mono; - // 40 characters + padding + border - max-width: calc(40ch + 22px); - } +.token { + input { + // API token display + font-family: mdl.$font_mono; } } -.optionsSection { - flex: 2; - text-align: right; - - button { - display: inline-block; - } +.options { + flex: 3; + display: flex; + justify-content: flex-end; } diff --git a/jsapp/js/account/security/email/emailSection.component.tsx b/jsapp/js/account/security/email/emailSection.component.tsx index 9df4cac7d5..70b3a12e0b 100644 --- a/jsapp/js/account/security/email/emailSection.component.tsx +++ b/jsapp/js/account/security/email/emailSection.component.tsx @@ -1,4 +1,8 @@ +// Libraries import React, {useEffect, useState} from 'react'; +import cx from 'classnames'; + +// Stores and email related import sessionStore from 'js/stores/session'; import { getUserEmails, @@ -6,12 +10,18 @@ import { deleteUnverifiedUserEmails, } from './emailSection.api'; import type {EmailResponse} from './emailSection.api'; -import style from './emailSection.module.scss'; + +// Partial components import Button from 'jsapp/js/components/common/button'; import TextBox from 'jsapp/js/components/common/textBox'; import Icon from 'jsapp/js/components/common/icon'; -import {formatTime} from 'jsapp/js/utils'; -import {notify} from 'js/utils'; + +// Utils +import {formatTime, notify} from 'js/utils'; + +// Styles +import styles from './emailSection.module.scss'; +import securityStyles from 'js/account/security/securityRoute.module.scss'; interface EmailState { emails: EmailResponse[]; @@ -23,9 +33,14 @@ interface EmailState { export default function EmailSection() { const [session] = useState(() => sessionStore); + let initialEmail = ''; + if ('email' in session.currentAccount) { + initialEmail = session.currentAccount.email; + } + const [email, setEmail] = useState({ emails: [], - newEmail: '', + newEmail: initialEmail, refreshedEmail: false, refreshedEmailDate: '', }); @@ -103,16 +118,21 @@ export default function EmailSection() { ); return ( -
-
-

{t('Email address')}

+
+
+

{t('Email address')}

-
+
{!session.isPending && session.isInitialLoadComplete && 'email' in currentAccount && ( -

{currentAccount.email}

+ )} {unverifiedEmail?.email && @@ -120,9 +140,9 @@ export default function EmailSection() { session.isInitialLoadComplete && 'email' in currentAccount && ( <> -
+
-

+

{t('Check your email ##UNVERIFIED_EMAIL##. ').replace( '##UNVERIFIED_EMAIL##', @@ -136,7 +156,7 @@ export default function EmailSection() {

-
+
+
); } diff --git a/jsapp/js/account/security/email/emailSection.module.scss b/jsapp/js/account/security/email/emailSection.module.scss index 23a2c7e377..912e1fb48b 100644 --- a/jsapp/js/account/security/email/emailSection.module.scss +++ b/jsapp/js/account/security/email/emailSection.module.scss @@ -1,49 +1,14 @@ @use 'scss/colors'; @use 'scss/sizes'; -@use 'scss/libs/_mdl'; -.root { - margin-top: sizes.$x20; - margin-bottom: sizes.$x60; - padding-top: sizes.$x14; - display: flex; - align-items: baseline; - column-gap: sizes.$x16; - border-top: sizes.$x1 solid; - border-color: colors.$kobo-gray-300; -} - -.titleSection { - flex: 2; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; -} - -.title { - margin: 0; - color: colors.$kobo-gray-700; - font-weight: 600; - line-height: 1.6; -} - -.bodySection { +.body { flex: 5; - flex-direction: row; - align-items: center; - flex-wrap: wrap; - padding-left: sizes.$x16; } -.optionsSection { - flex: 3; // widen for email input - - // stack input and button, right-aligned. +.options { + flex: 3; display: flex; - flex-direction: column; - row-gap: sizes.$x12; - align-items: flex-end; + justify-content: flex-end; } .currentEmail { diff --git a/jsapp/js/account/security/mfa/mfaSection.component.tsx b/jsapp/js/account/security/mfa/mfaSection.component.tsx index b2749b76cb..7ac0dffbcc 100644 --- a/jsapp/js/account/security/mfa/mfaSection.component.tsx +++ b/jsapp/js/account/security/mfa/mfaSection.component.tsx @@ -1,21 +1,33 @@ +// Libraries import React from 'react'; import bem, {makeBem} from 'js/bem'; + +// Partial components import Button from 'js/components/common/button'; import ToggleSwitch from 'js/components/common/toggleSwitch'; import Icon from 'js/components/common/icon'; import InlineMessage from 'js/components/common/inlineMessage'; import LoadingSpinner from 'js/components/common/loadingSpinner'; + +// Reflux import type { MfaUserMethodsResponse, MfaActivatedResponse, } from 'js/actions/mfaActions'; import mfaActions from 'js/actions/mfaActions'; + +// Constants and utils import {MODAL_TYPES} from 'jsapp/js/constants'; -import envStore from 'js/envStore'; -import './mfaSection.scss'; import {formatTime, formatDate} from 'js/utils'; + +// Stores +import envStore from 'js/envStore'; import pageState from 'js/pageState.store'; +// Styles +import './mfaSection.scss'; +import securityStyles from 'js/account/security/securityRoute.module.scss'; + bem.SecurityRow = makeBem(null, 'security-row'); bem.SecurityRow__header = makeBem(bem.SecurityRow, 'header'); bem.SecurityRow__title = makeBem(bem.SecurityRow, 'title', 'h2'); @@ -179,7 +191,7 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { } return ( - <> +
@@ -257,6 +269,7 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { /> )} + {this.state.isPlansMessageVisible && ( { } /> )} - +
); } } diff --git a/jsapp/js/account/security/mfa/mfaSection.scss b/jsapp/js/account/security/mfa/mfaSection.scss index e7a03e97ee..869667c5cd 100644 --- a/jsapp/js/account/security/mfa/mfaSection.scss +++ b/jsapp/js/account/security/mfa/mfaSection.scss @@ -3,16 +3,10 @@ @use 'scss/sizes'; .security-row { - margin-bottom: sizes.$x60; - .security-row__header { - margin-top: sizes.$x20; - padding-top: sizes.$x14; display: flex; align-items: baseline; column-gap: sizes.$x16; - border-top: sizes.$x1 solid; - border-color: colors.$kobo-gray-300; > * { margin: 0; @@ -53,7 +47,7 @@ .security-row--unauthorized { opacity: 0.5; pointer-events: none; - margin-bottom: sizes.$x36; + // margin-bottom: sizes.$x36; } .mfa-options { diff --git a/jsapp/js/account/security/password/passwordSection.component.tsx b/jsapp/js/account/security/password/passwordSection.component.tsx index 74eb9b6e20..c8cb62fef5 100644 --- a/jsapp/js/account/security/password/passwordSection.component.tsx +++ b/jsapp/js/account/security/password/passwordSection.component.tsx @@ -1,37 +1,51 @@ +// Libraries import React from 'react'; -import {PATHS} from 'js/router/routerConstants'; -import Button from 'jsapp/js/components/common/button'; -import styles from './passwordSection.module.scss'; + +// Partial components import {NavLink} from 'react-router-dom'; +import Button from 'jsapp/js/components/common/button'; + +// Constants +import {PATHS} from 'js/router/routerConstants'; import {ACCOUNT_ROUTES} from 'js/account/routes.constants'; +// Styles +import styles from './passwordSection.module.scss'; +import securityStyles from 'js/account/security/securityRoute.module.scss'; + const HIDDEN_TOKEN_VALUE = '● '.repeat(10); export default function PasswordSection() { return ( -
-
-

{t('Password')}

+
+
+

{t('Password')}

-
+

{HIDDEN_TOKEN_VALUE}

-
- {t('forgot password')} +
+ +
-
+
); } diff --git a/jsapp/js/account/security/password/passwordSection.module.scss b/jsapp/js/account/security/password/passwordSection.module.scss index a741a39d13..5cffd6dadd 100644 --- a/jsapp/js/account/security/password/passwordSection.module.scss +++ b/jsapp/js/account/security/password/passwordSection.module.scss @@ -1,68 +1,22 @@ @use 'scss/colors'; @use 'scss/sizes'; -@use 'scss/libs/_mdl'; +@use 'scss/mixins'; -.root { - margin-top: sizes.$x20; - margin-bottom: sizes.$x60; - padding-top: sizes.$x14; - display: flex; - align-items: baseline; - column-gap: sizes.$x16; - border-top: sizes.$x1 solid; - border-color: colors.$kobo-gray-300; -} - -.titleSection { - flex: 2; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; -} - -.title { +.passwordDisplay { margin: 0; - color: colors.$kobo-gray-700; - font-weight: 600; - line-height: 1.6; -} - -.bodySection { - flex: 3; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; - padding-left: sizes.$x16; - .passwordDisplay { - margin: 0; - - // gray circles - color: colors.$kobo-gray-500; - cursor: default; - font-size: sizes.$x14; - // Allow cutoff if low on space - overflow-y: hidden; - height: sizes.$x20; - } + // gray circles + color: colors.$kobo-gray-500; + cursor: default; + font-size: sizes.$x14; + // Allow cutoff if low on space + overflow-y: hidden; + height: sizes.$x20; } -.optionsSection { - text-align: right; +.options { + @include mixins.centerRowFlex; + gap: 10px; flex: 5; - - a { - display: inline-block; - margin-bottom: sizes.$x12; - - &:hover { - text-decoration: underline; - } - } -} - -.optionsSection > *:not(:first-child) { - margin-left: sizes.$x12; + justify-content: flex-end; } diff --git a/jsapp/js/account/security/securityRoute.component.tsx b/jsapp/js/account/security/securityRoute.component.tsx index 83386920c7..e1e9902815 100644 --- a/jsapp/js/account/security/securityRoute.component.tsx +++ b/jsapp/js/account/security/securityRoute.component.tsx @@ -1,21 +1,36 @@ +// Libraries import React from 'react'; + +// Partial components import MfaSection from './mfa/mfaSection.component'; import PasswordSection from './password/passwordSection.component'; import EmailSection from './email/emailSection.component'; import ApiTokenSection from './apiToken/apiTokenSection.component'; import SsoSection from './sso/ssoSection.component'; -import style from './securityRoute.module.scss'; import AccessLogSection from './accessLog/accessLogSection.component'; +// Styles +import styles from './securityRoute.module.scss'; + export default function securityRoute() { return ( -
-

{t('Security')}

+
+
+

+ {t('Security')} +

+
+ + + + + +
); diff --git a/jsapp/js/account/security/securityRoute.module.scss b/jsapp/js/account/security/securityRoute.module.scss index 20ee7157c5..02484c5a68 100644 --- a/jsapp/js/account/security/securityRoute.module.scss +++ b/jsapp/js/account/security/securityRoute.module.scss @@ -1,14 +1,12 @@ +@use 'scss/mixins'; +@use 'scss/colors'; @use 'scss/sizes'; -.security-section { +.securityRouteRoot { padding: sizes.$x50; overflow-y: auto; height: 100%; - h1 { - margin-bottom: sizes.$x36; - } - :global { // Harmonize button widths .k-button__label { @@ -18,3 +16,64 @@ } } } + +header.securityHeader { + @include mixins.centerRowFlex; + margin: 24px 0; + + &:not(:first-child) { + margin-top: 44px; + } +} + +h2.securityHeaderText { + color: colors.$kobo-storm; + text-transform: uppercase; + font-size: 18px; + font-weight: 700; + flex: 1; + margin: 0; +} + +.securityHeaderActions { + @include mixins.centerRowFlex; +} + +// Shared styles for sections + +.securitySection { + display: flex; + align-items: baseline; + flex-wrap: wrap; + gap: 16px; + padding: 16px 0; + border-top: 1px solid colors.$kobo-gray-200; + + &:last-of-type { + border-bottom: 1px solid colors.$kobo-gray-200; + } +} + +.securitySectionTitle { + flex: 2; + display: flex; + flex-direction: row; + align-items: center; + flex-wrap: wrap; +} + +.securitySectionTitleText { + margin: 0; + color: colors.$kobo-gray-600; + font-weight: 600; + line-height: 1.6; + font-size: 16px; +} + +.securitySectionBody { + flex: 3; + display: flex; + flex-direction: row; + align-items: center; + flex-wrap: wrap; +} diff --git a/jsapp/js/account/security/sso/ssoSection.component.tsx b/jsapp/js/account/security/sso/ssoSection.component.tsx index 167466f0ac..0274c36718 100644 --- a/jsapp/js/account/security/sso/ssoSection.component.tsx +++ b/jsapp/js/account/security/sso/ssoSection.component.tsx @@ -1,11 +1,19 @@ +// Libraries import React, {useCallback} from 'react'; import {observer} from 'mobx-react-lite'; +import cx from 'classnames'; + +// Partial components +import Button from 'jsapp/js/components/common/button'; + +// Stores and utils import sessionStore from 'js/stores/session'; -import styles from './ssoSection.module.scss'; +import envStore, {type SocialApp} from 'jsapp/js/envStore'; import {deleteSocialAccount} from './sso.api'; -import Button from 'jsapp/js/components/common/button'; -import envStore, {SocialApp} from 'jsapp/js/envStore'; -import classNames from 'classnames'; + +// Styles +import styles from './ssoSection.module.scss'; +import securityStyles from 'js/account/security/securityRoute.module.scss'; const SsoSection = observer(() => { const socialApps = envStore.isReady ? envStore.data.social_apps : []; @@ -25,48 +33,49 @@ const SsoSection = observer(() => { const providerLink = useCallback((socialApp: SocialApp) => { let providerPath = ''; - if(socialApp.provider === 'openid_connect') { + if (socialApp.provider === 'openid_connect') { providerPath = 'oidc/' + socialApp.provider_id; } else { providerPath = socialApp.provider_id || socialApp.provider; } return `accounts/${providerPath}/login/?process=connect&next=%2F%23%2Faccount%2Fsecurity`; - }, [sessionStore.currentAccount]) + }, [sessionStore.currentAccount]); if (socialApps.length === 0 && socialAccounts.length === 0) { return <>; } return ( -
-
-

{t('Single-Sign On')}

+
+
+

{t('Single-Sign On')}

+ {socialAccounts.length === 0 ? ( -
-
- {t( - "Connect your KoboToolbox account with your organization's identity provider for single-sign on (SSO). Afterwards, you will only " + - 'be able to sign in via SSO unless you disable this setting here. This will also update your email address in case your current ' + - 'address is different.' - )} -
+
+ {t( + "Connect your KoboToolbox account with your organization's identity provider for single-sign on (SSO). Afterwards, you will only " + + 'be able to sign in via SSO unless you disable this setting here. This will also update your email address in case your current ' + + 'address is different.' + )}
) : ( -
{t('Already connected')}
+
+ {t('Already connected')} +
)} {socialAccounts.length === 0 ? ( -
); }); diff --git a/jsapp/js/account/security/sso/ssoSection.module.scss b/jsapp/js/account/security/sso/ssoSection.module.scss index f90a601ad9..356bab7ab9 100644 --- a/jsapp/js/account/security/sso/ssoSection.module.scss +++ b/jsapp/js/account/security/sso/ssoSection.module.scss @@ -1,43 +1,11 @@ @use 'scss/colors'; @use 'scss/sizes'; -@use 'scss/libs/_mdl'; -.root { - margin-top: sizes.$x20; - margin-bottom: sizes.$x60; - padding-top: sizes.$x14; - display: flex; - align-items: baseline; - column-gap: sizes.$x16; - border-top: sizes.$x1 solid; - border-color: colors.$kobo-gray-300; -} - -.titleSection { - flex: 2; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; -} - -.title { - margin: 0; - color: colors.$kobo-gray-700; - font-weight: 600; - line-height: 1.6; -} - -.bodySection { +.body { flex: 6; - display: flex; - flex-direction: row; - align-items: center; - flex-wrap: wrap; - padding-left: sizes.$x16; } -.optionsSection { +.options { text-align: right; flex: 2; From 0c797d01f76570127b4ce9b589d1c149d321a9bf Mon Sep 17 00:00:00 2001 From: Leszek Date: Mon, 16 Sep 2024 21:09:52 +0200 Subject: [PATCH 08/50] simplify MFA section styles and HTML by removing BEM and making it use the same consistent DRY CSS module as other security sections --- .../security/mfa/mfaSection.component.tsx | 172 ++++++++---------- .../security/mfa/mfaSection.module.scss | 51 ++++++ jsapp/js/account/security/mfa/mfaSection.scss | 75 -------- .../security/securityRoute.module.scss | 17 +- jsapp/js/actions/mfaActions.ts | 2 +- 5 files changed, 141 insertions(+), 176 deletions(-) create mode 100644 jsapp/js/account/security/mfa/mfaSection.module.scss delete mode 100644 jsapp/js/account/security/mfa/mfaSection.scss diff --git a/jsapp/js/account/security/mfa/mfaSection.component.tsx b/jsapp/js/account/security/mfa/mfaSection.component.tsx index 7ac0dffbcc..e7b2b64ce6 100644 --- a/jsapp/js/account/security/mfa/mfaSection.component.tsx +++ b/jsapp/js/account/security/mfa/mfaSection.component.tsx @@ -1,6 +1,6 @@ // Libraries import React from 'react'; -import bem, {makeBem} from 'js/bem'; +import cx from 'classnames'; // Partial components import Button from 'js/components/common/button'; @@ -25,29 +25,9 @@ import envStore from 'js/envStore'; import pageState from 'js/pageState.store'; // Styles -import './mfaSection.scss'; +import styles from './mfaSection.module.scss'; import securityStyles from 'js/account/security/securityRoute.module.scss'; -bem.SecurityRow = makeBem(null, 'security-row'); -bem.SecurityRow__header = makeBem(bem.SecurityRow, 'header'); -bem.SecurityRow__title = makeBem(bem.SecurityRow, 'title', 'h2'); -bem.SecurityRow__buttons = makeBem(bem.SecurityRow, 'buttons'); -bem.SecurityRow__description = makeBem(bem.SecurityRow, 'description', 'p'); -bem.SecurityRow__switch = makeBem(bem.SecurityRow, 'switch'); - -bem.MFAOptions = makeBem(null, 'mfa-options'); -bem.MFAOptions__row = makeBem(bem.MFAOptions, 'row'); -bem.MFAOptions__label = makeBem(bem.MFAOptions, 'label'); -bem.MFAOptions__buttons = makeBem(bem.MFAOptions, 'buttons'); -bem.MFAOptions__date = makeBem(bem.MFAOptions, 'date'); - -bem.TableMediaPreviewHeader = makeBem(null, 'table-media-preview-header'); -bem.TableMediaPreviewHeader__title = makeBem( - bem.TableMediaPreviewHeader, - 'title', - 'div' -); - interface SecurityState { isMfaAvailable?: boolean; isMfaActive: boolean; @@ -105,7 +85,7 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { } } - onGetMfaAvailability(response: {isMfaAvailable: boolean, isPlansMessageVisible: boolean}) { + onGetMfaAvailability(response: {isMfaAvailable: boolean; isPlansMessageVisible: boolean}) { // Determine whether MFA is allowed based on per-user availability and subscription status this.setState({ isMfaAvailable: response.isMfaAvailable, @@ -172,12 +152,12 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { renderCustomHeader() { return ( - - +
+
{t('Two-factor authentication')} - - +
+
); } @@ -191,73 +171,61 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { } return ( -
- - - - {t('Two-factor authentication')} - - - - {t( - 'Two-factor authentication (2FA) verifies your identity using an authenticator application in addition to your usual password. ' + - 'We recommend enabling two-factor authentication for an additional layer of protection.' - )} - - - - - - - - +
+
+

+ {t('Two-factor authentication')} +

+
+ +
+

+ {t( + 'Two-factor authentication (2FA) verifies your identity using an authenticator application in addition to your usual password. ' + + 'We recommend enabling two-factor authentication for an additional layer of protection.' + )} +

{this.state.isMfaActive && this.state.isMfaAvailable && ( - - - +
+
+

{t('Authenticator app')} - +

{this.state.dateModified && ( - +
{formatTime(this.state.dateModified)} - +
)} - -
+ +
+

{t('Recovery codes')} - - - -

+ +
+
)} {!this.state.isMfaActive && this.state.isMfaAvailable && this.state.dateDisabled && ( @@ -268,20 +236,28 @@ export default class SecurityRoute extends React.Component<{}, SecurityState> { ).replace('##date##', formatDate(this.state.dateDisabled))} /> )} - - - {this.state.isPlansMessageVisible && ( - - {t('This feature is not available on your current plan. Please visit the ')} - {t('Plans page')} - {t(' to upgrade your account.')} - - } + + {this.state.isPlansMessageVisible && ( + + {t('This feature is not available on your current plan. Please visit the ')} + {t('Plans page')} + {t(' to upgrade your account.')} + + } + /> + )} +
+ +
+ - )} +
); } diff --git a/jsapp/js/account/security/mfa/mfaSection.module.scss b/jsapp/js/account/security/mfa/mfaSection.module.scss new file mode 100644 index 0000000000..c8bf348a33 --- /dev/null +++ b/jsapp/js/account/security/mfa/mfaSection.module.scss @@ -0,0 +1,51 @@ +@use 'scss/mixins'; +@use 'scss/colors'; +@use 'scss/sizes'; + +.isUnauthorized { + opacity: 0.5; + pointer-events: none; +} + +// We need stronger specificity +.body.body { + flex: 6; + flex-direction: column; +} + +.options { + flex: 2; + display: flex; + justify-content: flex-end; +} + +.mfaDescription { + margin: 0; +} + +.mfaOptions { + display: flex; + flex-direction: column; + gap: 16px; + margin-top: 16px; + width: 100%; +} + +.mfaOptionsRow { + @include mixins.centerRowFlex; + flex-wrap: wrap; + gap: 10px; + padding: 16px; + justify-content: space-between; + background-color: colors.$kobo-gray-200; + border-radius: 6px; +} + +.mfaOptionsLabel { + flex: 1; + display: flex; + align-items: center; + font-weight: 600; + font-size: sizes.$x16; + margin: 0; +} diff --git a/jsapp/js/account/security/mfa/mfaSection.scss b/jsapp/js/account/security/mfa/mfaSection.scss deleted file mode 100644 index 869667c5cd..0000000000 --- a/jsapp/js/account/security/mfa/mfaSection.scss +++ /dev/null @@ -1,75 +0,0 @@ -@use 'scss/mixins'; -@use 'scss/colors'; -@use 'scss/sizes'; - -.security-row { - .security-row__header { - display: flex; - align-items: baseline; - column-gap: sizes.$x16; - - > * { - margin: 0; - } - } - - .security-row__title { - flex: 2; - color: colors.$kobo-gray-700; - font-weight: 600; - line-height: 1.6; - } - - .security-row__description { - flex: 6; - padding-left: sizes.$x16; - } - - .security-row__switch { - flex: 2; - text-align: right; - } - - .security-row__buttons { - margin-right: sizes.$x20; - - .toggle-switch__wrapper { - display: flex; - } - - .toggle-switch__label { - font-weight: 600; - color: colors.$kobo-gray-700; - } - } -} - -.security-row--unauthorized { - opacity: 0.5; - pointer-events: none; - // margin-bottom: sizes.$x36; -} - -.mfa-options { - .mfa-options__buttons { - margin: sizes.$x14 0; - padding: 0 sizes.$x14 0 sizes.$x28; - } - - .mfa-options__row { - @include mixins.centerRowFlex; - - margin: sizes.$x14 0; - padding: 0 sizes.$x14 0 sizes.$x28; - justify-content: space-between; - background-color: colors.$kobo-gray-300; - } - - .mfa-options__label { - flex: 1; - display: flex; - align-items: center; - font-weight: 600; - font-size: sizes.$x16; - } -} diff --git a/jsapp/js/account/security/securityRoute.module.scss b/jsapp/js/account/security/securityRoute.module.scss index 02484c5a68..4de6c0b598 100644 --- a/jsapp/js/account/security/securityRoute.module.scss +++ b/jsapp/js/account/security/securityRoute.module.scss @@ -1,9 +1,10 @@ @use 'scss/mixins'; @use 'scss/colors'; @use 'scss/sizes'; +@use 'scss/breakpoints'; .securityRouteRoot { - padding: sizes.$x50; + padding: sizes.$x20; overflow-y: auto; height: 100%; @@ -55,7 +56,8 @@ h2.securityHeaderText { } .securitySectionTitle { - flex: 2; + width: 100%; + flex: initial; display: flex; flex-direction: row; align-items: center; @@ -77,3 +79,14 @@ h2.securityHeaderText { align-items: center; flex-wrap: wrap; } + +@include breakpoints.breakpoint(mediumAndUp) { + .securitySectionTitle { + width: initial; + flex: 2; + } + + .securityRouteRoot { + padding: sizes.$x50; + } +} diff --git a/jsapp/js/actions/mfaActions.ts b/jsapp/js/actions/mfaActions.ts index 098c7afa1a..8128867a67 100644 --- a/jsapp/js/actions/mfaActions.ts +++ b/jsapp/js/actions/mfaActions.ts @@ -102,7 +102,7 @@ mfaActions.getMfaAvailability.listen(() => { mfaActions.getMfaAvailability.failed({isMfaAvailable: false, isPlansMessageVisible: false}); }); } - }) + }); }); mfaActions.confirmCode.listen((mfaCode: string) => { From 195d65d075436c6c4da8cbf894ce4bcf67bd1f1c Mon Sep 17 00:00:00 2001 From: Leszek Date: Wed, 18 Sep 2024 17:07:47 +0200 Subject: [PATCH 09/50] hook up log out all button to API --- .../accessLog/accessLogSection.component.tsx | 3 ++- jsapp/js/api.endpoints.ts | 1 + jsapp/js/stores/session.ts | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx index 6aaabe2cde..fdde10b024 100644 --- a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx +++ b/jsapp/js/account/security/accessLog/accessLogSection.component.tsx @@ -8,13 +8,14 @@ import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniver // Utilities import useAccessLogQuery, {type AccessLog} from 'js/query/queries/accessLog.query'; import {formatTime} from 'js/utils'; +import sessionStore from 'js/stores/session'; // Styles import securityStyles from 'js/account/security/securityRoute.module.scss'; export default function AccessLogSection() { function logOutAllSessions() { - console.log('log\'em all'); + sessionStore.logOutAll(); } return ( diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts index c1b6a86d64..0d22e7eda8 100644 --- a/jsapp/js/api.endpoints.ts +++ b/jsapp/js/api.endpoints.ts @@ -11,4 +11,5 @@ export const endpoints = { /** Expected parameters: price_id and subscription_id **/ CHANGE_PLAN_URL: '/api/v2/stripe/change-plan', ACCESS_LOG_URL: '/api/v2/access-logs/', + LOGOUT_ALL: '/api/v2/logout-all', }; diff --git a/jsapp/js/stores/session.ts b/jsapp/js/stores/session.ts index a862740c4e..b9dcf5c2a4 100644 --- a/jsapp/js/stores/session.ts +++ b/jsapp/js/stores/session.ts @@ -5,6 +5,8 @@ import type {AccountResponse, FailResponse} from 'js/dataInterface'; import {log, currentLang} from 'js/utils'; import type {Json} from 'js/components/common/common.interfaces'; import type {ProjectViewsSettings} from 'js/projects/customViewStore'; +import {fetchPost, handleApiFail} from 'js/api'; +import {endpoints} from 'js/api.endpoints'; class SessionStore { currentAccount: AccountResponse | {username: string; date_joined: string} = { @@ -97,6 +99,20 @@ class SessionStore { ); } + /** + * Useful if you need to log out all sessions. + */ + public async logOutAll() { + try { + // Logging out is simply POSTing to this endpoint + await fetchPost(endpoints.LOGOUT_ALL, {}); + // After that we force reload + window.location.replace(''); + } catch (err) { + // `fetchPost` is handling the error + } + } + private saveUiLanguage() { // We want to save the language if it differs from the one we saved or if // none is saved yet. From 8b9576d07ea71d4b04036480491c29c059132d97 Mon Sep 17 00:00:00 2001 From: Leszek Date: Wed, 18 Sep 2024 18:15:44 +0200 Subject: [PATCH 10/50] tiny fixes for failing access log endpoint --- jsapp/js/query/queries/accessLog.query.ts | 5 +++++ jsapp/js/universalTable/universalTable.component.tsx | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jsapp/js/query/queries/accessLog.query.ts b/jsapp/js/query/queries/accessLog.query.ts index 6da583bc35..614728a00c 100644 --- a/jsapp/js/query/queries/accessLog.query.ts +++ b/jsapp/js/query/queries/accessLog.query.ts @@ -44,5 +44,10 @@ export default function useAccessLogQuery( queryKey: ['accessLog', itemLimit, pageOffset], queryFn: () => getAccessLog(itemLimit, pageOffset), placeholderData: keepPreviousData, + // We might want to improve this in future, for now let's not retry + retry: false, + // The `refetchOnWindowFocus` option is `true` by default, I'm setting it + // here so we don't forget about it. + refetchOnWindowFocus: true, }); } diff --git a/jsapp/js/universalTable/universalTable.component.tsx b/jsapp/js/universalTable/universalTable.component.tsx index 490cb1a770..eafe0c980d 100644 --- a/jsapp/js/universalTable/universalTable.component.tsx +++ b/jsapp/js/universalTable/universalTable.component.tsx @@ -162,7 +162,7 @@ export default function UniversalTable( .map((col) => col.key); options.state.columnPinning = {left: pinnedColumns || []}; - const hasPagination = ( + const hasPagination = Boolean( props.pageIndex !== undefined && props.pageCount && props.pageSize && From 2073b24fd93ec5796b7eb4e8b52d67475957396d Mon Sep 17 00:00:00 2001 From: Leszek Date: Wed, 18 Sep 2024 23:27:48 +0200 Subject: [PATCH 11/50] rename access log to access logs for consistency with BE code --- .../accessLogsSection.component.tsx} | 8 ++++---- jsapp/js/account/security/securityRoute.component.tsx | 4 ++-- jsapp/js/api.endpoints.ts | 2 +- .../{accessLog.query.ts => accessLogs.query.ts} | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) rename jsapp/js/account/security/{accessLog/accessLogSection.component.tsx => accessLogs/accessLogsSection.component.tsx} (84%) rename jsapp/js/query/queries/{accessLog.query.ts => accessLogs.query.ts} (82%) diff --git a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx b/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx similarity index 84% rename from jsapp/js/account/security/accessLog/accessLogSection.component.tsx rename to jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx index fdde10b024..ac3ee860e3 100644 --- a/jsapp/js/account/security/accessLog/accessLogSection.component.tsx +++ b/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx @@ -6,14 +6,14 @@ import Button from 'js/components/common/button'; import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component'; // Utilities -import useAccessLogQuery, {type AccessLog} from 'js/query/queries/accessLog.query'; +import useAccessLogsQuery, {type AccessLog} from 'js/query/queries/accessLogs.query'; import {formatTime} from 'js/utils'; import sessionStore from 'js/stores/session'; // Styles import securityStyles from 'js/account/security/securityRoute.module.scss'; -export default function AccessLogSection() { +export default function AccessLogsSection() { function logOutAllSessions() { sessionStore.logOutAll(); } @@ -37,10 +37,10 @@ export default function AccessLogSection() { - queryHook={useAccessLogQuery} + queryHook={useAccessLogsQuery} columns={[ // The `key`s of these columns are matching the `AccessLog` interface - // properties (from `accessLog.query.ts` file) using dot notation. + // properties (from `accessLogs.query.ts` file) using dot notation. {key: 'metadata.source', label: t('Source')}, { key: 'date_created', diff --git a/jsapp/js/account/security/securityRoute.component.tsx b/jsapp/js/account/security/securityRoute.component.tsx index e1e9902815..4446e4227c 100644 --- a/jsapp/js/account/security/securityRoute.component.tsx +++ b/jsapp/js/account/security/securityRoute.component.tsx @@ -7,7 +7,7 @@ import PasswordSection from './password/passwordSection.component'; import EmailSection from './email/emailSection.component'; import ApiTokenSection from './apiToken/apiTokenSection.component'; import SsoSection from './sso/ssoSection.component'; -import AccessLogSection from './accessLog/accessLogSection.component'; +import AccessLogsSection from './accessLogs/accessLogsSection.component'; // Styles import styles from './securityRoute.module.scss'; @@ -31,7 +31,7 @@ export default function securityRoute() { - +
); } diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts index 0d22e7eda8..1e3778b1c6 100644 --- a/jsapp/js/api.endpoints.ts +++ b/jsapp/js/api.endpoints.ts @@ -10,6 +10,6 @@ export const endpoints = { PORTAL_URL: '/api/v2/stripe/customer-portal', /** Expected parameters: price_id and subscription_id **/ CHANGE_PLAN_URL: '/api/v2/stripe/change-plan', - ACCESS_LOG_URL: '/api/v2/access-logs/', + ACCESS_LOGS_URL: '/api/v2/access-logs/', LOGOUT_ALL: '/api/v2/logout-all', }; diff --git a/jsapp/js/query/queries/accessLog.query.ts b/jsapp/js/query/queries/accessLogs.query.ts similarity index 82% rename from jsapp/js/query/queries/accessLog.query.ts rename to jsapp/js/query/queries/accessLogs.query.ts index 614728a00c..ea2bf8df1d 100644 --- a/jsapp/js/query/queries/accessLog.query.ts +++ b/jsapp/js/query/queries/accessLogs.query.ts @@ -23,26 +23,26 @@ export interface AccessLog { log_type: 'access' | string; } -async function getAccessLog(limit: number, offset: number) { +async function getAccessLogs(limit: number, offset: number) { const params = new URLSearchParams({ limit: limit.toString(), offset: offset.toString(), }); return fetchGet>( - endpoints.ACCESS_LOG_URL + '?' + params, + endpoints.ACCESS_LOGS_URL + '?' + params, { errorMessageDisplay: t('There was an error getting the list.'), } ); } -export default function useAccessLogQuery( +export default function useAccessLogsQuery( itemLimit: number, pageOffset: number ) { return useQuery({ - queryKey: ['accessLog', itemLimit, pageOffset], - queryFn: () => getAccessLog(itemLimit, pageOffset), + queryKey: ['accessLogs', itemLimit, pageOffset], + queryFn: () => getAccessLogs(itemLimit, pageOffset), placeholderData: keepPreviousData, // We might want to improve this in future, for now let's not retry retry: false, From 7d711ca4dd426afd2cad70228938be6fbb4b3d61 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Wed, 18 Sep 2024 18:09:29 -0400 Subject: [PATCH 12/50] Fix linters errors --- kobo/apps/openrosa/libs/utils/gravatar.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/kobo/apps/openrosa/libs/utils/gravatar.py b/kobo/apps/openrosa/libs/utils/gravatar.py index 4e1815390f..8eb8f525de 100644 --- a/kobo/apps/openrosa/libs/utils/gravatar.py +++ b/kobo/apps/openrosa/libs/utils/gravatar.py @@ -5,21 +5,22 @@ from kpi.utils.hash import calculate_hash -DEFAULT_GRAVATAR = "https://formhub.org/static/images/formhub_avatar.png" -GRAVATAR_ENDPOINT = "https://secure.gravatar.com/avatar/" +DEFAULT_GRAVATAR = 'https://formhub.org/static/images/formhub_avatar.png' +GRAVATAR_ENDPOINT = 'https://secure.gravatar.com/avatar/' GRAVATAR_SIZE = str(60) def get_gravatar_img_link(user): - url = GRAVATAR_ENDPOINT +\ - calculate_hash(user.email.lower()) + "?" + urlencode({ - 'd': DEFAULT_GRAVATAR, 's': GRAVATAR_SIZE - }) + url = ( + GRAVATAR_ENDPOINT + + calculate_hash(user.email.lower()) + + '?' + + urlencode({'d': DEFAULT_GRAVATAR, 's': GRAVATAR_SIZE}) + ) return url def gravatar_exists(user): - url = GRAVATAR_ENDPOINT +\ - get_hash(user.email.lower()) + "?" + "d=404" + url = GRAVATAR_ENDPOINT + calculate_hash(user.email.lower()) + '?' + 'd=404' exists = urlopen(url).getcode() != 404 return exists From 8039a00f94874cdadd0645e26305bc832bff864e Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Wed, 18 Sep 2024 18:21:37 -0400 Subject: [PATCH 13/50] Ignore Flake8 rW503, W504 rules --- .github/workflows/darker.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/darker.yml b/.github/workflows/darker.yml index 18dd2763d1..b22e560775 100644 --- a/.github/workflows/darker.yml +++ b/.github/workflows/darker.yml @@ -24,6 +24,6 @@ jobs: # darker still exit with code 1 even with no errors on changes - name: Run Darker with base commit run: | - output=$(darker --check --isort -L "flake8 --max-line-length=88 --ignore=F821" kpi kobo hub -r ${{ github.event.pull_request.base.sha }}) + output=$(darker --check --isort -L "flake8 --max-line-length=88 --ignore=F821,W503,W504" kpi kobo hub -r ${{ github.event.pull_request.base.sha }}) [[ -n "$output" ]] && echo "$output" && exit 1 || exit 0 shell: /usr/bin/bash {0} From 0fe1e33438a9eeab2ae668eaa46f1e98a78bf539 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 17 Sep 2024 16:23:41 -0400 Subject: [PATCH 14/50] feat: group submission logs when fetching access logs --- kobo/apps/audit_log/models.py | 24 +++++++++++-- kobo/apps/audit_log/serializers.py | 17 ++++++++++ kobo/apps/audit_log/views.py | 34 +++++++++++++++++-- .../apps/api/viewsets/xform_submission_api.py | 6 ++-- kpi/fields/username_hyperlinked.py | 15 ++++++++ 5 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 kpi/fields/username_hyperlinked.py diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 81e1555a25..21b6a296c6 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,6 +1,13 @@ +import logging +from importlib.metadata import metadata + from django.conf import settings +from django.contrib.auth.models import AnonymousUser +from django.contrib.contenttypes.models import ContentType from django.db import models from django.utils import timezone +from django.db.models.functions import Coalesce, Trunc, Concat, Cast +from django.db.models import When, Count, Case from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.libs.utils.viewer_tools import ( @@ -93,7 +100,16 @@ def save( class AccessLogManager(models.Manager): def get_queryset(self): - return super().get_queryset().filter(log_type=AuditType.ACCESS) + return super().get_queryset().filter(log_type=AuditType.ACCESS).annotate( + group_key=Case( + When(metadata__auth_type='submission', + then=Concat( + Cast(Trunc('date_created', 'hour'), output_field=models.CharField()), + 'user_uid') + ), + default=Cast('id', output_field=models.CharField()) + ) + ) def create(self, **kwargs): # remove any attempt to set fields that should @@ -111,6 +127,7 @@ def create(self, **kwargs): if log_type is not None: logging.warning(f'Ignoring attempt to set {log_type=} on access log') user = kwargs.pop('user') + print(f'{user=}') return super().create( # set the fields that are always the same for access logs, # pass along the rest to the original constructor @@ -152,9 +169,10 @@ def create_from_request( request.resolver_match is not None and request.resolver_match.url_name == 'loginas-user-login' ) + print(f'{request.resolver_match.url_name}') is_submission = ( request.resolver_match is not None - and request.resolver_match.url_name == 'submissions' + and request.resolver_match.url_name in ['submissions','submissions-list'] and request.method == 'POST' ) # a regular login may have an anonymous user as _cached_user, ignore that @@ -165,6 +183,7 @@ def create_from_request( ) is_loginas = is_loginas_url and user_changed if is_submission: + print('Is submission') # Submissions are special snowflakes and need to be grouped together, no matter the auth type auth_type = ACCESS_LOG_SUBMISSION_AUTH_TYPE elif authentication_type and authentication_type != '': @@ -182,6 +201,7 @@ def create_from_request( else: # default: unknown auth_type = ACCESS_LOG_UNKNOWN_AUTH_TYPE + print(f'{auth_type=}') # gather information about the source of the request ip = get_client_ip(request) diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py index a0d755d6b1..22e783ac2b 100644 --- a/kobo/apps/audit_log/serializers.py +++ b/kobo/apps/audit_log/serializers.py @@ -1,6 +1,7 @@ from django.contrib.auth import get_user_model from rest_framework import serializers +from kpi.fields.username_hyperlinked import UsernameHyperlinkField from .models import AuditAction, AuditLog @@ -45,3 +46,19 @@ def get_date_created(self, audit_log): def get_username(self, audit_log): return audit_log.user.username + +class AccessLogSerializer(serializers.Serializer): + + user = UsernameHyperlinkField(source='user__username') + date_created = serializers.SerializerMethodField() + username = serializers.CharField(source='user__username') + metadata = serializers.JSONField() + user_uid = serializers.CharField() + count = serializers.SerializerMethodField() + + def get_date_created(self, audit_log): + return audit_log['date_created'].strftime('%Y-%m-%dT%H:%M:%SZ') + + def get_count(self, audit_log): + # subtract one so submission groups don't count themselves as additional submissions + return audit_log['count'] diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 4d2ab951e0..ba1a8931f9 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -1,3 +1,6 @@ +from django.db.models.functions import Coalesce, Trunc, Concat, Cast +from django.db.models import When, Count, Case, F, Value, Min +from django.db import models from rest_framework import mixins, viewsets from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer @@ -6,9 +9,12 @@ from .filters import AccessLogPermissionsFilter from .models import AccessLog, AuditAction, AuditLog from .permissions import SuperUserPermission -from .serializers import AuditLogSerializer +from .serializers import AuditLogSerializer, AccessLogSerializer +def get_submission_dict(): + return {'auth_type': 'submission-group'} + class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): """ Audit logs @@ -184,6 +190,30 @@ class AccessLogViewSet(AuditLogViewSet): """ - queryset = AccessLog.objects.select_related('user').order_by('-date_created') + queryset = ( + AccessLog.objects.select_related('user') + .filter(action=AuditAction.AUTH) + .values( + 'user__username', + 'object_id', + 'user_uid', + 'group_key' + ) + # if we have a submission group, use its metadata/date_created for all its submissions + # so they will be grouped correctly + # otherwise use the information from the log itself + .annotate( + count=Count('pk'), + metadata=Case( + When( + metadata__auth_type='submission', + then=Value(get_submission_dict(), models.JSONField()) + ), + default=F('metadata') + ), + date_created=Min('date_created') + ) + ) permission_classes = (IsAuthenticated,) filter_backends = (AccessLogPermissionsFilter,) + serializer_class = AccessLogSerializer diff --git a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py index 5ca73aaa58..cbe9fd99b3 100644 --- a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py +++ b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py @@ -7,10 +7,8 @@ from rest_framework import permissions from rest_framework import status from rest_framework import mixins -from rest_framework.authentication import ( - BasicAuthentication, - TokenAuthentication, - SessionAuthentication,) +from rest_framework.authentication import SessionAuthentication +from kpi.authentication import TokenAuthentication, BasicAuthentication from rest_framework.exceptions import NotAuthenticated from rest_framework.response import Response from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer diff --git a/kpi/fields/username_hyperlinked.py b/kpi/fields/username_hyperlinked.py new file mode 100644 index 0000000000..81f26fb34b --- /dev/null +++ b/kpi/fields/username_hyperlinked.py @@ -0,0 +1,15 @@ +from django.contrib.auth import get_user_model +from rest_framework import serializers + +class UsernameHyperlinkField(serializers.HyperlinkedRelatedField): + """ + Special hyperlinked field to handle when a query returns a dict rather than a User object + """ + + queryset = get_user_model().objects.all() + view_name = 'user-kpi-detail' + + def get_url(self, obj, view_name, request, format): + return self.reverse( + view_name, kwargs={'username': obj}, request=request, format=format + ) From db3ffe75160aac415c7764c27277dba6bfdcc9b0 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 17 Sep 2024 16:30:05 -0400 Subject: [PATCH 15/50] fixup!: rm old stuff --- kobo/apps/audit_log/serializers.py | 5 +---- kobo/apps/audit_log/views.py | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py index 22e783ac2b..9e8ba1a564 100644 --- a/kobo/apps/audit_log/serializers.py +++ b/kobo/apps/audit_log/serializers.py @@ -54,11 +54,8 @@ class AccessLogSerializer(serializers.Serializer): username = serializers.CharField(source='user__username') metadata = serializers.JSONField() user_uid = serializers.CharField() - count = serializers.SerializerMethodField() + count = serializers.IntegerField() def get_date_created(self, audit_log): return audit_log['date_created'].strftime('%Y-%m-%dT%H:%M:%SZ') - def get_count(self, audit_log): - # subtract one so submission groups don't count themselves as additional submissions - return audit_log['count'] diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index ba1a8931f9..06281233b6 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -199,9 +199,6 @@ class AccessLogViewSet(AuditLogViewSet): 'user_uid', 'group_key' ) - # if we have a submission group, use its metadata/date_created for all its submissions - # so they will be grouped correctly - # otherwise use the information from the log itself .annotate( count=Count('pk'), metadata=Case( From 626afc6b74b9d3bd03ea66d8dc82ccebe4047f08 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 17 Sep 2024 16:42:31 -0400 Subject: [PATCH 16/50] fixup!: endpoint documentation --- kobo/apps/audit_log/views.py | 47 +++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 06281233b6..d4dcd492f6 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -117,9 +117,8 @@ class AllAccessLogViewSet(AuditLogViewSet): > "app_label": "kobo_auth", > "model_name": "User", > "user": "http://localhost/api/v2/users/admin/", - > "user_uid": "uBMZxx9tVfepvTRp3e9Twj", + > "user_uid": "u12345", > "username": "admin", - > "action": "AUTH", > "metadata": { > "source": "Firefox (Ubuntu)", > "auth_type": "Digest", @@ -128,6 +127,15 @@ class AllAccessLogViewSet(AuditLogViewSet): > "date_created": "2024-08-19T16:48:58Z", > "log_type": "access" > }, + > { + > "user": "http://localhost/api/v2/users/admin/", + > "user_uid": "u12345", + > "username": "admin", + > "metadata": { + > "auth_type": "submission-group", + > }, + > "date_created": "2024-08-19T16:00:00Z" + > }, > ... > ] > } @@ -139,8 +147,26 @@ class AllAccessLogViewSet(AuditLogViewSet): queryset = ( AccessLog.objects.select_related('user') .filter(action=AuditAction.AUTH) - .order_by('-date_created') + .values( + 'user__username', + 'object_id', + 'user_uid', + 'group_key' + ) + .annotate( + count=Count('pk'), + metadata=Case( + When( + metadata__auth_type='submission', + then=Value(get_submission_dict(), models.JSONField()) + ), + default=F('metadata') + ), + date_created=Min('date_created') + ) ) + serializer_class = AccessLogSerializer + class AccessLogViewSet(AuditLogViewSet): @@ -149,6 +175,8 @@ class AccessLogViewSet(AuditLogViewSet): Lists all access logs for the authenticated user + Submissions will be grouped together by hour + 
     GET /api/v2/access-logs/
@@ -168,16 +196,23 @@ class AccessLogViewSet(AuditLogViewSet): > "app_label": "kobo_auth", > "model_name": "User", > "user": "http://localhost/api/v2/users/admin/", - > "user_uid": "uBMZxx9tVfepvTRp3e9Twj", + > "user_uid": "u12345", > "username": "admin", - > "action": "AUTH", > "metadata": { > "source": "Firefox (Ubuntu)", > "auth_type": "Digest", > "ip_address": "172.18.0.6" > }, > "date_created": "2024-08-19T16:48:58Z" - > "log_type": "access" + > }, + > { + > "user": "http://localhost/api/v2/users/admin/", + > "user_uid": "u12345", + > "username": "admin", + > "metadata": { + > "auth_type": "submission-group", + > }, + > "date_created": "2024-08-19T16:00:00Z" > }, > ... > ] From 7106519d8dd6ea54bbd723ca5a438b3d900d6078 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 18 Sep 2024 12:12:30 -0400 Subject: [PATCH 17/50] fixup!: i will refactor you and everything you love --- kobo/apps/audit_log/models.py | 79 ++++++++--- .../tests/api/v2/test_api_audit_log.py | 50 ++++++- kobo/apps/audit_log/tests/test_models.py | 128 +++++++++++++++++- kobo/apps/audit_log/views.py | 48 +------ kpi/constants.py | 1 + 5 files changed, 236 insertions(+), 70 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 21b6a296c6..969d5c0148 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -7,7 +7,7 @@ from django.db import models from django.utils import timezone from django.db.models.functions import Coalesce, Trunc, Concat, Cast -from django.db.models import When, Count, Case +from django.db.models import When, Count, Case, F, Value, Min from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.libs.utils.viewer_tools import ( @@ -17,7 +17,7 @@ from kpi.constants import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_SUBMISSION_AUTH_TYPE, - ACCESS_LOG_UNKNOWN_AUTH_TYPE, + ACCESS_LOG_UNKNOWN_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ) from kpi.fields.kpi_uid import UUID_LENGTH from kpi.utils.log import logging @@ -100,16 +100,7 @@ def save( class AccessLogManager(models.Manager): def get_queryset(self): - return super().get_queryset().filter(log_type=AuditType.ACCESS).annotate( - group_key=Case( - When(metadata__auth_type='submission', - then=Concat( - Cast(Trunc('date_created', 'hour'), output_field=models.CharField()), - 'user_uid') - ), - default=Cast('id', output_field=models.CharField()) - ) - ) + return super().get_queryset().filter(log_type=AuditType.ACCESS) def create(self, **kwargs): # remove any attempt to set fields that should @@ -127,7 +118,6 @@ def create(self, **kwargs): if log_type is not None: logging.warning(f'Ignoring attempt to set {log_type=} on access log') user = kwargs.pop('user') - print(f'{user=}') return super().create( # set the fields that are always the same for access logs, # pass along the rest to the original constructor @@ -141,6 +131,63 @@ def create(self, **kwargs): **kwargs, ) + def with_group_key(self): + """ + Adds a group key to every access log. Used for grouping submissions. + """ + # add a group key to every access log + return self.annotate( + group_key=Case( + # for submissions, the group key is hour created + user_uid + # this enables us to group submissions by user by hour + When( + metadata__auth_type=ACCESS_LOG_SUBMISSION_AUTH_TYPE, + then=Concat( + # get the time, rounded down to the hour, as a string + Cast( + Trunc('date_created', 'hour'), + output_field=models.CharField(), + ), + 'user_uid', + ), + ), + # for everything else, the group key is just the id since they won't be grouped + default=Cast('id', output_field=models.CharField()), + ) + ) + + def with_submissions_grouped(self): + """ + Returns minimal audit log representation with submissions grouped by user by hour + """ + return ( + self.with_group_key() + .select_related('user') + # adding 'group_key' in the values lets us group submissions + # for performance and clarity, ignore things like action and log_type, + # which are the same for all audit logs + .values('user__username', 'object_id', 'user_uid', 'group_key') + .annotate( + # include the number of submissions per group + # will be '1' for everything else + count=Count('pk'), + metadata=Case( + When( + # override the metadata for submission groups + metadata__auth_type=ACCESS_LOG_SUBMISSION_AUTH_TYPE, + then=Value( + {'auth_type': ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE}, + models.JSONField(), + ), + ), + # keep the metadata the same for everything else + default=F('metadata'), + ), + # for submission groups, use the earliest submission as the date_created + date_created=Min('date_created'), + ) + ) + class AccessLog(AuditLog): objects = AccessLogManager() @@ -169,10 +216,10 @@ def create_from_request( request.resolver_match is not None and request.resolver_match.url_name == 'loginas-user-login' ) - print(f'{request.resolver_match.url_name}') is_submission = ( request.resolver_match is not None - and request.resolver_match.url_name in ['submissions','submissions-list'] + and request.resolver_match.url_name + in ['submissions', 'submissions-list'] and request.method == 'POST' ) # a regular login may have an anonymous user as _cached_user, ignore that @@ -183,7 +230,6 @@ def create_from_request( ) is_loginas = is_loginas_url and user_changed if is_submission: - print('Is submission') # Submissions are special snowflakes and need to be grouped together, no matter the auth type auth_type = ACCESS_LOG_SUBMISSION_AUTH_TYPE elif authentication_type and authentication_type != '': @@ -201,7 +247,6 @@ def create_from_request( else: # default: unknown auth_type = ACCESS_LOG_UNKNOWN_AUTH_TYPE - print(f'{auth_type=}') # gather information about the source of the request ip = get_client_ip(request) diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 942f7b298a..247c8d8d88 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -1,11 +1,17 @@ from datetime import timedelta +from unittest.mock import patch, Mock from django.contrib.auth import get_user_model from django.utils import timezone from rest_framework import status from rest_framework.reverse import reverse -from kobo.apps.audit_log.models import AccessLog, AuditAction, AuditLog, AuditType +from kobo.apps.audit_log.models import ( + AccessLog, + AuditAction, + AuditLog, + AuditType, AccessLogManager, +) from kobo.apps.audit_log.serializers import AuditLogSerializer from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User @@ -158,6 +164,7 @@ def setUp(self): super().setUp() user1 = User.objects.get(username='someuser') user2 = User.objects.get(username='anotheruser') + # generate 3 access logs, 2 for user1, 1 for user2 AccessLog.objects.create(user=user1) AccessLog.objects.create(user=user1) @@ -182,12 +189,11 @@ def test_list_as_anonymous_returns_unauthorized(self): def test_show_user_access_logs_correctly_filters_to_user(self): user1 = User.objects.get(username='someuser') self.force_login_user(user1) + response = self.client.get(self.url) - # only return user1's access logs - self.assert_audit_log_results_equal( - response=response, - expected_kwargs={'action': AuditAction.AUTH, 'user': user1}, - ) + self.assertEqual(response.data['count'], 2) + for audit_log_dict in response.data['results']: + self.assertEqual(audit_log_dict['username'], 'someuser') def test_endpoint_ignores_querystring(self): # make sure a user can't get someone else's access logs @@ -195,9 +201,41 @@ def test_endpoint_ignores_querystring(self): self.force_login_user(user1) response = self.client.get(f'{self.url}?q=user__username:anotheruser') # check we still only got logs for the logged-in user + self.assertEqual(response.data['count'], 2) for audit_log_dict in response.data['results']: self.assertEquals(audit_log_dict['username'], 'someuser') + def test_endpoint_orders_results_by_date_desc(self): + today = timezone.now() + yesterday = today - timedelta(days=1) + admin = User.objects.get(username='admin') + self.force_login_user(admin) + AccessLog.objects.create(user=admin, date_created=today) + AccessLog.objects.create(user=admin, date_created=yesterday) + response = self.client.get(self.url) + first_result = response.data['results'][0] + second_result = response.data['results'][1] + self.assertEqual( + first_result['date_created'], today.strftime('%Y-%m-%dT%H:%M:%SZ') + ) + self.assertEqual( + second_result['date_created'], + yesterday.strftime('%Y-%m-%dT%H:%M:%SZ'), + ) + + def test_endpoint_queries_for_submission_groups(self): + # basic plumbing test to ensure we're using the right queryset + # the queryset itself is tested in test_models.py + admin = User.objects.get(username='admin') + self.force_login_user(admin) + mock_manager = Mock(spec=AccessLogManager) + mock_manager.with_submissions_grouped.return_value = AccessLog.objects.none() + with patch('kobo.apps.audit_log.views.AccessLog.objects', + return_value=mock_manager, + ) as patched_query: + response = self.client.get(self.url) + # paginated endpoints call the query twice, once for data, once for count + self.assertEqual(patched_query.call_count, 2) class AllApiAccessLogsTestCase(BaseAuditLogTestCase): diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index f60e1b31b9..31b6a12750 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -1,3 +1,4 @@ +import datetime from datetime import timedelta from unittest.mock import patch @@ -11,9 +12,10 @@ ACCESS_LOG_UNKNOWN_AUTH_TYPE, AccessLog, AuditAction, - AuditType, + AuditType, AuditLog, ) from kobo.apps.kobo_auth.shortcuts import User +from kpi.constants import ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE from kpi.tests.base_test_case import BaseTestCase @@ -189,3 +191,127 @@ def test_create_auth_log_with_extra_metadata( 'foo': 'bar', }, ) + +class AccessLogModelManagerTestCase(BaseTestCase): + fixtures = ['test_data'] + + def test_access_log_manager_only_gets_access_logs(self): + user = User.objects.get(username='someuser') + non_access_log = AuditLog.objects.create( + user=user, + log_type = AuditType.DATA_EDITING, + action = AuditAction.CREATE, + object_id = 12345, + ) + access_log_1 = AccessLog.objects.create( + user=user, + ) + access_log_2 = AccessLog.objects.create( + user=user + ) + all_access_logs_query = AccessLog.objects.all() + self.assertEqual(all_access_logs_query.count(), 2) + self.assertEqual(all_access_logs_query.first().id, access_log_1.id) + self.assertEqual(all_access_logs_query.last().id, access_log_2.id) + + def test_with_group_key_uses_id_for_non_submissions(self): + user = User.objects.get(username='someuser') + access_log = AccessLog.objects.create( + user=user, + metadata={'foo':'bar'} + ) + # the group key is calculated when fetching from the db + refetched = AccessLog.objects.with_group_key().get(pk=access_log.id) + self.assertEqual(refetched.group_key, str(refetched.id)) + + def test_with_group_key_uses_hour_plus_user_for_submissions(self): + user = User.objects.get(username='someuser') + jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + access_log = AccessLog.objects.create( + user=user, + metadata={'auth_type':ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created = jan_1_1_30_am + ) + # group key should be date truncated to the hour followed by user uid + expected_group_key = f'2024-01-01 01:00:00{user.extra_details.uid}' + refetched = AccessLog.objects.with_group_key().get(pk=access_log.id) + + self.assertEqual(refetched.group_key, expected_group_key) + + def test_with_submissions_grouped_preserves_non_submissions(self): + jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_45_am = datetime.datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') + user = User.objects.get(username='someuser') + log_1 = AccessLog.objects.create( + user=user, + metadata={'auth_type': 'Token', 'identify_me':'1'}, + date_created = jan_1_1_30_am + ) + log_2 = AccessLog.objects.create( + user=user, + metadata={'auth_type': 'Token', 'identify_me': '2'}, + date_created=jan_1_1_45_am + ) + # order by date created so we can use first() and last() + results = AccessLog.objects.with_submissions_grouped().order_by('date_created') + self.assertEqual(results.count(), 2) + + first_result = results.first() + self.assertDictEqual(first_result['metadata'], {'auth_type': 'Token', 'identify_me':'1'}) + self.assertEqual(first_result['date_created'], jan_1_1_30_am) + + second_result = results.last() + self.assertDictEqual(second_result['metadata'], {'auth_type': 'Token', 'identify_me':'2'}) + self.assertEqual(second_result['date_created'], jan_1_1_45_am) + + def test_with_submissions_grouped_groups_submissions(self): + jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_45_am = datetime.datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') + jan_1_2_15_am = datetime.datetime.fromisoformat('2024-01-01T02:15:25.123456+00:00') + + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + user_1_submission_1 = AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created = jan_1_1_30_am + ) + user1_submission_2 = AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created = jan_1_1_45_am + ) + user1_submission_3 = AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created = jan_1_2_15_am + ) + user2_submission_1 = AccessLog.objects.create( + user=user2, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created = jan_1_1_30_am + ) + + # order by date created so we can use first() and last() + results = AccessLog.objects.with_submissions_grouped().order_by('date_created') + # should get 3 submission groups, 2 for user1, and 1 for user2 + self.assertEqual(results.count(), 3) + + user_1_groups = results.filter(user__username='someuser') + self.assertEqual(user_1_groups.count(), 2) + # first group should have 1/1/2024 1:30am as the date created + user_1_group_1 = user_1_groups.first() + self.assertEqual(user_1_group_1['date_created'], jan_1_1_30_am) + self.assertEqual(user_1_group_1['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) + # second group should have 1/1/2024 2:15am as the date created + user_1_group_2 = user_1_groups.last() + self.assertEqual(user_1_group_2['date_created'], jan_1_2_15_am) + self.assertEqual(user_1_group_2['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) + + # should only + user_2_groups = results.filter(user__username='anotheruser') + self.assertEqual(user_2_groups.count(), 1) + user_2_group_1 = user_2_groups.first() + self.assertEqual(user_2_group_1['count'], 1) + self.assertEqual(user_2_group_1['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) + diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index d4dcd492f6..4b7b913d43 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -12,9 +12,6 @@ from .serializers import AuditLogSerializer, AccessLogSerializer -def get_submission_dict(): - return {'auth_type': 'submission-group'} - class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): """ Audit logs @@ -144,27 +141,7 @@ class AllAccessLogViewSet(AuditLogViewSet): """ - queryset = ( - AccessLog.objects.select_related('user') - .filter(action=AuditAction.AUTH) - .values( - 'user__username', - 'object_id', - 'user_uid', - 'group_key' - ) - .annotate( - count=Count('pk'), - metadata=Case( - When( - metadata__auth_type='submission', - then=Value(get_submission_dict(), models.JSONField()) - ), - default=F('metadata') - ), - date_created=Min('date_created') - ) - ) + queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') serializer_class = AccessLogSerializer @@ -224,28 +201,7 @@ class AccessLogViewSet(AuditLogViewSet): will return entries 100-149 """ - - queryset = ( - AccessLog.objects.select_related('user') - .filter(action=AuditAction.AUTH) - .values( - 'user__username', - 'object_id', - 'user_uid', - 'group_key' - ) - .annotate( - count=Count('pk'), - metadata=Case( - When( - metadata__auth_type='submission', - then=Value(get_submission_dict(), models.JSONField()) - ), - default=F('metadata') - ), - date_created=Min('date_created') - ) - ) + queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') permission_classes = (IsAuthenticated,) filter_backends = (AccessLogPermissionsFilter,) serializer_class = AccessLogSerializer diff --git a/kpi/constants.py b/kpi/constants.py index 663f9c6ab8..d4ed407070 100644 --- a/kpi/constants.py +++ b/kpi/constants.py @@ -139,4 +139,5 @@ ACCESS_LOG_LOGINAS_AUTH_TYPE = 'django-loginas' ACCESS_LOG_UNKNOWN_AUTH_TYPE = 'unknown' ACCESS_LOG_SUBMISSION_AUTH_TYPE = 'submission' +ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE = 'submission-group' ACCESS_LOG_AUTHORIZED_APP_TYPE = 'authorized-application' From bba4153205fd7b81bccf765b00774468ba03aebd Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 18 Sep 2024 14:23:02 -0400 Subject: [PATCH 18/50] fixup!: test --- .../tests/api/v2/test_api_audit_log.py | 319 +++++++++++++----- 1 file changed, 231 insertions(+), 88 deletions(-) diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 247c8d8d88..11792da028 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -1,4 +1,4 @@ -from datetime import timedelta +from datetime import datetime, timedelta from unittest.mock import patch, Mock from django.contrib.auth import get_user_model @@ -10,11 +10,16 @@ AccessLog, AuditAction, AuditLog, - AuditType, AccessLogManager, + AuditType, + AccessLogManager, ) from kobo.apps.audit_log.serializers import AuditLogSerializer from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User +from kpi.constants import ( + ACCESS_LOG_SUBMISSION_AUTH_TYPE, + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, +) from kpi.tests.base_test_case import BaseTestCase from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE @@ -41,18 +46,6 @@ def force_login_user(self, user): with skip_login_access_log(): self.client.force_login(user) - def assert_audit_log_results_equal(self, response, expected_kwargs): - # utility method for tests that are just comparing the results of an api call to the results of - # manually applying the expected query (simple filters only) - expected = AccessLog.objects.filter(**expected_kwargs).order_by('-date_created') - expected_count = expected.count() - serializer = AuditLogSerializer( - expected, many=True, context=response.renderer_context - ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data['count'], expected_count) - self.assertEqual(response.data['results'], serializer.data) - class ApiAuditLogTestCase(BaseAuditLogTestCase): @@ -160,27 +153,6 @@ class ApiAccessLogTestCase(BaseAuditLogTestCase): def get_endpoint_basename(self): return 'access-log-list' - def setUp(self): - super().setUp() - user1 = User.objects.get(username='someuser') - user2 = User.objects.get(username='anotheruser') - - # generate 3 access logs, 2 for user1, 1 for user2 - AccessLog.objects.create(user=user1) - AccessLog.objects.create(user=user1) - AccessLog.objects.create(user=user2) - - # create a random non-auth audit log - log = AuditLog.objects.create( - user=user1, - app_label='foo', - model_name='bar', - object_id=1, - action=AuditAction.DELETE, - ) - log.save() - self.assertEqual(AuditLog.objects.count(), 4) - def test_list_as_anonymous_returns_unauthorized(self): self.client.logout() response = self.client.get(self.url) @@ -188,30 +160,34 @@ def test_list_as_anonymous_returns_unauthorized(self): def test_show_user_access_logs_correctly_filters_to_user(self): user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + AccessLog.objects.create(user=user1) + AccessLog.objects.create(user=user2) self.force_login_user(user1) response = self.client.get(self.url) - self.assertEqual(response.data['count'], 2) - for audit_log_dict in response.data['results']: - self.assertEqual(audit_log_dict['username'], 'someuser') + self.assertEqual(response.data['count'], 1) + self.assertEqual(response.data['results'][0]['username'], 'someuser') def test_endpoint_ignores_querystring(self): # make sure a user can't get someone else's access logs user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + AccessLog.objects.create(user=user1) + AccessLog.objects.create(user=user2) self.force_login_user(user1) response = self.client.get(f'{self.url}?q=user__username:anotheruser') # check we still only got logs for the logged-in user - self.assertEqual(response.data['count'], 2) - for audit_log_dict in response.data['results']: - self.assertEquals(audit_log_dict['username'], 'someuser') + self.assertEqual(response.data['count'], 1) + self.assertEqual(response.data['results'][0]['username'], 'someuser') def test_endpoint_orders_results_by_date_desc(self): + user1 = User.objects.get(username='someuser') today = timezone.now() yesterday = today - timedelta(days=1) - admin = User.objects.get(username='admin') - self.force_login_user(admin) - AccessLog.objects.create(user=admin, date_created=today) - AccessLog.objects.create(user=admin, date_created=yesterday) + self.force_login_user(user1) + AccessLog.objects.create(user=user1, date_created=today) + AccessLog.objects.create(user=user1, date_created=yesterday) response = self.client.get(self.url) first_result = response.data['results'][0] second_result = response.data['results'][1] @@ -223,44 +199,43 @@ def test_endpoint_orders_results_by_date_desc(self): yesterday.strftime('%Y-%m-%dT%H:%M:%SZ'), ) - def test_endpoint_queries_for_submission_groups(self): - # basic plumbing test to ensure we're using the right queryset - # the queryset itself is tested in test_models.py - admin = User.objects.get(username='admin') - self.force_login_user(admin) - mock_manager = Mock(spec=AccessLogManager) - mock_manager.with_submissions_grouped.return_value = AccessLog.objects.none() - with patch('kobo.apps.audit_log.views.AccessLog.objects', - return_value=mock_manager, - ) as patched_query: - response = self.client.get(self.url) - # paginated endpoints call the query twice, once for data, once for count - self.assertEqual(patched_query.call_count, 2) + def test_endpoint_groups_submissions(self): + # the logic of grouping submissions is tested more thoroughly in test_models, + # this is just to ensure that we're using the grouping query + user = User.objects.get(username='someuser') + self.force_login_user(user) + jan_1_1_30_am = datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) + AccessLog.objects.create( + user=user, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_30_am, + ) + AccessLog.objects.create( + user=user, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_45_am, + ) + response = self.client.get(self.url) + # should return 1 submission group with 2 submissions + self.assertEqual(response.data['count'], 1) + result = response.data['results'][0] + self.assertEqual( + result['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) + self.assertEqual(result['count'], 2) + class AllApiAccessLogsTestCase(BaseAuditLogTestCase): def get_endpoint_basename(self): return 'all-access-logs-list' - def setUp(self): - super().setUp() - super_user = User.objects.get(username='admin') - user2 = User.objects.get(username='anotheruser') - # generate 3 access logs, 2 for superuser, 1 for user2 - AccessLog.objects.create(user=super_user) - AccessLog.objects.create(user=super_user) - AccessLog.objects.create(user=user2) - - # create a random non-auth audit log - AuditLog.objects.create( - user=User.objects.get(username='someuser'), - app_label='foo', - model_name='bar', - object_id=1, - action=AuditAction.DELETE, - ) - self.assertEqual(AuditLog.objects.count(), 4) - def test_list_as_anonymous_returns_unauthorized(self): self.client.logout() response = self.client.get(self.url) @@ -274,32 +249,200 @@ def test_regular_user_access_returns_forbidden(self): def test_show_all_access_logs_succeeds_for_superuser(self): self.force_login_user(User.objects.get(username='admin')) response = self.client.get(self.url) - self.assert_audit_log_results_equal( - response=response, expected_kwargs={'action': AuditAction.AUTH} + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_show_all_access_logs_includes_all_users(self): + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + admin = User.objects.get(username='admin') + AccessLog.objects.create(user=user1) + AccessLog.objects.create(user=user2) + self.force_login_user(admin) + response = self.client.get(self.url) + self.assertEqual(response.data['count'], 2) + self.assertEqual(response.data['results'][0]['username'], 'anotheruser') + self.assertEqual(response.data['results'][1]['username'], 'someuser') + + def test_endpoint_groups_submissions(self): + # the logic of grouping submissions is tested more thoroughly in test_models, + # this is just to ensure that we're using the grouping query + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + admin = User.objects.get(username='admin') + + self.force_login_user(admin) + jan_1_1_30_am = datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) + jan_1_1_50_am = datetime.fromisoformat( + '2024-01-01T01:50:25.123456+00:00' + ) + + # create 2 submissions for user1 + AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_30_am, + ) + AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_45_am, ) + # 2 submissions for user2, after the ones for user1 so we know the expected + # order of the results + AccessLog.objects.create( + user=user2, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_45_am, + ) + AccessLog.objects.create( + user=user2, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_50_am, + ) + response = self.client.get(self.url) + # should return 2 submission group with 2 submissions each + self.assertEqual(response.data['count'], 2) + + # user2's submission group should be first + group1 = response.data['results'][0] + self.assertEqual( + group1['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) + self.assertEqual(group1['username'], 'anotheruser') + self.assertEqual(group1['count'], 2) + + #user1's group + group2 = response.data['results'][1] + self.assertEqual( + group2['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) + self.assertEqual(group2['username'], 'someuser') + self.assertEqual(group2['count'], 2) + def test_can_search_access_logs_by_username(self): + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + admin = User.objects.get(username='admin') + AccessLog.objects.create(user=user1) + AccessLog.objects.create(user=user2) self.force_login_user(User.objects.get(username='admin')) response = self.client.get(f'{self.url}?q=user__username:anotheruser') - another_user = User.objects.get(username='anotheruser') - self.assert_audit_log_results_equal( - response=response, - expected_kwargs={'action': AuditAction.AUTH, 'user': another_user}, + + # only return logs from user1 + for audit_log_dict in response.data['results']: + self.assertEqual(audit_log_dict['username'], 'anotheruser') + self.assertEqual(response.data['count'], 1) + + def test_can_search_access_logs_by_username_including_submission_groups( + self, + ): + user1 = User.objects.get(username='someuser') + user2 = User.objects.get(username='anotheruser') + admin = User.objects.get(username='admin') + self.force_login_user(admin) + + # create two submissions that will be grouped together + # these are the only two logs for user admin + jan_1_1_30_am = datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) + AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_30_am, + ) + AccessLog.objects.create( + user=user1, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_45_am, + ) + + # create an extra log for user2 so we know it gets filtered out + AccessLog.objects.create(user=user2) + + response = self.client.get(f'{self.url}?q=user__username:someuser') + self.assertEqual(response.data['count'], 1) + result = response.data['results'][0] + self.assertEqual(result['username'], 'someuser') + self.assertEqual( + result['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ) def test_can_search_access_logs_by_date(self): - another_user = User.objects.get(username='anotheruser') + user = User.objects.get(username='someuser') with skip_login_access_log(): self.client.force_login(User.objects.get(username='admin')) tomorrow = timezone.now() + timedelta(days=1) tomorrow_str = tomorrow.strftime('%Y-%m-%d') - log = AccessLog.objects.create(user=another_user, date_created=tomorrow) + # create one log from today and one from tomorrow + AccessLog.objects.create(user=user) + AccessLog.objects.create( + user=user, + date_created=tomorrow, + ) response = self.client.get( f'{self.url}?q=date_created__gte:"{tomorrow_str}"' ) - serializer = AuditLogSerializer( - [log], many=True, context=response.renderer_context + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # should only return the log from tomorrow + self.assertEqual(response.data['count'], 1) + result = response.data['results'][0] + self.assertEqual( + result['date_created'], tomorrow.strftime('%Y-%m-%dT%H:%M:%SZ') + ) + + def test_can_search_access_logs_by_date_including_submission_groups(self): + user = User.objects.get(username='someuser') + with skip_login_access_log(): + self.client.force_login(User.objects.get(username='admin')) + tomorrow = timezone.now() + timedelta(days=1) + two_days_from_now = tomorrow + timedelta(days=1) + tomorrow_str = tomorrow.strftime('%Y-%m-%d') + + # create one log from today + AccessLog.objects.create(user=user) + + # create 2 new submissions with a forced date_created of tomorrow + AccessLog.objects.create( + user=user, + date_created=two_days_from_now, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + ) + AccessLog.objects.create( + user=user, + date_created=two_days_from_now, + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + ) + + # search for logs created after tomorrow + response = self.client.get( + f'{self.url}?q=date_created__gte:"{tomorrow_str}"' ) self.assertEqual(response.status_code, status.HTTP_200_OK) + + # should only return the submission group self.assertEqual(response.data['count'], 1) - self.assertEqual(response.data['results'], serializer.data) + group = response.data['results'][0] + self.assertEqual( + group['date_created'], + two_days_from_now.strftime('%Y-%m-%dT%H:%M:%SZ'), + ) + self.assertEqual( + group['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) + From 5387901f10c2a196cba12028761c6b87a83b1509 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 18 Sep 2024 14:24:56 -0400 Subject: [PATCH 19/50] fixup!: format everything --- kobo/apps/audit_log/models.py | 11 ++- kobo/apps/audit_log/serializers.py | 2 +- .../tests/api/v2/test_api_audit_log.py | 7 +- kobo/apps/audit_log/tests/test_models.py | 97 ++++++++++++------- kobo/apps/audit_log/views.py | 16 +-- 5 files changed, 85 insertions(+), 48 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 969d5c0148..fc4799b8df 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -5,9 +5,9 @@ from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.models import ContentType from django.db import models +from django.db.models import Case, Count, F, Min, Value, When +from django.db.models.functions import Cast, Coalesce, Concat, Trunc from django.utils import timezone -from django.db.models.functions import Coalesce, Trunc, Concat, Cast -from django.db.models import When, Count, Case, F, Value, Min from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.libs.utils.viewer_tools import ( @@ -17,7 +17,8 @@ from kpi.constants import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_SUBMISSION_AUTH_TYPE, - ACCESS_LOG_UNKNOWN_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ACCESS_LOG_UNKNOWN_AUTH_TYPE, ) from kpi.fields.kpi_uid import UUID_LENGTH from kpi.utils.log import logging @@ -176,7 +177,9 @@ def with_submissions_grouped(self): # override the metadata for submission groups metadata__auth_type=ACCESS_LOG_SUBMISSION_AUTH_TYPE, then=Value( - {'auth_type': ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE}, + { + 'auth_type': ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE + }, models.JSONField(), ), ), diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py index 9e8ba1a564..9378ef5763 100644 --- a/kobo/apps/audit_log/serializers.py +++ b/kobo/apps/audit_log/serializers.py @@ -47,6 +47,7 @@ def get_date_created(self, audit_log): def get_username(self, audit_log): return audit_log.user.username + class AccessLogSerializer(serializers.Serializer): user = UsernameHyperlinkField(source='user__username') @@ -58,4 +59,3 @@ class AccessLogSerializer(serializers.Serializer): def get_date_created(self, audit_log): return audit_log['date_created'].strftime('%Y-%m-%dT%H:%M:%SZ') - diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 11792da028..41297bf8a7 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -1,5 +1,5 @@ from datetime import datetime, timedelta -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch from django.contrib.auth import get_user_model from django.utils import timezone @@ -8,10 +8,10 @@ from kobo.apps.audit_log.models import ( AccessLog, + AccessLogManager, AuditAction, AuditLog, AuditType, - AccessLogManager, ) from kobo.apps.audit_log.serializers import AuditLogSerializer from kobo.apps.audit_log.tests.test_signals import skip_login_access_log @@ -318,7 +318,7 @@ def test_endpoint_groups_submissions(self): self.assertEqual(group1['username'], 'anotheruser') self.assertEqual(group1['count'], 2) - #user1's group + # user1's group group2 = response.data['results'][1] self.assertEqual( group2['metadata']['auth_type'], @@ -445,4 +445,3 @@ def test_can_search_access_logs_by_date_including_submission_groups(self): group['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ) - diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 31b6a12750..444e48414a 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -12,10 +12,14 @@ ACCESS_LOG_UNKNOWN_AUTH_TYPE, AccessLog, AuditAction, - AuditType, AuditLog, + AuditLog, + AuditType, ) from kobo.apps.kobo_auth.shortcuts import User -from kpi.constants import ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE +from kpi.constants import ( + ACCESS_LOG_SUBMISSION_AUTH_TYPE, + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, +) from kpi.tests.base_test_case import BaseTestCase @@ -192,6 +196,7 @@ def test_create_auth_log_with_extra_metadata( }, ) + class AccessLogModelManagerTestCase(BaseTestCase): fixtures = ['test_data'] @@ -199,16 +204,14 @@ def test_access_log_manager_only_gets_access_logs(self): user = User.objects.get(username='someuser') non_access_log = AuditLog.objects.create( user=user, - log_type = AuditType.DATA_EDITING, - action = AuditAction.CREATE, - object_id = 12345, + log_type=AuditType.DATA_EDITING, + action=AuditAction.CREATE, + object_id=12345, ) access_log_1 = AccessLog.objects.create( user=user, ) - access_log_2 = AccessLog.objects.create( - user=user - ) + access_log_2 = AccessLog.objects.create(user=user) all_access_logs_query = AccessLog.objects.all() self.assertEqual(all_access_logs_query.count(), 2) self.assertEqual(all_access_logs_query.first().id, access_log_1.id) @@ -217,8 +220,7 @@ def test_access_log_manager_only_gets_access_logs(self): def test_with_group_key_uses_id_for_non_submissions(self): user = User.objects.get(username='someuser') access_log = AccessLog.objects.create( - user=user, - metadata={'foo':'bar'} + user=user, metadata={'foo': 'bar'} ) # the group key is calculated when fetching from the db refetched = AccessLog.objects.with_group_key().get(pk=access_log.id) @@ -226,11 +228,13 @@ def test_with_group_key_uses_id_for_non_submissions(self): def test_with_group_key_uses_hour_plus_user_for_submissions(self): user = User.objects.get(username='someuser') - jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_30_am = datetime.datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) access_log = AccessLog.objects.create( user=user, - metadata={'auth_type':ACCESS_LOG_SUBMISSION_AUTH_TYPE}, - date_created = jan_1_1_30_am + metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, + date_created=jan_1_1_30_am, ) # group key should be date truncated to the hour followed by user uid expected_group_key = f'2024-01-01 01:00:00{user.extra_details.uid}' @@ -239,61 +243,80 @@ def test_with_group_key_uses_hour_plus_user_for_submissions(self): self.assertEqual(refetched.group_key, expected_group_key) def test_with_submissions_grouped_preserves_non_submissions(self): - jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') - jan_1_1_45_am = datetime.datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') + jan_1_1_30_am = datetime.datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) user = User.objects.get(username='someuser') log_1 = AccessLog.objects.create( user=user, - metadata={'auth_type': 'Token', 'identify_me':'1'}, - date_created = jan_1_1_30_am + metadata={'auth_type': 'Token', 'identify_me': '1'}, + date_created=jan_1_1_30_am, ) log_2 = AccessLog.objects.create( user=user, metadata={'auth_type': 'Token', 'identify_me': '2'}, - date_created=jan_1_1_45_am + date_created=jan_1_1_45_am, ) # order by date created so we can use first() and last() - results = AccessLog.objects.with_submissions_grouped().order_by('date_created') + results = AccessLog.objects.with_submissions_grouped().order_by( + 'date_created' + ) self.assertEqual(results.count(), 2) first_result = results.first() - self.assertDictEqual(first_result['metadata'], {'auth_type': 'Token', 'identify_me':'1'}) + self.assertDictEqual( + first_result['metadata'], {'auth_type': 'Token', 'identify_me': '1'} + ) self.assertEqual(first_result['date_created'], jan_1_1_30_am) second_result = results.last() - self.assertDictEqual(second_result['metadata'], {'auth_type': 'Token', 'identify_me':'2'}) + self.assertDictEqual( + second_result['metadata'], + {'auth_type': 'Token', 'identify_me': '2'}, + ) self.assertEqual(second_result['date_created'], jan_1_1_45_am) def test_with_submissions_grouped_groups_submissions(self): - jan_1_1_30_am = datetime.datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') - jan_1_1_45_am = datetime.datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') - jan_1_2_15_am = datetime.datetime.fromisoformat('2024-01-01T02:15:25.123456+00:00') + jan_1_1_30_am = datetime.datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) + jan_1_2_15_am = datetime.datetime.fromisoformat( + '2024-01-01T02:15:25.123456+00:00' + ) user1 = User.objects.get(username='someuser') user2 = User.objects.get(username='anotheruser') user_1_submission_1 = AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, - date_created = jan_1_1_30_am + date_created=jan_1_1_30_am, ) user1_submission_2 = AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, - date_created = jan_1_1_45_am + date_created=jan_1_1_45_am, ) user1_submission_3 = AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, - date_created = jan_1_2_15_am + date_created=jan_1_2_15_am, ) user2_submission_1 = AccessLog.objects.create( user=user2, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, - date_created = jan_1_1_30_am + date_created=jan_1_1_30_am, ) # order by date created so we can use first() and last() - results = AccessLog.objects.with_submissions_grouped().order_by('date_created') + results = AccessLog.objects.with_submissions_grouped().order_by( + 'date_created' + ) # should get 3 submission groups, 2 for user1, and 1 for user2 self.assertEqual(results.count(), 3) @@ -302,16 +325,24 @@ def test_with_submissions_grouped_groups_submissions(self): # first group should have 1/1/2024 1:30am as the date created user_1_group_1 = user_1_groups.first() self.assertEqual(user_1_group_1['date_created'], jan_1_1_30_am) - self.assertEqual(user_1_group_1['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) + self.assertEqual( + user_1_group_1['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) # second group should have 1/1/2024 2:15am as the date created user_1_group_2 = user_1_groups.last() self.assertEqual(user_1_group_2['date_created'], jan_1_2_15_am) - self.assertEqual(user_1_group_2['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) + self.assertEqual( + user_1_group_2['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) # should only user_2_groups = results.filter(user__username='anotheruser') self.assertEqual(user_2_groups.count(), 1) user_2_group_1 = user_2_groups.first() self.assertEqual(user_2_group_1['count'], 1) - self.assertEqual(user_2_group_1['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE) - + self.assertEqual( + user_2_group_1['metadata']['auth_type'], + ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + ) diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 4b7b913d43..85361f7811 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -1,6 +1,6 @@ -from django.db.models.functions import Coalesce, Trunc, Concat, Cast -from django.db.models import When, Count, Case, F, Value, Min from django.db import models +from django.db.models import Case, Count, F, Min, Value, When +from django.db.models.functions import Cast, Coalesce, Concat, Trunc from rest_framework import mixins, viewsets from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer @@ -9,7 +9,7 @@ from .filters import AccessLogPermissionsFilter from .models import AccessLog, AuditAction, AuditLog from .permissions import SuperUserPermission -from .serializers import AuditLogSerializer, AccessLogSerializer +from .serializers import AccessLogSerializer, AuditLogSerializer class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): @@ -141,11 +141,12 @@ class AllAccessLogViewSet(AuditLogViewSet): """ - queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') + queryset = AccessLog.objects.with_submissions_grouped().order_by( + '-date_created' + ) serializer_class = AccessLogSerializer - class AccessLogViewSet(AuditLogViewSet): """ Access logs @@ -201,7 +202,10 @@ class AccessLogViewSet(AuditLogViewSet): will return entries 100-149 """ - queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') + + queryset = AccessLog.objects.with_submissions_grouped().order_by( + '-date_created' + ) permission_classes = (IsAuthenticated,) filter_backends = (AccessLogPermissionsFilter,) serializer_class = AccessLogSerializer From 86128a52d349cf69bf42785c4c3256d664598352 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 18 Sep 2024 14:27:23 -0400 Subject: [PATCH 20/50] fixup!: rm unused imports --- kobo/apps/audit_log/models.py | 1 - kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index fc4799b8df..f3cb4d8f37 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,5 +1,4 @@ import logging -from importlib.metadata import metadata from django.conf import settings from django.contrib.auth.models import AnonymousUser diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 41297bf8a7..61f049e3b6 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -1,5 +1,4 @@ from datetime import datetime, timedelta -from unittest.mock import Mock, patch from django.contrib.auth import get_user_model from django.utils import timezone @@ -8,12 +7,10 @@ from kobo.apps.audit_log.models import ( AccessLog, - AccessLogManager, AuditAction, AuditLog, AuditType, ) -from kobo.apps.audit_log.serializers import AuditLogSerializer from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( From 8483c46cc35bff44422553a659187cc1e2000152 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 18 Sep 2024 14:46:19 -0400 Subject: [PATCH 21/50] fixup!: oops sub tests --- .../audit_log/tests/test_one_time_auth.py | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index 205709c2e3..447d8c0db5 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -126,9 +126,40 @@ def side_effect(request): self.assertTrue(log_exists) self.assertEqual(AuditLog.objects.count(), 1) - def test_digest_auth_for_submission(self): + @data( + 'kpi.authentication.DRFTokenAuthentication.authenticate', + 'kpi.authentication.DRFBasicAuthentication.authenticate', + 'kpi.authentication.OPOAuth2Authentication.authenticate', + 'kobo.apps.openrosa.libs.authentication.BasicAuthentication.authenticate', + ) + def test_any_auth_for_submissions(self, authetication_method): + """ + Test any normal one-time authenticated submission results in a submission access log + """ + + with patch( + authetication_method, + return_value=(TestOneTimeAuthentication.user, 'something'), + ): + # assume the submission works, we don't actually care + with patch( + 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', + return_value=HttpResponse(status=200), + ): + # try both v1 and v2 + self.client.post(reverse('submissions')) + self.client.post(reverse('submissions-list')) + log_exists = AuditLog.objects.filter( + user_uid=TestOneTimeAuthentication.user.extra_details.uid, + action=AuditAction.AUTH, + metadata__auth_type='submission', + ).exists() + self.assertTrue(log_exists) + self.assertEqual(AuditLog.objects.count(), 2) + + def test_digest_auth_for_submissions(self): """ - Test digest authentications for submissions result in an audit log being created with the 'Submission' type + Test digest-authenticated submissions result in a submission access log """ def side_effect(request): @@ -146,14 +177,17 @@ def side_effect(request): 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', return_value=HttpResponse(status=200), ): + # check v1 and v2 self.client.post(reverse('submissions'), **header) + self.client.post(reverse('submissions-list'), **header) + log_exists = AuditLog.objects.filter( user_uid=TestOneTimeAuthentication.user.extra_details.uid, action=AuditAction.AUTH, metadata__auth_type='submission', ).exists() self.assertTrue(log_exists) - self.assertEqual(AuditLog.objects.count(), 1) + self.assertEqual(AuditLog.objects.count(), 2) def test_authorized_application_auth_creates_log(self): app: AuthorizedApplication = AuthorizedApplication(name='Auth app') From a7e2fd6c504dc9a7b805f798928036aac4d4f7af Mon Sep 17 00:00:00 2001 From: rgraber Date: Thu, 19 Sep 2024 10:15:16 -0400 Subject: [PATCH 22/50] fixup!: run formatter --- kobo/apps/audit_log/models.py | 11 ++---- kobo/apps/audit_log/serializers.py | 2 +- .../tests/api/v2/test_api_audit_log.py | 39 +++++-------------- kobo/apps/audit_log/tests/test_models.py | 12 ++---- .../audit_log/tests/test_one_time_auth.py | 4 +- kobo/apps/audit_log/views.py | 13 ++----- .../apps/api/viewsets/xform_submission_api.py | 21 +++++----- kpi/fields/username_hyperlinked.py | 1 + 8 files changed, 33 insertions(+), 70 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index f3cb4d8f37..f43dc499bd 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,11 +1,9 @@ import logging from django.conf import settings -from django.contrib.auth.models import AnonymousUser -from django.contrib.contenttypes.models import ContentType from django.db import models from django.db.models import Case, Count, F, Min, Value, When -from django.db.models.functions import Cast, Coalesce, Concat, Trunc +from django.db.models.functions import Cast, Concat, Trunc from django.utils import timezone from kobo.apps.kobo_auth.shortcuts import User @@ -176,9 +174,7 @@ def with_submissions_grouped(self): # override the metadata for submission groups metadata__auth_type=ACCESS_LOG_SUBMISSION_AUTH_TYPE, then=Value( - { - 'auth_type': ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE - }, + {'auth_type': ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE}, models.JSONField(), ), ), @@ -220,8 +216,7 @@ def create_from_request( ) is_submission = ( request.resolver_match is not None - and request.resolver_match.url_name - in ['submissions', 'submissions-list'] + and request.resolver_match.url_name in ['submissions', 'submissions-list'] and request.method == 'POST' ) # a regular login may have an anonymous user as _cached_user, ignore that diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py index 9378ef5763..78940fd34f 100644 --- a/kobo/apps/audit_log/serializers.py +++ b/kobo/apps/audit_log/serializers.py @@ -2,7 +2,7 @@ from rest_framework import serializers from kpi.fields.username_hyperlinked import UsernameHyperlinkField -from .models import AuditAction, AuditLog +from .models import AuditLog class AuditLogSerializer(serializers.ModelSerializer): diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 61f049e3b6..62ea9ee9e0 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -5,12 +5,7 @@ from rest_framework import status from rest_framework.reverse import reverse -from kobo.apps.audit_log.models import ( - AccessLog, - AuditAction, - AuditLog, - AuditType, -) +from kobo.apps.audit_log.models import AccessLog, AuditAction, AuditLog, AuditType from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( @@ -201,12 +196,8 @@ def test_endpoint_groups_submissions(self): # this is just to ensure that we're using the grouping query user = User.objects.get(username='someuser') self.force_login_user(user) - jan_1_1_30_am = datetime.fromisoformat( - '2024-01-01T01:30:25.123456+00:00' - ) - jan_1_1_45_am = datetime.fromisoformat( - '2024-01-01T01:45:25.123456+00:00' - ) + jan_1_1_30_am = datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_45_am = datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') AccessLog.objects.create( user=user, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, @@ -268,15 +259,9 @@ def test_endpoint_groups_submissions(self): admin = User.objects.get(username='admin') self.force_login_user(admin) - jan_1_1_30_am = datetime.fromisoformat( - '2024-01-01T01:30:25.123456+00:00' - ) - jan_1_1_45_am = datetime.fromisoformat( - '2024-01-01T01:45:25.123456+00:00' - ) - jan_1_1_50_am = datetime.fromisoformat( - '2024-01-01T01:50:25.123456+00:00' - ) + jan_1_1_30_am = datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_45_am = datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') + jan_1_1_50_am = datetime.fromisoformat('2024-01-01T01:50:25.123456+00:00') # create 2 submissions for user1 AccessLog.objects.create( @@ -348,12 +333,8 @@ def test_can_search_access_logs_by_username_including_submission_groups( # create two submissions that will be grouped together # these are the only two logs for user admin - jan_1_1_30_am = datetime.fromisoformat( - '2024-01-01T01:30:25.123456+00:00' - ) - jan_1_1_45_am = datetime.fromisoformat( - '2024-01-01T01:45:25.123456+00:00' - ) + jan_1_1_30_am = datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') + jan_1_1_45_am = datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, @@ -426,9 +407,7 @@ def test_can_search_access_logs_by_date_including_submission_groups(self): ) # search for logs created after tomorrow - response = self.client.get( - f'{self.url}?q=date_created__gte:"{tomorrow_str}"' - ) + response = self.client.get(f'{self.url}?q=date_created__gte:"{tomorrow_str}"') self.assertEqual(response.status_code, status.HTTP_200_OK) # should only return the submission group diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 444e48414a..f1225b330d 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -219,9 +219,7 @@ def test_access_log_manager_only_gets_access_logs(self): def test_with_group_key_uses_id_for_non_submissions(self): user = User.objects.get(username='someuser') - access_log = AccessLog.objects.create( - user=user, metadata={'foo': 'bar'} - ) + access_log = AccessLog.objects.create(user=user, metadata={'foo': 'bar'}) # the group key is calculated when fetching from the db refetched = AccessLog.objects.with_group_key().get(pk=access_log.id) self.assertEqual(refetched.group_key, str(refetched.id)) @@ -261,9 +259,7 @@ def test_with_submissions_grouped_preserves_non_submissions(self): date_created=jan_1_1_45_am, ) # order by date created so we can use first() and last() - results = AccessLog.objects.with_submissions_grouped().order_by( - 'date_created' - ) + results = AccessLog.objects.with_submissions_grouped().order_by('date_created') self.assertEqual(results.count(), 2) first_result = results.first() @@ -314,9 +310,7 @@ def test_with_submissions_grouped_groups_submissions(self): ) # order by date created so we can use first() and last() - results = AccessLog.objects.with_submissions_grouped().order_by( - 'date_created' - ) + results = AccessLog.objects.with_submissions_grouped().order_by('date_created') # should get 3 submission groups, 2 for user1, and 1 for user2 self.assertEqual(results.count(), 3) diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index 447d8c0db5..3068b90681 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -1,9 +1,9 @@ -from unittest import TestCase, mock +from unittest import mock from unittest.mock import patch from ddt import data, ddt, unpack from django.http import HttpResponse -from django.urls import resolve, reverse +from django.urls import reverse from rest_framework.authtoken.models import Token from trench.utils import get_mfa_model diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 85361f7811..4472df94a3 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -1,13 +1,10 @@ -from django.db import models -from django.db.models import Case, Count, F, Min, Value, When -from django.db.models.functions import Cast, Coalesce, Concat, Trunc from rest_framework import mixins, viewsets from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer from kpi.filters import SearchFilter from kpi.permissions import IsAuthenticated from .filters import AccessLogPermissionsFilter -from .models import AccessLog, AuditAction, AuditLog +from .models import AccessLog, AuditLog from .permissions import SuperUserPermission from .serializers import AccessLogSerializer, AuditLogSerializer @@ -141,9 +138,7 @@ class AllAccessLogViewSet(AuditLogViewSet): """ - queryset = AccessLog.objects.with_submissions_grouped().order_by( - '-date_created' - ) + queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') serializer_class = AccessLogSerializer @@ -203,9 +198,7 @@ class AccessLogViewSet(AuditLogViewSet): """ - queryset = AccessLog.objects.with_submissions_grouped().order_by( - '-date_created' - ) + queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') permission_classes = (IsAuthenticated,) filter_backends = (AccessLogPermissionsFilter,) serializer_class = AccessLogSerializer diff --git a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py index cbe9fd99b3..1e8f52cab7 100644 --- a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py +++ b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py @@ -1,17 +1,14 @@ # coding: utf-8 -import re import io +import re from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as t -from rest_framework import permissions -from rest_framework import status -from rest_framework import mixins +from rest_framework import mixins, permissions, status from rest_framework.authentication import SessionAuthentication -from kpi.authentication import TokenAuthentication, BasicAuthentication from rest_framework.exceptions import NotAuthenticated -from rest_framework.response import Response from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer +from rest_framework.response import Response from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.logger.models import Instance @@ -20,12 +17,16 @@ from kobo.apps.openrosa.libs.renderers.renderers import TemplateXMLRenderer from kobo.apps.openrosa.libs.serializers.data_serializer import SubmissionSerializer from kobo.apps.openrosa.libs.utils.logger_tools import ( + UnauthenticatedEditAttempt, dict2xform, safe_create_instance, - UnauthenticatedEditAttempt, ) from kobo.apps.openrosa.libs.utils.string import dict_lists2strings -from kpi.authentication import DigestAuthentication +from kpi.authentication import ( + BasicAuthentication, + DigestAuthentication, + TokenAuthentication, +) from kpi.utils.object_permission import get_database_user from ..utils.rest_framework.viewsets import OpenRosaGenericViewSet @@ -51,7 +52,7 @@ def create_instance_from_json(username, request): if submission is None: # return an error - return [t("No submission key provided."), None] + return [t('No submission key provided.'), None] # convert lists in submission dict to joined strings submission_joined = dict_lists2strings(submission) @@ -194,7 +195,7 @@ def create(self, request, *args, **kwargs): def error_response(self, error, is_json_request, request): if not error: - error_msg = t("Unable to create submission.") + error_msg = t('Unable to create submission.') status_code = status.HTTP_400_BAD_REQUEST elif isinstance(error, str): error_msg = error diff --git a/kpi/fields/username_hyperlinked.py b/kpi/fields/username_hyperlinked.py index 81f26fb34b..60b4cf47bb 100644 --- a/kpi/fields/username_hyperlinked.py +++ b/kpi/fields/username_hyperlinked.py @@ -1,6 +1,7 @@ from django.contrib.auth import get_user_model from rest_framework import serializers + class UsernameHyperlinkField(serializers.HyperlinkedRelatedField): """ Special hyperlinked field to handle when a query returns a dict rather than a User object From 42eadd0fefba5b33492ae95f328e33f5f401f0fb Mon Sep 17 00:00:00 2001 From: rgraber Date: Thu, 19 Sep 2024 10:18:55 -0400 Subject: [PATCH 23/50] fixup!: rm accidental import --- kobo/apps/audit_log/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index f43dc499bd..af7a5bb731 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -1,5 +1,3 @@ -import logging - from django.conf import settings from django.db import models from django.db.models import Case, Count, F, Min, Value, When From d7b3bfaabced2a45023671245f777ac546742a84 Mon Sep 17 00:00:00 2001 From: rgraber Date: Thu, 19 Sep 2024 10:50:56 -0400 Subject: [PATCH 24/50] fixup!: more formatting --- kobo/apps/audit_log/models.py | 5 +++-- .../tests/api/v2/test_api_audit_log.py | 2 +- kobo/apps/audit_log/tests/test_models.py | 20 +++++++++++-------- .../audit_log/tests/test_one_time_auth.py | 9 +++++---- kpi/fields/username_hyperlinked.py | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index af7a5bb731..4fd3908849 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -147,14 +147,15 @@ def with_group_key(self): 'user_uid', ), ), - # for everything else, the group key is just the id since they won't be grouped + # for everything else, the group key is just the id + # since they won't be grouped default=Cast('id', output_field=models.CharField()), ) ) def with_submissions_grouped(self): """ - Returns minimal audit log representation with submissions grouped by user by hour + Returns minimal representation with submissions grouped by user by hour """ return ( self.with_group_key() diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 62ea9ee9e0..8bf23dedec 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -315,7 +315,7 @@ def test_can_search_access_logs_by_username(self): admin = User.objects.get(username='admin') AccessLog.objects.create(user=user1) AccessLog.objects.create(user=user2) - self.force_login_user(User.objects.get(username='admin')) + self.force_login_user(admin) response = self.client.get(f'{self.url}?q=user__username:anotheruser') # only return logs from user1 diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index f1225b330d..a0af285c81 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -202,7 +202,8 @@ class AccessLogModelManagerTestCase(BaseTestCase): def test_access_log_manager_only_gets_access_logs(self): user = User.objects.get(username='someuser') - non_access_log = AuditLog.objects.create( + # non-access log + AuditLog.objects.create( user=user, log_type=AuditType.DATA_EDITING, action=AuditAction.CREATE, @@ -248,12 +249,12 @@ def test_with_submissions_grouped_preserves_non_submissions(self): '2024-01-01T01:45:25.123456+00:00' ) user = User.objects.get(username='someuser') - log_1 = AccessLog.objects.create( + AccessLog.objects.create( user=user, metadata={'auth_type': 'Token', 'identify_me': '1'}, date_created=jan_1_1_30_am, ) - log_2 = AccessLog.objects.create( + AccessLog.objects.create( user=user, metadata={'auth_type': 'Token', 'identify_me': '2'}, date_created=jan_1_1_45_am, @@ -288,22 +289,25 @@ def test_with_submissions_grouped_groups_submissions(self): user1 = User.objects.get(username='someuser') user2 = User.objects.get(username='anotheruser') - user_1_submission_1 = AccessLog.objects.create( + # two submissions for user1 between 1-2am + AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, date_created=jan_1_1_30_am, ) - user1_submission_2 = AccessLog.objects.create( + AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, date_created=jan_1_1_45_am, ) - user1_submission_3 = AccessLog.objects.create( + # one submission for user1 after 2am + AccessLog.objects.create( user=user1, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, date_created=jan_1_2_15_am, ) - user2_submission_1 = AccessLog.objects.create( + # one submission for user2 between 1-2am + AccessLog.objects.create( user=user2, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, date_created=jan_1_1_30_am, @@ -331,7 +335,7 @@ def test_with_submissions_grouped_groups_submissions(self): ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ) - # should only + # one group for user2 user_2_groups = results.filter(user__username='anotheruser') self.assertEqual(user_2_groups.count(), 1) user_2_group_1 = user_2_groups.first() diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index 3068b90681..a4bb289dc2 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -107,7 +107,8 @@ def test_digest_auth_creates_logs(self): """ def side_effect(request): - # Digest authentication sets request.user, so include that as a side effect when necessary + # Digest authentication sets request.user, + # so include that as a side effect when necessary request.user = TestOneTimeAuthentication.user return mock.DEFAULT @@ -134,7 +135,7 @@ def side_effect(request): ) def test_any_auth_for_submissions(self, authetication_method): """ - Test any normal one-time authenticated submission results in a submission access log + Test most one-time authenticated submissions result in a submission access log """ with patch( @@ -143,7 +144,7 @@ def test_any_auth_for_submissions(self, authetication_method): ): # assume the submission works, we don't actually care with patch( - 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', + 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', # noqa return_value=HttpResponse(status=200), ): # try both v1 and v2 @@ -174,7 +175,7 @@ def side_effect(request): ): # assume the submission works, we don't actually care with patch( - 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', + 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', # noqa return_value=HttpResponse(status=200), ): # check v1 and v2 diff --git a/kpi/fields/username_hyperlinked.py b/kpi/fields/username_hyperlinked.py index 60b4cf47bb..3dd73b4097 100644 --- a/kpi/fields/username_hyperlinked.py +++ b/kpi/fields/username_hyperlinked.py @@ -4,7 +4,7 @@ class UsernameHyperlinkField(serializers.HyperlinkedRelatedField): """ - Special hyperlinked field to handle when a query returns a dict rather than a User object + Special hyperlinked field to handle a dict rather than a User object """ queryset = get_user_model().objects.all() From 435ee420e02c6c700641a7646736e371daa78c5b Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 19 Sep 2024 16:20:53 -0400 Subject: [PATCH 25/50] fixup!: rm unneeded import --- kpi/urls/router_api_v2.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kpi/urls/router_api_v2.py b/kpi/urls/router_api_v2.py index 033fc1566c..7c7dbd2575 100644 --- a/kpi/urls/router_api_v2.py +++ b/kpi/urls/router_api_v2.py @@ -21,7 +21,6 @@ from kpi.views.v2.data import DataViewSet from kpi.views.v2.export_task import ExportTaskViewSet from kpi.views.v2.import_task import ImportTaskViewSet -from kpi.views.v2.logout import logout_from_all_devices from kpi.views.v2.paired_data import PairedDataViewset from kpi.views.v2.permission import PermissionViewSet from kpi.views.v2.service_usage import ServiceUsageViewSet From e95d916f26037e0ca1b1784b4bd5d40b6315333f Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 26 Sep 2024 08:45:02 -0400 Subject: [PATCH 26/50] fixup!: doc --- kobo/apps/audit_log/views.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 4472df94a3..15406f14a5 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -91,6 +91,8 @@ class AllAccessLogViewSet(AuditLogViewSet): Access logs Lists all access logs for all users. Only available to superusers. + + Submissions will be grouped together by hour 
     GET /api/v2/access-logs/all
@@ -108,8 +110,6 @@ class AllAccessLogViewSet(AuditLogViewSet):
     >           "previous": null,
     >           "results": [
     >                {
-    >                   "app_label": "kobo_auth",
-    >                    "model_name": "User",
     >                    "user": "http://localhost/api/v2/users/admin/",
     >                    "user_uid": "u12345",
     >                    "username": "admin",
@@ -119,7 +119,6 @@ class AllAccessLogViewSet(AuditLogViewSet):
     >                        "ip_address": "172.18.0.6"
     >                   },
     >                    "date_created": "2024-08-19T16:48:58Z",
-    >                    "log_type": "access"
     >                },
     >                {
     >                    "user": "http://localhost/api/v2/users/admin/",
@@ -166,8 +165,6 @@ class AccessLogViewSet(AuditLogViewSet):
     >           "previous": null,
     >           "results": [
     >                {
-    >                   "app_label": "kobo_auth",
-    >                    "model_name": "User",
     >                    "user": "http://localhost/api/v2/users/admin/",
     >                    "user_uid": "u12345",
     >                    "username": "admin",

From aab475c09efbe2404edac4abc7c0a0b93f395cc1 Mon Sep 17 00:00:00 2001
From: Rebecca Graber 
Date: Thu, 26 Sep 2024 09:43:05 -0400
Subject: [PATCH 27/50] fixup!: darker

---
 kobo/apps/audit_log/views.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py
index 15406f14a5..c3128df7a7 100644
--- a/kobo/apps/audit_log/views.py
+++ b/kobo/apps/audit_log/views.py
@@ -91,7 +91,7 @@ class AllAccessLogViewSet(AuditLogViewSet):
     Access logs
 
     Lists all access logs for all users. Only available to superusers.
-    
+
     Submissions will be grouped together by hour
 
     

From e88f974dc70d74a52333e72e9b30ef24f7fb3e60 Mon Sep 17 00:00:00 2001
From: James Kiger 
Date: Thu, 26 Sep 2024 13:52:14 -0400
Subject: [PATCH 28/50] Adjust logout-all endpoint in frontend constants.

---
 jsapp/js/api.endpoints.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts
index 1e3778b1c6..7588e3d076 100644
--- a/jsapp/js/api.endpoints.ts
+++ b/jsapp/js/api.endpoints.ts
@@ -11,5 +11,5 @@ export const endpoints = {
   /** Expected parameters: price_id and subscription_id **/
   CHANGE_PLAN_URL: '/api/v2/stripe/change-plan',
   ACCESS_LOGS_URL: '/api/v2/access-logs/',
-  LOGOUT_ALL: '/api/v2/logout-all',
+  LOGOUT_ALL: '/logout-all/',
 };

From f40395ae9e176d217fe4fd4c5ff11c80139ec6fb Mon Sep 17 00:00:00 2001
From: rgraber 
Date: Fri, 27 Sep 2024 09:36:56 -0400
Subject: [PATCH 29/50] fixup!: changes from review

---
 kobo/apps/audit_log/serializers.py               | 11 ++++++++---
 .../relative_prefix_hyperlinked_related.py       | 15 +++++++++++++++
 kpi/fields/username_hyperlinked.py               | 16 ----------------
 3 files changed, 23 insertions(+), 19 deletions(-)
 delete mode 100644 kpi/fields/username_hyperlinked.py

diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py
index 78940fd34f..55a02a4e76 100644
--- a/kobo/apps/audit_log/serializers.py
+++ b/kobo/apps/audit_log/serializers.py
@@ -1,7 +1,7 @@
 from django.contrib.auth import get_user_model
 from rest_framework import serializers
 
-from kpi.fields.username_hyperlinked import UsernameHyperlinkField
+from kpi.fields import RelativePrefixHyperlinkedRelatedField
 from .models import AuditLog
 
 
@@ -49,8 +49,13 @@ def get_username(self, audit_log):
 
 
 class AccessLogSerializer(serializers.Serializer):
-
-    user = UsernameHyperlinkField(source='user__username')
+    user = RelativePrefixHyperlinkedRelatedField(
+        view_name='user-kpi-detail',
+        lookup_field='user__username',
+        lookup_url_kwarg='username',
+        read_only=True,
+        source='user__username',
+    )
     date_created = serializers.SerializerMethodField()
     username = serializers.CharField(source='user__username')
     metadata = serializers.JSONField()
diff --git a/kpi/fields/relative_prefix_hyperlinked_related.py b/kpi/fields/relative_prefix_hyperlinked_related.py
index 85f9501855..02ee240f60 100644
--- a/kpi/fields/relative_prefix_hyperlinked_related.py
+++ b/kpi/fields/relative_prefix_hyperlinked_related.py
@@ -7,6 +7,21 @@
 
 class RelativePrefixHyperlinkedRelatedField(HyperlinkedRelatedField):
 
+    def get_url(self, obj, view_name, request, format):
+        # mostly copied from HyperLinkedRelatedField
+        if hasattr(obj, 'pk') and obj.pk in (None, ''):
+            return None
+
+        # special logic: if obj is a string, just use the value
+        # this allows us to pass dictionaries to the serializer
+        # as well as model instances
+        if isinstance(obj, str):
+            lookup_value = obj
+        else:
+            lookup_value = getattr(obj, self.lookup_field)
+        kwargs = {self.lookup_url_kwarg: lookup_value}
+        return self.reverse(view_name, kwargs=kwargs, request=request, format=format)
+
     def to_internal_value(self, data):
         try:
             http_prefix = data.startswith(('http:', 'https:'))
diff --git a/kpi/fields/username_hyperlinked.py b/kpi/fields/username_hyperlinked.py
deleted file mode 100644
index 3dd73b4097..0000000000
--- a/kpi/fields/username_hyperlinked.py
+++ /dev/null
@@ -1,16 +0,0 @@
-from django.contrib.auth import get_user_model
-from rest_framework import serializers
-
-
-class UsernameHyperlinkField(serializers.HyperlinkedRelatedField):
-    """
-    Special hyperlinked field to handle a dict rather than a User object
-    """
-
-    queryset = get_user_model().objects.all()
-    view_name = 'user-kpi-detail'
-
-    def get_url(self, obj, view_name, request, format):
-        return self.reverse(
-            view_name, kwargs={'username': obj}, request=request, format=format
-        )

From 2330438f86c6a853506fddbf6abb0a28842210d7 Mon Sep 17 00:00:00 2001
From: rgraber 
Date: Fri, 27 Sep 2024 10:25:24 -0400
Subject: [PATCH 30/50] feat: switch access log endpoints

---
 kobo/apps/audit_log/urls.py  | 4 ++--
 kobo/apps/audit_log/views.py | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kobo/apps/audit_log/urls.py b/kobo/apps/audit_log/urls.py
index 7820bca243..be877f8fdc 100644
--- a/kobo/apps/audit_log/urls.py
+++ b/kobo/apps/audit_log/urls.py
@@ -5,9 +5,9 @@
 
 router = DefaultRouter()
 router.register(r'audit-logs', AuditLogViewSet, basename='audit-log')
-router.register(r'access-logs', AccessLogViewSet, basename='access-log')
+router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-log')
 router.register(
-    r'access-logs/all', AllAccessLogViewSet, basename='all-access-logs'
+    r'access-logs/user', AccessLogViewSet, basename='access-logs'
 )
 
 urlpatterns = []
diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py
index 4d2ab951e0..1981246416 100644
--- a/kobo/apps/audit_log/views.py
+++ b/kobo/apps/audit_log/views.py
@@ -93,12 +93,12 @@ class AllAccessLogViewSet(AuditLogViewSet):
     Lists all access logs for all users. Only available to superusers.
 
     
-    GET /api/v2/access-logs/all
+    GET /api/v2/access-logs/
     

 
     > Example
     >
-    >       curl -X GET https://[kpi-url]/access-logs/all
+    >       curl -X GET https://[kpi-url]/access-logs/
 
     > Response 200
 
@@ -144,12 +144,12 @@ class AccessLogViewSet(AuditLogViewSet):
     Lists all access logs for the authenticated user
 
     
-    GET /api/v2/access-logs/
+    GET /api/v2/access-logs/user
     

 
     > Example
     >
-    >       curl -X GET https://[kpi-url]/access-logs/
+    >       curl -X GET https://[kpi-url]/access-logs/user
 
     > Response 200
 

From 623d1fb6463b5b247d9783533c9b4f4659fae72e Mon Sep 17 00:00:00 2001
From: Leszek 
Date: Mon, 30 Sep 2024 14:39:55 +0200
Subject: [PATCH 31/50] code review fixes

---
 .../security/email/emailSection.component.tsx |  4 +-
 .../security/email/emailSection.module.scss   |  7 +-
 .../security/mfa/mfaSection.component.tsx     | 92 ++++++++++---------
 .../security/mfa/mfaSection.module.scss       |  3 +-
 4 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/jsapp/js/account/security/email/emailSection.component.tsx b/jsapp/js/account/security/email/emailSection.component.tsx
index 70b3a12e0b..e350affae5 100644
--- a/jsapp/js/account/security/email/emailSection.component.tsx
+++ b/jsapp/js/account/security/email/emailSection.component.tsx
@@ -63,7 +63,7 @@ export default function EmailSection() {
           newEmail: '',
         });
       });
-    });
+    }, () => {/* Avoid crashing app when 500 error happens */});
   }
 
   function deleteNewUserEmail() {
@@ -156,7 +156,7 @@ export default function EmailSection() {
-
+
-

- {t( - 'Two-factor authentication (2FA) verifies your identity using an authenticator application in addition to your usual password. ' + - 'We recommend enabling two-factor authentication for an additional layer of protection.' - )} -

- - {this.state.isMfaActive && this.state.isMfaAvailable && ( -
-
-

- {t('Authenticator app')} -

- - {this.state.dateModified && ( -
- {formatTime(this.state.dateModified)} -
- )} - -
- -
-

- {t('Recovery codes')} -

- -
+ +
+

+ {t('Recovery codes')} +

+ +
-
- )} + )} +
{!this.state.isMfaActive && this.state.isMfaAvailable && this.state.dateDisabled && ( Date: Mon, 30 Sep 2024 08:54:25 -0400 Subject: [PATCH 32/50] fixup!: change indiv name --- kobo/apps/audit_log/urls.py | 2 +- kobo/apps/audit_log/views.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kobo/apps/audit_log/urls.py b/kobo/apps/audit_log/urls.py index be877f8fdc..d2ad8f2044 100644 --- a/kobo/apps/audit_log/urls.py +++ b/kobo/apps/audit_log/urls.py @@ -7,7 +7,7 @@ router.register(r'audit-logs', AuditLogViewSet, basename='audit-log') router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-log') router.register( - r'access-logs/user', AccessLogViewSet, basename='access-logs' + r'access-logs/me', AccessLogViewSet, basename='access-logs' ) urlpatterns = [] diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 1981246416..5a81b54765 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -144,12 +144,12 @@ class AccessLogViewSet(AuditLogViewSet): Lists all access logs for the authenticated user 
-    GET /api/v2/access-logs/user
+    GET /api/v2/access-logs/me
> Example > - > curl -X GET https://[kpi-url]/access-logs/user + > curl -X GET https://[kpi-url]/access-logs/me > Response 200 From 8e1bdedd362300bfe478a8ec4323cb6d1f373eba Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 30 Sep 2024 08:59:29 -0400 Subject: [PATCH 33/50] fixup!: oops s --- kobo/apps/audit_log/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/urls.py b/kobo/apps/audit_log/urls.py index d2ad8f2044..5ed044a994 100644 --- a/kobo/apps/audit_log/urls.py +++ b/kobo/apps/audit_log/urls.py @@ -5,7 +5,7 @@ router = DefaultRouter() router.register(r'audit-logs', AuditLogViewSet, basename='audit-log') -router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-log') +router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-logs') router.register( r'access-logs/me', AccessLogViewSet, basename='access-logs' ) From 91b83409568f358a5908d974ffe0f2c73cd3f5e0 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 30 Sep 2024 09:00:06 -0400 Subject: [PATCH 34/50] fixup!: oops another s --- kobo/apps/audit_log/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/urls.py b/kobo/apps/audit_log/urls.py index 5ed044a994..fac5f9ad15 100644 --- a/kobo/apps/audit_log/urls.py +++ b/kobo/apps/audit_log/urls.py @@ -7,7 +7,7 @@ router.register(r'audit-logs', AuditLogViewSet, basename='audit-log') router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-logs') router.register( - r'access-logs/me', AccessLogViewSet, basename='access-logs' + r'access-logs/me', AccessLogViewSet, basename='access-log' ) urlpatterns = [] From b23ff712f5225edaee8334965a48f5b2c9dac65c Mon Sep 17 00:00:00 2001 From: Leszek Date: Tue, 1 Oct 2024 12:54:16 +0200 Subject: [PATCH 35/50] update endpoint --- jsapp/js/api.endpoints.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts index 7588e3d076..5a4c6e899d 100644 --- a/jsapp/js/api.endpoints.ts +++ b/jsapp/js/api.endpoints.ts @@ -10,6 +10,6 @@ export const endpoints = { PORTAL_URL: '/api/v2/stripe/customer-portal', /** Expected parameters: price_id and subscription_id **/ CHANGE_PLAN_URL: '/api/v2/stripe/change-plan', - ACCESS_LOGS_URL: '/api/v2/access-logs/', + ACCESS_LOGS_URL: '/api/v2/access-logs/me', LOGOUT_ALL: '/logout-all/', }; From d9b0eacd4306bd8e95306a2486e81ad7e2bde230 Mon Sep 17 00:00:00 2001 From: Leszek Date: Wed, 2 Oct 2024 11:07:25 +0200 Subject: [PATCH 36/50] DiffValue component for displaying two values --- .../components/common/diffValue.component.tsx | 28 +++++++++++++++++++ .../components/common/diffValue.module.scss | 21 ++++++++++++++ .../components/common/diffValue.stories.tsx | 15 ++++++++++ 3 files changed, 64 insertions(+) create mode 100644 jsapp/js/components/common/diffValue.component.tsx create mode 100644 jsapp/js/components/common/diffValue.module.scss create mode 100644 jsapp/js/components/common/diffValue.stories.tsx diff --git a/jsapp/js/components/common/diffValue.component.tsx b/jsapp/js/components/common/diffValue.component.tsx new file mode 100644 index 0000000000..584adadb57 --- /dev/null +++ b/jsapp/js/components/common/diffValue.component.tsx @@ -0,0 +1,28 @@ +import React from 'react'; +import cx from 'classnames'; +import styles from './diffValue.module.scss'; + +interface DiffValueProps { + before: React.ReactNode; + after: React.ReactNode; + isInline?: boolean; +} + +/** + * This component displays a diff of two pieces of text. One (`before`) is red + * and has a strikethrough, and the other (`after`) is green. + * + * TODO (some ideas for future): + * - Allow passing just the `before`, so that this could be used more + * organically + * - Introduce some more complex way of showing the partial differences between + * two pieces of text + */ +export default function DiffValue(props: DiffValueProps) { + return ( + + {props.before} + {props.after} + + ); +} diff --git a/jsapp/js/components/common/diffValue.module.scss b/jsapp/js/components/common/diffValue.module.scss new file mode 100644 index 0000000000..591f7f8cfb --- /dev/null +++ b/jsapp/js/components/common/diffValue.module.scss @@ -0,0 +1,21 @@ +@use 'scss/colors'; + +.root { + display: inline; +} + +.before { + color: colors.$kobo-red !important; + text-decoration: line-through; +} + +.after { + color: colors.$kobo-dark-green !important; +} + +.root:not(.isInline) { + .before, + .after { + display: block; + } +} diff --git a/jsapp/js/components/common/diffValue.stories.tsx b/jsapp/js/components/common/diffValue.stories.tsx new file mode 100644 index 0000000000..78dd6ab82c --- /dev/null +++ b/jsapp/js/components/common/diffValue.stories.tsx @@ -0,0 +1,15 @@ +import React from 'react'; +import type {StoryObj} from '@storybook/react'; +import DiffValue from 'js/components/common/diffValue.component'; + +export default { + title: 'Common/Diff Value', + component: DiffValue, +}; + +export const Primary: StoryObj = { + args: { + before: 'The document movie is about the weird production of Fitzcarraldo bu Werner Herzog made in 1982.', + after: 'The film is a making-of documentary about the chaotic production of Werner Herzog\'s 1982 film Fitzcarraldo.', + }, +}; From a39f2b818d281a42bf729233eafba5e3f501719d Mon Sep 17 00:00:00 2001 From: rgraber Date: Thu, 26 Sep 2024 14:23:17 -0400 Subject: [PATCH 37/50] feat: create project history logs on new deployment --- .../migrations/0011_projecthistorylog.py | 23 +++ kobo/apps/audit_log/models.py | 99 +++++++++-- kobo/apps/audit_log/tests/test_models.py | 161 +++++++++++++++--- kpi/constants.py | 2 + kpi/serializers/v2/deployment.py | 1 + kpi/tests/api/v2/test_api_assets.py | 55 +++++- kpi/views/v2/asset.py | 20 +++ 7 files changed, 326 insertions(+), 35 deletions(-) create mode 100644 kobo/apps/audit_log/migrations/0011_projecthistorylog.py diff --git a/kobo/apps/audit_log/migrations/0011_projecthistorylog.py b/kobo/apps/audit_log/migrations/0011_projecthistorylog.py new file mode 100644 index 0000000000..a643e5aa18 --- /dev/null +++ b/kobo/apps/audit_log/migrations/0011_projecthistorylog.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.15 on 2024-09-26 16:46 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('audit_log', '0010_accesslog'), + ] + + operations = [ + migrations.CreateModel( + name='ProjectHistoryLog', + fields=[], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('audit_log.auditlog',), + ), + ] diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 81e1555a25..2c0df2169d 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -11,8 +11,11 @@ ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_UNKNOWN_AUTH_TYPE, + ASSET_TYPE_SURVEY, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) +from kpi.exceptions import BadAssetTypeException from kpi.fields.kpi_uid import UUID_LENGTH +from kpi.models import Asset from kpi.utils.log import logging @@ -24,6 +27,10 @@ class AuditAction(models.TextChoices): REMOVE = 'remove' UPDATE = 'update' AUTH = 'auth' + DEPLOY = 'deploy' + ARCHIVE = 'archive' + UNARCHIVE = 'unarchive' + REDEPLOY = 'redeploy' class AuditType(models.TextChoices): @@ -91,25 +98,32 @@ def save( ) -class AccessLogManager(models.Manager): +class IgnoreCommonFieldsMixin: + def remove_common_fields_and_warn(self, log_type, create_kwargs): + # remove any attempt to set fields that should + # always be the same on an particular log type + app_label = create_kwargs.pop('app_label', None) + if app_label is not None: + logging.warning(f'Ignoring attempt to set {app_label=} on {log_type} log') + model_name = create_kwargs.pop('model_name', None) + if model_name is not None: + logging.warning(f'Ignoring attempt to set {model_name=} on {log_type} log') + log_type = create_kwargs.pop('log_type', None) + if log_type is not None: + logging.warning(f'Ignoring attempt to set {log_type=} on {log_type} log') + + +class AccessLogManager(models.Manager, IgnoreCommonFieldsMixin): def get_queryset(self): return super().get_queryset().filter(log_type=AuditType.ACCESS) def create(self, **kwargs): # remove any attempt to set fields that should # always be the same on an access log - app_label = kwargs.pop('app_label', None) - if app_label is not None: - logging.warning(f'Ignoring attempt to set {app_label=} on access log') - model_name = kwargs.pop('model_name', None) - if model_name is not None: - logging.warning(f'Ignoring attempt to set {model_name=} on access log') + self.remove_common_fields_and_warn('access', kwargs) action = kwargs.pop('action', None) if action is not None: logging.warning(f'Ignoring attempt to set {action=} on access log') - log_type = kwargs.pop('log_type', None) - if log_type is not None: - logging.warning(f'Ignoring attempt to set {log_type=} on access log') user = kwargs.pop('user') return super().create( # set the fields that are always the same for access logs, @@ -199,3 +213,68 @@ def create_from_request( if extra_metadata is not None: metadata.update(extra_metadata) return AccessLog.objects.create(user=logged_in_user, metadata=metadata) + + +class ProjectHistoryLogManager(models.Manager, IgnoreCommonFieldsMixin): + def get_queryset(self): + return super().get_queryset().filter(log_type=AuditType.PROJECT_HISTORY) + + def create(self, **kwargs): + # remove any attempt to set fields that should + # always be the same on a project history log + self.remove_common_fields_and_warn('project history', kwargs) + user = kwargs.pop('user') + asset = kwargs.pop('asset') + if not asset.asset_type or asset.asset_type != ASSET_TYPE_SURVEY: + raise BadAssetTypeException( + 'Cannot create project history log for non-survey asset' + ) + return super().create( + # set the fields that are always the same for all project history logs, + # along with the ones derived from the user and asset + app_label=Asset._meta.app_label, + model_name=Asset._meta.model_name, + log_type=AuditType.PROJECT_HISTORY, + user=user, + user_uid=user.extra_details.uid, + object_id=asset.id, + **kwargs, + ) + + +class ProjectHistoryLog(AuditLog): + objects = ProjectHistoryLogManager() + + class Meta: + proxy = True + + @staticmethod + def create_from_deployment_request( + request, asset, first_deployment=False, only_active_changed=False + ): + ip = get_client_ip(request) + source = get_human_readable_client_user_agent(request) + if only_active_changed: + action = ( + AuditAction.UNARCHIVE + if asset.deployment.active + else AuditAction.ARCHIVE + ) + else: + action = AuditAction.DEPLOY if first_deployment else AuditAction.REDEPLOY + metadata = { + 'ip_address': ip, + 'source': source, + 'asset_uid': asset.uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + } + if action in [AuditAction.REDEPLOY, AuditAction.DEPLOY]: + version = asset.latest_deployed_version + metadata['version_uid'] = version.uid + user = request.user + return ProjectHistoryLog.objects.create( + user=user, + action=action, + metadata=metadata, + asset=asset, + ) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index f60e1b31b9..745e8f0053 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -1,10 +1,12 @@ from datetime import timedelta from unittest.mock import patch +from ddt import data, ddt, unpack from django.contrib.auth.models import AnonymousUser from django.test.client import RequestFactory from django.urls import resolve, reverse from django.utils import timezone +from model_bakery import baker from kobo.apps.audit_log.models import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, @@ -12,16 +14,15 @@ AccessLog, AuditAction, AuditType, + ProjectHistoryLog, ) from kobo.apps.kobo_auth.shortcuts import User +from kpi.constants import ASSET_TYPE_QUESTION, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE +from kpi.exceptions import BadAssetTypeException +from kpi.models import Asset from kpi.tests.base_test_case import BaseTestCase -@patch( - 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', - return_value='source', -) -@patch('kobo.apps.audit_log.models.get_client_ip', return_value='127.0.0.1') class AccessLogModelTestCase(BaseTestCase): @classmethod @@ -34,6 +35,19 @@ def setUpClass(cls): cls.super_user.backend = 'django.contrib.auth.backends.ModelBackend' cls.super_user.save() + def setUp(self): + source_patcher = patch( + 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', + return_value='source', + ) + ip_patcher = patch( + 'kobo.apps.audit_log.models.get_client_ip', return_value='127.0.0.1' + ) + source_patcher.start() + ip_patcher.start() + self.addCleanup(source_patcher.stop) + self.addCleanup(ip_patcher.stop) + def _create_request(self, url: str, cached_user, new_user): factory = RequestFactory() request = factory.post(url) @@ -51,7 +65,7 @@ def _check_common_fields(self, access_log: AccessLog, user): self.assertEqual(access_log.action, AuditAction.AUTH) self.assertEqual(access_log.log_type, AuditType.ACCESS) - def test_create_access_log_sets_standard_fields(self, patched_ip, patched_source): + def test_create_access_log_sets_standard_fields(self): yesterday = timezone.now() - timedelta(days=1) log = AccessLog.objects.create( user=AccessLogModelTestCase.super_user, @@ -64,7 +78,7 @@ def test_create_access_log_sets_standard_fields(self, patched_ip, patched_source @patch('kobo.apps.audit_log.models.logging.warning') def test_create_access_log_ignores_attempt_to_override_standard_fields( - self, patched_warning, patched_ip, patched_source + self, patched_warning ): log = AccessLog.objects.create( log_type=AuditType.DATA_EDITING, @@ -78,9 +92,7 @@ def test_create_access_log_ignores_attempt_to_override_standard_fields( # we logged a warning for each attempt to override a field self.assertEquals(patched_warning.call_count, 4) - def test_basic_create_auth_log_from_request( - self, patched_ip, patched_source - ): + def test_basic_create_auth_log_from_request(self): request = self._create_request( reverse('kobo_login'), AnonymousUser(), @@ -97,9 +109,7 @@ def test_basic_create_auth_log_from_request( }, ) - def test_create_auth_log_from_loginas_request( - self, patched_ip, patched_source - ): + def test_create_auth_log_from_loginas_request(self): second_user = User.objects.create_user( 'second_user', 'second@example.com', 'pass' ) @@ -122,9 +132,7 @@ def test_create_auth_log_from_loginas_request( }, ) - def test_create_auth_log_with_different_auth_type( - self, patched_ip, patched_source - ): + def test_create_auth_log_with_different_auth_type(self): request = self._create_request( reverse('api_v2:asset-list'), AnonymousUser(), @@ -143,9 +151,7 @@ def test_create_auth_log_with_different_auth_type( }, ) - def test_create_auth_log_unknown_authenticator( - self, patched_ip, patched_source - ): + def test_create_auth_log_unknown_authenticator(self): # no backend attached to the user object second_user = User.objects.create_user( 'second_user', 'second@example.com', 'pass' @@ -167,9 +173,7 @@ def test_create_auth_log_unknown_authenticator( }, ) - def test_create_auth_log_with_extra_metadata( - self, patched_ip, patched_source - ): + def test_create_auth_log_with_extra_metadata(self): request = self._create_request( reverse('api_v2:asset-list'), AnonymousUser(), @@ -189,3 +193,116 @@ def test_create_auth_log_with_extra_metadata( 'foo': 'bar', }, ) + + +@ddt +class ProjectHistoryLogModelTestCase(BaseTestCase): + + fixtures = ['test_data'] + + def setUp(self): + source_patcher = patch( + 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', + return_value='source', + ) + ip_patcher = patch( + 'kobo.apps.audit_log.models.get_client_ip', return_value='1.2.3.4' + ) + source_patcher.start() + ip_patcher.start() + self.addCleanup(source_patcher.stop) + self.addCleanup(ip_patcher.stop) + + def _check_common_fields(self, log: ProjectHistoryLog, user, asset): + self.assertEqual(log.user.id, user.id) + self.assertEqual(log.app_label, 'kpi') + self.assertEqual(log.model_name, 'asset') + self.assertEqual(log.object_id, asset.id) + self.assertEqual(log.user_uid, user.extra_details.uid) + self.assertEqual(log.log_type, AuditType.PROJECT_HISTORY) + + def test_create_project_history_log_sets_standard_fields(self): + user = User.objects.get(username='someuser') + asset = Asset.objects.get(pk=1) + yesterday = timezone.now() - timedelta(days=1) + log = ProjectHistoryLog.objects.create( + user=user, + metadata={'foo': 'bar'}, + date_created=yesterday, + asset=asset, + ) + self._check_common_fields(log, user, asset) + self.assertEquals(log.date_created, yesterday) + self.assertDictEqual(log.metadata, {'foo': 'bar'}) + + @patch('kobo.apps.audit_log.models.logging.warning') + def test_create_project_history_log_ignores_attempt_to_override_standard_fields( + self, patched_warning + ): + user = User.objects.get(username='someuser') + asset = Asset.objects.get(pk=1) + log = ProjectHistoryLog.objects.create( + log_type=AuditType.DATA_EDITING, + model_name='foo', + app_label='bar', + asset=asset, + user=user, + ) + # the standard fields should be set the same as any other project history logs + self._check_common_fields(log, user, asset) + # we logged a warning for each attempt to override a field + self.assertEquals(patched_warning.call_count, 3) + + def test_cannot_create_project_history_log_from_non_survey_asset(self): + block_asset = baker.make(Asset) + block_asset.asset_type = ASSET_TYPE_QUESTION + with self.assertRaises(BadAssetTypeException): + ProjectHistoryLog.objects.create( + user=User.objects.get(username='someuser'), asset=block_asset + ) + + @data( + # first_deployment, only_active_changed, is_active, + # expected_action, expect_extra_metadata + (True, False, True, AuditAction.DEPLOY, True), + (False, False, True, AuditAction.REDEPLOY, True), + (False, True, False, AuditAction.ARCHIVE, False), + (False, True, True, AuditAction.UNARCHIVE, False), + ) + @unpack + def test_create_log_for_deployment_changes( + self, + first_deployment, + only_active_changed, + is_active, + expected_action, + expect_extra_metadata, + ): + user = User.objects.get(username='someuser') + factory = RequestFactory() + request = factory.post('') + request.user = user + asset = Asset.objects.get(pk=1) + asset.save() + asset.deploy(backend='mock', active=is_active) + log = ProjectHistoryLog.create_from_deployment_request( + request, + asset, + first_deployment=first_deployment, + only_active_changed=only_active_changed, + ) + self._check_common_fields(log, user, asset) + expected_metadata = { + 'ip_address': '1.2.3.4', + 'source': 'source', + 'asset_uid': asset.uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + } + if expect_extra_metadata: + expected_metadata.update( + { + 'version_uid': asset.latest_deployed_version_uid, + } + ) + self.assertDictEqual(log.metadata, expected_metadata) + self.assertEqual(log.action, expected_action) diff --git a/kpi/constants.py b/kpi/constants.py index 663f9c6ab8..e40e26eefd 100644 --- a/kpi/constants.py +++ b/kpi/constants.py @@ -140,3 +140,5 @@ ACCESS_LOG_UNKNOWN_AUTH_TYPE = 'unknown' ACCESS_LOG_SUBMISSION_AUTH_TYPE = 'submission' ACCESS_LOG_AUTHORIZED_APP_TYPE = 'authorized-application' + +PROJECT_HISTORY_LOG_PROJECT_SUBTYPE = 'project' diff --git a/kpi/serializers/v2/deployment.py b/kpi/serializers/v2/deployment.py index e140800ca9..15f2f0802f 100644 --- a/kpi/serializers/v2/deployment.py +++ b/kpi/serializers/v2/deployment.py @@ -37,6 +37,7 @@ def create(self, validated_data): asset.deploy( backend=backend_id, active=validated_data.get('active', False) ) + return asset.deployment def update(self, instance, validated_data): diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index ea877add4c..4d29c973e9 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -9,6 +9,8 @@ from django.urls import reverse from django.utils import timezone from rest_framework import status +from unittest.mock import patch + from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.project_views.models.project_view import ProjectView @@ -1729,7 +1731,8 @@ def test_upload_form_media_with_slashes(self): class AssetDeploymentTest(BaseAssetDetailTestCase): - def test_asset_deployment(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_asset_deployment(self, patched_create_log): deployment_url = reverse(self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid}) @@ -1740,6 +1743,11 @@ def test_asset_deployment(self): self.assertEqual(response1.data['asset']['deployment__active'], True) self.assertEqual(response1.data['asset']['has_deployment'], True) + request = response1.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, first_deployment=True + ) + patched_create_log.reset_mock() response2 = self.client.get(self.asset_url, format='json') @@ -1749,8 +1757,11 @@ def test_asset_deployment(self): response2.data['deployment_status'] == AssetDeploymentStatus.DEPLOYED.value ) + # nothing should be logged for a GET request + patched_create_log.assert_not_called() - def test_asset_redeployment(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_asset_redeployment(self, patched_create_log): self.test_asset_deployment() # Update asset to redeploy it @@ -1776,6 +1787,8 @@ def test_asset_redeployment(self): }) self.assertEqual(redeploy_response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + # no log should be created for failed request + patched_create_log.assert_not_called() # ... but we can with `PATCH` redeploy_response = self.client.patch(deployment_url, { @@ -1785,6 +1798,13 @@ def test_asset_redeployment(self): }) self.assertEqual(redeploy_response.status_code, status.HTTP_200_OK) + + # check project history log + request = redeploy_response.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=False + ) + # Validate version id self.asset.refresh_from_db() self.assertEqual(self.asset.deployment.version_id, @@ -1870,7 +1890,8 @@ def test_asset_deployment_dates(self): ' modification date of that version' ) - def test_archive_asset(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_archive_asset(self, patched_create_log): self.test_asset_deployment() deployment_url = reverse(self._get_endpoint('asset-deployment'), @@ -1886,6 +1907,10 @@ def test_archive_asset(self): response1.data['asset']['deployment_status'] == AssetDeploymentStatus.ARCHIVED.value ) + request = response1.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=True + ) response2 = self.client.get(self.asset_url, format='json') self.assertEqual(response2.data['deployment__active'], False) @@ -1921,6 +1946,30 @@ def test_archive_asset_does_not_modify_date_deployed(self): self.asset.refresh_from_db() assert self.asset.date_deployed == original_date_deployed + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_unarchive_asset_creates_log(self, patched_create_log): + # manually deploy as archived + self.asset.deploy(backend='mock', active=False) + + # unarchive + deployment_url = reverse( + self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid} + ) + + response = self.client.patch( + deployment_url, + { + 'backend': 'mock', + 'active': True, + }, + ) + assert response.status_code == status.HTTP_200_OK + self.asset.refresh_from_db() + request = response.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=True + ) + class TestCreatedByAndLastModifiedByAsset(BaseAssetTestCase): fixtures = ['test_data'] diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index b44de7640e..017e0bc1e1 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -12,6 +12,7 @@ from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin +from kobo.apps.audit_log.models import ProjectHistoryLog from kpi.constants import ( ASSET_TYPES, ASSET_TYPE_ARG_NAME, @@ -494,6 +495,9 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() + ProjectHistoryLog.create_from_deployment_request( + request, asset, first_deployment=True + ) # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) @@ -518,6 +522,22 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() + + # create a project history log + # note a log will be created even if nothing changed, since + # we care about request intent more than effect + + # copy the request data so we don't change the original object + request_data_copy = request.data.copy() + # remove the 'backend' key, we don't care about it for this part + if request_data_copy.get('backend', None): + request_data_copy.pop('backend') + + updated_fields = request_data_copy.keys() + only_active_changed = list(updated_fields) == ['active'] + ProjectHistoryLog.create_from_deployment_request( + request, asset, only_active_changed=only_active_changed + ) # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) From bcfd949a772258aab9a31988e3270c9d1d248ec1 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 1 Oct 2024 10:55:03 -0400 Subject: [PATCH 38/50] fixup!: test refactor --- kobo/apps/audit_log/tests/test_models.py | 45 +++++++++--------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 745e8f0053..313bb1e379 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -22,19 +22,7 @@ from kpi.models import Asset from kpi.tests.base_test_case import BaseTestCase - -class AccessLogModelTestCase(BaseTestCase): - - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.super_user = User.objects.create_user( - 'user', 'user@example.com', 'userpass' - ) - cls.super_user.is_super = True - cls.super_user.backend = 'django.contrib.auth.backends.ModelBackend' - cls.super_user.save() - +class BaseAuditLogTestCase(BaseTestCase): def setUp(self): source_patcher = patch( 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', @@ -48,6 +36,20 @@ def setUp(self): self.addCleanup(source_patcher.stop) self.addCleanup(ip_patcher.stop) + +class AccessLogModelTestCase(BaseAuditLogTestCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.super_user = User.objects.create_user( + 'user', 'user@example.com', 'userpass' + ) + cls.super_user.is_super = True + cls.super_user.backend = 'django.contrib.auth.backends.ModelBackend' + cls.super_user.save() + + def _create_request(self, url: str, cached_user, new_user): factory = RequestFactory() request = factory.post(url) @@ -196,23 +198,10 @@ def test_create_auth_log_with_extra_metadata(self): @ddt -class ProjectHistoryLogModelTestCase(BaseTestCase): +class ProjectHistoryLogModelTestCase(BaseAuditLogTestCase): fixtures = ['test_data'] - def setUp(self): - source_patcher = patch( - 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', - return_value='source', - ) - ip_patcher = patch( - 'kobo.apps.audit_log.models.get_client_ip', return_value='1.2.3.4' - ) - source_patcher.start() - ip_patcher.start() - self.addCleanup(source_patcher.stop) - self.addCleanup(ip_patcher.stop) - def _check_common_fields(self, log: ProjectHistoryLog, user, asset): self.assertEqual(log.user.id, user.id) self.assertEqual(log.app_label, 'kpi') @@ -293,7 +282,7 @@ def test_create_log_for_deployment_changes( ) self._check_common_fields(log, user, asset) expected_metadata = { - 'ip_address': '1.2.3.4', + 'ip_address': '127.0.0.1', 'source': 'source', 'asset_uid': asset.uid, 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, From 18fc695a3f86adc33f7f13858de43f8bde8d8b61 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 1 Oct 2024 10:56:49 -0400 Subject: [PATCH 39/50] fixup!: what is up with that space --- kobo/apps/audit_log/tests/test_models.py | 1 - kpi/serializers/v2/deployment.py | 1 - 2 files changed, 2 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 313bb1e379..1fa0dfe190 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -49,7 +49,6 @@ def setUpClass(cls): cls.super_user.backend = 'django.contrib.auth.backends.ModelBackend' cls.super_user.save() - def _create_request(self, url: str, cached_user, new_user): factory = RequestFactory() request = factory.post(url) diff --git a/kpi/serializers/v2/deployment.py b/kpi/serializers/v2/deployment.py index 15f2f0802f..e140800ca9 100644 --- a/kpi/serializers/v2/deployment.py +++ b/kpi/serializers/v2/deployment.py @@ -37,7 +37,6 @@ def create(self, validated_data): asset.deploy( backend=backend_id, active=validated_data.get('active', False) ) - return asset.deployment def update(self, instance, validated_data): From b96625a2a13c8113698b06c7407f67cc685ea6b5 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 1 Oct 2024 10:59:27 -0400 Subject: [PATCH 40/50] fixup!: space --- kobo/apps/audit_log/tests/test_models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 1fa0dfe190..4299737ca5 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -22,6 +22,7 @@ from kpi.models import Asset from kpi.tests.base_test_case import BaseTestCase + class BaseAuditLogTestCase(BaseTestCase): def setUp(self): source_patcher = patch( From 776a78319d0668912780839d494883e36d2a0ba2 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 2 Oct 2024 08:25:35 -0400 Subject: [PATCH 41/50] fixup!: suggestion from review --- kpi/views/v2/asset.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index 017e0bc1e1..04634a4c94 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -530,8 +530,7 @@ def deployment(self, request, uid): # copy the request data so we don't change the original object request_data_copy = request.data.copy() # remove the 'backend' key, we don't care about it for this part - if request_data_copy.get('backend', None): - request_data_copy.pop('backend') + request_data_copy.pop('backend', None) updated_fields = request_data_copy.keys() only_active_changed = list(updated_fields) == ['active'] From 1edd0d680bc9604442913b61199b3a9037b08ed9 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 2 Oct 2024 09:10:42 -0400 Subject: [PATCH 42/50] fixup!: comments --- kobo/apps/audit_log/tests/test_one_time_auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_one_time_auth.py b/kobo/apps/audit_log/tests/test_one_time_auth.py index a4bb289dc2..2ba28beb4c 100644 --- a/kobo/apps/audit_log/tests/test_one_time_auth.py +++ b/kobo/apps/audit_log/tests/test_one_time_auth.py @@ -147,7 +147,7 @@ def test_any_auth_for_submissions(self, authetication_method): 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', # noqa return_value=HttpResponse(status=200), ): - # try both v1 and v2 + # try both OpenRosa and v1 endpoints self.client.post(reverse('submissions')) self.client.post(reverse('submissions-list')) log_exists = AuditLog.objects.filter( @@ -178,7 +178,7 @@ def side_effect(request): 'kobo.apps.openrosa.apps.api.viewsets.xform_submission_api.XFormSubmissionApi.create', # noqa return_value=HttpResponse(status=200), ): - # check v1 and v2 + # try both OpenRosa and v1 endpoints self.client.post(reverse('submissions'), **header) self.client.post(reverse('submissions-list'), **header) From 1aa3c788963df875be170ac0a4924662beeb98a8 Mon Sep 17 00:00:00 2001 From: rajpatel24 Date: Fri, 4 Oct 2024 14:56:24 +0530 Subject: [PATCH 43/50] Update flake8 ignore list in darker check --- .github/workflows/darker.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/darker.yml b/.github/workflows/darker.yml index 4997280e9c..4b6ee01246 100644 --- a/.github/workflows/darker.yml +++ b/.github/workflows/darker.yml @@ -24,6 +24,6 @@ jobs: # darker still exit with code 1 even with no errors on changes - name: Run Darker with base commit run: | - output=$(darker --check --isort -L "flake8 --max-line-length=88 --extend-ignore=F821,W503,W504" kpi kobo hub -r ${{ github.event.pull_request.base.sha }}) + output=$(darker --check --isort -L "flake8 --max-line-length=88 --extend-ignore=F821" kpi kobo hub -r ${{ github.event.pull_request.base.sha }}) [[ -n "$output" ]] && echo "$output" && exit 1 || exit 0 shell: /usr/bin/bash {0} From 7da7d5be71948f196889b37af27a309253f5c70a Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Fri, 4 Oct 2024 09:30:49 -0400 Subject: [PATCH 44/50] Record project history logs for deployment changes (#5134) --- .../migrations/0011_projecthistorylog.py | 23 +++ kobo/apps/audit_log/models.py | 99 ++++++++++-- kobo/apps/audit_log/tests/test_models.py | 153 +++++++++++++++--- kpi/constants.py | 2 + kpi/tests/api/v2/test_api_assets.py | 55 ++++++- kpi/views/v2/asset.py | 19 +++ 6 files changed, 315 insertions(+), 36 deletions(-) create mode 100644 kobo/apps/audit_log/migrations/0011_projecthistorylog.py diff --git a/kobo/apps/audit_log/migrations/0011_projecthistorylog.py b/kobo/apps/audit_log/migrations/0011_projecthistorylog.py new file mode 100644 index 0000000000..a643e5aa18 --- /dev/null +++ b/kobo/apps/audit_log/migrations/0011_projecthistorylog.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.15 on 2024-09-26 16:46 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('audit_log', '0010_accesslog'), + ] + + operations = [ + migrations.CreateModel( + name='ProjectHistoryLog', + fields=[], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('audit_log.auditlog',), + ), + ] diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 4fd3908849..aa8287bdc4 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -14,8 +14,11 @@ ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ACCESS_LOG_UNKNOWN_AUTH_TYPE, + ASSET_TYPE_SURVEY, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) +from kpi.exceptions import BadAssetTypeException from kpi.fields.kpi_uid import UUID_LENGTH +from kpi.models import Asset from kpi.utils.log import logging @@ -27,6 +30,10 @@ class AuditAction(models.TextChoices): REMOVE = 'remove' UPDATE = 'update' AUTH = 'auth' + DEPLOY = 'deploy' + ARCHIVE = 'archive' + UNARCHIVE = 'unarchive' + REDEPLOY = 'redeploy' class AuditType(models.TextChoices): @@ -94,25 +101,32 @@ def save( ) -class AccessLogManager(models.Manager): +class IgnoreCommonFieldsMixin: + def remove_common_fields_and_warn(self, log_type, create_kwargs): + # remove any attempt to set fields that should + # always be the same on an particular log type + app_label = create_kwargs.pop('app_label', None) + if app_label is not None: + logging.warning(f'Ignoring attempt to set {app_label=} on {log_type} log') + model_name = create_kwargs.pop('model_name', None) + if model_name is not None: + logging.warning(f'Ignoring attempt to set {model_name=} on {log_type} log') + log_type = create_kwargs.pop('log_type', None) + if log_type is not None: + logging.warning(f'Ignoring attempt to set {log_type=} on {log_type} log') + + +class AccessLogManager(models.Manager, IgnoreCommonFieldsMixin): def get_queryset(self): return super().get_queryset().filter(log_type=AuditType.ACCESS) def create(self, **kwargs): # remove any attempt to set fields that should # always be the same on an access log - app_label = kwargs.pop('app_label', None) - if app_label is not None: - logging.warning(f'Ignoring attempt to set {app_label=} on access log') - model_name = kwargs.pop('model_name', None) - if model_name is not None: - logging.warning(f'Ignoring attempt to set {model_name=} on access log') + self.remove_common_fields_and_warn('access', kwargs) action = kwargs.pop('action', None) if action is not None: logging.warning(f'Ignoring attempt to set {action=} on access log') - log_type = kwargs.pop('log_type', None) - if log_type is not None: - logging.warning(f'Ignoring attempt to set {log_type=} on access log') user = kwargs.pop('user') return super().create( # set the fields that are always the same for access logs, @@ -260,3 +274,68 @@ def create_from_request( if extra_metadata is not None: metadata.update(extra_metadata) return AccessLog.objects.create(user=logged_in_user, metadata=metadata) + + +class ProjectHistoryLogManager(models.Manager, IgnoreCommonFieldsMixin): + def get_queryset(self): + return super().get_queryset().filter(log_type=AuditType.PROJECT_HISTORY) + + def create(self, **kwargs): + # remove any attempt to set fields that should + # always be the same on a project history log + self.remove_common_fields_and_warn('project history', kwargs) + user = kwargs.pop('user') + asset = kwargs.pop('asset') + if not asset.asset_type or asset.asset_type != ASSET_TYPE_SURVEY: + raise BadAssetTypeException( + 'Cannot create project history log for non-survey asset' + ) + return super().create( + # set the fields that are always the same for all project history logs, + # along with the ones derived from the user and asset + app_label=Asset._meta.app_label, + model_name=Asset._meta.model_name, + log_type=AuditType.PROJECT_HISTORY, + user=user, + user_uid=user.extra_details.uid, + object_id=asset.id, + **kwargs, + ) + + +class ProjectHistoryLog(AuditLog): + objects = ProjectHistoryLogManager() + + class Meta: + proxy = True + + @staticmethod + def create_from_deployment_request( + request, asset, first_deployment=False, only_active_changed=False + ): + ip = get_client_ip(request) + source = get_human_readable_client_user_agent(request) + if only_active_changed: + action = ( + AuditAction.UNARCHIVE + if asset.deployment.active + else AuditAction.ARCHIVE + ) + else: + action = AuditAction.DEPLOY if first_deployment else AuditAction.REDEPLOY + metadata = { + 'ip_address': ip, + 'source': source, + 'asset_uid': asset.uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + } + if action in [AuditAction.REDEPLOY, AuditAction.DEPLOY]: + version = asset.latest_deployed_version + metadata['version_uid'] = version.uid + user = request.user + return ProjectHistoryLog.objects.create( + user=user, + action=action, + metadata=metadata, + asset=asset, + ) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index a0af285c81..40379277db 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -2,10 +2,12 @@ from datetime import timedelta from unittest.mock import patch +from ddt import data, ddt, unpack from django.contrib.auth.models import AnonymousUser from django.test.client import RequestFactory from django.urls import resolve, reverse from django.utils import timezone +from model_bakery import baker from kobo.apps.audit_log.models import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, @@ -14,21 +16,36 @@ AuditAction, AuditLog, AuditType, + ProjectHistoryLog, ) from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( + ASSET_TYPE_QUESTION, ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) +from kpi.exceptions import BadAssetTypeException +from kpi.models import Asset from kpi.tests.base_test_case import BaseTestCase -@patch( - 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', - return_value='source', -) -@patch('kobo.apps.audit_log.models.get_client_ip', return_value='127.0.0.1') -class AccessLogModelTestCase(BaseTestCase): +class BaseAuditLogTestCase(BaseTestCase): + def setUp(self): + source_patcher = patch( + 'kobo.apps.audit_log.models.get_human_readable_client_user_agent', + return_value='source', + ) + ip_patcher = patch( + 'kobo.apps.audit_log.models.get_client_ip', return_value='127.0.0.1' + ) + source_patcher.start() + ip_patcher.start() + self.addCleanup(source_patcher.stop) + self.addCleanup(ip_patcher.stop) + + +class AccessLogModelTestCase(BaseAuditLogTestCase): @classmethod def setUpClass(cls): @@ -57,7 +74,7 @@ def _check_common_fields(self, access_log: AccessLog, user): self.assertEqual(access_log.action, AuditAction.AUTH) self.assertEqual(access_log.log_type, AuditType.ACCESS) - def test_create_access_log_sets_standard_fields(self, patched_ip, patched_source): + def test_create_access_log_sets_standard_fields(self): yesterday = timezone.now() - timedelta(days=1) log = AccessLog.objects.create( user=AccessLogModelTestCase.super_user, @@ -70,7 +87,7 @@ def test_create_access_log_sets_standard_fields(self, patched_ip, patched_source @patch('kobo.apps.audit_log.models.logging.warning') def test_create_access_log_ignores_attempt_to_override_standard_fields( - self, patched_warning, patched_ip, patched_source + self, patched_warning ): log = AccessLog.objects.create( log_type=AuditType.DATA_EDITING, @@ -84,9 +101,7 @@ def test_create_access_log_ignores_attempt_to_override_standard_fields( # we logged a warning for each attempt to override a field self.assertEquals(patched_warning.call_count, 4) - def test_basic_create_auth_log_from_request( - self, patched_ip, patched_source - ): + def test_basic_create_auth_log_from_request(self): request = self._create_request( reverse('kobo_login'), AnonymousUser(), @@ -103,9 +118,7 @@ def test_basic_create_auth_log_from_request( }, ) - def test_create_auth_log_from_loginas_request( - self, patched_ip, patched_source - ): + def test_create_auth_log_from_loginas_request(self): second_user = User.objects.create_user( 'second_user', 'second@example.com', 'pass' ) @@ -128,9 +141,7 @@ def test_create_auth_log_from_loginas_request( }, ) - def test_create_auth_log_with_different_auth_type( - self, patched_ip, patched_source - ): + def test_create_auth_log_with_different_auth_type(self): request = self._create_request( reverse('api_v2:asset-list'), AnonymousUser(), @@ -149,9 +160,7 @@ def test_create_auth_log_with_different_auth_type( }, ) - def test_create_auth_log_unknown_authenticator( - self, patched_ip, patched_source - ): + def test_create_auth_log_unknown_authenticator(self): # no backend attached to the user object second_user = User.objects.create_user( 'second_user', 'second@example.com', 'pass' @@ -173,9 +182,7 @@ def test_create_auth_log_unknown_authenticator( }, ) - def test_create_auth_log_with_extra_metadata( - self, patched_ip, patched_source - ): + def test_create_auth_log_with_extra_metadata(self): request = self._create_request( reverse('api_v2:asset-list'), AnonymousUser(), @@ -344,3 +351,103 @@ def test_with_submissions_grouped_groups_submissions(self): user_2_group_1['metadata']['auth_type'], ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, ) + + +@ddt +class ProjectHistoryLogModelTestCase(BaseAuditLogTestCase): + + fixtures = ['test_data'] + + def _check_common_fields(self, log: ProjectHistoryLog, user, asset): + self.assertEqual(log.user.id, user.id) + self.assertEqual(log.app_label, 'kpi') + self.assertEqual(log.model_name, 'asset') + self.assertEqual(log.object_id, asset.id) + self.assertEqual(log.user_uid, user.extra_details.uid) + self.assertEqual(log.log_type, AuditType.PROJECT_HISTORY) + + def test_create_project_history_log_sets_standard_fields(self): + user = User.objects.get(username='someuser') + asset = Asset.objects.get(pk=1) + yesterday = timezone.now() - timedelta(days=1) + log = ProjectHistoryLog.objects.create( + user=user, + metadata={'foo': 'bar'}, + date_created=yesterday, + asset=asset, + ) + self._check_common_fields(log, user, asset) + self.assertEquals(log.date_created, yesterday) + self.assertDictEqual(log.metadata, {'foo': 'bar'}) + + @patch('kobo.apps.audit_log.models.logging.warning') + def test_create_project_history_log_ignores_attempt_to_override_standard_fields( + self, patched_warning + ): + user = User.objects.get(username='someuser') + asset = Asset.objects.get(pk=1) + log = ProjectHistoryLog.objects.create( + log_type=AuditType.DATA_EDITING, + model_name='foo', + app_label='bar', + asset=asset, + user=user, + ) + # the standard fields should be set the same as any other project history logs + self._check_common_fields(log, user, asset) + # we logged a warning for each attempt to override a field + self.assertEquals(patched_warning.call_count, 3) + + def test_cannot_create_project_history_log_from_non_survey_asset(self): + block_asset = baker.make(Asset) + block_asset.asset_type = ASSET_TYPE_QUESTION + with self.assertRaises(BadAssetTypeException): + ProjectHistoryLog.objects.create( + user=User.objects.get(username='someuser'), asset=block_asset + ) + + @data( + # first_deployment, only_active_changed, is_active, + # expected_action, expect_extra_metadata + (True, False, True, AuditAction.DEPLOY, True), + (False, False, True, AuditAction.REDEPLOY, True), + (False, True, False, AuditAction.ARCHIVE, False), + (False, True, True, AuditAction.UNARCHIVE, False), + ) + @unpack + def test_create_log_for_deployment_changes( + self, + first_deployment, + only_active_changed, + is_active, + expected_action, + expect_extra_metadata, + ): + user = User.objects.get(username='someuser') + factory = RequestFactory() + request = factory.post('') + request.user = user + asset = Asset.objects.get(pk=1) + asset.save() + asset.deploy(backend='mock', active=is_active) + log = ProjectHistoryLog.create_from_deployment_request( + request, + asset, + first_deployment=first_deployment, + only_active_changed=only_active_changed, + ) + self._check_common_fields(log, user, asset) + expected_metadata = { + 'ip_address': '127.0.0.1', + 'source': 'source', + 'asset_uid': asset.uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + } + if expect_extra_metadata: + expected_metadata.update( + { + 'version_uid': asset.latest_deployed_version_uid, + } + ) + self.assertDictEqual(log.metadata, expected_metadata) + self.assertEqual(log.action, expected_action) diff --git a/kpi/constants.py b/kpi/constants.py index d4ed407070..82588938dd 100644 --- a/kpi/constants.py +++ b/kpi/constants.py @@ -141,3 +141,5 @@ ACCESS_LOG_SUBMISSION_AUTH_TYPE = 'submission' ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE = 'submission-group' ACCESS_LOG_AUTHORIZED_APP_TYPE = 'authorized-application' + +PROJECT_HISTORY_LOG_PROJECT_SUBTYPE = 'project' diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index ea877add4c..4d29c973e9 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -9,6 +9,8 @@ from django.urls import reverse from django.utils import timezone from rest_framework import status +from unittest.mock import patch + from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.project_views.models.project_view import ProjectView @@ -1729,7 +1731,8 @@ def test_upload_form_media_with_slashes(self): class AssetDeploymentTest(BaseAssetDetailTestCase): - def test_asset_deployment(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_asset_deployment(self, patched_create_log): deployment_url = reverse(self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid}) @@ -1740,6 +1743,11 @@ def test_asset_deployment(self): self.assertEqual(response1.data['asset']['deployment__active'], True) self.assertEqual(response1.data['asset']['has_deployment'], True) + request = response1.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, first_deployment=True + ) + patched_create_log.reset_mock() response2 = self.client.get(self.asset_url, format='json') @@ -1749,8 +1757,11 @@ def test_asset_deployment(self): response2.data['deployment_status'] == AssetDeploymentStatus.DEPLOYED.value ) + # nothing should be logged for a GET request + patched_create_log.assert_not_called() - def test_asset_redeployment(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_asset_redeployment(self, patched_create_log): self.test_asset_deployment() # Update asset to redeploy it @@ -1776,6 +1787,8 @@ def test_asset_redeployment(self): }) self.assertEqual(redeploy_response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + # no log should be created for failed request + patched_create_log.assert_not_called() # ... but we can with `PATCH` redeploy_response = self.client.patch(deployment_url, { @@ -1785,6 +1798,13 @@ def test_asset_redeployment(self): }) self.assertEqual(redeploy_response.status_code, status.HTTP_200_OK) + + # check project history log + request = redeploy_response.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=False + ) + # Validate version id self.asset.refresh_from_db() self.assertEqual(self.asset.deployment.version_id, @@ -1870,7 +1890,8 @@ def test_asset_deployment_dates(self): ' modification date of that version' ) - def test_archive_asset(self): + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_archive_asset(self, patched_create_log): self.test_asset_deployment() deployment_url = reverse(self._get_endpoint('asset-deployment'), @@ -1886,6 +1907,10 @@ def test_archive_asset(self): response1.data['asset']['deployment_status'] == AssetDeploymentStatus.ARCHIVED.value ) + request = response1.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=True + ) response2 = self.client.get(self.asset_url, format='json') self.assertEqual(response2.data['deployment__active'], False) @@ -1921,6 +1946,30 @@ def test_archive_asset_does_not_modify_date_deployed(self): self.asset.refresh_from_db() assert self.asset.date_deployed == original_date_deployed + @patch('kpi.views.v2.asset.ProjectHistoryLog.create_from_deployment_request') + def test_unarchive_asset_creates_log(self, patched_create_log): + # manually deploy as archived + self.asset.deploy(backend='mock', active=False) + + # unarchive + deployment_url = reverse( + self._get_endpoint('asset-deployment'), kwargs={'uid': self.asset_uid} + ) + + response = self.client.patch( + deployment_url, + { + 'backend': 'mock', + 'active': True, + }, + ) + assert response.status_code == status.HTTP_200_OK + self.asset.refresh_from_db() + request = response.renderer_context['request'] + patched_create_log.assert_called_once_with( + request, self.asset, only_active_changed=True + ) + class TestCreatedByAndLastModifiedByAsset(BaseAssetTestCase): fixtures = ['test_data'] diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index b44de7640e..04634a4c94 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -12,6 +12,7 @@ from rest_framework.response import Response from rest_framework_extensions.mixins import NestedViewSetMixin +from kobo.apps.audit_log.models import ProjectHistoryLog from kpi.constants import ( ASSET_TYPES, ASSET_TYPE_ARG_NAME, @@ -494,6 +495,9 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() + ProjectHistoryLog.create_from_deployment_request( + request, asset, first_deployment=True + ) # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) @@ -518,6 +522,21 @@ def deployment(self, request, uid): ) serializer.is_valid(raise_exception=True) serializer.save() + + # create a project history log + # note a log will be created even if nothing changed, since + # we care about request intent more than effect + + # copy the request data so we don't change the original object + request_data_copy = request.data.copy() + # remove the 'backend' key, we don't care about it for this part + request_data_copy.pop('backend', None) + + updated_fields = request_data_copy.keys() + only_active_changed = list(updated_fields) == ['active'] + ProjectHistoryLog.create_from_deployment_request( + request, asset, only_active_changed=only_active_changed + ) # TODO: Understand why this 404s when `serializer.data` is not # coerced to a dict return Response(dict(serializer.data)) From 93a8cced7babb16d6c548bc20cf6dfe7c30955a1 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 7 Oct 2024 09:42:08 -0400 Subject: [PATCH 45/50] docs: update PR template to include test plan --- .github/PULL_REQUEST_TEMPLATE.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1e8099eb21..5dab3b4823 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,6 +7,7 @@ 5. [ ] Write a title and, if necessary, a description of your work suitable for publishing in our [release notes](https://community.kobotoolbox.org/tag/release-notes) 6. [ ] Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE) 7. [ ] Open an issue in the [docs](https://github.com/kobotoolbox/docs/issues/new) if there are UI/UX changes +8. [ ] Create a testing plan for the reviewer and add it to the Testing section ## Description @@ -16,6 +17,10 @@ Describe the outcome of your work here. A non-programmer who is familiar with Ko Describe what you've changed and why. This should allow the reviewer to understand more easily the scope of the PR. It's good to be thorough here. +## Testing + +Add a testing plan for the reviewer. This should allow the reviewer to test the basic functionality of the new code. + ## Related issues Fixes #ISSUE From 5024e8dd4150d4728a96f4010c6a939563aa92e1 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 7 Oct 2024 09:43:04 -0400 Subject: [PATCH 46/50] Revert "docs: update PR template to include test plan" This reverts commit 93a8cced7babb16d6c548bc20cf6dfe7c30955a1. --- .github/PULL_REQUEST_TEMPLATE.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 5dab3b4823..1e8099eb21 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,7 +7,6 @@ 5. [ ] Write a title and, if necessary, a description of your work suitable for publishing in our [release notes](https://community.kobotoolbox.org/tag/release-notes) 6. [ ] Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE) 7. [ ] Open an issue in the [docs](https://github.com/kobotoolbox/docs/issues/new) if there are UI/UX changes -8. [ ] Create a testing plan for the reviewer and add it to the Testing section ## Description @@ -17,10 +16,6 @@ Describe the outcome of your work here. A non-programmer who is familiar with Ko Describe what you've changed and why. This should allow the reviewer to understand more easily the scope of the PR. It's good to be thorough here. -## Testing - -Add a testing plan for the reviewer. This should allow the reviewer to test the basic functionality of the new code. - ## Related issues Fixes #ISSUE From e5e52b99616321ef1cc928bde1ca66794e7f5ec9 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 7 Oct 2024 09:46:08 -0400 Subject: [PATCH 47/50] docs: update PR template to include test plan --- .github/PULL_REQUEST_TEMPLATE.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1e8099eb21..5dab3b4823 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,6 +7,7 @@ 5. [ ] Write a title and, if necessary, a description of your work suitable for publishing in our [release notes](https://community.kobotoolbox.org/tag/release-notes) 6. [ ] Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE) 7. [ ] Open an issue in the [docs](https://github.com/kobotoolbox/docs/issues/new) if there are UI/UX changes +8. [ ] Create a testing plan for the reviewer and add it to the Testing section ## Description @@ -16,6 +17,10 @@ Describe the outcome of your work here. A non-programmer who is familiar with Ko Describe what you've changed and why. This should allow the reviewer to understand more easily the scope of the PR. It's good to be thorough here. +## Testing + +Add a testing plan for the reviewer. This should allow the reviewer to test the basic functionality of the new code. + ## Related issues Fixes #ISSUE From f773c0c52607dcb69721077e5778943b4eec2754 Mon Sep 17 00:00:00 2001 From: Leszek Date: Mon, 7 Oct 2024 17:59:20 +0200 Subject: [PATCH 48/50] comment out log out all button for now --- .../security/accessLogs/accessLogsSection.component.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx b/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx index ac3ee860e3..096f0f6b0d 100644 --- a/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx +++ b/jsapp/js/account/security/accessLogs/accessLogsSection.component.tsx @@ -25,7 +25,9 @@ export default function AccessLogsSection() { {t('Recent account activity')} -
+ {/* TODO: we comment this out until we know how to handle exsiting + sessions for the moment of release of the feature. */} + {/*
+
*/} From b8ed7c9e769270d4dd48ef496292d206470591b9 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 7 Oct 2024 18:32:17 -0400 Subject: [PATCH 49/50] Fix missing import after merge --- kobo/apps/audit_log/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 8db3c854a1..178854a4d8 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -4,7 +4,7 @@ from kpi.filters import SearchFilter from kpi.permissions import IsAuthenticated from .filters import AccessLogPermissionsFilter -from .models import AccessLog, AuditLog +from .models import AccessLog, AuditAction, AuditLog from .permissions import SuperUserPermission from .serializers import AccessLogSerializer, AuditLogSerializer From a77b81f58a78ef85e597156a06fdfb8a623fded2 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 7 Oct 2024 19:04:30 -0400 Subject: [PATCH 50/50] Fix another bad merge conflict --- .../tests/api/v2/test_api_audit_log.py | 17 ++++-- kobo/apps/audit_log/views.py | 52 ++++++++++++------- .../apps/api/viewsets/xform_submission_api.py | 1 + 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index 8bf23dedec..ee8f5fd940 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -5,7 +5,12 @@ from rest_framework import status from rest_framework.reverse import reverse -from kobo.apps.audit_log.models import AccessLog, AuditAction, AuditLog, AuditType +from kobo.apps.audit_log.models import ( + AccessLog, + AuditAction, + AuditLog, + AuditType, +) from kobo.apps.audit_log.tests.test_signals import skip_login_access_log from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ( @@ -196,8 +201,12 @@ def test_endpoint_groups_submissions(self): # this is just to ensure that we're using the grouping query user = User.objects.get(username='someuser') self.force_login_user(user) - jan_1_1_30_am = datetime.fromisoformat('2024-01-01T01:30:25.123456+00:00') - jan_1_1_45_am = datetime.fromisoformat('2024-01-01T01:45:25.123456+00:00') + jan_1_1_30_am = datetime.fromisoformat( + '2024-01-01T01:30:25.123456+00:00' + ) + jan_1_1_45_am = datetime.fromisoformat( + '2024-01-01T01:45:25.123456+00:00' + ) AccessLog.objects.create( user=user, metadata={'auth_type': ACCESS_LOG_SUBMISSION_AUTH_TYPE}, @@ -315,7 +324,7 @@ def test_can_search_access_logs_by_username(self): admin = User.objects.get(username='admin') AccessLog.objects.create(user=user1) AccessLog.objects.create(user=user2) - self.force_login_user(admin) + self.force_login_user(User.objects.get(username='admin')) response = self.client.get(f'{self.url}?q=user__username:anotheruser') # only return logs from user1 diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index 178854a4d8..bd84924e6d 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -4,7 +4,7 @@ from kpi.filters import SearchFilter from kpi.permissions import IsAuthenticated from .filters import AccessLogPermissionsFilter -from .models import AccessLog, AuditAction, AuditLog +from .models import AccessLog, AuditLog from .permissions import SuperUserPermission from .serializers import AccessLogSerializer, AuditLogSerializer @@ -43,7 +43,8 @@ class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): > ] > } - Results from this endpoint can be filtered by a Boolean query specified in the `q` parameter. + Results from this endpoint can be filtered by a Boolean query specified in the + `q` parameter. **Some examples:** @@ -62,7 +63,8 @@ class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): 5. All deleted submissions submitted after a specific date **and time**
`/api/v2/audit-logs/?q=action:delete AND date_created__gte:"2022-11-15 20:34"` - *Notes: Do not forget to wrap search terms in double-quotes if they contain spaces (e.g. date and time "2022-11-15 20:34")* + *Notes: Do not forget to wrap search terms in double-quotes if they contain spaces + (e.g. date and time "2022-11-15 20:34")* ### CURRENT ENDPOINT """ @@ -92,6 +94,8 @@ class AllAccessLogViewSet(AuditLogViewSet): Lists all access logs for all users. Only available to superusers. + Submissions will be grouped together by hour + 
     GET /api/v2/access-logs/
@@ -108,19 +112,24 @@ class AllAccessLogViewSet(AuditLogViewSet): > "previous": null, > "results": [ > { - > "app_label": "kobo_auth", - > "model_name": "User", > "user": "http://localhost/api/v2/users/admin/", - > "user_uid": "uBMZxx9tVfepvTRp3e9Twj", + > "user_uid": "u12345", > "username": "admin", - > "action": "AUTH", > "metadata": { > "source": "Firefox (Ubuntu)", > "auth_type": "Digest", > "ip_address": "172.18.0.6" > }, > "date_created": "2024-08-19T16:48:58Z", - > "log_type": "access" + > }, + > { + > "user": "http://localhost/api/v2/users/admin/", + > "user_uid": "u12345", + > "username": "admin", + > "metadata": { + > "auth_type": "submission-group", + > }, + > "date_created": "2024-08-19T16:00:00Z" > }, > ... > ] @@ -130,11 +139,8 @@ class AllAccessLogViewSet(AuditLogViewSet): """ - queryset = ( - AccessLog.objects.select_related('user') - .filter(action=AuditAction.AUTH) - .order_by('-date_created') - ) + queryset = AccessLog.objects.with_submissions_grouped().order_by('-date_created') + serializer_class = AccessLogSerializer class AccessLogViewSet(AuditLogViewSet): @@ -154,31 +160,39 @@ class AccessLogViewSet(AuditLogViewSet): > curl -X GET https://[kpi-url]/access-logs/me > Response 200 + > { > "count": 10, > "next": null, > "previous": null, > "results": [ > { - > "app_label": "kobo_auth", - > "model_name": "User", > "user": "http://localhost/api/v2/users/admin/", - > "user_uid": "uBMZxx9tVfepvTRp3e9Twj", + > "user_uid": "u12345", > "username": "admin", - > "action": "AUTH", > "metadata": { > "source": "Firefox (Ubuntu)", > "auth_type": "Digest", > "ip_address": "172.18.0.6" > }, > "date_created": "2024-08-19T16:48:58Z" - > "log_type": "access" + > }, + > { + > "user": "http://localhost/api/v2/users/admin/", + > "user_uid": "u12345", + > "username": "admin", + > "metadata": { + > "auth_type": "submission-group", + > }, + > "date_created": "2024-08-19T16:00:00Z" > }, > ... > ] > } + This endpoint can be paginated with 'offset' and 'limit' parameters, eg - > curl -X GET https://[kpi-url]/access-logs/me/?offset=100&limit=50 + > curl -X GET https://[kpi-url]/access-logs/me?offset=100&limit=50 + will return entries 100-149 """ diff --git a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py index 182a0135d0..0df1022d27 100644 --- a/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py +++ b/kobo/apps/openrosa/apps/api/viewsets/xform_submission_api.py @@ -23,6 +23,7 @@ from kpi.authentication import ( BasicAuthentication, DigestAuthentication, + SessionAuthentication, TokenAuthentication, ) from kpi.utils.object_permission import get_database_user