From 846ed8d0679fdd991b064b7d5ea5e5f69a8248c2 Mon Sep 17 00:00:00 2001
From: Ryan Barrett
Date: Wed, 1 Nov 2023 10:48:00 -0700
Subject: [PATCH] implement account delete for Bluesky
for #1593
---
bluesky.py | 48 +++++++++++++++---------
pages.py | 13 ++++---
templates/provide_app_password.html | 1 +
tests/test_bluesky.py | 33 +++++++++++++++--
tests/test_pages.py | 57 +++++++++++++++++++++++++++--
util.py | 11 +++---
6 files changed, 127 insertions(+), 36 deletions(-)
diff --git a/bluesky.py b/bluesky.py
index 1371af51..69daa090 100644
--- a/bluesky.py
+++ b/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'])
diff --git a/pages.py b/pages.py
index 653593b8..70ac8124 100644
--- a/pages.py
+++ b/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,
diff --git a/templates/provide_app_password.html b/templates/provide_app_password.html
index 15b9614a..d96dc894 100644
--- a/templates/provide_app_password.html
+++ b/templates/provide_app_password.html
@@ -16,6 +16,7 @@
+
diff --git a/tests/test_bluesky.py b/tests/test_bluesky.py
index edc7c076..030446f8 100644
--- a/tests/test_bluesky.py
+++ b/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]))
diff --git a/tests/test_pages.py b/tests/test_pages.py
index d7bcb21e..2832063d 100644
--- a/tests/test_pages.py
+++ b/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())
diff --git a/util.py b/util.py
index 811e4d76..b333ed73 100644
--- a/util.py
+++ b/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)