diff --git a/swh/web/auth/mailmap.py b/swh/web/auth/mailmap.py index 0bed6653..28e5cd08 100644 --- a/swh/web/auth/mailmap.py +++ b/swh/web/auth/mailmap.py @@ -1,97 +1,116 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +import json + from django.conf.urls import url from django.db import IntegrityError from django.db.models import Q from django.http.response import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound, ) from rest_framework import serializers from rest_framework.decorators import api_view from rest_framework.request import Request from rest_framework.response import Response -from swh.web.auth.models import UserMailmap +from swh.web.auth.models import UserMailmap, UserMailmapEvent from swh.web.auth.utils import MAILMAP_PERMISSION class UserMailmapSerializer(serializers.ModelSerializer): class Meta: model = UserMailmap fields = "__all__" @api_view(["GET"]) def profile_list_mailmap(request: Request) -> HttpResponse: if not request.user.has_perm(MAILMAP_PERMISSION): return HttpResponseForbidden() mms = UserMailmap.objects.filter(user_id=str(request.user.id),).all() return Response(UserMailmapSerializer(mms, many=True).data) @api_view(["POST"]) def profile_add_mailmap(request: Request) -> HttpResponse: if not request.user.has_perm(MAILMAP_PERMISSION): return HttpResponseForbidden() + event = UserMailmapEvent.objects.create( + user_id=str(request.user.id), + request_type="add", + request=json.dumps(request.data), + ) + from_email = request.data.pop("from_email", None) if not from_email: return HttpResponseBadRequest("'from_email' must be provided and non-empty.") try: UserMailmap.objects.create( user_id=str(request.user.id), from_email=from_email, **request.data ) except IntegrityError: return HttpResponseBadRequest("This 'from_email' already exists.") - mm = UserMailmap.objects.get( - user_id=str(request.user.id), from_email=from_email - ) + + event.successful = True + event.save() + + mm = UserMailmap.objects.get(user_id=str(request.user.id), from_email=from_email) return Response(UserMailmapSerializer(mm).data) @api_view(["POST"]) def profile_update_mailmap(request: Request) -> HttpResponse: if not request.user.has_perm(MAILMAP_PERMISSION): return HttpResponseForbidden() + event = UserMailmapEvent.objects.create( + user_id=str(request.user.id), + request_type="update", + request=json.dumps(request.data), + ) + from_email = request.data.pop("from_email", None) if not from_email: return HttpResponseBadRequest("'from_email' must be provided and non-empty.") user_id = str(request.user.id) try: to_update = ( UserMailmap.objects.filter(Q(user_id__isnull=True) | Q(user_id=user_id)) .filter(from_email=from_email) .get() ) except UserMailmap.DoesNotExist: return HttpResponseNotFound() for attr, value in request.data.items(): setattr(to_update, attr, value) to_update.save() + event.successful = True + event.save() + mm = UserMailmap.objects.get(user_id=user_id, from_email=from_email) return Response(UserMailmapSerializer(mm).data) urlpatterns = [ url(r"^profile/mailmap/list$", profile_list_mailmap, name="profile-mailmap-list",), url(r"^profile/mailmap/add$", profile_add_mailmap, name="profile-mailmap-add",), url( r"^profile/mailmap/update$", profile_update_mailmap, name="profile-mailmap-update", ), ] diff --git a/swh/web/auth/migrations/0005_usermailmapevent.py b/swh/web/auth/migrations/0005_usermailmapevent.py new file mode 100644 index 00000000..d256eff8 --- /dev/null +++ b/swh/web/auth/migrations/0005_usermailmapevent.py @@ -0,0 +1,39 @@ +# Generated by Django 2.2.27 on 2022-02-07 16:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("swh_web_auth", "0004_usermailmap"), + ] + + operations = [ + migrations.CreateModel( + name="UserMailmapEvent", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("timestamp", models.DateTimeField(auto_now=True)), + ("user_id", models.CharField(max_length=50)), + ("request_type", models.CharField(max_length=50)), + ("request", models.TextField()), + ("successful", models.BooleanField(default=False)), + ], + options={"db_table": "user_mailmap_event",}, + ), + migrations.AddIndex( + model_name="usermailmapevent", + index=models.Index( + fields=["timestamp"], name="user_mailma_timesta_1f7aef_idx" + ), + ), + ] diff --git a/swh/web/auth/models.py b/swh/web/auth/models.py index 6d9b1b47..331fe6cc 100644 --- a/swh/web/auth/models.py +++ b/swh/web/auth/models.py @@ -1,93 +1,123 @@ # Copyright (C) 2020-2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information from django.db import models class OIDCUserOfflineTokens(models.Model): """ Model storing encrypted bearer tokens generated by users. """ user_id = models.CharField(max_length=50) creation_date = models.DateTimeField(auto_now_add=True) offline_token = models.BinaryField() class Meta: app_label = "swh_web_auth" db_table = "oidc_user_offline_tokens" class UserMailmapManager(models.Manager): """A queryset manager which defers all :class:`models.DateTimeField` fields, to avoid resetting them to an old value involuntarily.""" @classmethod def deferred_fields(cls): try: return cls._deferred_fields except AttributeError: cls._deferred_fields = [ field.name for field in UserMailmap._meta.get_fields() if isinstance(field, models.DateTimeField) and not field.auto_now ] return cls._deferred_fields def get_queryset(self): return super().get_queryset().defer(*self.deferred_fields()) class UserMailmap(models.Model): """ Model storing mailmap settings submitted by users. """ user_id = models.CharField(max_length=50, null=True) """Optional user id from Keycloak""" from_email = models.TextField(unique=True, null=False) """Email address to find author in the archive""" from_email_verified = models.BooleanField(default=False) """Indicates if the from email has been verified""" from_email_verification_request_date = models.DateTimeField(null=True) """Last from email verification request date""" display_name = models.TextField(null=False) """Display name to use for the author instead of the archived one""" display_name_activated = models.BooleanField(default=False) """Indicates if the new display name should be used""" to_email = models.TextField(null=True) """Optional new email to use in the display name instead of the archived one""" to_email_verified = models.BooleanField(default=False) """Indicates if the to email has been verified""" to_email_verification_request_date = models.DateTimeField(null=True) """Last to email verification request date""" mailmap_last_processing_date = models.DateTimeField(null=True) """Last mailmap synchronisation date with swh-storage""" last_update_date = models.DateTimeField(auto_now=True) """Last date that mailmap model was updated""" class Meta: app_label = "swh_web_auth" db_table = "user_mailmap" # Defer _date fields by default to avoid updating them by mistake objects = UserMailmapManager() @property def full_display_name(self) -> str: if self.to_email is not None and self.to_email_verified: return "%s <%s>" % (self.display_name, self.to_email) else: return self.display_name + + +class UserMailmapEvent(models.Model): + """ + Represents an update to a mailmap object + """ + + timestamp = models.DateTimeField(auto_now=True, null=False) + """Timestamp of the moment the event was submitted""" + + user_id = models.CharField(max_length=50, null=False) + """User id from Keycloak of the user who changed the mailmap. + (Not necessarily the one who the mail belongs to.)""" + + request_type = models.CharField(max_length=50, null=False) + """Either ``add`` or ``update``.""" + + request = models.TextField(null=False) + """JSON dump of the request received.""" + + successful = models.BooleanField(default=False, null=False) + """If False, then the request failed or crashed before completing, + and may or may not have altered the database's state.""" + + class Meta: + indexes = [ + models.Index(fields=["timestamp"]), + ] + app_label = "swh_web_auth" + db_table = "user_mailmap_event" diff --git a/swh/web/tests/auth/test_mailmap.py b/swh/web/tests/auth/test_mailmap.py index b3c67250..75278e21 100644 --- a/swh/web/tests/auth/test_mailmap.py +++ b/swh/web/tests/auth/test_mailmap.py @@ -1,410 +1,441 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information import datetime +import json from io import StringIO from typing import Dict from psycopg2.extras import execute_values import pytest from django.core.management import call_command from django.db import transaction from swh.model.model import Person -from swh.web.auth.models import UserMailmap +from swh.web.auth.models import UserMailmap, UserMailmapEvent from swh.web.auth.utils import MAILMAP_PERMISSION from swh.web.common.utils import reverse from swh.web.tests.utils import ( check_api_post_response, check_http_get_response, create_django_permission, ) @pytest.fixture def mailmap_user(regular_user): regular_user.user_permissions.add(create_django_permission(MAILMAP_PERMISSION)) return regular_user @pytest.mark.django_db(transaction=True) @pytest.mark.parametrize("view_name", ["profile-mailmap-add", "profile-mailmap-update"]) def test_mailmap_endpoints_anonymous_user(api_client, view_name): url = reverse(view_name) check_api_post_response(api_client, url, status_code=403) @pytest.mark.django_db(transaction=True) def test_mailmap_endpoints_user_with_permission(api_client, mailmap_user): api_client.force_login(mailmap_user) + + request_data = {"from_email": "bar@example.org", "display_name": "bar"} + for view_name in ("profile-mailmap-add", "profile-mailmap-update"): url = reverse(view_name) check_api_post_response( - api_client, - url, - data={"from_email": "bar@example.org", "display_name": "bar"}, - status_code=200, + api_client, url, data=request_data, status_code=200, ) # FIXME: use check_api_get_responses; currently this crashes without # content_type="application/json" resp = check_http_get_response( api_client, reverse("profile-mailmap-list"), status_code=200, content_type="application/json", ).data assert len(resp) == 1 assert resp[0]["from_email"] == "bar@example.org" assert resp[0]["display_name"] == "bar" + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 2 + assert events[0].request_type == "add" + assert json.loads(events[0].request) == request_data + assert events[1].request_type == "update" + assert json.loads(events[1].request) == request_data + @pytest.mark.django_db(transaction=True) def test_mailmap_add_duplicate(api_client, mailmap_user): api_client.force_login(mailmap_user) check_api_post_response( api_client, reverse("profile-mailmap-add"), data={"from_email": "foo@example.org", "display_name": "bar"}, status_code=200, ) check_api_post_response( api_client, reverse("profile-mailmap-add"), data={"from_email": "foo@example.org", "display_name": "baz"}, status_code=400, ) @pytest.mark.django_db(transaction=True) def test_mailmap_add_full(api_client, mailmap_user): api_client.force_login(mailmap_user) request_data = { "from_email": "foo@example.org", "from_email_verified": True, "from_email_verification_request_date": "2021-02-07T14:04:15Z", "display_name": "bar", "display_name_activated": True, "to_email": "bar@example.org", "to_email_verified": True, "to_email_verification_request_date": "2021-02-07T15:54:59Z", } check_api_post_response( api_client, reverse("profile-mailmap-add"), data=request_data, status_code=200, ) resp = check_http_get_response( api_client, reverse("profile-mailmap-list"), status_code=200, content_type="application/json", ).data assert len(resp) == 1 assert resp[0].items() >= request_data.items() + events = UserMailmapEvent.objects.all() + assert len(events) == 1 + assert events[0].request_type == "add" + assert json.loads(events[0].request) == request_data + assert events[0].successful + @pytest.mark.django_db(transaction=True) def test_mailmap_endpoints_error_response(api_client, mailmap_user): api_client.force_login(mailmap_user) url = reverse("profile-mailmap-add") resp = check_api_post_response(api_client, url, status_code=400) assert b"from_email" in resp.content url = reverse("profile-mailmap-update") resp = check_api_post_response(api_client, url, status_code=400) assert b"from_email" in resp.content + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 2 + + assert events[0].request_type == "add" + assert json.loads(events[0].request) == {} + assert not events[0].successful + + assert events[1].request_type == "update" + assert json.loads(events[1].request) == {} + assert not events[1].successful + @pytest.mark.django_db(transaction=True) def test_mailmap_update(api_client, mailmap_user): api_client.force_login(mailmap_user) before_add = datetime.datetime.now(tz=datetime.timezone.utc) check_api_post_response( api_client, reverse("profile-mailmap-add"), data={"from_email": "orig1@example.org", "display_name": "Display Name 1"}, status_code=200, ) check_api_post_response( api_client, reverse("profile-mailmap-add"), data={"from_email": "orig2@example.org", "display_name": "Display Name 2"}, status_code=200, ) after_add = datetime.datetime.now(tz=datetime.timezone.utc) mailmaps = list(UserMailmap.objects.order_by("from_email").all()) assert len(mailmaps) == 2, mailmaps assert mailmaps[0].from_email == "orig1@example.org", mailmaps assert mailmaps[0].display_name == "Display Name 1", mailmaps assert before_add <= mailmaps[0].last_update_date <= after_add assert mailmaps[1].from_email == "orig2@example.org", mailmaps assert mailmaps[1].display_name == "Display Name 2", mailmaps assert before_add <= mailmaps[0].last_update_date <= after_add before_update = datetime.datetime.now(tz=datetime.timezone.utc) check_api_post_response( api_client, reverse("profile-mailmap-update"), data={"from_email": "orig1@example.org", "display_name": "Display Name 1b"}, status_code=200, ) after_update = datetime.datetime.now(tz=datetime.timezone.utc) mailmaps = list(UserMailmap.objects.order_by("from_email").all()) assert len(mailmaps) == 2, mailmaps assert mailmaps[0].from_email == "orig1@example.org", mailmaps assert mailmaps[0].display_name == "Display Name 1b", mailmaps assert before_update <= mailmaps[0].last_update_date <= after_update assert mailmaps[1].from_email == "orig2@example.org", mailmaps assert mailmaps[1].display_name == "Display Name 2", mailmaps assert before_add <= mailmaps[1].last_update_date <= after_add + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 3 + assert events[0].request_type == "add" + assert events[1].request_type == "add" + assert events[2].request_type == "update" + def populate_mailmap(): for (verified, activated) in ( (False, False), (False, True), (True, False), (True, True), ): verified_str = "V" if verified else "" activated_str = "A" if activated else "" UserMailmap.objects.create( from_email=f"from_email{verified_str}{activated_str}@example.com", display_name=f"Display Name {verified_str} {activated_str}".strip(), from_email_verified=verified, display_name_activated=activated, ) def call_sync_mailmaps(*args) -> str: out = StringIO() err = StringIO() call_command("sync_mailmaps", *args, stdout=out, stderr=err) out.seek(0) err.seek(0) assert err.read() == "" return out.read() MAILMAP_KNOWN_FULLNAMES = ( "Original Name ", "Original Name V ", "Original Name A ", "Original Name V A ", "Original Name V A 2 ", "Original Name V A 3 ", ) MAILMAP_KNOWN_PEOPLE = tuple( Person.from_fullname(f.encode()) for f in MAILMAP_KNOWN_FULLNAMES ) def init_stub_storage_db(postgresql): cur = postgresql.cursor() cur.execute( """ CREATE TABLE person ( fullname bytea PRIMARY KEY, name bytea, email bytea, displayname bytea ) """ ) execute_values( cur, "INSERT INTO person (fullname, name, email) VALUES %s", (p.to_dict() for p in MAILMAP_KNOWN_PEOPLE), template="(%(fullname)s, %(name)s, %(email)s)", ) cur.execute("CREATE INDEX ON person (email)") postgresql.commit() cur.close() return postgresql.dsn def get_displaynames(postgresql) -> Dict[str, str]: with postgresql.cursor() as cur: cur.execute( "SELECT fullname, displayname FROM person WHERE displayname IS NOT NULL" ) return {bytes(f).decode("utf-8"): bytes(d).decode("utf-8") for (f, d) in cur} @pytest.mark.django_db(transaction=True) def test_sync_mailmaps_dry_run(postgresql): with transaction.atomic(): populate_mailmap() dsn = init_stub_storage_db(postgresql) out = call_sync_mailmaps(dsn) assert "(dry run)" in out assert "Synced 1 mailmaps to swh.storage database" in out assert get_displaynames(postgresql) == {} assert ( UserMailmap.objects.filter( from_email_verified=True, display_name_activated=True, mailmap_last_processing_date__isnull=False, ).count() == 0 ) @pytest.mark.django_db(transaction=True) def test_sync_mailmaps_perform(postgresql): with transaction.atomic(): populate_mailmap() dsn = init_stub_storage_db(postgresql) out = call_sync_mailmaps("--perform", dsn) assert "(dry run)" not in out assert "Synced 1 mailmaps to swh.storage database" in out expected_displaynames = { "Original Name V A ": "Display Name V A", "Original Name V A 2 ": "Display Name V A", "Original Name V A 3 ": "Display Name V A", } assert get_displaynames(postgresql) == expected_displaynames assert ( UserMailmap.objects.filter( from_email_verified=True, display_name_activated=True, mailmap_last_processing_date__isnull=False, ).count() == 1 ) @pytest.mark.django_db(transaction=True) def test_sync_mailmaps_with_to_email(postgresql): with transaction.atomic(): populate_mailmap() dsn = init_stub_storage_db(postgresql) call_sync_mailmaps("--perform", dsn) expected_displaynames = { "Original Name V A ": "Display Name V A", "Original Name V A 2 ": "Display Name V A", "Original Name V A 3 ": "Display Name V A", } assert get_displaynames(postgresql) == expected_displaynames # Add a non-valid to_email with transaction.atomic(): for mailmap in UserMailmap.objects.filter( from_email_verified=True, display_name_activated=True ): mailmap.to_email = "to_email@example.com" mailmap.save() call_sync_mailmaps("--perform", dsn) assert get_displaynames(postgresql) == expected_displaynames # Verify the relevant to_email with transaction.atomic(): for mailmap in UserMailmap.objects.filter( from_email_verified=True, display_name_activated=True ): mailmap.to_email_verified = True mailmap.save() call_sync_mailmaps("--perform", dsn) new_displayname = "Display Name V A " expected_displaynames = { "Original Name V A ": new_displayname, "Original Name V A 2 ": new_displayname, "Original Name V A 3 ": new_displayname, } assert get_displaynames(postgresql) == expected_displaynames @pytest.mark.django_db(transaction=True) def test_sync_mailmaps_disable(postgresql): """Check that disabling a mailmap only happens once""" with transaction.atomic(): populate_mailmap() dsn = init_stub_storage_db(postgresql) # Do the initial mailmap sync call_sync_mailmaps("--perform", dsn) assert len(get_displaynames(postgresql)) == 3 updated = 0 # Disable a display name with transaction.atomic(): # Cannot use update() because `last_update_date` would not be updated for mailmap in UserMailmap.objects.filter( from_email_verified=True, display_name_activated=True ): mailmap.display_name_activated = False mailmap.save() updated += 1 assert updated == 1 # Sync mailmaps again out = call_sync_mailmaps("--perform", dsn) assert "1 mailmaps to disable" in out assert get_displaynames(postgresql) == {} # Update a displayname by hand with postgresql.cursor() as cur: cur.execute( "UPDATE person SET displayname='Manual Display Name' " "WHERE fullname='Original Name V A '" ) expected_displaynames = { "Original Name V A ": "Manual Display Name" } assert get_displaynames(postgresql) == expected_displaynames # Sync mailmaps one last time. No mailmaps should be disabled out = call_sync_mailmaps("--perform", dsn) assert "0 mailmaps to disable" in out assert get_displaynames(postgresql) == expected_displaynames