diff --git a/CTFd/models.py b/CTFd/models.py index 8334b4ad..8f382385 100644 --- a/CTFd/models.py +++ b/CTFd/models.py @@ -210,14 +210,14 @@ class Teams(db.Model): db.func.sum(Challenges.value).label('score'), db.func.max(Solves.id).label('id'), db.func.max(Solves.date).label('date') - ).join(Challenges).group_by(Solves.teamid) + ).join(Challenges).filter(Challenges.value != 0).group_by(Solves.teamid) awards = db.session.query( Awards.teamid.label('teamid'), db.func.sum(Awards.value).label('score'), db.func.max(Awards.id).label('id'), db.func.max(Awards.date).label('date') - ).group_by(Awards.teamid) + ).filter(Awards.value != 0).group_by(Awards.teamid) if not admin: freeze = Config.query.filter_by(key='freeze').first() diff --git a/CTFd/scoreboard.py b/CTFd/scoreboard.py index 37730708..bb45f4e4 100644 --- a/CTFd/scoreboard.py +++ b/CTFd/scoreboard.py @@ -9,19 +9,31 @@ scoreboard = Blueprint('scoreboard', __name__) def get_standings(admin=False, count=None): + """ + Get team standings as a list of tuples containing team_id, team_name, and score e.g. [(team_id, team_name, score)]. + + Ties are broken by who reached a given score first based on the solve ID. Two users can have the same score but one + user will have a solve ID that is before the others. That user will be considered the tie-winner. + + Challenges & Awards with a value of zero are filtered out of the calculations to avoid incorrect tie breaks. + """ scores = db.session.query( Solves.teamid.label('teamid'), db.func.sum(Challenges.value).label('score'), db.func.max(Solves.id).label('id'), db.func.max(Solves.date).label('date') - ).join(Challenges).group_by(Solves.teamid) + ).join(Challenges)\ + .filter(Challenges.value != 0)\ + .group_by(Solves.teamid) awards = db.session.query( Awards.teamid.label('teamid'), db.func.sum(Awards.value).label('score'), db.func.max(Awards.id).label('id'), db.func.max(Awards.date).label('date') - ).group_by(Awards.teamid) + )\ + .filter(Awards.value != 0)\ + .group_by(Awards.teamid) """ Filter out solves and awards that are before a specific time point. @@ -44,7 +56,8 @@ def get_standings(admin=False, count=None): db.func.sum(results.columns.score).label('score'), db.func.max(results.columns.id).label('id'), db.func.max(results.columns.date).label('date') - ).group_by(results.columns.teamid).subquery() + ).group_by(results.columns.teamid)\ + .subquery() """ Admins can see scores for all users but the public cannot see banned users. diff --git a/tests/user/test_scoreboard.py b/tests/user/test_scoreboard.py index 61d55a1d..494be25a 100644 --- a/tests/user/test_scoreboard.py +++ b/tests/user/test_scoreboard.py @@ -78,3 +78,155 @@ def test_top_10(): received = json.loads(response) assert saved == received destroy_ctfd(app) + + +def test_scoring_logic(): + """Test that scoring logic is correct""" + app = create_ctfd() + with app.app_context(): + admin = login_as_user(app, name="admin", password="password") + + register_user(app, name="user1", email="user1@ctfd.io", password="password") + client1 = login_as_user(app, name="user1", password="password") + register_user(app, name="user2", email="user2@ctfd.io", password="password") + client2 = login_as_user(app, name="user2", password="password") + + chal1 = gen_challenge(app.db) + flag1 = gen_flag(app.db, chal=chal1.id, flag='flag') + chal1_id = chal1.id + + chal2 = gen_challenge(app.db) + flag2 = gen_flag(app.db, chal=chal2.id, flag='flag') + chal2_id = chal2.id + + # user1 solves chal1 + with freeze_time("2017-10-3 03:21:34"): + with client1.session_transaction() as sess: + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client1.post('/chal/{}'.format(chal1_id), data=data) + + # user1 is now on top + scores = get_scores(admin) + assert scores[0]['team'] == 'user1' + + # user2 solves chal1 and chal2 + with freeze_time("2017-10-4 03:30:34"): + with client2.session_transaction() as sess: + # solve chal1 + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client2.post('/chal/{}'.format(chal1_id), data=data) + # solve chal2 + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client2.post('/chal/{}'.format(chal2_id), data=data) + + # user2 is now on top + scores = get_scores(admin) + assert scores[0]['team'] == 'user2' + + # user1 solves chal2 + with freeze_time("2017-10-5 03:50:34"): + with client1.session_transaction() as sess: + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client1.post('/chal/{}'.format(chal2_id), data=data) + + # user2 should still be on top because they solved chal2 first + scores = get_scores(admin) + assert scores[0]['team'] == 'user2' + destroy_ctfd(app) + + +def test_scoring_logic_with_zero_point_challenges(): + """Test that scoring logic is correct with zero point challenges. Zero point challenges should not tie break""" + app = create_ctfd() + with app.app_context(): + admin = login_as_user(app, name="admin", password="password") + + register_user(app, name="user1", email="user1@ctfd.io", password="password") + client1 = login_as_user(app, name="user1", password="password") + register_user(app, name="user2", email="user2@ctfd.io", password="password") + client2 = login_as_user(app, name="user2", password="password") + + chal1 = gen_challenge(app.db) + flag1 = gen_flag(app.db, chal=chal1.id, flag='flag') + chal1_id = chal1.id + + chal2 = gen_challenge(app.db) + flag2 = gen_flag(app.db, chal=chal2.id, flag='flag') + chal2_id = chal2.id + + # A 0 point challenge shouldn't influence the scoreboard (see #577) + chal0 = gen_challenge(app.db, value=0) + flag0 = gen_flag(app.db, chal=chal0.id, flag='flag') + chal0_id = chal0.id + + # user1 solves chal1 + with freeze_time("2017-10-3 03:21:34"): + with client1.session_transaction() as sess: + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client1.post('/chal/{}'.format(chal1_id), data=data) + + # user1 is now on top + scores = get_scores(admin) + assert scores[0]['team'] == 'user1' + + # user2 solves chal1 and chal2 + with freeze_time("2017-10-4 03:30:34"): + with client2.session_transaction() as sess: + # solve chal1 + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client2.post('/chal/{}'.format(chal1_id), data=data) + # solve chal2 + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client2.post('/chal/{}'.format(chal2_id), data=data) + + # user2 is now on top + scores = get_scores(admin) + assert scores[0]['team'] == 'user2' + + # user1 solves chal2 + with freeze_time("2017-10-5 03:50:34"): + with client1.session_transaction() as sess: + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client1.post('/chal/{}'.format(chal2_id), data=data) + + # user2 should still be on top because they solved chal2 first + scores = get_scores(admin) + assert scores[0]['team'] == 'user2' + + # user2 solves a 0 point challenge + with freeze_time("2017-10-5 03:55:34"): + with client2.session_transaction() as sess: + data = { + "key": 'flag', + "nonce": sess.get('nonce') + } + r = client2.post('/chal/{}'.format(chal0_id), data=data) + + # user2 should still be on top because 0 point challenges should not tie break + scores = get_scores(admin) + assert scores[0]['team'] == 'user2' + destroy_ctfd(app) diff --git a/tests/user/test_user_facing.py b/tests/user/test_user_facing.py index c3b1ec0b..ce45d2d3 100644 --- a/tests/user/test_user_facing.py +++ b/tests/user/test_user_facing.py @@ -320,73 +320,6 @@ def test_user_get_reset_password(): destroy_ctfd(app) -def test_scoring_logic(): - """Test that scoring logic is correct""" - app = create_ctfd() - with app.app_context(): - admin = login_as_user(app, name="admin", password="password") - - register_user(app, name="user1", email="user1@ctfd.io", password="password") - client1 = login_as_user(app, name="user1", password="password") - register_user(app, name="user2", email="user2@ctfd.io", password="password") - client2 = login_as_user(app, name="user2", password="password") - - chal1 = gen_challenge(app.db) - flag1 = gen_flag(app.db, chal=chal1.id, flag='flag') - chal1_id = chal1.id - - chal2 = gen_challenge(app.db) - flag2 = gen_flag(app.db, chal=chal2.id, flag='flag') - chal2_id = chal2.id - - # user1 solves chal1 - with freeze_time("2017-10-3 03:21:34"): - with client1.session_transaction() as sess: - data = { - "key": 'flag', - "nonce": sess.get('nonce') - } - r = client1.post('/chal/{}'.format(chal1_id), data=data) - - # user1 is now on top - scores = get_scores(admin) - assert scores[0]['team'] == 'user1' - - # user2 solves chal1 and chal2 - with freeze_time("2017-10-4 03:30:34"): - with client2.session_transaction() as sess: - # solve chal1 - data = { - "key": 'flag', - "nonce": sess.get('nonce') - } - r = client2.post('/chal/{}'.format(chal1_id), data=data) - # solve chal2 - data = { - "key": 'flag', - "nonce": sess.get('nonce') - } - r = client2.post('/chal/{}'.format(chal2_id), data=data) - - # user2 is now on top - scores = get_scores(admin) - assert scores[0]['team'] == 'user2' - - # user1 solves chal2 - with freeze_time("2017-10-5 03:50:34"): - with client1.session_transaction() as sess: - data = { - "key": 'flag', - "nonce": sess.get('nonce') - } - r = client1.post('/chal/{}'.format(chal2_id), data=data) - - # user2 should still be on top because they solved chal2 first - scores = get_scores(admin) - assert scores[0]['team'] == 'user2' - destroy_ctfd(app) - - def test_user_score_is_correct(): '''Test that a user's score is correct''' app = create_ctfd()