From 781fe69470125f888f5c7583db157e0ecc8129f9 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 6 Dec 2021 09:29:51 +1100 Subject: [PATCH] more descriptive remote follow errors - distinguish between invalid username, user not found, and remote follow not supported - make helpers DRYer --- bookwyrm/templates/ostatus/error.html | 28 ++++++++++--- bookwyrm/templates/ostatus/subscribe.html | 6 +-- bookwyrm/views/follow.py | 36 +++++++++++++--- bookwyrm/views/helpers.py | 51 +++++++++++++---------- 4 files changed, 83 insertions(+), 38 deletions(-) diff --git a/bookwyrm/templates/ostatus/error.html b/bookwyrm/templates/ostatus/error.html index b5a3bf40..a603529f 100644 --- a/bookwyrm/templates/ostatus/error.html +++ b/bookwyrm/templates/ostatus/error.html @@ -2,7 +2,28 @@ {% block content %}
- {% if not request.user.is_authenticated and not error == 'remote_subscribe' %} + {% if error == 'invalid_username' %} +
+ {% blocktrans %} +

{{ account }} is not a valid username.

+

Check you have the correct username before trying again.

+ {% endblocktrans %} +
+ {% elif error == 'user_not_found' %} +
+ {% blocktrans %} +

{{ account }} could not be found or {{ remote_domain }} does not support identity discovery.

+

Check you have the correct username before trying again.

+ {% endblocktrans %} +
+ {% elif error == 'not_supported' %} +
+ {% blocktrans %} +

{{ account }} was found but {{ remote_domain }} does not support 'remote follow'.

+

Try searching for {{ user }} on {{ remote_domain }} instead.

+ {% endblocktrans %} +
+ {% elif not request.user.is_authenticated %}
@@ -31,11 +52,6 @@

{% blocktrans %}Something went wrong trying to follow {{ account }}{% endblocktrans %}

{% trans 'Check you have the correct username before trying again.' %}

- {% elif error == 'remote_subscribe' %} -
-

{% blocktrans %}Something went wrong trying to follow from {{ account }}{% endblocktrans %}

-

{% trans 'Check you have the correct username before trying again.' %}

-
{% elif error == 'is_blocked' %}

{% blocktrans %}You have blocked {{ account }}{% endblocktrans %}

diff --git a/bookwyrm/templates/ostatus/subscribe.html b/bookwyrm/templates/ostatus/subscribe.html index 51dec438..ee622542 100644 --- a/bookwyrm/templates/ostatus/subscribe.html +++ b/bookwyrm/templates/ostatus/subscribe.html @@ -14,10 +14,10 @@ {% endblock %} {% block heading %} -{% if not request.user.is_authenticated and not error == 'remote_subscribe' %} -{% trans "Let's log in first..." %} -{% elif error %} +{% if error %} {% trans 'Uh oh...' %} +{% elif not request.user.is_authenticated %} +{% trans "Let's log in first..." %} {% else %} {% blocktrans with sitename=site.name %}Follow from {{ sitename }}{% endblocktrans %} {% endif %} diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index 7a9d7bad..6b1de5e6 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -12,6 +12,7 @@ from .helpers import ( get_user_from_username, handle_remote_webfinger, subscribe_remote_webfinger, + WebFingerError, ) @@ -142,7 +143,7 @@ def ostatus_follow_success(request): def remote_follow_page(request): - """Display remote follow page""" + """display remote follow page""" user = get_user_from_username(request.user, request.GET.get("user")) data = {"user": user} return TemplateResponse(request, "ostatus/remote_follow.html", data) @@ -152,10 +153,33 @@ def remote_follow_page(request): def remote_follow(request): """direct user to follow from remote account using ostatus subscribe protocol""" remote_user = request.POST.get("remote_user") - template = subscribe_remote_webfinger(remote_user) - if template is None: - data = {"account": remote_user, "user": None, "error": "remote_subscribe"} - return TemplateResponse(request, "ostatus/subscribe.html", data) + try: + if remote_user[0] == "@": + remote_user = remote_user[1:] + remote_domain = remote_user.split("@")[1] + except: + remote_domain = None + + wf_response = subscribe_remote_webfinger(remote_user) user = get_object_or_404(models.User, id=request.POST.get("user")) - url = template.replace("{uri}", urllib.parse.quote(user.remote_id)) + + if wf_response is None: + data = { + "account": remote_user, + "user": user, + "error": "not_supported", + "remote_domain": remote_domain, + } + return TemplateResponse(request, "ostatus/subscribe.html", data) + + if type(wf_response) == WebFingerError: + data = { + "account": remote_user, + "user": user, + "error": str(wf_response), + "remote_domain": remote_domain, + } + return TemplateResponse(request, "ostatus/subscribe.html", data) + + url = wf_response.replace("{uri}", urllib.parse.quote(user.remote_id)) return redirect(url) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 650087ec..eb15637e 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -15,6 +15,27 @@ from bookwyrm.status import create_generated_note from bookwyrm.utils import regex +class WebFingerError(Exception): + """error class for problems finding user information with webfinger""" + + pass + + +def get_domain_from_username(username): + # usernames could be @user@domain or user@domain + if not username: + return None + + if username[0] == "@": + username = username[1:] + + try: + domain = username.split("@")[1] + return domain + except IndexError: + return None + + def get_user_from_username(viewer, username): """helper function to resolve a localname or a username to a user""" if viewer.is_authenticated and viewer.localname == username: @@ -52,17 +73,8 @@ def is_bookwyrm_request(request): def handle_remote_webfinger(query): """webfingerin' other servers""" user = None - - # usernames could be @user@domain or user@domain - if not query: - return None - - if query[0] == "@": - query = query[1:] - - try: - domain = query.split("@")[1] - except IndexError: + domain = get_domain_from_username(query) + if domain is None: return None try: @@ -88,24 +100,17 @@ def handle_remote_webfinger(query): def subscribe_remote_webfinger(query): """get subscribe template from other servers""" template = None + domain = get_domain_from_username(query) - # usernames could be @user@domain or user@domain - if not query: - return None - - if query[0] == "@": - query = query[1:] - - try: - domain = query.split("@")[1] - except IndexError: - return None + if domain is None: + return WebFingerError("invalid_username") url = f"https://{domain}/.well-known/webfinger?resource=acct:{query}" + try: data = get_data(url) except (ConnectorException, HTTPError): - return None + return WebFingerError("user_not_found") for link in data.get("links"): if link.get("rel") == "http://ostatus.org/schema/1.0/subscribe":