Repository: CTFd/CTFd
Pull request #2495 (2024-03-15 10:56:00)

Add bracker filter in ScoreboardDetail
Linked to issue https://github.com/CTFd/CTFd/issues/2493

This patch proposes to add an extra GET parameter in ScoreboardDetail to filter by bracket.
This is useful to create custom scoreboards in downstream CTFd themes.

**I need help with `@cache.cached(timeout=60, key_prefix=make_cache_key, query_string=True)`.** It seems that CTFd custom cache keys does not support `query_string` option.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) Report
Attention: Patch coverage is `87.50000%` with `1 lines` in your changes are missing coverage. Please review.
> Project coverage is 85.31%. Comparing base [(`fde98ce`)](https://app.codecov.io/gh/CTFd/CTFd/commit/fde98ce6f7e3e0a5f0a750374e8263d8f7f4054a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) to head [(`fa3d09b`)](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).

| [Files](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) | Patch % | Lines |
|---|---|---|
| [CTFd/utils/scores/\_\_init\_\_.py](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd#diff-Q1RGZC91dGlscy9zY29yZXMvX19pbml0X18ucHk=) | 66.66% | [1 Missing :warning: ](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2495      +/-   ##
==========================================
- Coverage   87.73%   85.31%   -2.42%     
==========================================
  Files         155      153       -2     
  Lines        9610     9562      -48     
==========================================
- Hits         8431     8158     -273     
- Misses       1179     1404     +225     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/CTFd/CTFd/pull/2495?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CTFd).

ColdHeat: Yes I believe query params don't work for some reason. I think the function was mostly copied out of Flask-Caching though. I will take a look. 
ColdHeat: @erdnaxe I have a rough strategy for query string caching. I have made it so that we only cache based on specific parameters. How does this work for you?

This was tricker than I expected but likely this will be beneficial for other uses as well as it shows how to cache based on a specific item and then clear it as well meaning we can cache individual pages more easily. 
ColdHeat: FYI it looks like Flask-Caching does not allow controlling the query string logic if you specify the query_string argument which is why I think it's better to side step the behavior entirely.

https://github.com/pallets-eco/flask-caching/blob/a367b9f13109d35c1e3df069f5da4c29110a689c/src/flask_caching/__init__.py#L473-L477
ColdHeat: Seperately I have no idea why this endpoint returns a dictionary instead of a list. Might be an old holdover from when Flask could not return a list at the top level of a JSON response. 

Could be worth changing in 4.0. 

Patch:
diff --git a/CTFd/api/v1/scoreboard.py b/CTFd/api/v1/scoreboard.py
index b81702d7..13792d49 100644
--- a/CTFd/api/v1/scoreboard.py
+++ b/CTFd/api/v1/scoreboard.py
@@ -1,9 +1,10 @@
 from collections import defaultdict
 
+from flask import request
 from flask_restx import Namespace, Resource
 from sqlalchemy import select
 
-from CTFd.cache import cache, make_cache_key
+from CTFd.cache import cache, make_cache_key, make_cache_key_with_query_string
 from CTFd.models import Awards, Brackets, Solves, Users, db
 from CTFd.utils import get_config
 from CTFd.utils.dates import isoformat, unix_time_to_utc
@@ -90,11 +91,17 @@ class ScoreboardList(Resource):
 class ScoreboardDetail(Resource):
     @check_account_visibility
     @check_score_visibility
-    @cache.cached(timeout=60, key_prefix=make_cache_key)
+    @cache.cached(
+        timeout=60,
+        key_prefix=make_cache_key_with_query_string(allowed_params=["bracket_id"]),
+    )
     def get(self, count):
         response = {}
 
-        standings = get_standings(count=count)
+        # Optional filters
+        bracket_id = request.args.get("bracket_id")
+
+        standings = get_standings(count=count, bracket_id=bracket_id)
 
         team_ids = [team.account_id for team in standings]
 
diff --git a/CTFd/cache/__init__.py b/CTFd/cache/__init__.py
index 0511e127..b34f031b 100644
--- a/CTFd/cache/__init__.py
+++ b/CTFd/cache/__init__.py
@@ -1,4 +1,5 @@
 from functools import lru_cache, wraps
+from hashlib import md5
 from time import monotonic_ns
 
 from flask import request
@@ -52,6 +53,39 @@ def make_cache_key(path=None, key_prefix="view/%s"):
     return cache_key
 
 
+def make_cache_key_with_query_string(allowed_params=None, query_string_hash=None):
+    if allowed_params is None:
+        allowed_params = []
+
+    def _make_cache_key_with_query_string(path=None, key_prefix="view/%s/%s"):
+        if path is None:
+            path = request.endpoint
+
+        if query_string_hash:
+            args_hash = query_string_hash
+        else:
+            args_hash = calculate_param_hash(
+                params=tuple(request.args.items(multi=True)),
+                allowed_params=allowed_params,
+            )
+        cache_key = key_prefix % (path, args_hash)
+        return cache_key
+
+    return _make_cache_key_with_query_string
+
+
+def calculate_param_hash(params, allowed_params=None):
+    # Copied from Flask-Caching but modified to allow only accepted parameters
+    if allowed_params:
+        args_as_sorted_tuple = tuple(
+            sorted(pair for pair in params if pair[0] in allowed_params)
+        )
+    else:
+        args_as_sorted_tuple = tuple(sorted(pair for pair in params))
+    args_hash = md5(str(args_as_sorted_tuple).encode()).hexdigest()  # nosec B303
+    return args_hash
+
+
 def clear_config():
     from CTFd.utils import _get_config, get_app_config
 
@@ -63,7 +97,7 @@ def clear_standings():
     from CTFd.api import api
     from CTFd.api.v1.scoreboard import ScoreboardDetail, ScoreboardList
     from CTFd.constants.static import CacheKeys
-    from CTFd.models import Teams, Users  # noqa: I001
+    from CTFd.models import Brackets, Teams, Users  # noqa: I001
     from CTFd.utils.scores import get_standings, get_team_standings, get_user_standings
     from CTFd.utils.user import (
         get_team_place,
@@ -93,6 +127,19 @@ def clear_standings():
     cache.delete(make_cache_key(path=api.name + "." + ScoreboardList.endpoint))
     cache.delete(make_cache_key(path=api.name + "." + ScoreboardDetail.endpoint))
     cache.delete_memoized(ScoreboardList.get)
+    cache.delete_memoized(ScoreboardDetail.get)
+
+    # Clear out scoreboard detail
+    keys = [()]  # Empty tuple to handle case with no parameters
+    brackets = Brackets.query.all()
+    for bracket in brackets:
+        keys.append((("bracket_id", str(bracket.id)),))
+    for k in keys:
+        cache_func = make_cache_key_with_query_string(
+            query_string_hash=calculate_param_hash(params=k)
+        )
+        cache_key = cache_func(path=api.name + "." + ScoreboardDetail.endpoint)
+        cache.delete(cache_key)
 
     # Clear out scoreboard templates
     cache.delete(make_template_fragment_key(CacheKeys.PUBLIC_SCOREBOARD_TABLE))
diff --git a/CTFd/utils/scores/__init__.py b/CTFd/utils/scores/__init__.py
index 1ac4c915..171f7b1f 100644
--- a/CTFd/utils/scores/__init__.py
+++ b/CTFd/utils/scores/__init__.py
@@ -8,7 +8,7 @@ from CTFd.utils.modes import get_model
 
 
 @cache.memoize(timeout=60)
-def get_standings(count=None, admin=False, fields=None):
+def get_standings(count=None, bracket_id=None, admin=False, fields=None):
     """
     Get standings as a list of tuples containing account_id, name, and score e.g. [(account_id, team_name, score)].
 
@@ -121,9 +121,11 @@ def get_standings(count=None, admin=False, fields=None):
             )
         )
 
-    """
-    Only select a certain amount of users if asked.
-    """
+    # Filter on a bracket if asked
+    if bracket_id is not None:
+        standings_query = standings_query.filter(Model.bracket_id == bracket_id)
+
+    # Only select a certain amount of users if asked.
     if count is None:
         standings = standings_query.all()
     else:
@@ -133,7 +135,7 @@ def get_standings(count=None, admin=False, fields=None):
 
 
 @cache.memoize(timeout=60)
-def get_team_standings(count=None, admin=False, fields=None):
+def get_team_standings(count=None, bracket_id=None, admin=False, fields=None):
     if fields is None:
         fields = []
     scores = (
@@ -214,6 +216,9 @@ def get_team_standings(count=None, admin=False, fields=None):
             )
         )
 
+    if bracket_id is not None:
+        standings_query = standings_query.filter(Teams.bracket_id == bracket_id)
+
     if count is None:
         standings = standings_query.all()
     else:
@@ -223,7 +228,7 @@ def get_team_standings(count=None, admin=False, fields=None):
 
 
 @cache.memoize(timeout=60)
-def get_user_standings(count=None, admin=False, fields=None):
+def get_user_standings(count=None, bracket_id=None, admin=False, fields=None):
     if fields is None:
         fields = []
     scores = (
@@ -305,6 +310,9 @@ def get_user_standings(count=None, admin=False, fields=None):
             )
         )
 
+    if bracket_id is not None:
+        standings_query = standings_query.filter(Users.bracket_id == bracket_id)
+
     if count is None:
         standings = standings_query.all()
     else:


Test patch:
diff --git a/tests/api/v1/test_scoreboard.py b/tests/api/v1/test_scoreboard.py
index 6347c7e6..0dc75092 100644
--- a/tests/api/v1/test_scoreboard.py
+++ b/tests/api/v1/test_scoreboard.py
@@ -37,12 +37,21 @@ def test_scoreboard_is_cached():
             # No cached data
             assert app.cache.get("view/api.scoreboard_scoreboard_list") is None
             assert app.cache.get("view/api.scoreboard_scoreboard_detail") is None
+            assert (
+                app.cache.get(
+                    "view/api.scoreboard_scoreboard_detail/bcd8b0c2eb1fce714eab6cef0d771acc"
+                )
+                is None
+            )
 
             # Load and check cached data
             client.get("/api/v1/scoreboard")
             assert app.cache.get("view/api.scoreboard_scoreboard_list")
+            assert app.cache.get("view/api.scoreboard_scoreboard_detail") is None
             client.get("/api/v1/scoreboard/top/10")
-            assert app.cache.get("view/api.scoreboard_scoreboard_detail")
+            assert app.cache.get(
+                "view/api.scoreboard_scoreboard_detail/bcd8b0c2eb1fce714eab6cef0d771acc"
+            )
 
             # Check scoreboard page
             assert (
@@ -56,6 +65,12 @@ def test_scoreboard_is_cached():
             clear_standings()
             assert app.cache.get("view/api.scoreboard_scoreboard_list") is None
             assert app.cache.get("view/api.scoreboard_scoreboard_detail") is None
+            assert (
+                app.cache.get(
+                    "view/api.scoreboard_scoreboard_detail/bcd8b0c2eb1fce714eab6cef0d771acc"
+                )
+                is None
+            )
             assert (
                 app.cache.get(make_template_fragment_key("public_scoreboard_table"))
                 is None
diff --git a/tests/users/test_scoreboard.py b/tests/users/test_scoreboard.py
index 87cb7569..434939b1 100644
--- a/tests/users/test_scoreboard.py
+++ b/tests/users/test_scoreboard.py
@@ -8,6 +8,7 @@ from tests.helpers import (
     create_ctfd,
     destroy_ctfd,
     gen_award,
+    gen_bracket,
     gen_challenge,
     gen_flag,
     gen_solve,
@@ -84,9 +85,11 @@ def test_top_10():
     """Make sure top10 returns correct information"""
     app = create_ctfd()
     with app.app_context():
-        register_user(app, name="user1", email="user1@examplectf.com")
-        register_user(app, name="user2", email="user2@examplectf.com")
-        register_user(app)
+        gen_bracket(app.db, name="players1")
+        gen_bracket(app.db, name="players2")
+        register_user(app, name="user1", email="user1@examplectf.com", bracket_id=1)
+        register_user(app, name="user2", email="user2@examplectf.com", bracket_id=2)
+        register_user(app, bracket_id=1)
 
         chal1 = gen_challenge(app.db)
         gen_flag(app.db, challenge_id=chal1.id, content="flag")
@@ -117,8 +120,8 @@ def test_top_10():
                 "account_url": "/users/2",
                 "name": "user1",
                 "score": 200,
-                "bracket_id": None,
-                "bracket_name": None,
+                "bracket_id": 1,
+                "bracket_name": "players1",
                 "solves": [
                     {
                         "date": "2017-10-03T03:21:34Z",
@@ -143,8 +146,32 @@ def test_top_10():
                 "account_url": "/users/3",
                 "name": "user2",
                 "score": 100,
-                "bracket_id": None,
-                "bracket_name": None,
+                "bracket_id": 2,
+                "bracket_name": "players2",
+                "solves": [
+                    {
+                        "date": "2017-10-03T03:21:34Z",
+                        "challenge_id": 1,
+                        "account_id": 3,
+                        "user_id": 3,
+                        "team_id": None,
+                        "value": 100,
+                    }
+                ],
+            },
+        }
+        assert saved == response
+
+        r = client.get("/api/v1/scoreboard/top/10?bracket_id=2")
+        response = r.get_json()["data"]
+        saved = {
+            "1": {
+                "id": 3,
+                "account_url": "/users/3",
+                "name": "user2",
+                "score": 100,
+                "bracket_id": 2,
+                "bracket_name": "players2",
                 "solves": [
                     {
                         "date": "2017-10-03T03:21:34Z",