Fix issue with scoreboard ordering when an award results in a tie (#2212)

* Fix issue with scoreboard ordering when an award results in a tie
* Closes #833

CTFd/models/__init__.py

@@ -3,6 +3,7 @@ from collections import defaultdict
 
 from flask_marshmallow import Marshmallow
 from flask_sqlalchemy import SQLAlchemy
+from sqlalchemy.ext.compiler import compiles
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import column_property, validates
 
@@ -25,6 +26,16 @@ def get_class_by_tablename(tablename):
     return None
 
 
+@compiles(db.DateTime, "mysql")
+def compile_datetime_mysql(_type, _compiler, **kw):
+    """
+    This decorator makes the default db.DateTime class always enable fsp to enable millisecond precision
+    https://dev.mysql.com/doc/refman/5.7/en/fractional-seconds.html
+    https://docs.sqlalchemy.org/en/14/core/custom_types.html#overriding-type-compilation
+    """
+    return "DATETIME(6)"
+
+
 class Notifications(db.Model):
     __tablename__ = "notifications"
     id = db.Column(db.Integer, primary_key=True)

CTFd/utils/scores/__init__.py

@@ -91,7 +91,11 @@ def get_standings(count=None, admin=False, fields=None):
                 *fields,
             )
             .join(sumscores, Model.id == sumscores.columns.account_id)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
     else:
         standings_query = (
@@ -104,7 +108,11 @@ def get_standings(count=None, admin=False, fields=None):
             )
             .join(sumscores, Model.id == sumscores.columns.account_id)
             .filter(Model.banned == False, Model.hidden == False)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
 
     """
@@ -175,7 +183,11 @@ def get_team_standings(count=None, admin=False, fields=None):
                 *fields,
             )
             .join(sumscores, Teams.id == sumscores.columns.team_id)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
     else:
         standings_query = (
@@ -189,7 +201,11 @@ def get_team_standings(count=None, admin=False, fields=None):
             .join(sumscores, Teams.id == sumscores.columns.team_id)
             .filter(Teams.banned == False)
             .filter(Teams.hidden == False)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
 
     if count is None:
@@ -258,7 +274,11 @@ def get_user_standings(count=None, admin=False, fields=None):
                 *fields,
             )
             .join(sumscores, Users.id == sumscores.columns.user_id)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
     else:
         standings_query = (
@@ -272,7 +292,11 @@ def get_user_standings(count=None, admin=False, fields=None):
             )
             .join(sumscores, Users.id == sumscores.columns.user_id)
             .filter(Users.banned == False, Users.hidden == False)
-            .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
+            .order_by(
+                sumscores.columns.score.desc(),
+                sumscores.columns.date.asc(),
+                sumscores.columns.id.asc(),
+            )
         )
 
     if count is None:

migrations/versions/46a278193a94_enable_millisecond_precision_in_mysql_.py

@@ -0,0 +1,46 @@
+"""Enable millisecond precision in MySQL datetime
+
+Revision ID: 46a278193a94
+Revises: 4d3c1b59d011
+Create Date: 2022-11-01 23:27:44.620893
+
+"""
+from alembic import op
+from sqlalchemy.dialects import mysql
+
+
+# revision identifiers, used by Alembic.
+revision = "46a278193a94"
+down_revision = "4d3c1b59d011"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    bind = op.get_bind()
+    url = str(bind.engine.url)
+    if url.startswith("mysql"):
+        get_columns = "SELECT `TABLE_NAME`, `COLUMN_NAME` FROM `information_schema`.`COLUMNS` WHERE `table_schema`=DATABASE() AND `DATA_TYPE`='datetime' AND `COLUMN_TYPE`='datetime';"
+        conn = op.get_bind()
+        columns = conn.execute(get_columns).fetchall()
+        for table_name, column_name in columns:
+            op.alter_column(
+                table_name=table_name,
+                column_name=column_name,
+                type_=mysql.DATETIME(fsp=6),
+            )
+
+
+def downgrade():
+    bind = op.get_bind()
+    url = str(bind.engine.url)
+    if url.startswith("mysql"):
+        get_columns = "SELECT `TABLE_NAME`, `COLUMN_NAME` FROM `information_schema`.`COLUMNS` WHERE `table_schema`=DATABASE() AND `DATA_TYPE`='datetime' AND `COLUMN_TYPE`='datetime(6)';"
+        conn = op.get_bind()
+        columns = conn.execute(get_columns).fetchall()
+        for table_name, column_name in columns:
+            op.alter_column(
+                table_name=table_name,
+                column_name=column_name,
+                type_=mysql.DATETIME(fsp=0),
+            )

tests/api/v1/test_scoreboard.py

@@ -4,12 +4,15 @@
 from flask_caching import make_template_fragment_key
 
 from CTFd.cache import clear_standings
+from CTFd.models import Users
 from tests.helpers import (
     create_ctfd,
     destroy_ctfd,
+    gen_award,
     gen_challenge,
     gen_flag,
     gen_solve,
+    gen_team,
     login_as_user,
     register_user,
 )
@@ -58,3 +61,95 @@ def test_scoreboard_is_cached():
                 is None
             )
     destroy_ctfd(app)
+
+
+def test_scoreboard_tie_break_ordering_with_awards():
+    """
+    Test that scoreboard tie break ordering respects the addition of awards
+    """
+    app = create_ctfd()
+    with app.app_context():
+        # create user1
+        register_user(app, name="user1", email="user1@examplectf.com")
+        # create user2
+        register_user(app, name="user2", email="user2@examplectf.com")
+
+        chal = gen_challenge(app.db, value=100)
+        gen_flag(app.db, challenge_id=chal.id, content="flag")
+
+        chal = gen_challenge(app.db, value=200)
+        gen_flag(app.db, challenge_id=chal.id, content="flag")
+
+        # create solves for the challenges. (the user_ids are off by 1 because of the admin)
+        gen_solve(app.db, user_id=2, challenge_id=1)
+        gen_solve(app.db, user_id=3, challenge_id=2)
+
+        with login_as_user(app, "user1") as client:
+            r = client.get("/api/v1/scoreboard")
+            resp = r.get_json()
+            assert len(resp["data"]) == 2
+            assert resp["data"][0]["name"] == "user2"
+            assert resp["data"][0]["score"] == 200
+            assert resp["data"][1]["name"] == "user1"
+            assert resp["data"][1]["score"] == 100
+
+        # Give user1 an award for 100 points.
+        # At this point user2 should still be ahead
+        gen_award(app.db, user_id=2, value=100)
+
+        with login_as_user(app, "user1") as client:
+            r = client.get("/api/v1/scoreboard")
+            resp = r.get_json()
+            assert len(resp["data"]) == 2
+            assert resp["data"][0]["name"] == "user2"
+            assert resp["data"][0]["score"] == 200
+            assert resp["data"][1]["name"] == "user1"
+            assert resp["data"][1]["score"] == 200
+    destroy_ctfd(app)
+
+
+def test_scoreboard_tie_break_ordering_with_awards_under_teams():
+    """
+    Test that team mode scoreboard tie break ordering respects the addition of awards
+    """
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        gen_team(app.db, name="team1", email="team1@examplectf.com")
+        gen_team(app.db, name="team2", email="team2@examplectf.com")
+
+        chal = gen_challenge(app.db, value=100)
+        gen_flag(app.db, challenge_id=chal.id, content="flag")
+
+        chal = gen_challenge(app.db, value=200)
+        gen_flag(app.db, challenge_id=chal.id, content="flag")
+
+        # create solves for the challenges. (the user_ids are off by 1 because of the admin)
+        gen_solve(app.db, user_id=2, team_id=1, challenge_id=1)
+        gen_solve(app.db, user_id=6, team_id=2, challenge_id=2)
+
+        user = Users.query.filter_by(id=2).first()
+
+        with login_as_user(app, user.name) as client:
+            r = client.get("/api/v1/scoreboard")
+            resp = r.get_json()
+            print(resp)
+            assert len(resp["data"]) == 2
+            assert resp["data"][0]["name"] == "team2"
+            assert resp["data"][0]["score"] == 200
+            assert resp["data"][1]["name"] == "team1"
+            assert resp["data"][1]["score"] == 100
+
+        # Give a user on the team an award for 100 points.
+        # At this point team2 should still be ahead
+        gen_award(app.db, user_id=3, team_id=1, value=100)
+
+        with login_as_user(app, user.name) as client:
+            r = client.get("/api/v1/scoreboard")
+            resp = r.get_json()
+            print(resp)
+            assert len(resp["data"]) == 2
+            assert resp["data"][0]["name"] == "team2"
+            assert resp["data"][0]["score"] == 200
+            assert resp["data"][1]["name"] == "team1"
+            assert resp["data"][1]["score"] == 200
+    destroy_ctfd(app)