From 632e3844b9c2b0dd1f67567b9c4411bf6b022a2e Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 10 Apr 2023 17:32:49 +1000
Subject: [PATCH 01/15] Don't assume user id is key id minus fragment

Fixes #2801
Related to #2794

It is legitimate to use any url for the user's key id. We have been assuming this id is the user id plus a fragment (#key-id) but this is not always the case, notably in the case of GoToSocial it is at /key-id. This commit instead checks the remote user's information to see if the key id listed matches the key id of the message allegedly received from them.

Whilst troubleshooting this it also became apparent that there is a mismatch between Bookwyrm users' keyId and the KeyId we claim to be using in signed requests (there is a forward slash missing). Since everything after the slash is a fragment, this usually slips through but we should be consistent so I updated that.
---
 bookwyrm/activitypub/__init__.py |  2 +-
 bookwyrm/signatures.py           |  3 ++-
 bookwyrm/views/inbox.py          | 10 ++++------
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py
index 05ca444766..8449b4f503 100644
--- a/bookwyrm/activitypub/__init__.py
+++ b/bookwyrm/activitypub/__init__.py
@@ -4,7 +4,7 @@
 
 from .base_activity import ActivityEncoder, Signature, naive_parse
 from .base_activity import Link, Mention, Hashtag
-from .base_activity import ActivitySerializerError, resolve_remote_id
+from .base_activity import ActivitySerializerError, resolve_remote_id, get_activitypub_data
 from .image import Document, Image
 from .note import Note, GeneratedNote, Article, Comment, Quotation
 from .note import Review, Rating
diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index 3102f8da21..10d500f1c3 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -38,8 +38,9 @@ def make_signature(method, sender, destination, date, digest=None):
     message_to_sign = "\n".join(signature_headers)
     signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key))
     signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8")))
+    # TODO: this should be /#main-key to match our user key ids, even though that was probably a mistake in hindsight.
     signature = {
-        "keyId": f"{sender.remote_id}#main-key",
+        "keyId": f"{sender.remote_id}/#main-key",
         "algorithm": "rsa-sha256",
         "headers": headers,
         "signature": b64encode(signed_message).decode("utf8"),
diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index 89e644db8a..867b2b971a 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -130,15 +130,13 @@ def has_valid_signature(request, activity):
     """verify incoming signature"""
     try:
         signature = Signature.parse(request)
-
-        key_actor = urldefrag(signature.key_id).url
-        if key_actor != activity.get("actor"):
-            raise ValueError("Wrong actor created signature.")
-
-        remote_user = activitypub.resolve_remote_id(key_actor, model=models.User)
+        remote_user = activitypub.resolve_remote_id(activity.get("actor"), model=models.User)
         if not remote_user:
             return False
 
+        if signature.key_id != remote_user.key_pair.remote_id:
+            raise ValueError("Wrong actor created signature.")
+
         try:
             signature.verify(remote_user.key_pair.public_key, request)
         except ValueError:

From 49758f2383a675ebedac29fd21e8d43b2d13751d Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 10 Apr 2023 17:50:25 +1000
Subject: [PATCH 02/15] formatting fixes

---
 bookwyrm/activitypub/__init__.py | 2 +-
 bookwyrm/views/inbox.py          | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py
index 8449b4f503..05ca444766 100644
--- a/bookwyrm/activitypub/__init__.py
+++ b/bookwyrm/activitypub/__init__.py
@@ -4,7 +4,7 @@
 
 from .base_activity import ActivityEncoder, Signature, naive_parse
 from .base_activity import Link, Mention, Hashtag
-from .base_activity import ActivitySerializerError, resolve_remote_id, get_activitypub_data
+from .base_activity import ActivitySerializerError, resolve_remote_id
 from .image import Document, Image
 from .note import Note, GeneratedNote, Article, Comment, Quotation
 from .note import Review, Rating
diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index 867b2b971a..da32ebdb0c 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -130,7 +130,9 @@ def has_valid_signature(request, activity):
     """verify incoming signature"""
     try:
         signature = Signature.parse(request)
-        remote_user = activitypub.resolve_remote_id(activity.get("actor"), model=models.User)
+        remote_user = activitypub.resolve_remote_id(
+            activity.get("actor"), model=models.User
+        )
         if not remote_user:
             return False
 

From e112718d2d08369581dc14f0f12fef1525d0be74 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 10 Apr 2023 20:34:45 +1000
Subject: [PATCH 03/15] clean up troubleshooting comment

---
 bookwyrm/signatures.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index 10d500f1c3..3dfde4cb1f 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -38,7 +38,6 @@ def make_signature(method, sender, destination, date, digest=None):
     message_to_sign = "\n".join(signature_headers)
     signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key))
     signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8")))
-    # TODO: this should be /#main-key to match our user key ids, even though that was probably a mistake in hindsight.
     signature = {
         "keyId": f"{sender.remote_id}/#main-key",
         "algorithm": "rsa-sha256",

From ef85394a1633fb599df2ec31b29707c26042ce30 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 10 Apr 2023 20:35:13 +1000
Subject: [PATCH 04/15] Allow for tag value to be object

Previously the 'tag' value in an activitypub object was assumed to be a List (array).
Some AP software sends 'tag' as a Dict (object) if there is only a single tag value.
It's somewhat debatable whether this is spec compliant but we should aim to be robust.
This commit puts an individual mention tag inside a list if necessary.
---
 bookwyrm/models/status.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py
index 1fcc9ee757..bd2a98f354 100644
--- a/bookwyrm/models/status.py
+++ b/bookwyrm/models/status.py
@@ -136,10 +136,16 @@ def ignore_activity(cls, activity):  # pylint: disable=too-many-return-statement
         # keep notes if they mention local users
         if activity.tag == MISSING or activity.tag is None:
             return True
-        tags = [l["href"] for l in activity.tag if l["type"] == "Mention"]
+
+        tags = activity.tag if type(activity.tag) == list else [activity.tag]
         user_model = apps.get_model("bookwyrm.User", require_ready=True)
         for tag in tags:
-            if user_model.objects.filter(remote_id=tag, local=True).exists():
+            if (
+                tag["type"] == "Mention"
+                and user_model.objects.filter(
+                    remote_id=tag["href"], local=True
+                ).exists()
+            ):
                 # we found a mention of a known use boost
                 return False
         return True

From c9dcd4f7add416601464167f0d8a599ea44cff99 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 10 Apr 2023 20:38:20 +1000
Subject: [PATCH 05/15] Include initial '@' in mention tag name

GoToSocial expects the 'name' value of a mention tag to have an initial '@' symbol. Mastodon doesn't seem to mind either way.
---
 bookwyrm/models/fields.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py
index 6cfe4c10c2..c11a24630f 100644
--- a/bookwyrm/models/fields.py
+++ b/bookwyrm/models/fields.py
@@ -371,7 +371,7 @@ def field_to_activity(self, value):
             tags.append(
                 activitypub.Link(
                     href=item.remote_id,
-                    name=getattr(item, item.name_field),
+                    name=f"@{getattr(item, item.name_field)}",
                     type=activity_type,
                 )
             )

From 03f21b0f35609c07fedfc71e6b32979dbbda927f Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Tue, 11 Apr 2023 15:45:06 +1000
Subject: [PATCH 06/15] Use correct keyId with legacy fallback

Bookwyrm keyIds are at `userpath/#main-key`, however when signing AP objects we have claimed in the headers that the keyId is at `userpath#main-key`.
This is incorrect, and makes GoToSocial's strict checking break.
Simply updating the signatures to use the correct KeyId breaks legacy Bookwyrm's signature checks, becuase it assumes that the keyId path is the same as the user path plus a fragment.
This commit allows for either option, by sending the request a second time with the incorrect keyId if sending with the correct one causes an error.
---
 bookwyrm/models/activitypub_mixin.py | 13 ++++++++++++-
 bookwyrm/signatures.py               |  6 ++++--
 bookwyrm/views/inbox.py              |  3 ++-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 83ca90b0a5..83a7dd9982 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -540,10 +540,11 @@ async def sign_and_send(
 
     digest = make_digest(data)
 
+
     headers = {
         "Date": now,
         "Digest": digest,
-        "Signature": make_signature("post", sender, destination, now, digest),
+        "Signature": make_signature("post", sender, destination, now, digest, False),
         "Content-Type": "application/activity+json; charset=utf-8",
         "User-Agent": USER_AGENT,
     }
@@ -554,6 +555,16 @@ async def sign_and_send(
                 logger.exception(
                     "Failed to send broadcast to %s: %s", destination, response.reason
                 )
+                logger.info("Trying again with legacy keyId")
+                # try with incorrect keyId to enable communication with legacy Bookwyrm servers
+                legacy_signature = make_signature("post", sender, destination, now, digest, True)
+                headers["Signature"] = legacy_signature
+                async with session.post(destination, data=data, headers=headers) as response: 
+                    if not response.ok:
+                        logger.exception(
+                        "Failed to send broadcast with legacy keyId to %s: %s", destination, response.reason
+                    )
+                
             return response
     except asyncio.TimeoutError:
         logger.info("Connection timed out for url: %s", destination)
diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index 3dfde4cb1f..2a1f2e72a7 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -22,7 +22,7 @@ def create_key_pair():
     return private_key, public_key
 
 
-def make_signature(method, sender, destination, date, digest=None):
+def make_signature(method, sender, destination, date, digest=None, use_legacy_key=False):
     """uses a private key to sign an outgoing message"""
     inbox_parts = urlparse(destination)
     signature_headers = [
@@ -38,8 +38,10 @@ def make_signature(method, sender, destination, date, digest=None):
     message_to_sign = "\n".join(signature_headers)
     signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key))
     signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8")))
+    # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions
+    key_id = f"{sender.remote_id}#main-key" if use_legacy_key else f"{sender.remote_id}/#main-key"
     signature = {
-        "keyId": f"{sender.remote_id}/#main-key",
+        "keyId": key_id,
         "algorithm": "rsa-sha256",
         "headers": headers,
         "signature": b64encode(signed_message).decode("utf8"),
diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index da32ebdb0c..9eef6792f4 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -137,7 +137,8 @@ def has_valid_signature(request, activity):
             return False
 
         if signature.key_id != remote_user.key_pair.remote_id:
-            raise ValueError("Wrong actor created signature.")
+            if signature.key_id != f"{remote_user.remote_id}#main-key": # legacy Bookwyrm
+                raise ValueError("Wrong actor created signature.")
 
         try:
             signature.verify(remote_user.key_pair.public_key, request)

From 279fa3851b6d043410804b789954f5e42a049348 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Tue, 11 Apr 2023 16:49:11 +1000
Subject: [PATCH 07/15] add comment

---
 bookwyrm/models/status.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py
index bd2a98f354..cabf2cf8d8 100644
--- a/bookwyrm/models/status.py
+++ b/bookwyrm/models/status.py
@@ -137,6 +137,8 @@ def ignore_activity(cls, activity):  # pylint: disable=too-many-return-statement
         if activity.tag == MISSING or activity.tag is None:
             return True
 
+        # BUG: this fixes the TypeError but we still don't get any notifs
+        # and DMs are dropped
         tags = activity.tag if type(activity.tag) == list else [activity.tag]
         user_model = apps.get_model("bookwyrm.User", require_ready=True)
         for tag in tags:

From c450947eeee26727dd48a5dd63c42d6ed45bc889 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Tue, 11 Apr 2023 18:57:55 +1000
Subject: [PATCH 08/15] update comment to identify bug

---
 bookwyrm/models/status.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py
index cabf2cf8d8..1561295a78 100644
--- a/bookwyrm/models/status.py
+++ b/bookwyrm/models/status.py
@@ -137,9 +137,9 @@ def ignore_activity(cls, activity):  # pylint: disable=too-many-return-statement
         if activity.tag == MISSING or activity.tag is None:
             return True
 
-        # BUG: this fixes the TypeError but we still don't get any notifs
-        # and DMs are dropped
-        tags = activity.tag if type(activity.tag) == list else [activity.tag]
+        # BUG: this fixes the TypeError but if there is only one user mentioned
+        # we still don't get any notifs and DMs are dropped.
+        tags = activity.tag if type(activity.tag) == list else [ activity.tag ]
         user_model = apps.get_model("bookwyrm.User", require_ready=True)
         for tag in tags:
             if (

From e3261c6b88c8683bd50f4be94541bae1a134c1eb Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Thu, 13 Apr 2023 13:21:05 +1000
Subject: [PATCH 09/15] fix incoming GTS mentions and DMs

GoToSocial sends 'tag' values as a single object if there is only one
user mentioned, rather than an array with an object inside it.

This causes Bookwyrm to reject the tag since it comes through as a
dict rather than a list.

This commit fixes this at the point the incoming AP object is transformed
so that "mention" tags are turned into a mention_user.
---
 bookwyrm/models/fields.py | 7 ++++++-
 bookwyrm/models/status.py | 7 +++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py
index c11a24630f..1682709739 100644
--- a/bookwyrm/models/fields.py
+++ b/bookwyrm/models/fields.py
@@ -379,7 +379,12 @@ def field_to_activity(self, value):
 
     def field_from_activity(self, value, allow_external_connections=True):
         if not isinstance(value, list):
-            return None
+            # GoToSocial DMs and single-user mentions are
+            # sent as objects, not as an array of objects
+            if isinstance(value, dict):
+                value = [value]
+            else: 
+                return None
         items = []
         for link_json in value:
             link = activitypub.Link(**link_json)
diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py
index 1561295a78..a620f09b16 100644
--- a/bookwyrm/models/status.py
+++ b/bookwyrm/models/status.py
@@ -136,10 +136,9 @@ def ignore_activity(cls, activity):  # pylint: disable=too-many-return-statement
         # keep notes if they mention local users
         if activity.tag == MISSING or activity.tag is None:
             return True
-
-        # BUG: this fixes the TypeError but if there is only one user mentioned
-        # we still don't get any notifs and DMs are dropped.
-        tags = activity.tag if type(activity.tag) == list else [ activity.tag ]
+        # GoToSocial sends single tags as objects
+        # not wrapped in a list
+        tags = activity.tag if isinstance(activity.tag, list) else [ activity.tag ]
         user_model = apps.get_model("bookwyrm.User", require_ready=True)
         for tag in tags:
             if (

From a6676718cbd64cc2d259e81cf7cff339f533743a Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Thu, 13 Apr 2023 13:27:51 +1000
Subject: [PATCH 10/15] formatting

---
 bookwyrm/models/activitypub_mixin.py | 17 +++++++++++------
 bookwyrm/models/fields.py            |  2 +-
 bookwyrm/models/status.py            |  2 +-
 bookwyrm/signatures.py               | 10 ++++++++--
 bookwyrm/views/inbox.py              |  4 +++-
 5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 83a7dd9982..5e52576aa6 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -540,7 +540,6 @@ async def sign_and_send(
 
     digest = make_digest(data)
 
-
     headers = {
         "Date": now,
         "Digest": digest,
@@ -557,14 +556,20 @@ async def sign_and_send(
                 )
                 logger.info("Trying again with legacy keyId")
                 # try with incorrect keyId to enable communication with legacy Bookwyrm servers
-                legacy_signature = make_signature("post", sender, destination, now, digest, True)
+                legacy_signature = make_signature(
+                    "post", sender, destination, now, digest, True
+                )
                 headers["Signature"] = legacy_signature
-                async with session.post(destination, data=data, headers=headers) as response: 
+                async with session.post(
+                    destination, data=data, headers=headers
+                ) as response:
                     if not response.ok:
                         logger.exception(
-                        "Failed to send broadcast with legacy keyId to %s: %s", destination, response.reason
-                    )
-                
+                            "Failed to send broadcast with legacy keyId to %s: %s",
+                            destination,
+                            response.reason,
+                        )
+
             return response
     except asyncio.TimeoutError:
         logger.info("Connection timed out for url: %s", destination)
diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py
index 1682709739..62dc8f0d99 100644
--- a/bookwyrm/models/fields.py
+++ b/bookwyrm/models/fields.py
@@ -383,7 +383,7 @@ def field_from_activity(self, value, allow_external_connections=True):
             # sent as objects, not as an array of objects
             if isinstance(value, dict):
                 value = [value]
-            else: 
+            else:
                 return None
         items = []
         for link_json in value:
diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py
index a620f09b16..2f1999bf25 100644
--- a/bookwyrm/models/status.py
+++ b/bookwyrm/models/status.py
@@ -138,7 +138,7 @@ def ignore_activity(cls, activity):  # pylint: disable=too-many-return-statement
             return True
         # GoToSocial sends single tags as objects
         # not wrapped in a list
-        tags = activity.tag if isinstance(activity.tag, list) else [ activity.tag ]
+        tags = activity.tag if isinstance(activity.tag, list) else [activity.tag]
         user_model = apps.get_model("bookwyrm.User", require_ready=True)
         for tag in tags:
             if (
diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index 2a1f2e72a7..d3475edf61 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -22,7 +22,9 @@ def create_key_pair():
     return private_key, public_key
 
 
-def make_signature(method, sender, destination, date, digest=None, use_legacy_key=False):
+def make_signature(
+    method, sender, destination, date, digest=None, use_legacy_key=False
+):
     """uses a private key to sign an outgoing message"""
     inbox_parts = urlparse(destination)
     signature_headers = [
@@ -39,7 +41,11 @@ def make_signature(method, sender, destination, date, digest=None, use_legacy_ke
     signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key))
     signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8")))
     # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions
-    key_id = f"{sender.remote_id}#main-key" if use_legacy_key else f"{sender.remote_id}/#main-key"
+    key_id = (
+        f"{sender.remote_id}#main-key"
+        if use_legacy_key
+        else f"{sender.remote_id}/#main-key"
+    )
     signature = {
         "keyId": key_id,
         "algorithm": "rsa-sha256",
diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index 9eef6792f4..5670f35b99 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -137,7 +137,9 @@ def has_valid_signature(request, activity):
             return False
 
         if signature.key_id != remote_user.key_pair.remote_id:
-            if signature.key_id != f"{remote_user.remote_id}#main-key": # legacy Bookwyrm
+            if (
+                signature.key_id != f"{remote_user.remote_id}#main-key"
+            ):  # legacy Bookwyrm
                 raise ValueError("Wrong actor created signature.")
 
         try:

From c7adb62831f1fac673645bba67369955ffd00a39 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Thu, 13 Apr 2023 19:48:20 +1000
Subject: [PATCH 11/15] make get_legacy_key more DRY

---
 bookwyrm/models/activitypub_mixin.py | 29 +++++++++++++---------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 5e52576aa6..d8103c9c3c 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -529,7 +529,7 @@ async def async_broadcast(recipients: List[str], sender, data: str):
 
 
 async def sign_and_send(
-    session: aiohttp.ClientSession, sender, data: str, destination: str
+    session: aiohttp.ClientSession, sender, data: str, destination: str, **kwargs
 ):
     """Sign the messages and send them in an asynchronous bundle"""
     now = http_date()
@@ -539,11 +539,14 @@ async def sign_and_send(
         raise ValueError("No private key found for sender")
 
     digest = make_digest(data)
+    signature = make_signature(
+        "post", sender, destination, now, digest, kwargs.get("use_legacy_key")
+    )
 
     headers = {
         "Date": now,
         "Digest": digest,
-        "Signature": make_signature("post", sender, destination, now, digest, False),
+        "Signature": signature,
         "Content-Type": "application/activity+json; charset=utf-8",
         "User-Agent": USER_AGENT,
     }
@@ -554,21 +557,15 @@ async def sign_and_send(
                 logger.exception(
                     "Failed to send broadcast to %s: %s", destination, response.reason
                 )
-                logger.info("Trying again with legacy keyId")
-                # try with incorrect keyId to enable communication with legacy Bookwyrm servers
-                legacy_signature = make_signature(
-                    "post", sender, destination, now, digest, True
-                )
-                headers["Signature"] = legacy_signature
-                async with session.post(
-                    destination, data=data, headers=headers
-                ) as response:
-                    if not response.ok:
-                        logger.exception(
-                            "Failed to send broadcast with legacy keyId to %s: %s",
-                            destination,
-                            response.reason,
+                if kwargs.get("use_legacy_key") is not True:
+                    # try with incorrect keyId to enable communication with legacy Bookwyrm servers
+                    logger.info("Trying again with legacy keyId header value")
+
+                    asyncio.ensure_future(
+                        sign_and_send(
+                            session, sender, data, destination, use_legacy_key=True
                         )
+                    )
 
             return response
     except asyncio.TimeoutError:

From 56a062d01f58be9088de22774f09a17ab08f32b3 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Thu, 13 Apr 2023 20:21:35 +1000
Subject: [PATCH 12/15] pylint fixes

---
 bookwyrm/models/activitypub_mixin.py | 2 +-
 bookwyrm/signatures.py               | 5 +++--
 bookwyrm/views/inbox.py              | 1 -
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index d8103c9c3c..3e04fa408d 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -540,7 +540,7 @@ async def sign_and_send(
 
     digest = make_digest(data)
     signature = make_signature(
-        "post", sender, destination, now, digest, kwargs.get("use_legacy_key")
+        "post", sender, destination, now, digest=digest, use_legacy_key=kwargs.get("use_legacy_key")
     )
 
     headers = {
diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index d3475edf61..9730ec6a1e 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -23,7 +23,7 @@ def create_key_pair():
 
 
 def make_signature(
-    method, sender, destination, date, digest=None, use_legacy_key=False
+    method, sender, destination, date, **kwargs
 ):
     """uses a private key to sign an outgoing message"""
     inbox_parts = urlparse(destination)
@@ -33,6 +33,7 @@ def make_signature(
         f"date: {date}",
     ]
     headers = "(request-target) host date"
+    digest = kwargs.get("digest")
     if digest is not None:
         signature_headers.append(f"digest: {digest}")
         headers = "(request-target) host date digest"
@@ -43,7 +44,7 @@ def make_signature(
     # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions
     key_id = (
         f"{sender.remote_id}#main-key"
-        if use_legacy_key
+        if kwargs.get("use_legacy_key")
         else f"{sender.remote_id}/#main-key"
     )
     signature = {
diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py
index 5670f35b99..1c6c642284 100644
--- a/bookwyrm/views/inbox.py
+++ b/bookwyrm/views/inbox.py
@@ -3,7 +3,6 @@
 import re
 import logging
 
-from urllib.parse import urldefrag
 import requests
 
 from django.http import HttpResponse, Http404

From 123628c66a2f332c48b26e261200ad81e3041d45 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Thu, 13 Apr 2023 22:33:54 +1000
Subject: [PATCH 13/15] fix tests and formatting

---
 bookwyrm/models/activitypub_mixin.py | 7 ++++++-
 bookwyrm/signatures.py               | 4 +---
 bookwyrm/tests/models/test_fields.py | 2 +-
 bookwyrm/tests/test_signing.py       | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index 3e04fa408d..d9942746a6 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -540,7 +540,12 @@ async def sign_and_send(
 
     digest = make_digest(data)
     signature = make_signature(
-        "post", sender, destination, now, digest=digest, use_legacy_key=kwargs.get("use_legacy_key")
+        "post",
+        sender,
+        destination,
+        now,
+        digest=digest,
+        use_legacy_key=kwargs.get("use_legacy_key"),
     )
 
     headers = {
diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py
index 9730ec6a1e..08780b7317 100644
--- a/bookwyrm/signatures.py
+++ b/bookwyrm/signatures.py
@@ -22,9 +22,7 @@ def create_key_pair():
     return private_key, public_key
 
 
-def make_signature(
-    method, sender, destination, date, **kwargs
-):
+def make_signature(method, sender, destination, date, **kwargs):
     """uses a private key to sign an outgoing message"""
     inbox_parts = urlparse(destination)
     signature_headers = [
diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py
index 961fbd5224..fc8d7387da 100644
--- a/bookwyrm/tests/models/test_fields.py
+++ b/bookwyrm/tests/models/test_fields.py
@@ -404,7 +404,7 @@ def test_tag_field(self, *_):
         self.assertIsInstance(result, list)
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].href, "https://e.b/c")
-        self.assertEqual(result[0].name, "Name")
+        self.assertEqual(result[0].name, "@Name")
         self.assertEqual(result[0].type, "Serializable")
 
     def test_tag_field_from_activity(self, *_):
diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py
index cde193f080..12a164eb5b 100644
--- a/bookwyrm/tests/test_signing.py
+++ b/bookwyrm/tests/test_signing.py
@@ -87,7 +87,7 @@ def send_test_request(  # pylint: disable=too-many-arguments
         data = json.dumps(get_follow_activity(sender, self.rat))
         digest = digest or make_digest(data)
         signature = make_signature(
-            "post", signer or sender, self.rat.inbox, now, digest
+            "post", signer or sender, self.rat.inbox, now, digest=digest
         )
         with patch("bookwyrm.views.inbox.activity_task.apply_async"):
             with patch("bookwyrm.models.user.set_remote_server.delay"):

From 8a8af4e90927540077908b4474a10b427f9d7672 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Fri, 14 Apr 2023 18:03:51 +1000
Subject: [PATCH 14/15] fix tests and make pylint happier

---
 bookwyrm/models/activitypub_mixin.py | 2 --
 bookwyrm/tests/test_signing.py       | 4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py
index d9942746a6..ee8b0d26a6 100644
--- a/bookwyrm/models/activitypub_mixin.py
+++ b/bookwyrm/models/activitypub_mixin.py
@@ -563,9 +563,7 @@ async def sign_and_send(
                     "Failed to send broadcast to %s: %s", destination, response.reason
                 )
                 if kwargs.get("use_legacy_key") is not True:
-                    # try with incorrect keyId to enable communication with legacy Bookwyrm servers
                     logger.info("Trying again with legacy keyId header value")
-
                     asyncio.ensure_future(
                         sign_and_send(
                             session, sender, data, destination, use_legacy_key=True
diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py
index 12a164eb5b..ca68d3fd61 100644
--- a/bookwyrm/tests/test_signing.py
+++ b/bookwyrm/tests/test_signing.py
@@ -111,6 +111,7 @@ def test_remote_signer(self):
         datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json")
         data = json.loads(datafile.read_bytes())
         data["id"] = self.fake_remote.remote_id
+        data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key"
         data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key
         del data["icon"]  # Avoid having to return an avatar.
         responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200)
@@ -138,6 +139,7 @@ def test_key_needs_refresh(self):
         datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json")
         data = json.loads(datafile.read_bytes())
         data["id"] = self.fake_remote.remote_id
+        data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key"
         data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key
         del data["icon"]  # Avoid having to return an avatar.
         responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200)
@@ -157,7 +159,7 @@ def test_key_needs_refresh(self):
                 "bookwyrm.models.relationship.UserFollowRequest.accept"
             ) as accept_mock:
                 response = self.send_test_request(sender=self.fake_remote)
-            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.status_code, 200) #BUG this is 401
             self.assertTrue(accept_mock.called)
 
             # Old key is cached, so still works:

From 98726585f6dc8a36447519c630d093f81ef0adea Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Fri, 14 Apr 2023 18:20:06 +1000
Subject: [PATCH 15/15] oops black

---
 bookwyrm/tests/test_signing.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py
index ca68d3fd61..37d841b33c 100644
--- a/bookwyrm/tests/test_signing.py
+++ b/bookwyrm/tests/test_signing.py
@@ -159,7 +159,7 @@ def test_key_needs_refresh(self):
                 "bookwyrm.models.relationship.UserFollowRequest.accept"
             ) as accept_mock:
                 response = self.send_test_request(sender=self.fake_remote)
-            self.assertEqual(response.status_code, 200) #BUG this is 401
+            self.assertEqual(response.status_code, 200)  # BUG this is 401
             self.assertTrue(accept_mock.called)
 
             # Old key is cached, so still works: