implement account delete for Bluesky

for #1593

bluesky.py

@@ -1,20 +1,20 @@
-import models
+import logging
+from urllib.parse import quote
+
+from flask import flash, render_template, request
 from granary import bluesky as gr_bluesky
 from oauth_dropins import bluesky as oauth_bluesky
+from oauth_dropins.webutil.util import json_loads
+
 from flask_app import app
+import models
 import util
-import logging
-from flask import flash, render_template
-from oauth_dropins.webutil.util import json_loads
-from urllib.parse import quote
 
 logger = logging.getLogger(__name__)
 
 
 class Bluesky(models.Source):
-  """
-  A Bluesky account.
-  """
+  """A Bluesky account. Key id is DID."""
   SHORT_NAME = 'bluesky'
   GR_CLASS = gr_bluesky.Bluesky
   OAUTH_START = oauth_bluesky.Start
@@ -45,7 +45,7 @@ def new(auth_entity, **kwargs):
                    **kwargs)
 
   def silo_url(self):
-    """Returns the Bluesky account URL, e.g. https://bsky.app/profile/foo.bsky.social."""
+    """Returns the Bluesky account URL, e.g. ``https://bsky.app/profile/foo.com``."""
     return self.gr_source.user_url(self.username)
 
   def label_name(self):
@@ -55,7 +55,8 @@ def label_name(self):
   def format_for_source_url(self, id):
     """
     Bluesky keys (AT URIs) contain slashes, so must be double-encoded.
-    This is due to a particular behaviour in WSGI: https://github.com/pallets/flask/issues/900
+    This is due to a particular behaviour in WSGI:
+    https://github.com/pallets/flask/issues/900
     They do not need to be decoded correspondingly.
     """
     return quote(quote(id, safe=''))
@@ -91,16 +92,27 @@ def finish(self, auth_entity, state=None):
       flash("Failed to log in to Bluesky. Are your credentials correct?")
       return util.redirect("/bluesky/start")
 
-    util.maybe_add_or_delete_source(
-      Bluesky,
-      auth_entity,
-      util.construct_state_param_for_add(),
-    )
+    state = {
+      'operation': request.form['operation'],
+      'feature': request.form['feature'],
+    }
+    if request.form['operation'] == 'delete':
+      state['source'] = Bluesky(id=auth_entity.key.id()).key.urlsafe().decode()
+
+    util.maybe_add_or_delete_source(Bluesky, auth_entity,
+                                    util.encode_oauth_state(state))
+
 
 @app.route('/bluesky/start', methods=['GET'])
-def provide_app_password():
-  """Serves the Bluesky login form page."""
-  return render_template('provide_app_password.html')
+def bluesky_start():
+  """Serves the Bluesky login form page to sign up."""
+  return render_template('provide_app_password.html', operation='add')
+
+
+@app.route('/bluesky/delete/start', methods=['GET'])
+def bluesky_delete():
+  """Serves the Bluesky login form page to delete an existing account."""
+  return render_template('provide_app_password.html', operation='delete')
 
 
 app.add_url_rule('/bluesky/callback', view_func=Callback.as_view('bluesky_callback', 'unused'), methods=['POST'])

pages.py

@@ -328,9 +328,12 @@ def delete_start():
     'callback': request.values.get('callback'),
   })
 
-  # Blogger don't support redirect_url() yet
   if kind == 'Blogger':
+    # Blogger doesn't support redirect_url() yet
     return redirect(f'/blogger/delete/start?state={state}')
+  elif kind == 'Bluesky':
+    # Bluesky isn't OAuth at all yet
+    return redirect(f'/bluesky/delete/start')
 
   path = ('/reddit/callback' if kind == 'Reddit'
           else '/wordpress/add' if kind == 'WordPress'
@@ -384,12 +387,10 @@ def delete_finish():
 
   logins = None
   if logged_in_as and logged_in_as.is_authority_for(source.auth_entity):
-    existing = set(source.features)
-    remaining = existing - set(features)
-    if remaining != existing:
-      source.features = list(remaining)
-      source.put()
+    source.features = set(source.features) - set(features)
+    source.put()
 
+    if not source.features:
       # remove login cookie
       logins = util.get_logins()
       login = util.Login(path=source.bridgy_path(), site=source.SHORT_NAME,

templates/provide_app_password.html

@@ -16,6 +16,7 @@
     <input type="password" name="password" placeholder="app password" />
   </p>
   <p class="row big"><input type="submit" class="btn btn-default" value="OK" /></p>
+  <input name="operation" type="hidden" value="{{ operation }}" />
   <input name="feature" type="hidden" value="listen" />
 </form>
 

tests/test_bluesky.py

@@ -1,8 +1,15 @@
-from . import testutil
-from bluesky import Bluesky
+"""Unit tests for bluesky.py."""
+from unittest import mock
+from urllib.parse import parse_qs, urlparse
+
 from oauth_dropins import bluesky as oauth_bluesky
 from oauth_dropins.webutil.util import json_dumps
-from unittest import mock
+from werkzeug.routing import RequestRedirect
+
+from bluesky import Bluesky, Callback
+from flask_app import app
+import util
+from . import testutil
 
 
 class BlueskyTest(testutil.AppTest):
@@ -63,3 +70,23 @@ def test_canonicalize_url(self):
         ('at://did:web:alice.com/app.bsky.feed.post/123', 'https://bsky.app/profile/alice.com/post/123'),
     ]:
       self.assertEqual(expected, self.bsky.canonicalize_url(input))
+
+  def test_delete(self):
+    self.bsky.features = ['listen', 'publish']
+    self.bsky.put()
+
+    with self.assertRaises(RequestRedirect) as redir, app.test_request_context(data={
+        'operation': 'delete',
+        'feature': 'listen,publish',
+      }):
+      Callback('unused').finish(self.auth_entity)
+
+    location = urlparse(redir.exception.get_response().headers['Location'])
+    self.assertEqual('/delete/finish', location.path)
+    query = parse_qs(location.query)
+    self.assertEqual([self.auth_entity.key.urlsafe().decode()], query['auth_entity'])
+    self.assertEqual({
+      'operation': 'delete',
+      'feature': 'listen,publish',
+      'source': self.bsky.key.urlsafe().decode(),
+    }, util.decode_oauth_state(query['state'][0]))

tests/test_pages.py

@@ -1,7 +1,7 @@
 """Unit tests for pages.py."""
 from datetime import datetime, timedelta, timezone
 import urllib.request, urllib.parse, urllib.error
-from urllib.parse import urlencode
+from urllib.parse import urlencode, urlparse, parse_qs
 
 from flask import get_flashed_messages
 from google.cloud import ndb
@@ -11,6 +11,8 @@
 import tweepy
 
 from flask_app import app
+from blogger import Blogger
+from bluesky import Bluesky
 import models
 from models import Publish, PublishedPage, SyndicatedPost
 import util
@@ -221,7 +223,7 @@ def test_delete_start_redirect_url_http_error(self):
         'key': self.sources[0].key.urlsafe().decode(),
       })
     self.assertEqual(302, resp.status_code)
-    location = urllib.parse.urlparse(resp.headers['Location'])
+    location = urlparse(resp.headers['Location'])
     self.assertEqual('/fake/0123456789', location.path)
     self.assertEqual(['FakeSource API error 504: Connection closed unexpectedly...'],
                       get_flashed_messages())
@@ -236,7 +238,7 @@ def test_delete_start_redirect_url_value_error(self):
         'key': self.sources[0].key.urlsafe().decode(),
       })
     self.assertEqual(302, resp.status_code)
-    location = urllib.parse.urlparse(resp.headers['Location'])
+    location = urlparse(resp.headers['Location'])
     self.assertEqual('/fake/0123456789', location.path)
     self.assertEqual(['Error: foo bar'], get_flashed_messages())
 
@@ -260,6 +262,53 @@ def test_delete_removes_from_logins_cookie(self):
     self.assertIn('logins=/other/1?bob;',
                   resp.headers['Set-Cookie'].split(' '))
 
+  def test_delete_blogger(self):
+    source_key = Blogger(id='123').put().urlsafe().decode()
+
+    resp = self.client.post('/delete/start', data={
+        'feature': 'listen',
+        'key': source_key,
+      })
+    self.assertEqual(302, resp.status_code)
+
+    location = urlparse(resp.headers['Location'])
+    self.assertEqual('/blogger/delete/start', location.path)
+    self.assertEqual({
+      'operation': 'delete',
+      'feature': 'listen',
+      'source': source_key,
+    }, util.decode_oauth_state(parse_qs(location.query)['state'][0]))
+
+  def test_delete_bluesky(self):
+    source_key = Bluesky(id='did:foo').put().urlsafe().decode()
+
+    resp = self.client.post('/delete/start', data={
+        'feature': 'listen',
+        'key': source_key,
+      })
+    self.assertEqual(302, resp.status_code)
+    self.assertEqual('http://localhost/bluesky/delete/start',
+                     resp.headers['Location'])
+
+  def test_delete_finish_multiple_features(self):
+    self.sources[0].features = ['listen', 'publish']
+    self.sources[0].put()
+
+    with app.test_request_context():
+      state = util.construct_state_param_for_add(
+        feature='listen,publish',
+        operation='delete',
+        source=self.sources[0].key.urlsafe().decode())
+
+    self.auth_entities[0].put()
+    auth_entity_key = self.sources[0].auth_entity.urlsafe().decode()
+    resp = self.client.get(
+      f'/delete/finish?auth_entity={auth_entity_key}&state={state}')
+
+    self.assertEqual(302, resp.status_code)
+    got = self.sources[0].key.get()
+    self.assertEqual([], got.features)
+
   def test_user_page(self):
     self.sources[0].last_webmention_sent = util.now()
     self.sources[0].put()
@@ -615,7 +664,7 @@ def check_discover(self, url, expected_message):
         'url': url,
       })
       self.assertEqual(302, resp.status_code)
-      location = urllib.parse.urlparse(resp.headers['Location'])
+      location = urlparse(resp.headers['Location'])
       detail = ' '.join((url, str(resp.status_code), repr(location), repr(resp.get_data(as_text=True))))
       self.assertEqual(self.source.bridgy_path(), location.path, detail)
       self.assertEqual([expected_message], get_flashed_messages())

util.py

@@ -461,13 +461,15 @@ def maybe_add_or_delete_source(source_cls, auth_entity, state, **kwargs):
   """
   state_obj = util.decode_oauth_state(state)
   operation = state_obj.get('operation', 'add')
-  feature = state_obj.get('feature')
   callback = state_obj.get('callback')
   user_url = state_obj.get('user_url')
 
+  feat_str = state_obj.get('feature')
+  features = feat_str.split(',') if feat_str else []
+
   logger.debug(
-    'maybe_add_or_delete_source with operation=%s, feature=%s, callback=%s',
-    operation, feature, callback)
+    'maybe_add_or_delete_source with operation=%s feature=%s callback=%s',
+    operation, features, callback)
   logins = None
 
   if operation == 'add':  # this is an add/update
@@ -486,8 +488,7 @@ def maybe_add_or_delete_source(source_cls, auth_entity, state, **kwargs):
         redirect('/')
 
     logger.info(f'{source_cls.__class__.__name__}.create_new with {auth_entity.key}, {state}, {kwargs}')
-    source = source_cls.create_new(auth_entity=auth_entity,
-                                   features=feature.split(',') if feature else [],
+    source = source_cls.create_new(auth_entity=auth_entity, features=features,
                                    user_url=user_url, **kwargs)