From 797a0bbbb3032a845f0f839b3eefa05b5f836ea1 Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Wed, 7 Jun 2023 11:51:31 -0700 Subject: [PATCH] bug fix, don't redirect web users to readable_id fixes #534 --- pages.py | 66 ++++++++++++++++++++++++--------------------- tests/test_pages.py | 30 +++++++++++++++++---- web.py | 11 ++++++++ 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/pages.py b/pages.py index 291c72c..81f6283 100644 --- a/pages.py +++ b/pages.py @@ -31,6 +31,38 @@ with app.test_request_context('/'): logger = logging.getLogger(__name__) +def load_user(protocol, id): + """Loads the current request's user into `g.user`. + + Args: + protocol: str + id: str + + Raises: + :class:`werkzeug.exceptions.HTTPException` on error or redirect + """ + cls = PROTOCOLS[protocol] + g.user = cls.get_by_id(id) + + # TODO(#512): generalize across protocols + if protocol == 'activitypub': + if not g.user: + g.user = cls.query(cls.readable_id == id).get() + if g.user and g.user.use_instead: + g.user = g.user.use_instead.get() + + if g.user and id != g.user.readable_or_key_id(): + error('', status=302, location=g.user.user_page_path()) + + elif g.user and id != g.user.key.id(): # use_instead redirect + error('', status=302, location=g.user.user_page_path()) + + if not g.user or not g.user.direct: + error(USER_NOT_FOUND_HTML, status=404) + + assert not g.user.use_instead + + @app.route('/') @flask_util.cached(cache, datetime.timedelta(days=1)) def front_page(): @@ -55,17 +87,7 @@ def web_user_redirects(**kwargs): @app.get(f'//') def user(protocol, id): - # TODO: unify this with followers_or_following, others - 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.readable_or_key_id(): # this also handles use_instead - return redirect(g.user.user_page_path(), code=301) - - assert not g.user.use_instead + load_user(protocol, id) query = Object.query( Object.domains == id, @@ -94,17 +116,7 @@ def user(protocol, id): @app.get(f'///') def followers_or_following(protocol, id, collection): - # TODO: unify this with user, feed - 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.readable_or_key_id(): # this also handles use_instead - return redirect(g.user.user_page_path(), code=301) - - assert not g.user.use_instead + load_user(protocol, id) followers, before, after = Follower.fetch_page(id, collection) @@ -131,15 +143,7 @@ def feed(protocol, id): if format not in ('html', 'atom', 'rss'): error(f'format {format} not supported; expected html, atom, or rss') - # TODO: unify this with user, followers_or_following - 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.readable_or_key_id(): # this also handles use_instead - return redirect(g.user.user_page_path(), code=301) + load_user(protocol, id) assert not g.user.use_instead diff --git a/tests/test_pages.py b/tests/test_pages.py index f5dd32f..791feea 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -56,9 +56,28 @@ class PagesTest(TestCase): self.assert_equals(200, got.status_code) got = self.client.get('/activitypub/foo') - self.assert_equals(301, got.status_code) + self.assert_equals(302, got.status_code) self.assert_equals('/activitypub/@me@plus.google.com', got.headers['Location']) + def test_user_web_custom_username_doesnt_redirect(self): + """https://github.com/snarfed/bridgy-fed/issues/534""" + self.user.actor_as2 = { + **ACTOR_AS2, + 'url': 'acct:baz@user.com', + } + self.user.put() + self.assertEqual('baz', self.user.username()) + + got = self.client.get('/web/@baz@user.com') + self.assert_equals(404, got.status_code) + + got = self.client.get('/web/baz') + self.assert_equals(404, got.status_code) + + got = self.client.get('/web/user.com') + self.assert_equals(200, got.status_code) + self.assertIn('@baz@user.com', got.get_data(as_text=True)) + def test_user_objects(self): self.add_objects() got = self.client.get('/web/user.com') @@ -80,14 +99,15 @@ class PagesTest(TestCase): self.assert_equals('/web/user.com', got.headers['Location']) def test_user_use_instead(self): - bar = self.make_user('bar.com') - bar.use_instead = self.user.key - bar.put() + self.make_user('bar.com', use_instead=self.user.key) got = self.client.get('/web/bar.com') - self.assert_equals(301, got.status_code) + self.assert_equals(302, got.status_code) self.assert_equals('/web/user.com', got.headers['Location']) + got = self.client.get('/web/user.com') + self.assert_equals(200, got.status_code) + def test_user_object_bare_string_id(self): with self.request_context: Object(id='a', domains=['user.com'], labels=['notification'], diff --git a/web.py b/web.py index 15dffd5..0258d36 100644 --- a/web.py +++ b/web.py @@ -88,6 +88,17 @@ class Web(User, Protocol): url += f'/{rest}' return url + def user_page_path(self, rest=None): + """Always use domain.""" + path = f'/{self.LABEL}/{self.key.id()}' + + if rest: + if not rest.startswith('?'): + path += '/' + path += rest + + return path + def username(self): """Returns the user's preferred username.