From 9a9f65590d995bbccdf72e1b1cf54c75969f5c40 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Fri, 13 Dec 2024 17:31:57 +0530 Subject: [PATCH] LB-811: Submit listens with OAuth (#3068) Support access tokens generated using MetaBrainz OAuth provider. --- listenbrainz/tests/integration/test_api.py | 96 +++++++++++++--------- listenbrainz/webserver/views/api.py | 2 +- listenbrainz/webserver/views/api_tools.py | 45 ++++++++-- 3 files changed, 98 insertions(+), 45 deletions(-) diff --git a/listenbrainz/tests/integration/test_api.py b/listenbrainz/tests/integration/test_api.py index b9522b6cf3..8d1866f12d 100644 --- a/listenbrainz/tests/integration/test_api.py +++ b/listenbrainz/tests/integration/test_api.py @@ -1,14 +1,14 @@ import json import time +from datetime import datetime, timedelta from unittest.mock import patch -import pytest from psycopg2.extras import execute_values +import requests_mock import listenbrainz.db.user as db_user import listenbrainz.db.user_relationship as db_user_relationship from data.model.external_service import ExternalServiceType -from listenbrainz import db from listenbrainz.tests.integration import ListenAPIIntegrationTestCase from listenbrainz.webserver.views.api_tools import is_valid_uuid import listenbrainz.db.external_service_oauth as db_oauth @@ -388,7 +388,6 @@ def test_playlist_api_xml_internal_server_error(self): actual_xml = response.data.decode('utf-8') self.assertEqual(actual_xml, expected_error_xml) - def test_valid_playing_now(self): """ Test for valid submission of listen_type 'playing_now' """ @@ -943,6 +942,62 @@ def test_token_validation_auth_header(self): self.assertTrue(response.json['valid']) self.assertEqual(response.json['user_name'], self.user['musicbrainz_id']) + @requests_mock.Mocker() + def test_oauth_invalid_access_token(self, mock_requests): + """Test oauth access tokens for submit listens""" + with open(self.path_to_data_file('valid_single.json'), 'r') as f: + payload = json.load(f) + + mock_requests.post("https://musicbrainz.org/new-oauth2/introspect", json={"active": False}) + response = self.client.post( + self.custom_url_for('api_v1.submit_listen'), + data=json.dumps(payload), + headers={'Authorization': 'Bearer meba_123'}, + content_type='application/json' + ) + self.assert401(response) + self.assertEqual(response.json["error"], "Invalid access token.") + + mock_requests.post("https://musicbrainz.org/new-oauth2/introspect", json={ + "active": True, + "client_id": "abc", + "token_type": "Bearer", + "metabrainz_user_id": 123, + "scope": "profile", + "sub": "lucifer", + "issued_by": "https://metabrainz.org/", + "expires_at": int(datetime(2023, 1, 2).timestamp()), + "issued_at": int(datetime(2023, 1, 1).timestamp()), + }) + response = self.client.post( + self.custom_url_for('api_v1.submit_listen'), + data=json.dumps(payload), + headers={'Authorization': 'Bearer meba_123'}, + content_type='application/json' + ) + self.assert401(response) + self.assertEqual(response.json["error"], "Invalid access token.") + + mock_requests.post("https://musicbrainz.org/new-oauth2/introspect", json={ + "active": True, + "client_id": "abc", + "token_type": "Bearer", + "metabrainz_user_id": 123, + "scope": "profile", + "sub": "lucifer", + "issued_by": "https://metabrainz.org/", + "expires_at": int((datetime.now() + timedelta(hours=1)).timestamp()), + "issued_at": int(datetime.now().timestamp()), + }) + response = self.client.post( + self.custom_url_for('api_v1.submit_listen'), + data=json.dumps(payload), + headers={'Authorization': 'Bearer meba_123'}, + content_type='application/json' + ) + self.assert401(response) + self.assertEqual(response.json["error"], "Insufficient scope.") + def test_get_playing_now(self): """ Test for valid submission and retrieval of listen_type 'playing_now' """ @@ -975,41 +1030,6 @@ def test_get_playing_now(self): self.assertEqual(r.json['payload']['listens'][0] ['track_metadata']['track_name'], 'Fade') - @pytest.mark.skip(reason="Test seems to fail when running all integration tests, but passes when run individually. " - "Skip for now") - def test_delete_listen(self): - with open(self.path_to_data_file('valid_single.json'), 'r') as f: - payload = json.load(f) - - # send a listen - ts = int(time.time()) - payload['payload'][0]['listened_at'] = ts - response = self.send_data(payload) - self.assert200(response) - self.assertEqual(response.json['status'], 'ok') - - url = self.custom_url_for('api_v1.get_listens', - user_name=self.user['musicbrainz_id']) - response = self.wait_for_query_to_have_items(url, 1) - data = json.loads(response.data)['payload'] - self.assertEqual(len(data['listens']), 1) - - delete_listen_url = self.custom_url_for('api_v1.delete_listen') - data = { - "listened_at": ts, - "recording_msid": payload['payload'][0]['track_metadata']['additional_info']['recording_msid'] - } - - response = self.client.post( - delete_listen_url, - data=json.dumps(data), - headers={'Authorization': 'Token {}'.format( - self.user['auth_token'])}, - content_type='application/json' - ) - self.assert200(response) - self.assertEqual(response.json["status"], "ok") - def test_delete_listen_not_logged_in(self): delete_listen_url = self.custom_url_for('api_v1.delete_listen') data = { diff --git a/listenbrainz/webserver/views/api.py b/listenbrainz/webserver/views/api.py index 258b3c46aa..5b3dc8c469 100644 --- a/listenbrainz/webserver/views/api.py +++ b/listenbrainz/webserver/views/api.py @@ -72,7 +72,7 @@ def submit_listen(): :statuscode 401: invalid authorization. See error message for details. :resheader Content-Type: *application/json* """ - user = validate_auth_header(fetch_email=True) + user = validate_auth_header(fetch_email=True, scopes=["listenbrainz:submit-listens"]) if mb_engine and current_app.config["REJECT_LISTENS_WITHOUT_USER_EMAIL"] and not user["email"]: raise APIUnauthorized(REJECT_LISTENS_WITHOUT_EMAIL_ERROR) diff --git a/listenbrainz/webserver/views/api_tools.py b/listenbrainz/webserver/views/api_tools.py index 8f0ff577ad..a6fa715b6d 100644 --- a/listenbrainz/webserver/views/api_tools.py +++ b/listenbrainz/webserver/views/api_tools.py @@ -1,8 +1,10 @@ +import logging +from datetime import datetime from typing import Dict, Tuple from urllib.parse import urlparse import bleach -from kombu import Producer +import requests from kombu.entity import PERSISTENT_DELIVERY_MODE from more_itertools import chunked @@ -25,6 +27,8 @@ from listenbrainz.webserver.models import SubmitListenUserMetadata +logger = logging.getLogger(__name__) + #: Maximum overall listen size in bytes, to prevent egregious spamming. MAX_LISTEN_SIZE = 10240 @@ -461,29 +465,58 @@ def _validate_get_endpoint_params() -> Tuple[int, int, int]: return min_ts, max_ts, count -def validate_auth_header(*, optional: bool = False, fetch_email: bool = False): +def validate_auth_header(*, optional: bool = False, fetch_email: bool = False, scopes: list[str] = None): """ Examine the current request headers for an Authorization: Token header that identifies a LB user and then load the corresponding user - object from the database and return it, if succesful. Otherwise raise + object from the database and return it, if successful. Otherwise raise APIUnauthorized() exception. Args: optional: If the optional flag is given, do not raise an exception if the Authorization header is not set. fetch_email: if True, include email in the returned dict + scopes: the scopes the access token is required to have access to """ - - auth_token = request.headers.get('Authorization') + auth_token = request.headers.get("Authorization") if not auth_token: if optional: return None raise APIUnauthorized("You need to provide an Authorization header.") + try: auth_token = auth_token.split(" ")[1] except IndexError: raise APIUnauthorized("Provided Authorization header is invalid.") - user = db_user.get_by_token(db_conn, auth_token, fetch_email=fetch_email) + if auth_token.startswith("meba_"): + try: + response = requests.post( + current_app.config["OAUTH_INTROSPECTION_URL"], + data={ + "client_id": current_app.config["OAUTH_CLIENT_ID"], + "client_secret": current_app.config["OAUTH_CLIENT_SECRET"], + "token": auth_token, + "token_type_hint": "access_token", + } + ) + token = response.json() + except requests.exceptions.RequestException: + logger.error("Error while trying to introspect token:", exc_info=True) + raise APIServiceUnavailable("Something is wrong. Please try again later.") + + if not token["active"] or datetime.fromtimestamp(token["expires_at"]) < datetime.now(): + raise APIUnauthorized("Invalid access token.") + + if scopes: + token_scopes = token["scope"] + for scope in scopes: + if scope not in token_scopes: + raise APIUnauthorized("Insufficient scope.") + + user = db_user.get_by_mb_id(db_conn, token["sub"], fetch_email=fetch_email) + else: + user = db_user.get_by_token(db_conn, auth_token, fetch_email=fetch_email) + if user is None: raise APIUnauthorized("Invalid authorization token.")