From e982a87a7e91ba63aa8022842648870165e50587 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 21 Sep 2023 21:16:09 +0000 Subject: [PATCH 1/6] Bump cryptography from 41.0.3 to 41.0.4 Bumps [cryptography](https://github.com/pyca/cryptography) from 41.0.3 to 41.0.4. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pyca/cryptography/compare/41.0.3...41.0.4) --- updated-dependencies: - dependency-name: cryptography dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- requirements_test.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index ad21f188..2f9a75a7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -62,7 +62,7 @@ click-repl==0.3.0 # via celery cron-descriptor==1.4.0 # via django-celery-beat -cryptography==41.0.3 +cryptography==41.0.4 # via -r requirements.in directory-components==39.1.3 # via -r requirements.in diff --git a/requirements_test.txt b/requirements_test.txt index e302cd98..e4dfb7ef 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -72,7 +72,7 @@ coveralls==3.3.1 # via -r requirements_test.in cron-descriptor==1.4.0 # via django-celery-beat -cryptography==41.0.3 +cryptography==41.0.4 # via -r requirements.in directory-components==39.1.3 # via -r requirements.in From 68c1c3f6b121030d763769a2bf337d46d50ffe24 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Sep 2023 19:20:50 +0000 Subject: [PATCH 2/6] Bump gevent from 22.10.2 to 23.9.1 Bumps [gevent](https://github.com/gevent/gevent) from 22.10.2 to 23.9.1. - [Release notes](https://github.com/gevent/gevent/releases) - [Changelog](https://github.com/gevent/gevent/blob/master/docs/changelog_pre.rst) - [Commits](https://github.com/gevent/gevent/compare/22.10.2...23.9.1) --- updated-dependencies: - dependency-name: gevent dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.in | 2 +- requirements.txt | 2 +- requirements_test.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements.in b/requirements.in index 322059da..c0c82cd2 100644 --- a/requirements.in +++ b/requirements.in @@ -35,7 +35,7 @@ notifications-python-client==6.3.* num2words==0.5.10 pycountry==19.8.18 elastic-apm>6.0 -gevent==22.10.* +gevent==23.9.* psycogreen==1.0.2 wagtailmedia==0.14.* cryptography==41.* diff --git a/requirements.txt b/requirements.txt index ad21f188..e1546a11 100644 --- a/requirements.txt +++ b/requirements.txt @@ -151,7 +151,7 @@ elastic-apm==6.18.0 # via -r requirements.in et-xmlfile==1.1.0 # via openpyxl -gevent==22.10.2 +gevent==23.9.1 # via -r requirements.in greenlet==2.0.2 # via gevent diff --git a/requirements_test.txt b/requirements_test.txt index e302cd98..5556bad5 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -177,7 +177,7 @@ flake8==6.1.0 # via -r requirements_test.in freezegun==0.3.14 # via -r requirements_test.in -gevent==22.10.2 +gevent==23.9.1 # via -r requirements.in gitdb==4.0.10 # via gitpython From b8b50d4f2e5c8bf181b01782a164841b382f5720 Mon Sep 17 00:00:00 2001 From: Timothy Patterson Date: Mon, 25 Sep 2023 14:33:36 +0100 Subject: [PATCH 3/6] add activity stream for cms content --- activitystream/__init__.py | 0 activitystream/apps.py | 5 + activitystream/authentication.py | 61 +++++++++ activitystream/filters.py | 9 ++ activitystream/helpers.py | 62 ++++++++++ activitystream/pagination.py | 36 ++++++ activitystream/serializers.py | 36 ++++++ activitystream/urls.py | 15 +++ activitystream/views.py | 33 +++++ conf/settings.py | 1 + conf/urls.py | 2 + tests/activitystream/__init__.py | 0 tests/activitystream/test_helpers.py | 29 +++++ tests/activitystream/test_serializers.py | 36 ++++++ tests/activitystream/test_views.py | 150 +++++++++++++++++++++++ tests/conftest.py | 49 +++----- tests/great_international/factories.py | 1 + 17 files changed, 491 insertions(+), 34 deletions(-) create mode 100644 activitystream/__init__.py create mode 100644 activitystream/apps.py create mode 100644 activitystream/authentication.py create mode 100644 activitystream/filters.py create mode 100644 activitystream/helpers.py create mode 100644 activitystream/pagination.py create mode 100644 activitystream/serializers.py create mode 100644 activitystream/urls.py create mode 100644 activitystream/views.py create mode 100644 tests/activitystream/__init__.py create mode 100644 tests/activitystream/test_helpers.py create mode 100644 tests/activitystream/test_serializers.py create mode 100644 tests/activitystream/test_views.py diff --git a/activitystream/__init__.py b/activitystream/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/activitystream/apps.py b/activitystream/apps.py new file mode 100644 index 00000000..55a727c1 --- /dev/null +++ b/activitystream/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class ActivityStreamConfig(AppConfig): + name = 'activitystream' diff --git a/activitystream/authentication.py b/activitystream/authentication.py new file mode 100644 index 00000000..11c95407 --- /dev/null +++ b/activitystream/authentication.py @@ -0,0 +1,61 @@ +import logging + +from mohawk.exc import HawkFail +from rest_framework.authentication import BaseAuthentication +from rest_framework.exceptions import AuthenticationFailed + +from activitystream.helpers import authorise + +logger = logging.getLogger(__name__) +NO_CREDENTIALS_MESSAGE = 'Authentication credentials were not provided.' +INCORRECT_CREDENTIALS_MESSAGE = 'Incorrect authentication credentials.' + + +class ActivityStreamAuthentication(BaseAuthentication): + def authenticate_header(self, request): + """This is returned as the WWW-Authenticate header when + AuthenticationFailed is raised. DRF also requires this + to send a 401 (as opposed to 403) + """ + return 'Hawk' + + def authenticate(self, request): + """Authenticates a request using Hawk signature + + If either of these suggest we cannot authenticate, AuthenticationFailed + is raised, as required in the DRF authentication flow + """ + + return self.authenticate_by_hawk(request) + + def authenticate_by_hawk(self, request): + if 'HTTP_AUTHORIZATION' not in request.META: + raise AuthenticationFailed(NO_CREDENTIALS_MESSAGE) + + # Required to make mohawk auth work locally, and no harm in production, + # so saves having to have it as a temporary monkey-patch + request.META['CONTENT_TYPE'] = b'' + + try: + hawk_receiver = authorise(request) + except HawkFail as e: + logger.warning('Failed authentication {e}'.format(e=e)) + raise AuthenticationFailed(INCORRECT_CREDENTIALS_MESSAGE) + + return (None, hawk_receiver) + + +class ActivityStreamHawkResponseMiddleware: + """Adds the Server-Authorization header to the response, so the originator + of the request can authenticate the response + """ + + def __init__(self, *args, **kwargs): + pass + + def process_response(self, viewset, response): + response['Server-Authorization'] = viewset.request.auth.respond( + content=response.content, + content_type=response['Content-Type'], + ) + return response diff --git a/activitystream/filters.py b/activitystream/filters.py new file mode 100644 index 00000000..cac57728 --- /dev/null +++ b/activitystream/filters.py @@ -0,0 +1,9 @@ +from django_filters import NumberFilter, FilterSet + + +class ActivityStreamCMSContentFilter(FilterSet): + after = NumberFilter(method='filter_after') + + def filter_after(self, queryset, name, value): + value = value or 0 + return queryset.filter(id__gt=value) diff --git a/activitystream/helpers.py b/activitystream/helpers.py new file mode 100644 index 00000000..212746ca --- /dev/null +++ b/activitystream/helpers.py @@ -0,0 +1,62 @@ +import logging + +from django.conf import settings +from django.core.cache import cache +from django.utils.crypto import constant_time_compare +from mohawk import Receiver +from mohawk.exc import HawkFail + +logger = logging.getLogger(__name__) + + +def lookup_credentials(access_key_id): + """Raises a HawkFail if the passed ID is not equal to + settings.ACTIVITY_STREAM_ACCESS_KEY_ID + """ + if not constant_time_compare(access_key_id, settings.ACTIVITY_STREAM_ACCESS_KEY_ID): + raise HawkFail( + 'No Hawk ID of {access_key_id}'.format( + access_key_id=access_key_id, + ) + ) + + return { + 'id': settings.ACTIVITY_STREAM_ACCESS_KEY_ID, + 'key': settings.ACTIVITY_STREAM_SECRET_ACCESS_KEY, + 'algorithm': 'sha256', + } + + +def seen_nonce(access_key_id, nonce, _): + """Returns if the passed access_key_id/nonce combination has been + used within 60 seconds + """ + cache_key = 'activity_stream:{access_key_id}:{nonce}'.format( + access_key_id=access_key_id, + nonce=nonce, + ) + + # cache.add only adds key if it isn't present + seen_cache_key = not cache.add( + cache_key, + True, + timeout=60, + ) + + if seen_cache_key: + logger.warning('Already seen nonce {nonce}'.format(nonce=nonce)) + + return seen_cache_key + + +def authorise(request): + """Raises a HawkFail if the passed request cannot be authenticated""" + return Receiver( + lookup_credentials, + request.META['HTTP_AUTHORIZATION'], + request.build_absolute_uri(), + request.method, + content=request.body, + content_type=request.content_type, + seen_nonce=seen_nonce, + ) diff --git a/activitystream/pagination.py b/activitystream/pagination.py new file mode 100644 index 00000000..1b85eece --- /dev/null +++ b/activitystream/pagination.py @@ -0,0 +1,36 @@ +from rest_framework import pagination +from rest_framework.response import Response +from rest_framework.utils.urls import replace_query_param + + +class ActivityStreamBasePagination(pagination.BasePagination): + page_query_param = 'after' + + def get_paginated_response(self, data): + next_link = self.get_next_link() + return Response( + { + '@context': 'https://www.w3.org/ns/activitystreams', + 'type': 'Collection', + 'orderedItems': data, + **next_link, + } + ) + + def get_next_link(self): + if self.has_next: + url = self.request.build_absolute_uri() + link = replace_query_param(url, self.page_query_param, self.next_value) + return {'next': link} + return {} + + +class ActivityStreamCMSContentPagination(ActivityStreamBasePagination): + page_size = 20 + + def paginate_queryset(self, queryset, request, view=None): + self.has_next = queryset.count() > self.page_size + page = list(queryset[: self.page_size]) + self.next_value = page[-1].id if page else '' + self.request = request + return page diff --git a/activitystream/serializers.py b/activitystream/serializers.py new file mode 100644 index 00000000..3846237b --- /dev/null +++ b/activitystream/serializers.py @@ -0,0 +1,36 @@ +from rest_framework import serializers +from core.cache import PageCache + + +def get_cms_content_for_obj(obj): + result = {} + try: + result = dict( + url=obj.url_path, + seo_title=obj.seo_title, + search_description=obj.search_description, + content=PageCache.get(obj.id, lang='en-gb'), + first_published_at=obj.first_published_at.isoformat(), + last_published_at=obj.last_published_at.isoformat(), + content_type_id=obj.content_type_id, + ) + except Exception as e: + result = {f"Could not parse content for class {obj.specific_class}. Error: {e}"} + + return result + + +class WagtailPageSerializer(serializers.Serializer): + def to_representation(self, obj): + return { + 'id': ('dit:cmsContent:international:' + str(obj.id) + ':Update'), + 'type': 'Update', + 'published': obj.last_published_at.isoformat(), + 'object': { + 'type': 'dit:cmsContent', + 'id': 'dit:cmsContent:international:' + str(obj.id), + 'name': obj.title, + 'url': obj.url_path, + 'content': get_cms_content_for_obj(obj), + }, + } diff --git a/activitystream/urls.py b/activitystream/urls.py new file mode 100644 index 00000000..2e2c68c3 --- /dev/null +++ b/activitystream/urls.py @@ -0,0 +1,15 @@ +from django.urls import path + +# from great_components.decorators import skip_ga360 + +import activitystream.views + +app_name = 'activitystream' + +urlpatterns = [ + path( + r'cms-content/', + activitystream.views.CMSContentActivityStreamView.as_view(), + name='cms-content', + ), +] diff --git a/activitystream/views.py b/activitystream/views.py new file mode 100644 index 00000000..bff37769 --- /dev/null +++ b/activitystream/views.py @@ -0,0 +1,33 @@ +from rest_framework.generics import ListAPIView + +from activitystream.authentication import ( + ActivityStreamAuthentication, + ActivityStreamHawkResponseMiddleware, +) +from django.utils.decorators import decorator_from_middleware +from wagtail.models import Page +import django_filters.rest_framework +from activitystream.serializers import WagtailPageSerializer +from activitystream.pagination import ActivityStreamCMSContentPagination +from activitystream.filters import ActivityStreamCMSContentFilter + + +class ActivityStreamBaseView(ListAPIView): + authentication_classes = (ActivityStreamAuthentication,) + permission_classes = () + filter_backends = [django_filters.rest_framework.DjangoFilterBackend] + + @decorator_from_middleware(ActivityStreamHawkResponseMiddleware) + def list(self, request, *args, **kwargs): + """A single page of activities to be consumed by activity stream.""" + return super().list(request, *args, **kwargs) + + +class CMSContentActivityStreamView(ActivityStreamBaseView): + queryset = Page.objects.all().filter(live=True) + serializer_class = WagtailPageSerializer + pagination_class = ActivityStreamCMSContentPagination + filterset_class = ActivityStreamCMSContentFilter + + def get_queryset(self): + return self.queryset.order_by('id') diff --git a/conf/settings.py b/conf/settings.py index 1967ad6c..5925a573 100644 --- a/conf/settings.py +++ b/conf/settings.py @@ -91,6 +91,7 @@ 'django_celery_beat', 'drf_spectacular', 'wagtailmarkdown', + 'activitystream.apps.ActivityStreamConfig', ] MIDDLEWARE = [ diff --git a/conf/urls.py b/conf/urls.py index b6aa7802..2ff47a41 100644 --- a/conf/urls.py +++ b/conf/urls.py @@ -22,6 +22,7 @@ import core.views from groups.views import GroupInfoModalView +import activitystream.urls api_router = WagtailAPIRouter('api') api_router.register_endpoint('pages', core.views.PagesOptionalDraftAPIEndpoint) @@ -50,6 +51,7 @@ urlpatterns = [ + path('activity-stream/', include(activitystream.urls, namespace='activitystream')), re_path(r'^django-admin/', admin.site.urls), re_path(r'^api/', include((api_urls, 'api'))), re_path(r'^healthcheck/', include((healthcheck_urls, 'healthcheck'))), diff --git a/tests/activitystream/__init__.py b/tests/activitystream/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/activitystream/test_helpers.py b/tests/activitystream/test_helpers.py new file mode 100644 index 00000000..d187d57b --- /dev/null +++ b/tests/activitystream/test_helpers.py @@ -0,0 +1,29 @@ +import pytest +from django.test import override_settings +from mohawk.exc import HawkFail + +from activitystream.helpers import lookup_credentials, seen_nonce + + +@override_settings(ACTIVITY_STREAM_ACCESS_KEY_ID='good-key') +def test_lookup_credentials__mismatching_key(): + with pytest.raises(HawkFail): + lookup_credentials('bad-key') + + +@override_settings( + CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'KEY_PREFIX': 'test', + } + }, +) +def test_seen_nonce__seen_before(caplog): + assert not seen_nonce('access-key', '123-nonce-value', None) + assert not caplog.records + + assert seen_nonce('access-key', '123-nonce-value', None) + assert len(caplog.records) == 1 + assert caplog.records[0].message == 'Already seen nonce 123-nonce-value' + assert caplog.records[0].levelname == 'WARNING' diff --git a/tests/activitystream/test_serializers.py b/tests/activitystream/test_serializers.py new file mode 100644 index 00000000..729ca1ab --- /dev/null +++ b/tests/activitystream/test_serializers.py @@ -0,0 +1,36 @@ +import pytest + +from activitystream.serializers import WagtailPageSerializer + + +from tests.great_international.factories import InternationalArticlePageFactory + + +@pytest.mark.django_db +def test_wagtail_page_serializer(): + article = InternationalArticlePageFactory() + + serialized_article = WagtailPageSerializer().to_representation(article) + + # serialized_article.object.content.content is None because the page is not available in the Redis cache. + # Serializing of actual article content / caching is covered by other unit tests. + assert serialized_article == { + 'id': ('dit:cmsContent:international:' + str(article.id) + ':Update'), + 'type': 'Update', + 'published': article.last_published_at.isoformat(), + 'object': { + 'type': 'dit:cmsContent', + 'id': 'dit:cmsContent:international:' + str(article.id), + 'name': article.title, + 'url': article.url_path, + 'content': { + 'url': article.url_path, + 'seo_title': article.seo_title, + 'search_description': article.search_description, + 'content': None, + 'first_published_at': article.first_published_at.isoformat(), + 'last_published_at': article.last_published_at.isoformat(), + 'content_type_id': article.content_type_id, + }, + }, + } diff --git a/tests/activitystream/test_views.py b/tests/activitystream/test_views.py new file mode 100644 index 00000000..72c8967d --- /dev/null +++ b/tests/activitystream/test_views.py @@ -0,0 +1,150 @@ +import datetime +import mohawk +import pytest +from django.conf import settings +from django.utils import timezone +from freezegun import freeze_time +from rest_framework import status +from rest_framework.reverse import reverse +from rest_framework.test import APIClient + +URL = 'http://testserver' + reverse('activitystream:cms-content') +URL_INCORRECT_DOMAIN = 'http://incorrect' + reverse('activitystream:cms-content') +URL_INCORRECT_PATH = 'http://testserver' + reverse('activitystream:cms-content') + 'incorrect/' +EMPTY_COLLECTION = { + '@context': 'https://www.w3.org/ns/activitystreams', + 'type': 'Collection', + 'orderedItems': [], +} + + +@pytest.fixture +def api_client(): + return APIClient() + + +def article_attribute(activity, attribute): + return activity['object'][attribute] + + +def auth_sender( + key_id=settings.ACTIVITY_STREAM_ACCESS_KEY_ID, + secret_key=settings.ACTIVITY_STREAM_SECRET_ACCESS_KEY, + url=URL, + method='GET', + content='', + content_type='', +): + credentials = { + 'id': key_id, + 'key': secret_key, + 'algorithm': 'sha256', + } + + return mohawk.Sender( + credentials, + url, + method, + content=content, + content_type=content_type, + ) + + +@pytest.mark.django_db +def test_site_pages_returned_with_authentication(api_client, en_locale): + """If the Authorization and X-Forwarded-For headers are correct, then + the correct, and authentic, data is returned + """ + sender = auth_sender() + response = api_client.get( + URL, + content_type='', + HTTP_AUTHORIZATION=sender.request_header, + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == status.HTTP_200_OK + body = response.json() + assert body['@context'] == 'https://www.w3.org/ns/activitystreams' + assert body['type'] == 'Collection' + assert len(body['orderedItems']) == 3 + + # sender.accept_response will raise an error if the + # inputs are not valid + sender.accept_response( + response_header=response['Server-Authorization'], + content=response.content, + content_type=response['Content-Type'], + ) + with pytest.raises(mohawk.exc.MacMismatch): + sender.accept_response( + response_header=( + response['Server-Authorization'][:-12] + 'incorrect' + response['Server-Authorization'][-3:] + ), + content=response.content, + content_type=response['Content-Type'], + ) + with pytest.raises(mohawk.exc.BadHeaderValue): + sender.accept_response( + response_header=response['Server-Authorization'] + 'incorrect', + content=response.content, + content_type=response['Content-Type'], + ) + with pytest.raises(mohawk.exc.MisComputedContentHash): + sender.accept_response( + response_header=response['Server-Authorization'], + content='incorrect', + content_type=response['Content-Type'], + ) + with pytest.raises(mohawk.exc.MisComputedContentHash): + sender.accept_response( + response_header=response['Server-Authorization'], + content=response.content, + content_type='incorrect', + ) + + +@pytest.mark.django_db +def test_authentication_fails_if_url_mismatched(api_client, en_locale): + """Creates a Hawk header with incorrect domain""" + sender = auth_sender(url=URL_INCORRECT_DOMAIN) + response = api_client.get( + URL, + content_type='', + HTTP_AUTHORIZATION=sender.request_header, + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + """Creates a Hawk header with incorrect path""" + sender = auth_sender(url=URL_INCORRECT_PATH) + response = api_client.get( + URL, + content_type='', + HTTP_AUTHORIZATION=sender.request_header, + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.django_db +def test_if_61_seconds_in_past_401_returned(api_client, en_locale): + """If the Authorization header is generated 61 seconds in the past, then a + 401 is returned + """ + past = timezone.now() - datetime.timedelta(seconds=61) + with freeze_time(past): + auth = auth_sender().request_header + + response = api_client.get( + reverse('activitystream:cms-content'), + content_type='', + HTTP_AUTHORIZATION=auth, + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + error = {'detail': 'Incorrect authentication credentials.'} + assert response.json() == error diff --git a/tests/conftest.py b/tests/conftest.py index 0e6bd68c..f59c7414 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,6 +16,7 @@ from django.core.files.storage import default_storage from django.core.files.uploadedfile import SimpleUploadedFile from django.utils import translation +from datetime import datetime from core.models import RoutingSettings from groups.models import GroupInfo @@ -24,10 +25,8 @@ from .users.factories import UserFactory -@pytest.mark.django_db @pytest.fixture() -def en_locale(): - # Equivalent for unittest is in tests.core.helpers.SetUpLocaleMixin +def en_locale(transactional_db): return Locale.objects.get_or_create(language_code='en-gb') @@ -35,10 +34,7 @@ def en_locale(): def wagtail_initial_data(request, en_locale): if not request.node.get_closest_marker('django_db'): return - page_content_type, _ = ContentType.objects.get_or_create( - model='page', - app_label='wagtailcore' - ) + page_content_type, _ = ContentType.objects.get_or_create(model='page', app_label='wagtailcore') locale = Locale.objects.get(language_code='en-gb') root, _ = Page.objects.get_or_create( @@ -52,7 +48,8 @@ def wagtail_initial_data(request, en_locale): numchild=1, locale=locale, url_path='/', - ) + last_published_at=datetime.now(), + ), ) Page.objects.get_or_create( slug='home', @@ -65,17 +62,13 @@ def wagtail_initial_data(request, en_locale): numchild=0, url_path='/home/', locale=locale, - ) + last_published_at=datetime.now(), + ), ) - wagtailadmin_content_type, _ = ContentType.objects.get_or_create( - app_label='wagtailadmin', - model='admin' - ) + wagtailadmin_content_type, _ = ContentType.objects.get_or_create(app_label='wagtailadmin', model='admin') admin_permission, created = Permission.objects.get_or_create( - content_type=wagtailadmin_content_type, - codename='access_admin', - name='Can access Wagtail admin' + content_type=wagtailadmin_content_type, codename='access_admin', name='Can access Wagtail admin' ) moderators_group = Group.objects.create(name='Moderators') @@ -151,9 +144,7 @@ def enable_signature_check(mock_signature_check): @pytest.fixture def uploaded_file(): return SimpleUploadedFile( - name='test_image.png', - content=open('core/static/core/logo.png', 'rb').read(), - content_type='image/png' + name='test_image.png', content=open('core/static/core/logo.png', 'rb').read(), content_type='image/png' ) @@ -196,6 +187,7 @@ def feature_flags(settings): @pytest.fixture def groups_with_info(): from django.contrib.auth.models import Group + groups = [] for i, group in enumerate(Group.objects.select_related('info').all(), 1): try: @@ -268,17 +260,9 @@ def international_site(international_root_page, request): return site, created = Site.objects.get_or_create( - port=80, - hostname='great.gov.uk', - defaults={ - 'root_page': international_root_page, - 'site_name': 'international' - } - ) - RoutingSettings.objects.get_or_create( - site=site, - defaults={'root_path_prefix': '/international/content/'} + port=80, hostname='great.gov.uk', defaults={'root_page': international_root_page, 'site_name': 'international'} ) + RoutingSettings.objects.get_or_create(site=site, defaults={'root_path_prefix': '/international/content/'}) return site @@ -324,7 +308,7 @@ def site_with_translated_page_as_root(translated_page): @pytest.fixture def page_with_reversion(admin_user, translated_page): - translated_page.title = 'published-title', + translated_page.title = ('published-title',) translated_page.title_en_gb = 'published-title' translated_page.save() @@ -348,10 +332,7 @@ def site_with_revised_page_as_root(page_with_reversion): @pytest.fixture def page(international_root_page): - return InternationalArticlePageFactory( - parent=international_root_page, - slug='the-slug' - ) + return InternationalArticlePageFactory(parent=international_root_page, slug='the-slug') @pytest.fixture() diff --git a/tests/great_international/factories.py b/tests/great_international/factories.py index 47effd7a..1a09bdac 100644 --- a/tests/great_international/factories.py +++ b/tests/great_international/factories.py @@ -58,6 +58,7 @@ class Meta: slug = factory.Sequence(lambda n: '123-555-{0}'.format(n)) title_en_gb = factory.Sequence(lambda n: '123-555-{0}'.format(n)) last_published_at = timezone.now() + first_published_at = timezone.now() parent = None From 0fd650ec11fcca8ca3beae787d9b67079d5381d4 Mon Sep 17 00:00:00 2001 From: Timothy Patterson Date: Thu, 28 Sep 2023 14:31:04 +0100 Subject: [PATCH 4/6] fix tests --- tests/activitystream/test_helpers.py | 2 ++ tests/conftest.py | 13 ++++++++----- tests/great_international/factories.py | 1 + 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/activitystream/test_helpers.py b/tests/activitystream/test_helpers.py index d187d57b..4d944430 100644 --- a/tests/activitystream/test_helpers.py +++ b/tests/activitystream/test_helpers.py @@ -5,6 +5,7 @@ from activitystream.helpers import lookup_credentials, seen_nonce +@pytest.mark.django_db @override_settings(ACTIVITY_STREAM_ACCESS_KEY_ID='good-key') def test_lookup_credentials__mismatching_key(): with pytest.raises(HawkFail): @@ -19,6 +20,7 @@ def test_lookup_credentials__mismatching_key(): } }, ) +@pytest.mark.django_db def test_seen_nonce__seen_before(caplog): assert not seen_nonce('access-key', '123-nonce-value', None) assert not caplog.records diff --git a/tests/conftest.py b/tests/conftest.py index f59c7414..80ee2aff 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,8 +15,7 @@ from django.core.files.base import ContentFile from django.core.files.storage import default_storage from django.core.files.uploadedfile import SimpleUploadedFile -from django.utils import translation -from datetime import datetime +from django.utils import translation, timezone from core.models import RoutingSettings from groups.models import GroupInfo @@ -25,8 +24,10 @@ from .users.factories import UserFactory +@pytest.mark.django_db @pytest.fixture() -def en_locale(transactional_db): +def en_locale(): + # Equivalent for unittest is in tests.core.helpers.SetUpLocaleMixin return Locale.objects.get_or_create(language_code='en-gb') @@ -48,7 +49,8 @@ def wagtail_initial_data(request, en_locale): numchild=1, locale=locale, url_path='/', - last_published_at=datetime.now(), + last_published_at=timezone.now(), + first_published_at=timezone.now(), ), ) Page.objects.get_or_create( @@ -62,7 +64,8 @@ def wagtail_initial_data(request, en_locale): numchild=0, url_path='/home/', locale=locale, - last_published_at=datetime.now(), + last_published_at=timezone.now(), + first_published_at=timezone.now(), ), ) diff --git a/tests/great_international/factories.py b/tests/great_international/factories.py index 1a09bdac..e1751728 100644 --- a/tests/great_international/factories.py +++ b/tests/great_international/factories.py @@ -17,6 +17,7 @@ class Meta: slug = factory.Sequence(lambda n: '123-555-{0}'.format(n)) title_en_gb = factory.Sequence(lambda n: '123-555-{0}'.format(n)) last_published_at = timezone.now() + first_published_at = timezone.now() parent = None hero_title = factory.fuzzy.FuzzyText(length=10) From 2db0d49e400dd50621e73afd8ce41ee727e019d1 Mon Sep 17 00:00:00 2001 From: Timothy Patterson Date: Thu, 28 Sep 2023 16:45:47 +0100 Subject: [PATCH 5/6] increase codecov, flatten contents of object key in WagtailPageSerializer --- activitystream/filters.py | 5 +++ activitystream/serializers.py | 44 +++++++++++------------- activitystream/urls.py | 2 -- activitystream/views.py | 3 +- tests/activitystream/test_filters.py | 13 +++++++ tests/activitystream/test_pagination.py | 17 +++++++++ tests/activitystream/test_serializers.py | 43 ++++++++++++++--------- tests/activitystream/test_views.py | 12 +++++++ 8 files changed, 96 insertions(+), 43 deletions(-) create mode 100644 tests/activitystream/test_filters.py create mode 100644 tests/activitystream/test_pagination.py diff --git a/activitystream/filters.py b/activitystream/filters.py index cac57728..e5ef4bae 100644 --- a/activitystream/filters.py +++ b/activitystream/filters.py @@ -1,4 +1,5 @@ from django_filters import NumberFilter, FilterSet +from wagtail.models import Page class ActivityStreamCMSContentFilter(FilterSet): @@ -7,3 +8,7 @@ class ActivityStreamCMSContentFilter(FilterSet): def filter_after(self, queryset, name, value): value = value or 0 return queryset.filter(id__gt=value) + + class Meta: + model = Page + fields = ['id'] diff --git a/activitystream/serializers.py b/activitystream/serializers.py index 3846237b..e3333fd9 100644 --- a/activitystream/serializers.py +++ b/activitystream/serializers.py @@ -1,36 +1,32 @@ from rest_framework import serializers from core.cache import PageCache +from django.conf import settings -def get_cms_content_for_obj(obj): - result = {} - try: - result = dict( - url=obj.url_path, - seo_title=obj.seo_title, - search_description=obj.search_description, - content=PageCache.get(obj.id, lang='en-gb'), - first_published_at=obj.first_published_at.isoformat(), - last_published_at=obj.last_published_at.isoformat(), - content_type_id=obj.content_type_id, - ) - except Exception as e: - result = {f"Could not parse content for class {obj.specific_class}. Error: {e}"} - - return result +class WagtailPageSerializer(serializers.Serializer): + def get_cms_content_for_obj(self, obj): + try: + result = dict( + id='dit:cmsContent:international:' + str(obj.id), + type='dit:cmsContent', + title=obj.title, + url=settings.APP_URL_GREAT_INTERNATIONAL+'content'+obj.url, + seo_title=obj.seo_title, + search_description=obj.search_description, + first_published_at=obj.first_published_at.isoformat(), + last_published_at=obj.last_published_at.isoformat(), + content_type_id=obj.content_type_id, + content=PageCache.get(obj.id, lang='en-gb'), + ) + except Exception as e: + result = {"error": f"Could not parse content for class {obj.specific_class}. Error: {e}"} + return result -class WagtailPageSerializer(serializers.Serializer): def to_representation(self, obj): return { 'id': ('dit:cmsContent:international:' + str(obj.id) + ':Update'), 'type': 'Update', 'published': obj.last_published_at.isoformat(), - 'object': { - 'type': 'dit:cmsContent', - 'id': 'dit:cmsContent:international:' + str(obj.id), - 'name': obj.title, - 'url': obj.url_path, - 'content': get_cms_content_for_obj(obj), - }, + 'object': self.get_cms_content_for_obj(obj), } diff --git a/activitystream/urls.py b/activitystream/urls.py index 2e2c68c3..54596f5a 100644 --- a/activitystream/urls.py +++ b/activitystream/urls.py @@ -1,7 +1,5 @@ from django.urls import path -# from great_components.decorators import skip_ga360 - import activitystream.views app_name = 'activitystream' diff --git a/activitystream/views.py b/activitystream/views.py index bff37769..d42c4379 100644 --- a/activitystream/views.py +++ b/activitystream/views.py @@ -5,6 +5,7 @@ ActivityStreamHawkResponseMiddleware, ) from django.utils.decorators import decorator_from_middleware +from django.db.models import Q from wagtail.models import Page import django_filters.rest_framework from activitystream.serializers import WagtailPageSerializer @@ -24,7 +25,7 @@ def list(self, request, *args, **kwargs): class CMSContentActivityStreamView(ActivityStreamBaseView): - queryset = Page.objects.all().filter(live=True) + queryset = Page.objects.exclude(Q(live=False) | Q(first_published_at__isnull=True) | Q(last_published_at__isnull=True)) # noqa:E501 serializer_class = WagtailPageSerializer pagination_class = ActivityStreamCMSContentPagination filterset_class = ActivityStreamCMSContentFilter diff --git a/tests/activitystream/test_filters.py b/tests/activitystream/test_filters.py new file mode 100644 index 00000000..7b32b2f6 --- /dev/null +++ b/tests/activitystream/test_filters.py @@ -0,0 +1,13 @@ +import pytest +from activitystream.filters import ActivityStreamCMSContentFilter +from wagtail.models import Page + + +@pytest.mark.django_db +def test_cms_content_filter(international_site): + queryset = Page.objects.all() + assert queryset.count() == 3 + filtered = ActivityStreamCMSContentFilter().filter_after(queryset, '', 1) + assert filtered.count() == 2 + assert filtered[0].id == 2 + assert filtered[1].id == 3 diff --git a/tests/activitystream/test_pagination.py b/tests/activitystream/test_pagination.py new file mode 100644 index 00000000..f0f980fd --- /dev/null +++ b/tests/activitystream/test_pagination.py @@ -0,0 +1,17 @@ +import pytest +from activitystream.pagination import ActivityStreamCMSContentPagination +from wagtail.models import Page +from unittest.mock import MagicMock + + +@pytest.mark.django_db +def test_get_next_link(international_site, monkeypatch): + monkeypatch.setattr(ActivityStreamCMSContentPagination, 'page_size', 2) + request = MagicMock() + + queryset = Page.objects.all() + pagination_instance = ActivityStreamCMSContentPagination() + pagination_instance.paginate_queryset(queryset, request) + next_link = pagination_instance.get_next_link() + + assert '?after=2' in next_link['next'] diff --git a/tests/activitystream/test_serializers.py b/tests/activitystream/test_serializers.py index 729ca1ab..ba92b005 100644 --- a/tests/activitystream/test_serializers.py +++ b/tests/activitystream/test_serializers.py @@ -1,4 +1,6 @@ import pytest +from wagtail.models import Page +from django.conf import settings from activitystream.serializers import WagtailPageSerializer @@ -7,30 +9,39 @@ @pytest.mark.django_db -def test_wagtail_page_serializer(): - article = InternationalArticlePageFactory() +def test_wagtail_page_serializer(international_site): + article = InternationalArticlePageFactory(parent=international_site.root_page) serialized_article = WagtailPageSerializer().to_representation(article) - - # serialized_article.object.content.content is None because the page is not available in the Redis cache. + # serialized_article.object.content is None because the page is not available in the Redis cache. # Serializing of actual article content / caching is covered by other unit tests. assert serialized_article == { 'id': ('dit:cmsContent:international:' + str(article.id) + ':Update'), 'type': 'Update', 'published': article.last_published_at.isoformat(), 'object': { - 'type': 'dit:cmsContent', 'id': 'dit:cmsContent:international:' + str(article.id), - 'name': article.title, - 'url': article.url_path, - 'content': { - 'url': article.url_path, - 'seo_title': article.seo_title, - 'search_description': article.search_description, - 'content': None, - 'first_published_at': article.first_published_at.isoformat(), - 'last_published_at': article.last_published_at.isoformat(), - 'content_type_id': article.content_type_id, - }, + 'type': 'dit:cmsContent', + 'title': article.title, + 'url': settings.APP_URL_GREAT_INTERNATIONAL+'content'+article.url, + 'seo_title': article.seo_title, + 'search_description': article.search_description, + 'first_published_at': article.first_published_at.isoformat(), + 'last_published_at': article.last_published_at.isoformat(), + 'content_type_id': article.content_type_id, + 'content': None, }, } + + +@pytest.mark.django_db +def test_error_thrown_for_invalid_page(international_site): + page = Page.objects.get(id=3) + invalid_page = page.copy() + invalid_page.id = None + invalid_page.title = None + invalid_page.first_published_at = None + + serialized_article = WagtailPageSerializer().to_representation(invalid_page) + + assert 'Could not parse content for class' in serialized_article['object']['error'] diff --git a/tests/activitystream/test_views.py b/tests/activitystream/test_views.py index 72c8967d..d2451689 100644 --- a/tests/activitystream/test_views.py +++ b/tests/activitystream/test_views.py @@ -7,6 +7,7 @@ from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APIClient +from activitystream.authentication import NO_CREDENTIALS_MESSAGE URL = 'http://testserver' + reverse('activitystream:cms-content') URL_INCORRECT_DOMAIN = 'http://incorrect' + reverse('activitystream:cms-content') @@ -148,3 +149,14 @@ def test_if_61_seconds_in_past_401_returned(api_client, en_locale): assert response.status_code == status.HTTP_401_UNAUTHORIZED error = {'detail': 'Incorrect authentication credentials.'} assert response.json() == error + + +@pytest.mark.django_db +def test_error_for_no_authorization_field_in_header(api_client): + response = api_client.get( + URL, + content_type='', + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + assert response.status_code == 401 + assert response.json()['detail'] == NO_CREDENTIALS_MESSAGE From 400c6c4c1cfd6ab096050acd928860ed33d569d5 Mon Sep 17 00:00:00 2001 From: Timothy Patterson Date: Mon, 2 Oct 2023 11:13:26 +0100 Subject: [PATCH 6/6] change returned object to string --- activitystream/serializers.py | 2 +- tests/activitystream/test_serializers.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/activitystream/serializers.py b/activitystream/serializers.py index e3333fd9..baf2442e 100644 --- a/activitystream/serializers.py +++ b/activitystream/serializers.py @@ -16,7 +16,7 @@ def get_cms_content_for_obj(self, obj): first_published_at=obj.first_published_at.isoformat(), last_published_at=obj.last_published_at.isoformat(), content_type_id=obj.content_type_id, - content=PageCache.get(obj.id, lang='en-gb'), + content=f"{PageCache.get(obj.id, lang='en-gb')}", ) except Exception as e: result = {"error": f"Could not parse content for class {obj.specific_class}. Error: {e}"} diff --git a/tests/activitystream/test_serializers.py b/tests/activitystream/test_serializers.py index ba92b005..84ce8919 100644 --- a/tests/activitystream/test_serializers.py +++ b/tests/activitystream/test_serializers.py @@ -13,7 +13,7 @@ def test_wagtail_page_serializer(international_site): article = InternationalArticlePageFactory(parent=international_site.root_page) serialized_article = WagtailPageSerializer().to_representation(article) - # serialized_article.object.content is None because the page is not available in the Redis cache. + # serialized_article.object.content is "None" because the page is not available in the Redis cache. # Serializing of actual article content / caching is covered by other unit tests. assert serialized_article == { 'id': ('dit:cmsContent:international:' + str(article.id) + ':Update'), @@ -29,7 +29,7 @@ def test_wagtail_page_serializer(international_site): 'first_published_at': article.first_published_at.isoformat(), 'last_published_at': article.last_published_at.isoformat(), 'content_type_id': article.content_type_id, - 'content': None, + 'content': "None", }, }