implement server-side avatar refresh

closes #2504

js/10-base.js

@@ -105,10 +105,6 @@ Liberapay.init = function() {
     $('[data-email-reveal]').one('click', function () {
         $(this).html($(this).data('email-reveal'));
     });
-
-    $('button[data-action="reload"]').on('click', function() {
-        location.reload();
-    });
 };
 
 $(function(){

liberapay/constants.py

@@ -40,7 +40,6 @@ def check_bits(bits):
                                 "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                                 "-_.")
 
-AVATAR_QUERY = '?s=160&d=404'
 AVATAR_SOURCES = (
     'libravatar bitbucket github gitlab mastodon pleroma twitch twitter'
 ).split()

liberapay/models/account_elsewhere.py

@@ -1,6 +1,5 @@
 from datetime import timedelta
 import json
-from urllib.parse import urlsplit, urlunsplit
 import uuid
 
 from markupsafe import Markup
@@ -9,14 +8,14 @@
 from postgres.orm import Model
 from psycopg2 import IntegrityError
 
-from ..constants import AVATAR_QUERY, DOMAIN_RE, SUMMARY_MAX_SIZE
+from ..constants import DOMAIN_RE, SUMMARY_MAX_SIZE
 from ..cron import logger
 from ..elsewhere._base import (
     ElsewhereError, InvalidServerResponse, UserNotFound,
 )
 from ..exceptions import InvalidId
 from ..security.crypto import constant_time_compare
-from ..utils import excerpt_intro
+from ..utils import excerpt_intro, tweak_avatar_url
 from ..website import website
 
 
@@ -114,13 +113,7 @@ def upsert(cls, i):
 
         # Clean up avatar_url
         if i.avatar_url:
-            scheme, netloc, path, query, fragment = urlsplit(i.avatar_url)
-            fragment = ''
-            if netloc.endswith('githubusercontent.com') or \
-               netloc.endswith('gravatar.com') or \
-               netloc.endswith('libravatar.org'):
-                query = AVATAR_QUERY
-            i.avatar_url = urlunsplit((scheme, netloc, path, query, fragment))
+            i.avatar_url = tweak_avatar_url(i.avatar_url)
 
         d = dict(i.__dict__)
         d.pop('email', None)
@@ -193,7 +186,7 @@ def update():
                     raise
 
         # Return account after propagating avatar_url to participant
-        account.participant.update_avatar(check=False)
+        account.participant.update_avatar()
         return account
 
 

liberapay/models/participant.py

@@ -30,7 +30,7 @@
 
 from liberapay.billing.payday import compute_next_payday_date
 from liberapay.constants import (
-    ASCII_ALLOWED_IN_USERNAME, AVATAR_QUERY, BASE64URL_CHARS, CURRENCIES,
+    ASCII_ALLOWED_IN_USERNAME, BASE64URL_CHARS, CURRENCIES,
     DONATION_LIMITS, EMAIL_VERIFICATION_TIMEOUT, EVENTS, HTML_A,
     PASSWORD_MAX_SIZE, PASSWORD_MIN_SIZE, PAYPAL_CURRENCIES,
     PERIOD_CONVERSION_RATES, PRIVILEGES,
@@ -87,6 +87,7 @@
 from liberapay.security.crypto import constant_time_compare
 from liberapay.utils import (
     deserialize, erase_cookie, get_recordable_headers, serialize, set_cookie,
+    tweak_avatar_url,
     markdown,
 )
 from liberapay.utils.emails import (
@@ -2200,7 +2201,7 @@ def is_char_forbidden(char):
 
         return new_public_name
 
-    def update_avatar(self, src=None, cursor=None, avatar_email=None, check=True):
+    def update_avatar(self, src=None, cursor=None, avatar_email=None, refresh=False):
         if self.status == 'stub':
             assert src is None
 
@@ -2274,7 +2275,6 @@ def update_avatar(self, src=None, cursor=None, avatar_email=None, check=True):
                 website.tell_sentry(e)
             avatar_id = md5(normalized_email.encode('utf8')).hexdigest()
             avatar_url = avatar_origin + '/avatar/' + avatar_id
-            avatar_url += AVATAR_QUERY
 
         elif platform is None:
             avatar_url = (cursor or self.db).one("""
@@ -2298,7 +2298,14 @@ def update_avatar(self, src=None, cursor=None, avatar_email=None, check=True):
                  LIMIT 1
             """, (self.id, platform, user_id or None))
 
-        if avatar_url and avatar_url != self.avatar_url and check and website.app_conf.check_avatar_urls:
+        avatar_url = tweak_avatar_url(avatar_url, increment=refresh)
+        check_url = (
+            avatar_url and
+            avatar_url != self.avatar_url and
+            self.status != 'stub' and
+            website.app_conf.check_avatar_urls
+        )
+        if check_url:
             # Check that the new avatar URL returns a 200.
             try:
                 r = requests.head(avatar_url, allow_redirects=True, timeout=5)

liberapay/utils/__init__.py

@@ -9,6 +9,7 @@
 import os
 import re
 import socket
+from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit
 
 from pando import Response, json
 from pando.utils import to_rfc822, utcnow
@@ -730,3 +731,23 @@ def get_recordable_headers(request):
         for k, v in request.headers.items()
         if k != b'Cookie'
     }
+
+
+def tweak_avatar_url(avatar_url, increment=True):
+    if not avatar_url:
+        return ''
+    # Parse the URL
+    scheme, netloc, path, query, fragment = urlsplit(avatar_url)
+    query = parse_qs(query)
+    # Add parameters inherited from Gravatar (https://wiki.libravatar.org/api/)
+    query['s'] = '160'  # size = 160 pixels
+    query['d'] = '404'  # default = a 404 HTTP response
+    # Increment the serial number to avoid stale images in a browser's cache
+    try:
+        query[''] = str(int(query[''][-1]) + int(increment))
+    except (KeyError, ValueError):
+        query[''] = '1'
+    # Drop any fragment that might be there
+    fragment = ''
+    # Return the modified URL
+    return urlunsplit((scheme, netloc, path, urlencode(query), fragment))

tests/py/fixtures/TestAvatars.yml

@@ -3,7 +3,7 @@ interactions:
     body: null
     headers: {}
     method: HEAD
-    uri: https://seccdn.libravatar.org/avatar/20f0944fefc09a31e43c55bc30c25cdf?s=160&d=404
+    uri: https://seccdn.libravatar.org/avatar/20f0944fefc09a31e43c55bc30c25cdf?s=160&d=404&=1
   response:
     body: {string: ''}
     headers:
@@ -54,7 +54,7 @@ interactions:
     body: null
     headers: {}
     method: HEAD
-    uri: https://liberapay.com/assets/nonexistent.jpg
+    uri: https://liberapay.com/assets/nonexistent.jpg?s=160&d=404&=1
   response:
     body: {string: ''}
     headers:

tests/py/test_elsewhere.py

@@ -119,10 +119,10 @@ def test_upsert_correctly_updates_the_participant_avatar_url(self):
         alice = alice.refetch()
         assert alice.avatar_url == libravatar_url
         alice.update_avatar(src='github:')
-        assert alice.avatar_url == 'fake-github-avatar-url'
+        assert alice.avatar_url == 'fake-github-avatar-url?s=160&d=404&=1'
         alice_github_info.avatar_url = 'new-fake-github-avatar-url'
         alice_github = AccountElsewhere.upsert(alice_github_info)
-        assert alice_github.participant.avatar_url == 'new-fake-github-avatar-url'
+        assert alice_github.participant.avatar_url == 'new-fake-github-avatar-url?s=160&d=404&=1'
 
     @mock.patch('liberapay.elsewhere._base.Platform.get_user_info')
     def test_user_pages(self, get_user_info):

www/%username/edit/avatar.spt

@@ -5,17 +5,23 @@ from liberapay.utils import form_post_success, get_participant
 participant = get_participant(state, restrict=True, allow_member=True)
 
 if request.method == 'POST':
-    src, email = request.body['src'], request.body.get('email', '')
-
-    if src not in constants.AVATAR_SOURCES:
-        raise response.invalid_input(src, 'src', 'body')
-    if email and '@' not in email:
-        raise BadEmailAddress(email)
-
-    new_avatar_url = participant.update_avatar(src+':', avatar_email=email)
+    action = request.body.get('action', 'set_source')
+    if action == 'refresh':
+        new_avatar_url = participant.update_avatar(refresh=True)
+    elif action == 'set_source':
+        src, email = request.body['src'], request.body.get('email', '')
+        if src not in constants.AVATAR_SOURCES:
+            raise response.invalid_input(src, 'src', 'body')
+        if email and '@' not in email:
+            raise BadEmailAddress(email)
+        new_avatar_url = participant.update_avatar(src+':', avatar_email=email)
+    else:
+        raise response.invalid_input(action, 'action', 'body')
     if new_avatar_url:
         msg = _("Your new avatar URL is: {0}", new_avatar_url)
-        form_post_success(state, msg=msg)
+    else:
+        msg = _("Your profile no longer has an avatar.")
+    form_post_success(state, msg=msg)
 
 else:
     src = participant.avatar_src
@@ -80,10 +86,12 @@ subhead = _("Avatar")
     </a>
     <br><br>
     % if participant.avatar_url and request.method == 'GET'
-    <button class="btn btn-default" data-action="reload" type="button">
-       {{ icon('refresh') }} <span>{{ _("Refresh") }}</span>
-    </button>
-    <noscript><span class="text-danger">{{ _("JavaScript is required") }}</span></noscript>
+    <form action="" method="POST">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token }}" />
+        <button class="btn btn-default" name="action" value="refresh">
+           {{ icon('refresh') }} <span>{{ _("Refresh") }}</span>
+        </button>
+    </form>
     % endif
     </div>
     </div>
@@ -92,6 +100,7 @@ subhead = _("Avatar")
 
     <form action="" method="POST" class="form-inline">
         <input type="hidden" name="csrf_token" value="{{ csrf_token }}" />
+        <input type="hidden" name="action" value="set_source" />
 
         <div class="form-group">
             <label for="avatar-src">{{ _("Avatar source") }}</label><br>