From 53d94529175dface366a935eea2c2bcc1eeab15c Mon Sep 17 00:00:00 2001
From: Andrew Godwin <andrew@aeracode.org>
Date: Mon, 21 Nov 2022 21:18:13 -0700
Subject: [PATCH] Outgoing mentions mostly work (exc. cc followers)

---
 README.md                            |   2 +-
 activities/models/post.py            | 116 ++++++++++++++++++++-------
 activities/views/posts.py            |   3 +-
 core/html.py                         |   8 ++
 templates/activities/_post.html      |   2 +-
 tests/activities/models/test_post.py |  82 ++++++++++++++-----
 tests/conftest.py                    |   6 +-
 users/models/identity.py             |  12 +--
 users/views/activitypub.py           |   4 +-
 9 files changed, 171 insertions(+), 64 deletions(-)

diff --git a/README.md b/README.md
index 0e647ec..cfb4900 100644
--- a/README.md
+++ b/README.md
@@ -37,7 +37,7 @@ the less sure I am about it.
 - [x] Undo follows
 - [x] Receive and accept follows
 - [x] Receive follow undos
-- [ ] Do outgoing mentions properly
+- [x] Do outgoing mentions properly
 - [x] Home timeline (posts and boosts from follows)
 - [x] Notifications page (followed, boosted, liked)
 - [x] Local timeline
diff --git a/activities/models/post.py b/activities/models/post.py
index 7016077..da23742 100644
--- a/activities/models/post.py
+++ b/activities/models/post.py
@@ -4,12 +4,13 @@ from typing import Dict, Optional
 import httpx
 import urlman
 from django.db import models, transaction
+from django.template.defaultfilters import linebreaks_filter
 from django.utils import timezone
 from django.utils.safestring import mark_safe
 
 from activities.models.fan_out import FanOut
 from activities.models.timeline_event import TimelineEvent
-from core.html import sanitize_post
+from core.html import sanitize_post, strip_html
 from core.ld import canonicalise, format_ld_date, get_list, parse_ld_date
 from stator.models import State, StateField, StateGraph, StatorModel
 from users.models.follow import Follow
@@ -134,7 +135,6 @@ class Post(StatorModel):
 
     class urls(urlman.Urls):
         view = "{self.author.urls.view}posts/{self.id}/"
-        view_nice = "{self.author.urls.view_nice}posts/{self.id}/"
         object_uri = "{self.author.actor_uri}posts/{self.id}/"
         action_like = "{view}like/"
         action_unlike = "{view}unlike/"
@@ -153,42 +153,58 @@ class Post(StatorModel):
     def get_absolute_url(self):
         return self.urls.view
 
+    def absolute_object_uri(self):
+        """
+        Returns an object URI that is always absolute, for sending out to
+        other servers.
+        """
+        if self.local:
+            return self.author.absolute_profile_uri() + f"posts/{self.id}/"
+        else:
+            return self.object_uri
+
     ### Content cleanup and extraction ###
 
     mention_regex = re.compile(
-        r"([^\w\d\-_])(@[\w\d\-_]+(?:@[\w\d\-_]+\.[\w\d\-_\.]+)?)"
+        r"([^\w\d\-_])@([\w\d\-_]+(?:@[\w\d\-_]+\.[\w\d\-_\.]+)?)"
     )
 
-    def linkify_mentions(self, content):
+    def linkify_mentions(self, content, local=False):
         """
-        Links mentions _in the context of the post_ - meaning that if there's
-        a short @andrew mention, it will look at the mentions link to resolve
-        it rather than presuming it's local.
+        Links mentions _in the context of the post_ - as in, using the mentions
+        property as the only source (as we might be doing this without other
+        DB access allowed)
         """
 
+        possible_matches = {}
+        for mention in self.mentions.all():
+            if local:
+                url = str(mention.urls.view)
+            else:
+                url = mention.absolute_profile_uri()
+            possible_matches[mention.username] = url
+            possible_matches[f"{mention.username}@{mention.domain_id}"] = url
+
         def replacer(match):
             precursor = match.group(1)
             handle = match.group(2)
-            # If the handle has no domain, try to match it with a mention
-            if "@" not in handle.lstrip("@"):
-                username = handle.lstrip("@")
-                identity = self.mentions.filter(username=username).first()
-                if identity:
-                    url = identity.urls.view
-                else:
-                    url = f"/@{username}/"
-            else:
-                url = f"/{handle}/"
-            # If we have a URL, link to it, otherwise don't link
-            if url:
-                return f'{precursor}<a href="{url}">{handle}</a>'
+            if handle in possible_matches:
+                return f'{precursor}<a href="{possible_matches[handle]}">@{handle}</a>'
             else:
                 return match.group()
 
         return mark_safe(self.mention_regex.sub(replacer, content))
 
-    @property
-    def safe_content(self):
+    def safe_content_local(self):
+        """
+        Returns the content formatted for local display
+        """
+        return self.linkify_mentions(sanitize_post(self.content), local=True)
+
+    def safe_content_remote(self):
+        """
+        Returns the content formatted for remote consumption
+        """
         return self.linkify_mentions(sanitize_post(self.content))
 
     ### Async helpers ###
@@ -197,8 +213,10 @@ class Post(StatorModel):
         """
         Returns a version of the object with all relations pre-loaded
         """
-        return await Post.objects.select_related("author", "author__domain").aget(
-            pk=self.pk
+        return (
+            await Post.objects.select_related("author", "author__domain")
+            .prefetch_related("mentions", "mentions__domain")
+            .aget(pk=self.pk)
         )
 
     ### Local creation ###
@@ -212,6 +230,25 @@ class Post(StatorModel):
         visibility: int = Visibilities.public,
     ) -> "Post":
         with transaction.atomic():
+            # Find mentions in this post
+            mention_hits = cls.mention_regex.findall(content)
+            mentions = set()
+            for precursor, handle in mention_hits:
+                if "@" in handle:
+                    username, domain = handle.split("@", 1)
+                else:
+                    username = handle
+                    domain = author.domain_id
+                identity = Identity.by_username_and_domain(
+                    username=username,
+                    domain=domain,
+                    fetch=True,
+                )
+                if identity is not None:
+                    mentions.add(identity)
+            # Strip all HTML and apply linebreaks filter
+            content = linebreaks_filter(strip_html(content))
+            # Make the Post object
             post = cls.objects.create(
                 author=author,
                 content=content,
@@ -221,7 +258,8 @@ class Post(StatorModel):
                 visibility=visibility,
             )
             post.object_uri = post.urls.object_uri
-            post.url = post.urls.view_nice
+            post.url = post.absolute_object_uri()
+            post.mentions.set(mentions)
             post.save()
         return post
 
@@ -232,28 +270,48 @@ class Post(StatorModel):
         Returns the AP JSON for this object
         """
         value = {
+            "to": "as:Public",
+            "cc": [],
             "type": "Note",
             "id": self.object_uri,
             "published": format_ld_date(self.published),
             "attributedTo": self.author.actor_uri,
-            "content": self.safe_content,
-            "to": "as:Public",
+            "content": self.safe_content_remote(),
             "as:sensitive": self.sensitive,
-            "url": str(self.urls.view_nice if self.local else self.url),
+            "url": self.absolute_object_uri(),
+            "tag": [],
         }
         if self.summary:
             value["summary"] = self.summary
+        # Mentions
+        for mention in self.mentions.all():
+            value["tag"].append(
+                {
+                    "href": mention.actor_uri,
+                    "name": "@" + mention.handle,
+                    "type": "Mention",
+                }
+            )
+            value["cc"].append(mention.actor_uri)
+        # Remove tag and cc if they're empty
+        if not value["cc"]:
+            del value["cc"]
+        if not value["tag"]:
+            del value["tag"]
         return value
 
     def to_create_ap(self):
         """
         Returns the AP JSON to create this object
         """
+        object = self.to_ap()
         return {
+            "to": object["to"],
+            "cc": object.get("cc", []),
             "type": "Create",
             "id": self.object_uri + "#create",
             "actor": self.author.actor_uri,
-            "object": self.to_ap(),
+            "object": object,
         }
 
     ### ActivityPub (inbound) ###
diff --git a/activities/views/posts.py b/activities/views/posts.py
index 14da9ca..de11a09 100644
--- a/activities/views/posts.py
+++ b/activities/views/posts.py
@@ -1,6 +1,5 @@
 from django import forms
 from django.shortcuts import get_object_or_404, redirect, render
-from django.template.defaultfilters import linebreaks_filter
 from django.utils.decorators import method_decorator
 from django.views.generic import FormView, TemplateView, View
 
@@ -158,7 +157,7 @@ class Compose(FormView):
     def form_valid(self, form):
         Post.create_local(
             author=self.request.identity,
-            content=linebreaks_filter(form.cleaned_data["text"]),
+            content=form.cleaned_data["text"],
             summary=form.cleaned_data.get("content_warning"),
             visibility=form.cleaned_data["visibility"],
         )
diff --git a/core/html.py b/core/html.py
index 5045b16..3230284 100644
--- a/core/html.py
+++ b/core/html.py
@@ -30,3 +30,11 @@ def sanitize_post(post_html: str) -> str:
         strip=True,
     )
     return mark_safe(cleaner.clean(post_html))
+
+
+def strip_html(post_html: str) -> str:
+    """
+    Strips all tags from the text, then linkifies it.
+    """
+    cleaner = bleach.Cleaner(tags=[], strip=True, filters=[LinkifyFilter])
+    return mark_safe(cleaner.clean(post_html))
diff --git a/templates/activities/_post.html b/templates/activities/_post.html
index 3d455ea..80fa653 100644
--- a/templates/activities/_post.html
+++ b/templates/activities/_post.html
@@ -36,7 +36,7 @@
     {% endif %}
 
     <div class="content {% if post.summary %}hidden{% endif %}">
-        {{ post.safe_content }}
+        {{ post.safe_content_local }}
     </div>
 
     {% if post.attachments.exists %}
diff --git a/tests/activities/models/test_post.py b/tests/activities/models/test_post.py
index fb2e7a6..6d86781 100644
--- a/tests/activities/models/test_post.py
+++ b/tests/activities/models/test_post.py
@@ -32,32 +32,72 @@ def test_fetch_post(httpx_mock: HTTPXMock):
 
 
 @pytest.mark.django_db
-def test_linkify_mentions(identity, remote_identity):
+def test_linkify_mentions_remote(identity, remote_identity):
     """
-    Tests that we can linkify post mentions properly
+    Tests that we can linkify post mentions properly for remote use
     """
-    # Test a short username without a mention (presumed local)
-    post = Post.objects.create(
-        content="<p>Hello @test</p>",
-        author=identity,
-        local=True,
-    )
-    assert post.safe_content == '<p>Hello <a href="/@test/">@test</a></p>'
-    # Test a full username
-    post = Post.objects.create(
-        content="<p>@test@example.com, welcome!</p>",
-        author=identity,
-        local=True,
-    )
-    assert (
-        post.safe_content
-        == '<p><a href="/@test@example.com/">@test@example.com</a>, welcome!</p>'
-    )
-    # Test a short username with a mention resolving to remote
+    # Test a short username (remote)
     post = Post.objects.create(
         content="<p>Hello @test</p>",
         author=identity,
         local=True,
     )
     post.mentions.add(remote_identity)
-    assert post.safe_content == '<p>Hello <a href="/@test@remote.test/">@test</a></p>'
+    assert (
+        post.safe_content_remote()
+        == '<p>Hello <a href="https://remote.test/@test/">@test</a></p>'
+    )
+    # Test a full username (local)
+    post = Post.objects.create(
+        content="<p>@test@example.com, welcome!</p>",
+        author=identity,
+        local=True,
+    )
+    post.mentions.add(identity)
+    assert (
+        post.safe_content_remote()
+        == '<p><a href="https://example.com/@test/">@test@example.com</a>, welcome!</p>'
+    )
+    # Test that they don't get touched without a mention
+    post = Post.objects.create(
+        content="<p>@test@example.com, welcome!</p>",
+        author=identity,
+        local=True,
+    )
+    assert post.safe_content_remote() == "<p>@test@example.com, welcome!</p>"
+
+
+@pytest.mark.django_db
+def test_linkify_mentions_local(identity, remote_identity):
+    """
+    Tests that we can linkify post mentions properly for local use
+    """
+    # Test a short username (remote)
+    post = Post.objects.create(
+        content="<p>Hello @test</p>",
+        author=identity,
+        local=True,
+    )
+    post.mentions.add(remote_identity)
+    assert (
+        post.safe_content_local()
+        == '<p>Hello <a href="/@test@remote.test/">@test</a></p>'
+    )
+    # Test a full username (local)
+    post = Post.objects.create(
+        content="<p>@test@example.com, welcome!</p>",
+        author=identity,
+        local=True,
+    )
+    post.mentions.add(identity)
+    assert (
+        post.safe_content_local()
+        == '<p><a href="/@test@example.com/">@test@example.com</a>, welcome!</p>'
+    )
+    # Test that they don't get touched without a mention
+    post = Post.objects.create(
+        content="<p>@test@example.com, welcome!</p>",
+        author=identity,
+        local=True,
+    )
+    assert post.safe_content_local() == "<p>@test@example.com, welcome!</p>"
diff --git a/tests/conftest.py b/tests/conftest.py
index 24fac9a..536162c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -68,14 +68,15 @@ def identity():
     """
     user = User.objects.create(email="test@example.com")
     domain = Domain.objects.create(domain="example.com", local=True, public=True)
-    return Identity.objects.create(
+    identity = Identity.objects.create(
         actor_uri="https://example.com/test-actor/",
         username="test",
         domain=domain,
-        user=user,
         name="Test User",
         local=True,
     )
+    identity.users.set([user])
+    return identity
 
 
 @pytest.fixture
@@ -87,6 +88,7 @@ def remote_identity():
     domain = Domain.objects.create(domain="remote.test", local=False)
     return Identity.objects.create(
         actor_uri="https://remote.test/test-actor/",
+        profile_uri="https://remote.test/@test/",
         username="test",
         domain=domain,
         name="Test Remote User",
diff --git a/users/models/identity.py b/users/models/identity.py
index 7116021..7d3d7d5 100644
--- a/users/models/identity.py
+++ b/users/models/identity.py
@@ -97,7 +97,6 @@ class Identity(StatorModel):
         unique_together = [("username", "domain")]
 
     class urls(urlman.Urls):
-        view_nice = "{self._nice_view_url}"
         view = "/@{self.username}@{self.domain_id}/"
         action = "{view}action/"
         activate = "{view}activate/"
@@ -113,14 +112,15 @@ class Identity(StatorModel):
             return self.handle
         return self.actor_uri
 
-    def _nice_view_url(self):
+    def absolute_profile_uri(self):
         """
-        Returns the "nice" user URL if they're local, otherwise our general one
+        Returns a profile URI that is always absolute, for sending out to
+        other servers.
         """
         if self.local:
             return f"https://{self.domain.uri_domain}/@{self.username}/"
         else:
-            return f"/@{self.username}@{self.domain_id}/"
+            return self.profile_uri
 
     def local_icon_url(self):
         """
@@ -206,7 +206,7 @@ class Identity(StatorModel):
     def handle(self):
         if self.domain_id:
             return f"{self.username}@{self.domain_id}"
-        return f"{self.username}@UNKNOWN-DOMAIN"
+        return f"{self.username}@unknown.invalid"
 
     @property
     def data_age(self) -> float:
@@ -238,7 +238,7 @@ class Identity(StatorModel):
                 "publicKeyPem": self.public_key,
             },
             "published": self.created.strftime("%Y-%m-%dT%H:%M:%SZ"),
-            "url": str(self.urls.view_nice),
+            "url": self.absolute_profile_uri(),
         }
         if self.name:
             response["name"] = self.name
diff --git a/users/views/activitypub.py b/users/views/activitypub.py
index 0ba7d67..1ca80a1 100644
--- a/users/views/activitypub.py
+++ b/users/views/activitypub.py
@@ -128,13 +128,13 @@ class Webfinger(View):
                 {
                     "subject": f"acct:{identity.handle}",
                     "aliases": [
-                        str(identity.urls.view_nice),
+                        identity.absolute_profile_uri(),
                     ],
                     "links": [
                         {
                             "rel": "http://webfinger.net/rel/profile-page",
                             "type": "text/html",
-                            "href": str(identity.urls.view_nice),
+                            "href": identity.absolute_profile_uri(),
                         },
                         {
                             "rel": "self",