diff --git a/metabrainz/decorators.py b/metabrainz/decorators.py index d77242f1..69bbb872 100644 --- a/metabrainz/decorators.py +++ b/metabrainz/decorators.py @@ -3,7 +3,7 @@ from flask import request, current_app, make_response import six import requests -from metabrainz.errors import APIBadRequest, APIUnauthorized, APIForbidden +from metabrainz.errors import APIUnauthorized, APIForbidden NOTIFICATION_SCOPE = 'notification' @@ -80,7 +80,7 @@ def wrapped_function(*args, **kwargs): def ccg_token_required(f): """ This decorator protects an endpoint by validating an access token. - Token should be provided as a query parameter. + Token should be provided in the Authorization header. Token must be generated by an official MeB project and must contain 'notification' scope. Raises: @@ -90,10 +90,14 @@ def ccg_token_required(f): """ @wraps(f) - def decorated(*args,**kwargs): - token = request.args.get('token') + def decorated(*args, **kwargs): + auth_header = request.headers.get("Authorization") + if not auth_header or not auth_header.startswith("Bearer "): + raise APIUnauthorized("Missing or invalid Authorization header.") + token = auth_header.split(" ", 1)[1] if not token: - raise APIBadRequest('Missing access token.') + raise APIUnauthorized("Missing access token.") + data = { "client_id": current_app.config["MUSICBRAINZ_CLIENT_ID"], "client_secret": current_app.config["MUSICBRAINZ_CLIENT_SECRET"], diff --git a/metabrainz/errors.py b/metabrainz/errors.py index a93bd1e7..e9b9d128 100644 --- a/metabrainz/errors.py +++ b/metabrainz/errors.py @@ -1,4 +1,4 @@ -from flask import render_template, jsonify, Response +from flask import render_template, jsonify def init_error_handlers(app): diff --git a/metabrainz/notifications/views.py b/metabrainz/notifications/views.py index ae622d3c..1c79679f 100644 --- a/metabrainz/notifications/views.py +++ b/metabrainz/notifications/views.py @@ -18,8 +18,8 @@ def get_notifications(user_id: int): """ Fetch notifications for a user. - An access token must be provided as a query paramater. - + An access token must be provided in the Authorization header, formatted as `Bearer `. + If none of the optional parameters are specified, this endpoint returns the :data:`~DEFAULT_NOTIFICATION_FETCH_COUNT` most recent notifications across all projects, including both read and unread. @@ -103,13 +103,12 @@ def get_notifications(user_id: int): return jsonify(data) -@notification_bp.post("/mark-read") +@notification_bp.post("//mark-read") @ccg_token_required def mark_notifications(user_id: int): """ Mark notifications as read or unread for a user. - An access token must be provided as a query paramater. - + An access token must be provided in the Authorization header, formatted as `Bearer `. The request must include a JSON body with at least one of the ``read`` or ``unread`` arrays containing notification ID's. Example request body: @@ -128,7 +127,7 @@ def mark_notifications(user_id: int): { "status": "ok" } - + :param token: Required. Access token for authentication. :reqheader Content-Type: *application/json* :statuscode 200: Notifications successfully updated. @@ -174,12 +173,12 @@ def mark_notifications(user_id: int): return jsonify({'status': 'ok'}), 200 -@notification_bp.post("/delete") +@notification_bp.post("//delete") @ccg_token_required def remove_notifications(user_id: int): """ Delete notifications for a user. - An access token must be provided as a query paramater. + An access token must be provided in the Authorization header, formatted as `Bearer `. The request must include a JSON array of notification IDs that belongs to the user. @@ -234,7 +233,7 @@ def remove_notifications(user_id: int): def send_notifications(): """ Inserts batch of notifications for a project. - An access token must be provided as a query paramater. + An access token must be provided in the Authorization header, formatted as `Bearer `. The request must include a JSON array of notifications. @@ -335,12 +334,13 @@ def send_notifications(): return jsonify({'status':'ok'}), 200 -@notification_bp.route("/digest-preference", methods=["GET", "POST"]) +@notification_bp.route("//digest-preference", methods=["GET", "POST"]) @ccg_token_required def set_digest_preference(user_id): """ Get and update the digest preference of the user. - + An access token must be provided in the Authorization header, formatted as `Bearer `. + **To get the digest preference of the user, a GET request must be made to this endpoint.** Returns JSON of the following format: diff --git a/metabrainz/notifications/views_test.py b/metabrainz/notifications/views_test.py index 3038f12d..407580c0 100644 --- a/metabrainz/notifications/views_test.py +++ b/metabrainz/notifications/views_test.py @@ -21,6 +21,7 @@ def test_get_notifications(self, mock_requests, mock_fetch): until_ts = datetime.now(timezone.utc) expected_value=[{'t_id':1}, {'t_id':2}] url = f'notification/{user_id}/fetch' + headers = {"Authorization": "Bearer good_token"} mock_fetch.return_value=expected_value mock_requests.post(self.introspect_url, json={ @@ -30,7 +31,7 @@ def test_get_notifications(self, mock_requests, mock_fetch): }) # With no optional parameters. - response = self.client.get(url, query_string={'token':'good_token'}) + response = self.client.get(url, headers=headers) self.assert200(response) mock_fetch.assert_called_with( user_id, @@ -42,14 +43,18 @@ def test_get_notifications(self, mock_requests, mock_fetch): ) self.assertListEqual(response.json, expected_value) # With all optional paramters. - response = self.client.get(url, query_string={ - 'token': 'good_token', - 'project': 'listenbrainz,metabrainz', - 'count': 3, - 'offset': 1, - 'until_ts': until_ts.timestamp(), - 'unread_only': 't' - }) + response = self.client.get( + url, + headers=headers, + query_string={ + "token": "good_token", + "project": "listenbrainz,metabrainz", + "count": 3, + "offset": 1, + "until_ts": until_ts.timestamp(), + "unread_only": "t", + }, + ) self.assert200(response) mock_fetch.assert_called_with( user_id, @@ -65,10 +70,11 @@ def test_get_notifications(self, mock_requests, mock_fetch): ('until_ts','asdf','Invalid Until_Timestamp'), ('project','tidal,spotify','Invalid project name'), ('unread_only','true','Invalid unread_only option')] for param in bad_params: - res = self.client.get(url, query_string={ - 'token': 'good_token', - param[0]:param[1] - }) + res = self.client.get( + url, + headers=headers, + query_string={"token": "good_token", param[0]: param[1]}, + ) self.assert400(res) self.assertEqual(res.json['error'],param[2]) @@ -83,12 +89,12 @@ def test_mark_notifications(self, mock_requests, mock_mark): "client_id": "abc", "scope": ["notification"], }) - url = f'notification/{user_id}/mark-read?token=good_token' + url = f"notification/{user_id}/mark-read" + headers = {"Authorization": "Bearer good_token"} # Both read and unread arrays. res = self.client.post( - url, - json={"read": read, "unread": unread} - ) + url, headers=headers, json={"read": read, "unread": unread} + ) self.assert200(res) mock_mark.assert_called_with( user_id, @@ -97,10 +103,7 @@ def test_mark_notifications(self, mock_requests, mock_mark): ) self.assertEqual(res.json['status'],'ok') # Only read array. - res = self.client.post( - url, - json={"read": read} - ) + res = self.client.post(url, headers=headers, json={"read": read}) self.assert200(res) mock_mark.assert_called_with( user_id, @@ -109,10 +112,7 @@ def test_mark_notifications(self, mock_requests, mock_mark): ) self.assertEqual(res.json['status'],'ok') # Bad requests. - res = self.client.post( - url, - json={} - ) + res = self.client.post(url, headers=headers, json={}) self.assert400(res) bad_data=[([],[],'Missing Read and Unread IDs'), (1,[2],"'read' must be an Array" ), @@ -120,17 +120,15 @@ def test_mark_notifications(self, mock_requests, mock_mark): ] for d in bad_data: res = self.client.post( - url, - json={"read": d[0], "unread":d[1]} + url, headers=headers, json={"read": d[0], "unread": d[1]} ) self.assert400(res) self.assertEqual(res.json['error'],d[2]) # Database error. mock_mark.side_effect= Exception() res = self.client.post( - url, - json={"read": read, "unread": unread} - ) + url, headers=headers, json={"read": read, "unread": unread} + ) self.assert503(res) self.assertEqual(res.json['error'], 'Cannot update read values right now.') @@ -144,11 +142,10 @@ def test_remove_notifications(self, mock_requests, mock_delete): "client_id": "abc", "scope": ["notification"], }) - url = f'notification/{user_id}/delete?token=good_token' - res = self.client.post( - url, - json=[i for i in delete] - ) + url = f"notification/{user_id}/delete" + headers = {"Authorization": "Bearer good_token"} + + res = self.client.post(url, headers=headers, json=[i for i in delete]) self.assert200(res) mock_delete.assert_called_with( user_id, @@ -159,18 +156,12 @@ def test_remove_notifications(self, mock_requests, mock_delete): bad_data = [(1,"ID's must be in an Array"),(['1'],'ID values must be Integers'), ([],'Missing notification IDs for deletion')] for d in bad_data: - res = self.client.post( - url, - json=d[0] - ) + res = self.client.post(url, headers=headers, json=d[0]) self.assert400(res) self.assertEqual(res.json['error'],d[1]) # Database error. mock_delete.side_effect= Exception() - res = self.client.post( - url, - json=[i for i in delete] - ) + res = self.client.post(url, headers=headers, json=[i for i in delete]) self.assert503(res) self.assertEqual(res.json['error'], 'Cannot delete notifications right now.') @@ -211,11 +202,10 @@ def test_send_notifications(self, mock_requests, mock_insert): "send_email": True } ] - url = 'notification/send?token=good_token' - res = self.client.post( - url, - json=test_data - ) + url = "notification/send" + headers = {"Authorization": "Bearer good_token"} + + res = self.client.post(url, headers=headers, json=test_data) self.assert200(res) self.assertEqual(res.json['status'], 'ok') @@ -226,10 +216,7 @@ def test_send_notifications(self, mock_requests, mock_insert): ([test_data[1]], 'Missing required field/fields in notification 0.'), ([test_data[0]], 'Notification 0 should include either subject and body or template_id and template_params.')] for i in bad_data: - res = self.client.post( - url, - json=i[0] - ) + res = self.client.post(url, headers=headers, json=i[0]) self.assert400(res) self.assertEqual(res.json['error'], i[1]) @@ -237,20 +224,23 @@ def test_send_notifications(self, mock_requests, mock_insert): mock_insert.side_effect= Exception() res = self.client.post( url, - json=[{ - "user_id": 4, - "project": "musicbrainz", - "to": "user4@example.com", - "reply_to": "noreply@musicbrainz.org", - "sent_from": "noreply@musicbrainz.org", - "template_id": "verify-email", - "template_params": { "reason": "verify" }, - "important": False, - "expire_age": 30, - "email_id": "verify-email-213324", - "send_email": True - }] - ) + headers=headers, + json=[ + { + "user_id": 4, + "project": "musicbrainz", + "to": "user4@example.com", + "reply_to": "noreply@musicbrainz.org", + "sent_from": "noreply@musicbrainz.org", + "template_id": "verify-email", + "template_params": {"reason": "verify"}, + "important": False, + "expire_age": 30, + "email_id": "verify-email-213324", + "send_email": True, + } + ], + ) self.assert503(res) self.assertEqual(res.json['error'], 'Cannot send notifications right now.') @@ -264,23 +254,24 @@ def test_set_digest_preference(self, mock_requests, mock_digest): "scope": ["notification"], }) user_id = 1 - url = f'notification/{user_id}/digest-preference?token=good_token' + url = f"notification/{user_id}/digest-preference" + headers = {"Authorization": "Bearer good_token"} - # GET method test - res = self.client.get(url) + # GET method test + res = self.client.get(url, headers=headers) self.assert200(res) self.assertEqual(res.json, {"digest": True, "digest_age": 19}) mock_digest.get.assert_called_with(musicbrainz_row_id=user_id) mock_digest.get.return_value = None - res = self.client.get(url) + res = self.client.get(url, headers=headers) self.assert400(res) self.assertEqual(res.json['error'], 'Invalid user_id.') # POST method test mock_digest.set_digest_info.return_value = mock.MagicMock(digest=True, digest_age=21) params = {"digest": True, "digest_age": 21} - res = self.client.post(url, json=params) + res = self.client.post(url, headers=headers, json=params) self.assert200(res) self.assertEqual(res.json, params) @@ -291,64 +282,59 @@ def test_set_digest_preference(self, mock_requests, mock_digest): ({"digest": True, "digest_age": 0}, "Invalid digest age.") ] for b in bad_params: - res = self.client.post(url, json=b[0]) + res = self.client.post(url, headers=headers, json=b[0]) self.assert400(res) self.assertEqual(res.json['error'], b[1]) mock_digest.set_digest_info.side_effect = Exception() - res = self.client.post(url, json=params) + res = self.client.post(url, headers=headers, json=params) self.assert503(res) self.assertEqual(res.json['error'], "Cannot update digest preference right now.") @requests_mock.Mocker() def test_invalid_tokens(self, mock_requests): endpoints = [ - { - "url": "notification/1/fetch", - "method": self.client.get - }, + {"url": "notification/1/fetch", "method": self.client.get}, { "url": "notification/1/mark-read", "method": self.client.post, - "data": {"json": {"read": [1,2]}} - }, - { - "url": "notification/1/delete", - "method": self.client.post, - "data": {"json": [1]} + "data": {"read": [1, 2]}, }, + {"url": "notification/1/delete", "method": self.client.post, "data": [1]}, { "url": "notification/send", "method": self.client.post, - "data": {"json": [{"test_data": 1}]} + "data": [{"test_data": 1}], }, { "url": "notification/1/digest-preference", "method": self.client.post, - "data": {"json": {"digest": True, "digest_age": 19}} - - } + "data": {"digest": True, "digest_age": 19}, + }, ] + headers = {"Authorization": "Bearer token"} for e in endpoints: url = e["url"] - json = e.get("data") method = e["method"] + json = e.get("data") - res = method(url, **(json or {})) - self.assert400(res) - self.assertEqual(res.json['error'], 'Missing access token.') + res = method(url, json=json) + self.assert401(res) + self.assertEqual( + res.json["error"], "Missing or invalid Authorization header." + ) mock_requests.post(self.introspect_url, json={"active": False}) - res = method(url+'?token=bad_token', **(json or {})) + res = method(url, headers=headers, json=json) self.assert401(res) self.assertEqual(res.json['error'], 'Invalid or Expired access token.') mock_requests.post(self.introspect_url, json={"active": True, "scope": ["view", "profile"]}) - res = method(url+'?token=missing_scope_token', **(json or {})) + res = method(url, headers=headers, json=json) self.assert403(res) self.assertEqual(res.json['error'], 'Missing notification scope.') mock_requests.post(self.introspect_url, json={"active": True, "scope": ["notification"], "client_id": "xyz"}) - res = method(url+'?token=invalid_clientid_token', **(json or {})) + res = method(url, headers=headers, json=json) self.assert403(res) self.assertEqual(res.json['error'], 'Client is not an official MeB project.')