use Protocol.create_for for all protocols, not just HAS_COPIES ones

we can now use create_for for all protocol-specific setup for new users, eg creating AP keypairs. for #1881

first attempt at this may have caused #1929, so I'm paying more attention to it this time

activitypub.py

@@ -3,10 +3,12 @@ from base64 import b64encode
 from hashlib import sha256
 import itertools
 import logging
+import random
 import re
 from urllib.parse import quote_plus, urljoin, urlparse
 from unittest.mock import MagicMock
 
+from Crypto.PublicKey import RSA
 from flask import abort, g, redirect, request
 from google.cloud import ndb
 from google.cloud.ndb.query import FilterNode, OR, Query
@@ -34,6 +36,7 @@ from common import (
     error,
     host_url,
     LOCAL_DOMAINS,
+    long_to_base64,
     PRIMARY_DOMAIN,
     PROTOCOL_DOMAINS,
     redirect_wrap,
@@ -43,7 +46,7 @@ from common import (
 )
 from ids import BOT_ACTOR_AP_IDS
 import memcache
-from models import fetch_objects, Follower, Object, PROTOCOLS, User
+from models import fetch_objects, Follower, KEY_BITS, Object, PROTOCOLS, User
 from protocol import activity_id_memcache_key, DELETE_TASK_DELAY, Protocol
 import webfinger
 
@@ -85,7 +88,7 @@ def instance_actor():
     global _INSTANCE_ACTOR
     if _INSTANCE_ACTOR is None:
         import web
-        _INSTANCE_ACTOR = web.Web.get_or_create(PRIMARY_DOMAIN)
+        _INSTANCE_ACTOR = web.Web.get_or_create(PRIMARY_DOMAIN, propagate=True)
     return _INSTANCE_ACTOR
 
 
@@ -259,6 +262,28 @@ class ActivityPub(User, Protocol):
         kwargs['prefer_id'] = False
         return super().user_page_path(rest=rest, **kwargs)
 
+    @classmethod
+    def create_for(cls, user):
+        """Creates an AP keypair for a non-APProto user.
+
+        Can use urandom() and do nontrivial math, so this can take time
+        depending on the amount of randomness available and compute needed.
+
+        Args:
+          user (models.User)
+        """
+        assert not isinstance(user, ActivityPub)
+
+        if not user.public_exponent or not user.private_exponent or not user.mod:
+            assert (not user.public_exponent and not user.private_exponent
+                    and not user.mod), id
+            key = RSA.generate(KEY_BITS, randfunc=random.randbytes
+                               if appengine_info.DEBUG else None)
+            user.mod = long_to_base64(key.n)
+            user.public_exponent = long_to_base64(key.e)
+            user.private_exponent = long_to_base64(key.d)
+            user.put()
+
     @classmethod
     def target_for(cls, obj, shared=False):
         """Returns ``obj``'s or its author's/actor's inbox, if available."""
@@ -1083,7 +1108,8 @@ def _load_user(handle_or_id, create=False):
 
     assert id
     try:
-        user = proto.get_or_create(id) if create else proto.get_by_id(id)
+        user = (proto.get_or_create(id, propagate=True) if create
+                else proto.get_by_id(id))
     except ValueError as e:
         logging.warning(e)
         user = None

models.py

@@ -5,7 +5,6 @@ from functools import lru_cache
 import itertools
 import json
 import logging
-import random
 import re
 from threading import Lock
 from urllib.parse import quote, urlparse
@@ -30,7 +29,6 @@ import common
 from common import (
     base64_to_long,
     DOMAIN_RE,
-    long_to_base64,
     OLD_ACCOUNT_AGE,
     PROTOCOL_DOMAINS,
     report_error,
@@ -377,30 +375,13 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
                 proto = PROTOCOLS[label]
                 if proto == cls:
                     continue
-                elif proto.HAS_COPIES:
-                    if not user.get_copy(proto) and user.is_enabled(proto):
-                        try:
-                            proto.create_for(user)
-                        except (ValueError, AssertionError):
-                            logger.info(f'failed creating {proto.LABEL} copy',
-                                        exc_info=True)
-                            util.remove(user.enabled_protocols, proto.LABEL)
-                    else:
-                        logger.debug(f'{proto.LABEL} not enabled or user copy already exists, skipping propagate')
-
-        # generate keys for all protocols _except_ our own
-        #
-        # these can use urandom() and do nontrivial math, so they can take time
-        # depending on the amount of randomness available and compute needed.
-        if cls.LABEL != 'activitypub':
-            if (not user.public_exponent or not user.private_exponent or not user.mod):
-                assert (not user.public_exponent and not user.private_exponent
-                        and not user.mod), id
-                key = RSA.generate(KEY_BITS,
-                                   randfunc=random.randbytes if DEBUG else None)
-                user.mod = long_to_base64(key.n)
-                user.public_exponent = long_to_base64(key.e)
-                user.private_exponent = long_to_base64(key.d)
+                elif user.is_enabled(proto):
+                    try:
+                        proto.create_for(user)
+                    except (ValueError, AssertionError):
+                        logger.info(f'failed creating {proto.LABEL} copy',
+                                    exc_info=True)
+                        util.remove(user.enabled_protocols, proto.LABEL)
 
         try:
             user.put()
@@ -603,10 +584,9 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
                            text=ineligible.format(desc=e))
             common.error(str(e), status=299)
 
-        if to_proto.LABEL in ids.COPIES_PROTOCOLS:
-            # do this even if there's an existing copy since we might need to
-            # reactivate it, which create_for should do
-            to_proto.create_for(self)
+        # do this even if there's an existing copy since we might need to
+        # reactivate it, which create_for should do
+        to_proto.create_for(self)
 
         if to_proto.LABEL not in self.enabled_protocols:
             self.enabled_protocols.append(to_proto.LABEL)

protocol.py

@@ -497,20 +497,22 @@ class Protocol:
 
     @classmethod
     def create_for(cls, user):
-        """Creates or re-activate a copy user in this protocol.
+        """Creates or re-activate a user in this protocol.
 
-        Should add the copy user to :attr:`copies`.
+        If this protocol has copies, adds the new copy user to :attr:`copies`.
+        If the copy user already exists and active, does nothing.
 
-        If the copy user already exists and active, should do nothing.
+        By default, does nothing.
 
         Args:
           user (models.User): original source user. Shouldn't already have a
             copy user for this protocol in :attr:`copies`.
 
         Raises:
-          ValueError: if we can't create a copy of the given user in this protocol
+          ValueError: if we can't create the given user in this protocol
         """
-        raise NotImplementedError()
+        if cls.HAS_COPIES:
+            raise NotImplementedError()
 
     @classmethod
     def send(to_cls, obj, url, from_user=None, orig_obj_id=None):

tests/test_models.py

@@ -72,7 +72,8 @@ class UserTest(TestCase):
         self.assertIsNone(Web.get_by_id('y.za'))
 
     def test_get_or_create(self):
-        user = Fake.get_or_create('fake:user')
+        user = Fake.get_or_create('fake:user', enabled_protocols=['activitypub'],
+                                  propagate=True)
 
         assert not user.existing
         assert user.mod
@@ -187,11 +188,17 @@ class UserTest(TestCase):
         self.assertIsNone(Fake.get_or_create('fake:user', manual_opt_out=True))
 
     def test_public_pem(self):
+        bot = self.make_user(id='ap.brid.gy', cls=Web)
+        self.user.enable_protocol(ActivityPub)
+
         pem = self.user.public_pem()
         self.assertTrue(pem.decode().startswith('-----BEGIN PUBLIC KEY-----\n'), pem)
         self.assertTrue(pem.decode().endswith('-----END PUBLIC KEY-----'), pem)
 
     def test_private_pem(self):
+        bot = self.make_user(id='ap.brid.gy', cls=Web)
+        self.user.enable_protocol(ActivityPub)
+
         pem = self.user.private_pem()
         self.assertTrue(pem.decode().startswith('-----BEGIN RSA PRIVATE KEY-----\n'), pem)
         self.assertTrue(pem.decode().endswith('-----END RSA PRIVATE KEY-----'), pem)
@@ -445,6 +452,19 @@ class UserTest(TestCase):
 
         self.assertIsNone(OtherFake().get_copy(Fake))
 
+    def test_enable_protocol_calls_create_for(self):
+        bot = self.make_user(id='ap.brid.gy', cls=Web)
+        user = Fake(id='fake:user')
+        user.put()
+
+        self.assertIsNone(user.public_exponent)
+        self.assertIsNone(user.private_exponent)
+
+        user.enable_protocol(ActivityPub)
+
+        self.assertIsNotNone(user.public_exponent)
+        self.assertIsNotNone(user.private_exponent)
+
     def test_count_followers(self):
         self.assertEqual((0, 0), self.user.count_followers())
 

tests/testutil.py

@@ -295,7 +295,8 @@ Web.DEFAULT_ENABLED_PROTOCOLS += ('fake', 'other')
 # expensive to generate.
 requests.post(f'http://{ndb_client.host}/reset')
 with ndb_client.context():
-    global_user = activitypub._INSTANCE_ACTOR = Fake.get_or_create('fake:user')
+    global_user = activitypub._INSTANCE_ACTOR = Fake.get_or_create(
+        'fake:user', propagate=True, enabled_protocols=['activitypub'])
 
 
 class TestCase(unittest.TestCase, testutil.Asserts):

web.py

@@ -192,7 +192,7 @@ class Web(User, Protocol):
             return None
 
         if verify or (verify is None and not user.existing):
-            user = user.verify()
+            user = user.verify(**kwargs)
 
         if not allow_opt_out and user.status:
             return None
@@ -289,9 +289,13 @@ class Web(User, Protocol):
 
         return super().status
 
-    def verify(self):
+    def verify(self, **kwargs):
         """Fetches site a couple ways to check for redirects and h-card.
 
+        Args:
+          **kwargs: passed through to :meth:`Web.get_or_create` if this user is a www
+            domain and we need to call it to create a new root domain user.
+
         Returns:
           web.Web: user that was verified. May be different than self! eg if
           self's domain started with www and we switch to the root domain.
@@ -310,7 +314,7 @@ class Web(User, Protocol):
                     logger.info(f'{root_site} serves ok ; using {root} instead')
                     root_user = Web.get_or_create(
                         root, enabled_protocols=self.enabled_protocols,
-                        allow_opt_out=True)
+                        allow_opt_out=True, **kwargs)
                     self.use_instead = root_user.key
                     self.put()
                     return root_user.verify()