diff --git a/activitypub.py b/activitypub.py index d62f0f8b..05017189 100644 --- a/activitypub.py +++ b/activitypub.py @@ -52,25 +52,14 @@ class ActivityPub(User, Protocol): """ActivityPub protocol class. Key id is AP/AS2 actor id URL. (*Not* fediverse/WebFinger @-@ handle!) - - Includes extra `address` computed property for querying entities by handle. """ LABEL = 'activitypub' @ndb.ComputedProperty - def address(self): + def readable_id(self): + """Returns fediverse handle ie WebFinger address, eg '@me@snarfed.org'.""" return self.ap_address() - @classmethod - def get_by_id(cls, id): - """Override User.get_by_id to fall back to querying by address.""" - return (super(User, ActivityPub).get_by_id(id) - or ActivityPub.query(ActivityPub.address == id).get()) - - def label_id(self): - """Returns this user's human-readable unique id, eg '@me@snarfed.org'.""" - return self.address or self.key.id() - def web_url(self): """Returns this user's web URL aka web_url, eg 'https://foo.com/'.""" return util.get_url(self.actor_as2) or self.ap_actor() diff --git a/models.py b/models.py index 5977a056..84fdb1f9 100644 --- a/models.py +++ b/models.py @@ -98,6 +98,18 @@ class User(StringIdModel, metaclass=ProtocolUserMeta): created = ndb.DateTimeProperty(auto_now_add=True) updated = ndb.DateTimeProperty(auto_now=True) + @ndb.ComputedProperty + def readable_id(self): + """This user's human-readable unique id, eg '@me@snarfed.org'. + + To be implemented by subclasses. + """ + return None + + def readable_or_key_id(self): + """Returns readable_id if set, otherwise key id.""" + return self.readable_id or self.key.id() + @classmethod def new(cls, **kwargs): """Try to prevent instantiation. Use subclasses instead.""" @@ -181,11 +193,7 @@ def name(self): if name: return name - return self.label_id() - - def label_id(self): - """Returns this user's human-readable unique id, eg '@me@snarfed.org'.""" - return self.key.id() + return self.readable_or_key_id() def username(self): """Returns the user's preferred username. @@ -270,7 +278,7 @@ def ap_actor(self, rest=None): def user_page_path(self, rest=None): """Returns the user's Bridgy Fed user page path.""" - path = f'/{self.LABEL}/{self.label_id()}' + path = f'/{self.LABEL}/{self.readable_or_key_id()}' if rest: if not rest.startswith('?'): diff --git a/pages.py b/pages.py index 5b6377a5..5b33c2cd 100644 --- a/pages.py +++ b/pages.py @@ -55,10 +55,14 @@ def web_user_redirects(**kwargs): @app.get(f'//') def user(protocol, id): - g.user = PROTOCOLS[protocol].get_by_id(id) + cls = PROTOCOLS[protocol] + g.user = cls.get_by_id(id) + if not g.user: + g.user = cls.query(cls.readable_id == id).get() + if not g.user or not g.user.direct: return USER_NOT_FOUND_HTML, 404 - elif id != g.user.label_id(): + elif id != g.user.readable_or_key_id(): # this also handles use_instead return redirect(g.user.user_page_path(), code=301) assert not g.user.use_instead diff --git a/tests/test_activitypub.py b/tests/test_activitypub.py index 1740ac76..f5e2da61 100644 --- a/tests/test_activitypub.py +++ b/tests/test_activitypub.py @@ -1585,15 +1585,15 @@ def test_postprocess_as2_idempotent(self): def test_ap_address(self): user = ActivityPub(actor_as2={**ACTOR, 'preferredUsername': 'me'}) self.assertEqual('@me@mas.to', user.ap_address()) - self.assertEqual('@me@mas.to', user.address) + self.assertEqual('@me@mas.to', user.readable_id) user = ActivityPub(actor_as2=ACTOR) self.assertEqual('@swentel@mas.to', user.ap_address()) - self.assertEqual('@swentel@mas.to', user.address) + self.assertEqual('@swentel@mas.to', user.readable_id) user = ActivityPub(id='https://mas.to/users/alice') self.assertEqual('@alice@mas.to', user.ap_address()) - self.assertEqual('@alice@mas.to', user.address) + self.assertEqual('@alice@mas.to', user.readable_id) def test_ap_actor(self): user = self.make_user('http://foo/actor', cls=ActivityPub) @@ -1609,19 +1609,11 @@ def test_web_url(self): user.actor_as2['url'] = ['http://my/url'] self.assertEqual('http://my/url', user.web_url()) - def test_label_id(self): + def test_readable_id(self): user = self.make_user('http://foo', cls=ActivityPub) - self.assertEqual('http://foo', user.label_id()) + self.assertIsNone(user.readable_id) + self.assertEqual('http://foo', user.readable_or_key_id()) user.actor_as2 = ACTOR - self.assertEqual('@swentel@mas.to', user.label_id()) - - def test_get_by_id(self): - user = self.make_user('http://foo', cls=ActivityPub) - self.assert_entities_equal(user, ActivityPub.get_by_id('http://foo')) - self.assertIsNone(ActivityPub.get_by_id('@swentel@mas.to')) - - user.actor_as2 = ACTOR - user.put() - self.assert_entities_equal(user, ActivityPub.get_by_id('http://foo')) - self.assert_entities_equal(user, ActivityPub.get_by_id('@swentel@mas.to')) + self.assertEqual('@swentel@mas.to', user.readable_id) + self.assertEqual('@swentel@mas.to', user.readable_or_key_id()) diff --git a/tests/test_models.py b/tests/test_models.py index cbc68226..7972f080 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -103,8 +103,8 @@ def test_name(self): g.user.actor_as2 = {'name': 'alice'} self.assertEqual('alice', g.user.name()) - def test_label_id(self): - self.assertEqual('y.z', g.user.label_id()) + def test_readable_id(self): + self.assertIsNone(g.user.readable_id) class ObjectTest(TestCase): diff --git a/tests/test_pages.py b/tests/test_pages.py index 0a8965db..f5dd32fa 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -47,10 +47,10 @@ def test_user_fake(self): got = self.client.get('/fake/foo.com') self.assert_equals(200, got.status_code) - def test_user_activitypub_address(self): + def test_user_readable_id_activitypub_address(self): user = self.make_user('foo', cls=ActivityPub, actor_as2=ACTOR_WITH_PREFERRED_USERNAME) - self.assertEqual('@me@plus.google.com', user.address) + self.assertEqual('@me@plus.google.com', user.ap_address()) got = self.client.get('/activitypub/@me@plus.google.com') self.assert_equals(200, got.status_code) @@ -74,7 +74,7 @@ def test_user_not_direct(self): got = self.client.get('/web/user.com') self.assert_equals(404, got.status_code) - def test_user_redirect(self): + def test_user_web_redirect(self): got = self.client.get('/user/user.com') self.assert_equals(301, got.status_code) self.assert_equals('/web/user.com', got.headers['Location']) diff --git a/tests/test_web.py b/tests/test_web.py index e43bb2ad..c5de0abf 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1514,9 +1514,6 @@ def test_verify_override_preferredUsername(self, mock_get, _): 'preferredUsername': 'user.com', }) - def test_label_id(self, _, __): - self.assertEqual('user.com', self.user.label_id()) - def test_web_url(self, _, __): self.assertEqual('https://user.com/', self.user.web_url()) diff --git a/web.py b/web.py index 862d3a2e..a591d9e1 100644 --- a/web.py +++ b/web.py @@ -7,7 +7,7 @@ import feedparser from flask import g, redirect, render_template, request from flask.views import View -from google.cloud.ndb import Key +from google.cloud.ndb import ComputedProperty, Key from granary import as1, as2, microformats2 import mf2util from oauth_dropins.webutil import flask_util, util @@ -54,9 +54,12 @@ class Web(User, Protocol): def _get_kind(cls): return 'MagicKey' - def label_id(self): + @ComputedProperty + def readable_id(self): # prettify if domain, noop if username - return util.domain_from_link(self.username(), minimize=False) + username = self.username() + if username != self.key.id(): + return util.domain_from_link(username, minimize=False) def web_url(self): """Returns this user's web URL aka web_url, eg 'https://foo.com/'.""" diff --git a/webfinger.py b/webfinger.py index fb55a40e..ecc4ed99 100644 --- a/webfinger.py +++ b/webfinger.py @@ -112,7 +112,7 @@ def template_vars(self, domain=None, allow_indirect=False): 'rel': 'http://ostatus.org/schema/1.0/subscribe', # TODO(#512) switch to: # 'template': common.host_url(g.user.user_page_path('?url={uri}')), - # the problem is that user_page_path() uses label_id(), which uses + # the problem is that user_page_path() uses readable_id, which uses # custom username instead of domain, which may not be unique 'template': common.host_url(f'web/{domain}?url={{uri}}'), }]