Fix @crossdomain decorator

listenbrainz/webserver/decorators.py

@@ -1,13 +1,16 @@
 from functools import wraps
 
-from brainzutils import musicbrainz_db
 from flask import request, current_app, make_response, redirect, url_for
 
 from listenbrainz.webserver import timescale_connection
 
 
 def crossdomain(f):
-    """ Decorator to add CORS headers to flask endpoints """
+    """ Decorator to add CORS headers to flask endpoints.
+
+    This decorator should be applied just after the route to ensure the provide_automatic_options
+    is set correctly.
+    """
     @wraps(f)
     def decorator(*args, **kwargs):
         options_resp = current_app.make_default_options_response()
@@ -24,7 +27,8 @@ def decorator(*args, **kwargs):
         h["Access-Control-Allow-Headers"] = "Authorization, Content-Type"
         return resp
 
-    f.provide_automatic_options = False
+    decorator.provide_automatic_options = False
+    decorator.required_methods = ["OPTIONS"]
     return decorator
 
 

listenbrainz/webserver/test/test_routes.py

@@ -19,3 +19,23 @@ def test_routes_have_trailing_slash(self):
             if not rule.rule.startswith(ignored_prefixes) and rule.endpoint not in ignored_endpoints:
                 if not rule.rule.endswith('/'):
                     self.fail(f"Rule doesn't end with a trailing slash: {rule.rule} ({rule.endpoint})")
+
+
+    def test_api_routes_have_options(self):
+        """ Ensure that all API routes (which are cors-enabled) also add CORS headers to their options response.
+        The flask default doesn't.
+        """
+        CORS_DISABLED_ENDPOINTS = set()
+        for rule in self.app.url_map.iter_rules():
+            if rule.rule.startswith(API_PREFIX) and rule.endpoint not in CORS_DISABLED_ENDPOINTS:
+                with self.subTest(rule=rule):
+                    url = rule.rule
+                    # replace path args requiring an integer with arbitrary integer to ensure path matches
+                    for arg in rule.arguments:
+                        url = url.replace(f"<int:{arg}>", "1")
+                    response = self.client.options(url)
+                    headers = set(response.headers.keys())
+                    self.assertIn("Access-Control-Allow-Origin", headers)
+                    self.assertIn("Access-Control-Allow-Methods", headers)
+                    self.assertIn("Access-Control-Max-Age", headers)
+                    self.assertIn("Access-Control-Allow-Headers", headers)

listenbrainz/webserver/views/api.py

@@ -34,7 +34,7 @@
 SEARCH_USER_LIMIT = 10
 
 
-@api_bp.route('/search/users/', methods=['GET', 'OPTIONS'])
+@api_bp.get("/search/users/")
 @crossdomain
 @ratelimit()
 def search_user():
@@ -50,7 +50,7 @@ def search_user():
     return jsonify({'users': users})
 
 
-@api_bp.route("/submit-listens", methods=["POST", "OPTIONS"])
+@api_bp.post("/submit-listens")
 @crossdomain
 @ratelimit()
 def submit_listen():
@@ -134,7 +134,7 @@ def submit_listen():
     return jsonify({'status': 'ok'})
 
 
-@api_bp.route("/user/<user_name>/listens", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/listens")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -180,7 +180,7 @@ def get_listens(user_name):
     }})
 
 
-@api_bp.route("/user/<user_name>/listen-count", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/listen-count")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -210,7 +210,7 @@ def get_listen_count(user_name):
     }})
 
 
-@api_bp.route("/user/<user_name>/playing-now", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/playing-now")
 @crossdomain
 @ratelimit()
 def get_playing_now(user_name):
@@ -248,7 +248,7 @@ def get_playing_now(user_name):
     })
 
 
-@api_bp.route("/user/<user_name>/similar-users", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/similar-users")
 @crossdomain
 @ratelimit()
 def get_similar_users(user_name):
@@ -284,7 +284,7 @@ def get_similar_users(user_name):
     })
 
 
-@api_bp.route("/user/<user_name>/similar-to/<other_user_name>", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/similar-to/<other_user_name>")
 @crossdomain
 @ratelimit()
 def get_similar_to_user(user_name, other_user_name):
@@ -320,7 +320,7 @@ def get_similar_to_user(user_name, other_user_name):
         raise APINotFound("Similar-to user not found")
 
 
-@api_bp.route('/latest-import', methods=['GET', 'POST', 'OPTIONS'])
+@api_bp.route("/latest-import", methods=["GET", "POST"])
 @crossdomain
 @ratelimit()
 def latest_import():
@@ -392,7 +392,7 @@ def latest_import():
         return jsonify({'status': 'ok'})
 
 
-@api_bp.route('/validate-token', methods=['GET', 'OPTIONS'])
+@api_bp.get("/validate-token")
 @crossdomain
 @ratelimit()
 def validate_token():
@@ -459,7 +459,7 @@ def validate_token():
         })
 
 
-@api_bp.route('/delete-listen', methods=['POST', 'OPTIONS'])
+@api_bp.post("/delete-listen")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -537,7 +537,7 @@ def serialize_playlists(playlists, playlist_count, count, offset):
             "count": count}
 
 
-@api_bp.route("/user/<playlist_user_name>/playlists", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<playlist_user_name>/playlists")
 @crossdomain
 @ratelimit()
 def get_playlists_for_user(playlist_user_name):
@@ -572,7 +572,7 @@ def get_playlists_for_user(playlist_user_name):
     return jsonify(serialize_playlists(playlists, playlist_count, count, offset))
 
 
-@api_bp.route("/user/<playlist_user_name>/playlists/createdfor", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<playlist_user_name>/playlists/createdfor")
 @crossdomain
 @ratelimit()
 def get_playlists_created_for_user(playlist_user_name):
@@ -604,7 +604,7 @@ def get_playlists_created_for_user(playlist_user_name):
     return jsonify(serialize_playlists(playlists, playlist_count, count, offset))
 
 
-@api_bp.route("/user/<playlist_user_name>/playlists/collaborator", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<playlist_user_name>/playlists/collaborator")
 @crossdomain
 @ratelimit()
 def get_playlists_collaborated_on_for_user(playlist_user_name):
@@ -643,7 +643,7 @@ def get_playlists_collaborated_on_for_user(playlist_user_name):
     return jsonify(serialize_playlists(playlists, playlist_count, count, offset))
 
 
-@api_bp.route("/user/<playlist_user_name>/playlists/recommendations", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<playlist_user_name>/playlists/recommendations")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -666,7 +666,7 @@ def user_recommendations(playlist_user_name):
     return jsonify(serialize_playlists(playlists, len(playlists), 0, 0))
 
 
-@api_bp.route("/user/<playlist_user_name>/playlists/search", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<playlist_user_name>/playlists/search")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -696,7 +696,7 @@ def search_user_playlist(playlist_user_name):
     return jsonify(serialize_playlists(playlists, playlist_count, count, offset))
 
 
-@api_bp.route("/user/<user_name>/services", methods=['GET', 'OPTIONS'])
+@api_bp.get("/user/<user_name>/services")
 @crossdomain
 @ratelimit()
 def get_service_details(user_name):
@@ -725,7 +725,7 @@ def get_service_details(user_name):
     return jsonify({'user_name': user_name, 'services': services})
 
 
-@api_bp.route("/lb-radio/tags", methods=['GET', 'OPTIONS'])
+@api_bp.get("/lb-radio/tags")
 @crossdomain
 @ratelimit()
 def get_tags_dataset():
@@ -805,7 +805,7 @@ def _get_listen_type(listen_type):
     }.get(listen_type)
 
 
-@api_bp.route("/lb-radio/artist/<seed_artist_mbid>", methods=['GET', 'OPTIONS'])
+@api_bp.get("/lb-radio/artist/<seed_artist_mbid>")
 @crossdomain
 @ratelimit()
 def get_artist_radio_recordings(seed_artist_mbid):

listenbrainz/webserver/views/api_compat.py

@@ -24,7 +24,7 @@
 api_bp = Blueprint('api_compat', __name__)
 
 
-@api_bp.route('/api/auth/', methods=['GET'])
+@api_bp.get('/api/auth/')
 @ratelimit()
 @login_required
 def api_auth():
@@ -35,7 +35,7 @@ def api_auth():
     )
 
 
-@api_bp.route('/api/auth/', methods=['POST'])
+@api_bp.post('/api/auth/')
 @ratelimit()
 @login_required
 def api_auth_approve():

listenbrainz/webserver/views/api_tools.py

@@ -220,7 +220,7 @@ def validate_listen(listen: Dict, listen_type) -> Dict:
             validate_multiple_mbids_field(listen, key)
 
     # monitor performance of unicode null check because it might be a potential bottleneck
-    with sentry_sdk.start_span(op="null check", description="check for unicode null in submitted listen json"):
+    with sentry_sdk.start_span(op="null check", name="check for unicode null in submitted listen json"):
         # If unicode null is present in the listen, postgres will raise an
         # error while trying to insert it. hence, reject such listens.
         check_for_unicode_null_recursively(listen)

listenbrainz/webserver/views/art.py

@@ -3,7 +3,7 @@
 art_bp = Blueprint('art', __name__)
 
 
-@art_bp.route("/")
+@art_bp.get("/")
 def index():
     """ This page shows of a bit of what can be done with the cover art, as a sort of showcase. """
     return render_template("art/index.html", api_url=current_app.config["API_URL"])

listenbrainz/webserver/views/art_api.py

@@ -28,7 +28,7 @@ def _repeat_images(images, size=9):
     return images
 
 
-@art_api_bp.route("/grid/", methods=["POST", "OPTIONS"])
+@art_api_bp.post("/grid/")
 @crossdomain
 @ratelimit()
 def cover_art_grid_post():
@@ -171,8 +171,7 @@ def get_release_group_mbids() -> tuple[list, str]:
                            }
 
 
-@art_api_bp.route("/grid-stats/<user_name>/<time_range>/<int:dimension>/<int:layout>/<int:image_size>",
-                  methods=["GET"])
+@art_api_bp.get("/grid-stats/<user_name>/<time_range>/<int:dimension>/<int:layout>/<int:image_size>")
 @crossdomain
 @ratelimit()
 def cover_art_grid_stats(user_name, time_range, dimension, layout, image_size):
@@ -233,7 +232,7 @@ def cover_art_grid_stats(user_name, time_range, dimension, layout, image_size):
                            }
 
 
-@art_api_bp.route("/<custom_name>/<user_name>/<time_range>/<int:image_size>", methods=["GET"])
+@art_api_bp.get("/<custom_name>/<user_name>/<time_range>/<int:image_size>")
 @crossdomain
 @ratelimit()
 def cover_art_custom_stats(custom_name, user_name, time_range, image_size):
@@ -420,6 +419,7 @@ def _cover_art_yim_albums_2023(user_name, stats):
         images=images,
     )
 
+
 def _cover_art_yim_albums_2024(user_name, stats, yim24):
     """ Create the SVG using YIM top albums for 2024. """
     cac = CoverArtGenerator(current_app.config["MB_DATABASE_URI"], 3, 250)
@@ -724,7 +724,7 @@ def _cover_art_yim_overview(user_name, stats, year, yim24):
         return render_template("art/svg-templates/year-in-music-2024/yim-2024-overview.svg", **props, **yim24)
 
 
-@art_api_bp.route("/year-in-music/<int:year>/<user_name>", methods=["GET"])
+@art_api_bp.get("/year-in-music/<int:year>/<user_name>")
 @crossdomain
 @ratelimit()
 def cover_art_yim(user_name, year: int = 2024):

listenbrainz/webserver/views/atom.py

@@ -164,7 +164,7 @@ def _init_feed(id, title, alternate_url, self_url):
     return fg
 
 
-@atom_bp.route("/user/<user_name>/listens", methods=["GET"])
+@atom_bp.get("/user/<user_name>/listens")
 @crossdomain
 @ratelimit()
 @api_listenstore_needed
@@ -252,7 +252,7 @@ def get_listens(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/fresh-releases", methods=["GET"])
+@atom_bp.get("/fresh-releases")
 @crossdomain
 @ratelimit()
 def get_fresh_releases():
@@ -333,8 +333,9 @@ def get_fresh_releases():
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/fresh-releases", methods=["GET"])
+@atom_bp.get("/user/<user_name>/fresh-releases")
 @crossdomain
+@ratelimit()
 def get_user_fresh_releases(user_name):
     """
     Get fresh releases for a user, sorted by release date.
@@ -426,7 +427,7 @@ def _get_entity_stats(user_id: str, entity: str, range: str, count: int):
     return entity_list, stats.to_ts, stats.last_updated
 
 
-@atom_bp.route("/user/<user_name>/stats/top-artists")
+@atom_bp.get("/user/<user_name>/stats/top-artists")
 @crossdomain
 @ratelimit()
 def get_artist_stats(user_name):
@@ -507,7 +508,7 @@ def get_artist_stats(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/stats/top-albums")
+@atom_bp.get("/user/<user_name>/stats/top-albums")
 @crossdomain
 @ratelimit()
 def get_release_group_stats(user_name):
@@ -589,7 +590,7 @@ def get_release_group_stats(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/stats/top-tracks")
+@atom_bp.get("/user/<user_name>/stats/top-tracks")
 @crossdomain
 @ratelimit()
 def get_recording_stats(user_name):
@@ -670,7 +671,7 @@ def get_recording_stats(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/playlist/<playlist_mbid>")
+@atom_bp.get("/playlist/<playlist_mbid>")
 @crossdomain
 @api_listenstore_needed
 @ratelimit()
@@ -743,7 +744,7 @@ def get_playlist_recordings(playlist_mbid):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/recommendations")
+@atom_bp.get("/user/<user_name>/recommendations")
 @crossdomain
 @api_listenstore_needed
 @ratelimit()
@@ -833,7 +834,7 @@ def get_recommendation(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/stats/art/grid")
+@atom_bp.get("/user/<user_name>/stats/art/grid")
 @crossdomain
 @ratelimit()
 def get_cover_art_grid_stats(user_name):
@@ -941,7 +942,7 @@ def get_cover_art_grid_stats(user_name):
     return Response(atomfeed, mimetype="application/atom+xml")
 
 
-@atom_bp.route("/user/<user_name>/stats/art/custom")
+@atom_bp.get("/user/<user_name>/stats/art/custom")
 @crossdomain
 @ratelimit()
 def get_cover_art_custom_stats(user_name):
@@ -1088,7 +1089,7 @@ def _generate_event_title(event):
 
 # Commented out as new OAuth is not merged yet. Once merged, update this function to use the new OAuth API to 
 # authenticate the user and then fetch the user's events feed.
-# @atom_bp.route("/user/<user_name>/events", methods=["GET"])
+# @atom_bp.get("/user/<user_name>/events")
 # @crossdomain
 # @ratelimit()
 # @api_listenstore_needed