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)