diff --git a/follow.py b/follow.py index 58a554f..e79c7eb 100644 --- a/follow.py +++ b/follow.py @@ -122,7 +122,7 @@ class FollowCallback(indieauth.Callback): followee_id = followee.as1.get('id') timestamp = util.now().replace(microsecond=0, tzinfo=None).isoformat() - follow_id = common.host_url(user.user_page_path(f'following#{timestamp}-{addr}')) + follow_id = f'{user.web_url()}#follow-{timestamp}-{addr}' follow_as1 = { 'objectType': 'activity', 'verb': 'follow', @@ -204,7 +204,7 @@ class UnfollowCallback(indieauth.Callback): # TODO(#529): generalize timestamp = util.now().replace(microsecond=0, tzinfo=None).isoformat() - unfollow_id = common.host_url(user.user_page_path(f'following#undo-{timestamp}-{followee_id}')) + unfollow_id = f'{user.web_url()}#unfollow-{timestamp}-{followee_id}' unfollow_as1 = { 'objectType': 'activity', 'verb': 'stop-following', diff --git a/protocol.py b/protocol.py index 5726999..0068f51 100644 --- a/protocol.py +++ b/protocol.py @@ -547,18 +547,22 @@ class Protocol: raise NotImplementedError() @classmethod - def is_blocklisted(cls, url): + def is_blocklisted(cls, url, allow_internal=False): """Returns True if we block the given URL and shouldn't deliver to it. Default implementation here, subclasses may override. Args: url (str): + allow_internal (bool): whether to return False for internal domains + like ``fed.brid.gy``, ``bsky.brid.gy``, etc Returns: bool: """ - return util.domain_or_parent_in(util.domain_from_link(url), - DOMAIN_BLOCKLIST + DOMAINS) + blocklist = DOMAIN_BLOCKLIST + if not allow_internal: + blocklist += DOMAINS + return util.domain_or_parent_in(util.domain_from_link(url), blocklist) @classmethod def translate_ids(to_cls, obj): @@ -674,7 +678,7 @@ class Protocol: if not id: error('No id provided') - elif from_cls.is_blocklisted(id) and not internal: + elif from_cls.is_blocklisted(id, allow_internal=internal): error(f'Activity {id} is blocklisted') # short circuit if we've already seen this activity id. diff --git a/tests/test_activitypub.py b/tests/test_activitypub.py index b62d421..a80a0ee 100644 --- a/tests/test_activitypub.py +++ b/tests/test_activitypub.py @@ -1825,6 +1825,7 @@ class ActivityPubUtilsTest(TestCase): self.assertFalse(ActivityPub.owns_id('https://twitter.com/foo')) self.assertFalse(ActivityPub.owns_id('https://fed.brid.gy/foo')) + self.assertFalse(ActivityPub.owns_id('https://ap.brid.gy/foo')) def test_owns_handle(self): for handle in ('@user@instance', 'user@instance.com', 'user.com@instance.com', diff --git a/tests/test_follow.py b/tests/test_follow.py index d98f425..c5a0706 100644 --- a/tests/test_follow.py +++ b/tests/test_follow.py @@ -43,17 +43,17 @@ FOLLOWEE = { FOLLOW_ADDRESS = { '@context': 'https://www.w3.org/ns/activitystreams', 'type': 'Follow', - 'id': f'http://localhost/r/alice.com/following#2022-01-02T03:04:05-@foo@ba.r', + 'id': f'http://localhost/r/https://alice.com/#follow-2022-01-02T03:04:05-@foo@ba.r', 'actor': 'http://localhost/alice.com', 'object': FOLLOWEE['id'], 'to': [as2.PUBLIC_AUDIENCE], } FOLLOW_URL = copy.deepcopy(FOLLOW_ADDRESS) -FOLLOW_URL['id'] = f'http://localhost/r/alice.com/following#2022-01-02T03:04:05-https://ba.r/actor' +FOLLOW_URL['id'] = f'http://localhost/r/https://alice.com/#follow-2022-01-02T03:04:05-https://ba.r/actor' UNDO_FOLLOW = { '@context': 'https://www.w3.org/ns/activitystreams', 'type': 'Undo', - 'id': f'http://localhost/r/alice.com/following#undo-2022-01-02T03:04:05-https://ba.r/id', + 'id': f'http://localhost/r/https://alice.com/#unfollow-2022-01-02T03:04:05-https://ba.r/id', 'actor': 'http://localhost/alice.com', 'object': copy.deepcopy(FOLLOW_ADDRESS), } @@ -213,7 +213,7 @@ class FollowTest(TestCase): follow_with_profile_link = { **FOLLOW_URL, - 'id': f'http://localhost/r/alice.com/following#2022-01-02T03:04:05-https://ba.r/id', + 'id': f'http://localhost/r/https://alice.com/#follow-2022-01-02T03:04:05-https://ba.r/id', 'object': 'https://ba.r/id', } self.check('https://ba.r/id', resp, follow_with_profile_link, mock_get, @@ -355,7 +355,7 @@ class FollowTest(TestCase): sig_template.startswith('keyId="http://localhost/alice.com#key"'), sig_template) - follow_id = f'https://fed.brid.gy/web/alice.com/following#2022-01-02T03:04:05-{input}' + follow_id = f'https://alice.com/#follow-2022-01-02T03:04:05-{input}' followers = Follower.query().fetch() followee = ActivityPub(id='https://ba.r/id').key @@ -410,7 +410,7 @@ class FollowTest(TestCase): self.assertEqual(302, resp.status_code) self.assertEqual('/web/www.alice.com/following', resp.headers['Location']) - id = 'www.alice.com/following#2022-01-02T03:04:05-https://ba.r/actor' + id = 'https://www.alice.com/#follow-2022-01-02T03:04:05-https://ba.r/actor' expected_follow_as1 = as2.to_as1({ **FOLLOW_URL, 'id': id, @@ -418,16 +418,14 @@ class FollowTest(TestCase): }) del expected_follow_as1['to'] followee = ActivityPub(id='https://ba.r/id').key - follow_obj = self.assert_object( - f'https://fed.brid.gy/web/{id}', - users=[user.key], - notify=[followee], - status='complete', - labels=['user', 'activity'], - source_protocol='ui', - our_as1=expected_follow_as1, - delivered=['http://ba.r/inbox'], - delivered_protocol='activitypub') + follow_obj = self.assert_object(id, users=[user.key], + notify=[followee], + status='complete', + labels=['user', 'activity'], + source_protocol='ui', + our_as1=expected_follow_as1, + delivered=['http://ba.r/inbox'], + delivered_protocol='activitypub') followers = Follower.query().fetch() self.assert_entities_equal( @@ -607,7 +605,7 @@ class UnfollowTest(TestCase): self.assertEqual('inactive', follower.status) self.assert_object( - 'https://fed.brid.gy/web/alice.com/following#undo-2022-01-02T03:04:05-https://ba.r/id', + 'https://alice.com/#unfollow-2022-01-02T03:04:05-https://ba.r/id', users=[self.user.key], notify=[ActivityPub(id='https://ba.r/id').key], status='complete', @@ -646,7 +644,7 @@ class UnfollowTest(TestCase): self.assertEqual(302, resp.status_code) self.assertEqual('/web/www.alice.com/following', resp.headers['Location']) - id = 'http://localhost/r/www.alice.com/following#undo-2022-01-02T03:04:05-https://ba.r/id' + id = 'http://localhost/r/https://www.alice.com/#unfollow-2022-01-02T03:04:05-https://ba.r/id' expected_undo = { '@context': 'https://www.w3.org/ns/activitystreams', 'type': 'Undo', @@ -670,7 +668,7 @@ class UnfollowTest(TestCase): self.assertEqual('inactive', follower.status) self.assert_object( - 'https://fed.brid.gy/web/www.alice.com/following#undo-2022-01-02T03:04:05-https://ba.r/id', + 'https://www.alice.com/#unfollow-2022-01-02T03:04:05-https://ba.r/id', users=[user.key], notify=[ActivityPub(id='https://ba.r/id').key], status='complete', diff --git a/tests/test_web.py b/tests/test_web.py index e11aab7..8d55919 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -2541,8 +2541,6 @@ class WebUtilTest(TestCase): self.assertIs(False, Web.owns_id('http://localhost:8080/foo')) self.assertIs(False, Web.owns_id('https://twitter.com/')) self.assertIs(False, Web.owns_id('https://ap.brid.gy/foo')) - # TODO: this still needs to be false - # special-case PROTOCOL_DOMAINS homepages in Web.owns_id, otherwise False def test_owns_handle(self, *_): self.assertIsNone(Web.owns_handle('foo.com')) diff --git a/web.py b/web.py index 01072d6..4cba93b 100644 --- a/web.py +++ b/web.py @@ -67,18 +67,21 @@ MAX_FEED_POLL_PERIOD = timedelta(weeks=1) MAX_FEED_PROPERTY_SIZE = 500 * 1000 # Object.atom/rss -def is_valid_domain(domain): +def is_valid_domain(domain, allow_internal=True): """Returns True if this is a valid domain we can use, False otherwise. + Args: + domain (str): + allow_internal (bool): whether to return True for internal domains + like ``fed.brid.gy``, ``bsky.brid.gy``, etc + Valid means TLD is ok, not blacklisted, etc. """ if not domain or not re.match(DOMAIN_RE, domain): # logger.debug(f"{domain} doesn't look like a domain") return False - if (Web.is_blocklisted(domain) - and domain != PRIMARY_DOMAIN - and domain not in PROTOCOL_DOMAINS): + if Web.is_blocklisted(domain, allow_internal=allow_internal): logger.debug(f'{domain} is blocklisted') return False @@ -135,7 +138,7 @@ class Web(User, Protocol): """Validate domain id, don't allow upper case or invalid characters.""" super()._pre_put_hook() id = self.key.id() - assert is_valid_domain(id), id + assert is_valid_domain(id, allow_internal=False), id assert id.lower() == id, f'upper case is not allowed in Web key id: {id}' @classmethod @@ -324,7 +327,7 @@ class Web(User, Protocol): if parsed.path in ('', '/'): id = parsed.netloc - if is_valid_domain(id): + if is_valid_domain(id, allow_internal=True): return super().key_for(id) # logger.info(f'{id} is not a domain or usable home page URL') @@ -344,14 +347,21 @@ class Web(User, Protocol): return True if user and user.has_redirects else None elif is_valid_domain(id): return None - elif util.is_web(id) and is_valid_domain(util.domain_from_link(id)): + + # we allowed internal domains for protocol bot actors above, but we + # don't want to allow non-homepage URLs on those domains, eg + # https://bsky.brid.gy/foo, so don't allow internal here + domain = util.domain_from_link(id) + if util.is_web(id) and is_valid_domain(domain, allow_internal=False): return None return False @classmethod def owns_handle(cls, handle): - if not is_valid_domain(handle): + if handle in PROTOCOL_DOMAINS: + return True + elif not is_valid_domain(handle, allow_internal=False): return False @classmethod @@ -586,7 +596,7 @@ def check_web_site(): # this normalizes and lower cases domain domain = util.domain_from_link(url, minimize=False) - if not domain or not is_valid_domain(domain): + if not domain or not is_valid_domain(domain, allow_internal=False): flash(f'{url} is not a valid or supported web site') return render_template('enter_web_site.html'), 400