From 3d23ece370b642443f537d19d7fc5211dc312c90 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Sat, 11 May 2019 00:18:56 -0400 Subject: [PATCH] Fix freeze time regressions (#989) * Fix freeze time regressions in 2.x * Make `/api/v1/[users,teams]/[me,id]/[solves,fails,awards]` endpoints load as admin to load all rows and bypass freeze * Closes #988 * Make `/api/v1/challenges/[id]/solves` respect freeze time. `/api/v1/challenges/[id]/solves?preview=true` is exposed for admins to see solves as a user would. * Closes #986 --- .gitignore | 1 + CTFd/api/v1/challenges.py | 8 ++ CTFd/api/v1/teams.py | 18 ++--- CTFd/api/v1/users.py | 20 ++--- CTFd/models/__init__.py | 8 +- tests/api/v1/test_challenges.py | 50 +++++++++++- tests/api/v1/test_teams.py | 127 ++++++++++++++++++++++++++++--- tests/api/v1/test_users.py | 131 +++++++++++++++++++++++++++++++- 8 files changed, 321 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index 0c3c59d..42d344d 100644 --- a/.gitignore +++ b/.gitignore @@ -58,6 +58,7 @@ target/ *.db *.log .idea/ +.vscode/ CTFd/static/uploads CTFd/uploads .data/ diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py index d0616eb..c064c4d 100644 --- a/CTFd/api/v1/challenges.py +++ b/CTFd/api/v1/challenges.py @@ -38,6 +38,7 @@ from CTFd.utils.dates import ctf_ended, ctf_paused, ctftime from CTFd.utils.logging import log from CTFd.utils.security.signing import serialize from sqlalchemy.sql import and_ +import datetime challenges_namespace = Namespace('challenges', description="Endpoint to retrieve Challenges") @@ -523,6 +524,13 @@ class ChallengeSolves(Resource): .filter(Solves.challenge_id == challenge_id, Model.banned == False, Model.hidden == False)\ .order_by(Solves.date.asc()) + freeze = get_config('freeze') + if freeze: + preview = request.args.get('preview') + if (is_admin() is False) or (is_admin() is True and preview): + dt = datetime.datetime.utcfromtimestamp(freeze) + solves = solves.filter(Solves.date < dt) + endpoint = None if get_config('user_mode') == TEAMS_MODE: endpoint = 'teams.public' diff --git a/CTFd/api/v1/teams.py b/CTFd/api/v1/teams.py index 6af4ea2..a9410aa 100644 --- a/CTFd/api/v1/teams.py +++ b/CTFd/api/v1/teams.py @@ -304,6 +304,7 @@ class TeamSolves(Resource): if not authed(): abort(403) team = get_current_team() + solves = team.get_solves(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -311,10 +312,7 @@ class TeamSolves(Resource): if (team.banned or team.hidden) and is_admin() is False: abort(404) - - solves = team.get_solves( - admin=is_admin() - ) + solves = team.get_solves(admin=is_admin()) view = 'admin' if is_admin() else 'user' schema = SubmissionSchema(view=view, many=True) @@ -341,6 +339,7 @@ class TeamFails(Resource): if not authed(): abort(403) team = get_current_team() + fails = team.get_fails(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -348,10 +347,7 @@ class TeamFails(Resource): if (team.banned or team.hidden) and is_admin() is False: abort(404) - - fails = team.get_fails( - admin=is_admin() - ) + fails = team.get_fails(admin=is_admin()) view = 'admin' if is_admin() else 'user' @@ -388,6 +384,7 @@ class TeamAwards(Resource): if not authed(): abort(403) team = get_current_team() + awards = team.get_awards(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -395,10 +392,7 @@ class TeamAwards(Resource): if (team.banned or team.hidden) and is_admin() is False: abort(404) - - awards = team.get_awards( - admin=is_admin() - ) + awards = team.get_awards(admin=is_admin()) schema = AwardSchema(many=True) response = schema.dump(awards) diff --git a/CTFd/api/v1/users.py b/CTFd/api/v1/users.py index 9317209..a3a10a4 100644 --- a/CTFd/api/v1/users.py +++ b/CTFd/api/v1/users.py @@ -200,6 +200,7 @@ class UserSolves(Resource): if not authed(): abort(403) user = get_current_user() + solves = user.get_solves(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -207,12 +208,7 @@ class UserSolves(Resource): if (user.banned or user.hidden) and is_admin() is False: abort(404) - - solves = user.get_solves( - admin=is_admin() - ) - for solve in solves: - setattr(solve, 'value', 100) + solves = user.get_solves(admin=is_admin()) view = 'user' if not is_admin() else 'admin' response = SubmissionSchema(view=view, many=True).dump(solves) @@ -237,6 +233,7 @@ class UserFails(Resource): if not authed(): abort(403) user = get_current_user() + fails = user.get_fails(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -244,10 +241,7 @@ class UserFails(Resource): if (user.banned or user.hidden) and is_admin() is False: abort(404) - - fails = user.get_fails( - admin=is_admin() - ) + fails = user.get_fails(admin=is_admin()) view = 'user' if not is_admin() else 'admin' response = SubmissionSchema(view=view, many=True).dump(fails) @@ -280,6 +274,7 @@ class UserAwards(Resource): if not authed(): abort(403) user = get_current_user() + awards = user.get_awards(admin=True) else: if accounts_visible() is False or scores_visible() is False: abort(404) @@ -287,10 +282,7 @@ class UserAwards(Resource): if (user.banned or user.hidden) and is_admin() is False: abort(404) - - awards = user.get_awards( - admin=is_admin() - ) + awards = user.get_awards(admin=is_admin()) view = 'user' if not is_admin() else 'admin' response = AwardSchema(view=view, many=True).dump(awards) diff --git a/CTFd/models/__init__.py b/CTFd/models/__init__.py index b1c4a3b..d6819ab 100644 --- a/CTFd/models/__init__.py +++ b/CTFd/models/__init__.py @@ -310,7 +310,7 @@ class Users(db.Model): freeze = get_config('freeze') if freeze and admin is False: dt = datetime.datetime.utcfromtimestamp(freeze) - fails = fails.filter(Solves.date < dt) + fails = fails.filter(Fails.date < dt) return fails.all() def get_awards(self, admin=False): @@ -318,7 +318,7 @@ class Users(db.Model): freeze = get_config('freeze') if freeze and admin is False: dt = datetime.datetime.utcfromtimestamp(freeze) - awards = awards.filter(Solves.date < dt) + awards = awards.filter(Awards.date < dt) return awards.all() def get_score(self, admin=False): @@ -513,7 +513,7 @@ class Teams(db.Model): freeze = get_config('freeze') if freeze and admin is False: dt = datetime.datetime.utcfromtimestamp(freeze) - fails = fails.filter(Solves.date < dt) + fails = fails.filter(Fails.date < dt) return fails.all() @@ -529,7 +529,7 @@ class Teams(db.Model): freeze = get_config('freeze') if freeze and admin is False: dt = datetime.datetime.utcfromtimestamp(freeze) - awards = awards.filter(Solves.date < dt) + awards = awards.filter(Awards.date < dt) return awards.all() diff --git a/tests/api/v1/test_challenges.py b/tests/api/v1/test_challenges.py index a06e615..b9176e5 100644 --- a/tests/api/v1/test_challenges.py +++ b/tests/api/v1/test_challenges.py @@ -1,7 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -from CTFd.models import Users, Challenges, Tags, Hints, Flags +from CTFd.models import Users, Challenges, Tags, Hints, Flags, Solves from CTFd.utils import set_config from tests.helpers import ( create_ctfd, @@ -442,6 +442,54 @@ def test_api_challenge_get_solves_ctftime_public(): destroy_ctfd(app) +def test_api_challenge_get_solves_ctf_frozen(): + """Test users can only see challenge solves that happened before freeze time""" + app = create_ctfd() + with app.app_context(): + register_user(app, name="user1", email="user1@ctfd.io") + register_user(app, name="user2", email="user2@ctfd.io") + + # Friday, October 6, 2017 12:00:00 AM GMT-04:00 DST + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + chal = gen_challenge(app.db) + chal_id = chal.id + gen_solve(app.db, user_id=2, challenge_id=chal_id) + chal2 = gen_challenge(app.db) + chal2_id = chal2.id + + with freeze_time("2017-10-8"): + chal2 = gen_solve(app.db, user_id=2, challenge_id=chal2_id) + + # There should now be two solves assigned to the same user. + assert Solves.query.count() == 2 + + client = login_as_user(app, name="user2") + + # Challenge 1 should have one solve + r = client.get('/api/v1/challenges/1/solves') + data = r.get_json()['data'] + assert len(data) == 1 + + # Challenge 2 should have a solve shouldn't be shown to the user + r = client.get('/api/v1/challenges/2/solves') + data = r.get_json()['data'] + assert len(data) == 0 + + # Admins should see data as an admin with no modifications + admin = login_as_user(app, name="admin") + r = admin.get('/api/v1/challenges/2/solves') + data = r.get_json()['data'] + assert len(data) == 1 + + # But should see as a user if the preview param is passed + r = admin.get('/api/v1/challenges/2/solves?preview=true') + data = r.get_json()['data'] + assert len(data) == 0 + + destroy_ctfd(app) + + def test_api_challenge_get_solves_visibility_private(): """Can a private user get /api/v1/challenges//solves if challenge_visibility is private/public""" app = create_ctfd() diff --git a/tests/api/v1/test_teams.py b/tests/api/v1/test_teams.py index 65a4a07..52683bc 100644 --- a/tests/api/v1/test_teams.py +++ b/tests/api/v1/test_teams.py @@ -1,7 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -from CTFd.models import Teams, Users +from CTFd.models import Teams, Users, Solves, Fails, Awards from CTFd.utils import set_config from CTFd.utils.crypto import verify_password from tests.helpers import ( @@ -13,8 +13,12 @@ from tests.helpers import ( gen_team, gen_challenge, gen_flag, + gen_solve, + gen_award, + gen_fail, simulate_user_activity ) +from freezegun import freeze_time def test_api_teams_get_public(): @@ -42,7 +46,6 @@ def test_api_teams_get_private(): with login_as_user(app) as client: set_config('account_visibility', 'public') r = client.get('/api/v1/teams') - print(r.__dict__) assert r.status_code == 200 set_config('account_visibility', 'private') r = client.get('/api/v1/teams') @@ -191,7 +194,6 @@ def test_api_team_get_private(): set_config('account_visibility', 'public') gen_team(app.db) r = client.get('/api/v1/teams/1') - print(r.__dict__) assert r.status_code == 200 set_config('account_visibility', 'private') r = client.get('/api/v1/teams/1') @@ -408,7 +410,6 @@ def test_api_team_get_me_solves_logged_in(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/me/solves') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) @@ -424,11 +425,50 @@ def test_api_team_get_solves(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/1/solves') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) +def test_api_team_get_solves_after_freze_time(): + """Can a user get /api/v1/teams//solves after freeze time""" + app = create_ctfd(user_mode="teams") + with app.app_context(): + register_user(app) + team = gen_team(app.db, name='team1', email='team1@ctfd.io', member_count=1) + + team_member = team.members[0] + tm_name = team_member.name + + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + chal = gen_challenge(app.db) + chal_id = chal.id + gen_solve(app.db, user_id=3, team_id=1, challenge_id=chal_id) + chal2 = gen_challenge(app.db) + chal2_id = chal2.id + + with freeze_time("2017-10-8"): + gen_solve(app.db, user_id=3, team_id=1, challenge_id=chal2_id) + + assert Solves.query.count() == 2 + + with login_as_user(app) as client: + r = client.get('/api/v1/teams/1/solves') + data = r.get_json()['data'] + assert len(data) == 1 + + with login_as_user(app, name=tm_name) as client: + r = client.get('/api/v1/teams/me/solves') + data = r.get_json()['data'] + assert len(data) == 2 + + with login_as_user(app, name="admin") as client: + r = client.get('/api/v1/teams/1/solves') + data = r.get_json()['data'] + assert len(data) == 2 + destroy_ctfd(app) + + def test_api_team_get_me_fails_not_logged_in(): """Can a user get /api/v1/teams/me/fails if not logged in""" app = create_ctfd(user_mode="teams") @@ -450,7 +490,6 @@ def test_api_team_get_me_fails_logged_in(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/me/fails') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) @@ -466,11 +505,47 @@ def test_api_team_get_fails(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/1/fails') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) +def test_api_team_get_fails_after_freze_time(): + """Can a user get /api/v1/teams//fails after freeze time""" + app = create_ctfd(user_mode="teams") + with app.app_context(): + register_user(app) + team = gen_team(app.db, name='team1', email='team1@ctfd.io', member_count=1) + + team_member = team.members[0] + tm_name = team_member.name + + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + chal = gen_challenge(app.db) + chal_id = chal.id + chal2 = gen_challenge(app.db) + chal2_id = chal2.id + gen_fail(app.db, user_id=3, team_id=1, challenge_id=chal_id) + + with freeze_time("2017-10-8"): + gen_fail(app.db, user_id=3, team_id=1, challenge_id=chal2_id) + + assert Fails.query.count() == 2 + + with login_as_user(app) as client: + r = client.get('/api/v1/teams/1/fails') + assert r.get_json()['meta']['count'] == 1 + + with login_as_user(app, name=tm_name) as client: + r = client.get('/api/v1/teams/me/fails') + assert r.get_json()['meta']['count'] == 2 + + with login_as_user(app, name="admin") as client: + r = client.get('/api/v1/teams/1/fails') + assert r.get_json()['meta']['count'] == 2 + destroy_ctfd(app) + + def test_api_team_get_me_awards_not_logged_in(): """Can a user get /api/v1/teams/me/awards if not logged in""" app = create_ctfd(user_mode="teams") @@ -492,7 +567,6 @@ def test_api_team_get_me_awards_logged_in(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/me/awards') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) @@ -508,11 +582,46 @@ def test_api_team_get_awards(): app.db.session.commit() with login_as_user(app, name="user_name") as client: r = client.get('/api/v1/teams/1/awards') - print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) +def test_api_team_get_awards_after_freze_time(): + """Can a user get /api/v1/teams//awards after freeze time""" + app = create_ctfd(user_mode="teams") + with app.app_context(): + register_user(app) + team = gen_team(app.db, name='team1', email='team1@ctfd.io', member_count=1) + + team_member = team.members[0] + tm_name = team_member.name + + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + gen_award(app.db, user_id=3) + + with freeze_time("2017-10-8"): + gen_award(app.db, user_id=3) + + assert Awards.query.count() == 2 + + with login_as_user(app) as client: + r = client.get('/api/v1/teams/1/awards') + data = r.get_json()['data'] + assert len(data) == 1 + + with login_as_user(app, name=tm_name) as client: + r = client.get('/api/v1/teams/me/awards') + data = r.get_json()['data'] + assert len(data) == 2 + + with login_as_user(app, name="admin") as client: + r = client.get('/api/v1/teams/1/awards') + data = r.get_json()['data'] + assert len(data) == 2 + destroy_ctfd(app) + + def test_api_team_patch_password(): """Can a user change their team password /api/v1/teams/me if logged in as the captain""" app = create_ctfd(user_mode="teams") diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py index 38af141..8e515fe 100644 --- a/tests/api/v1/test_users.py +++ b/tests/api/v1/test_users.py @@ -1,7 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -from CTFd.models import Users +from CTFd.models import Users, Solves, Awards, Fails from CTFd.utils import set_config from CTFd.utils.crypto import verify_password from CTFd.schemas.users import UserSchema @@ -10,9 +10,14 @@ from tests.helpers import ( destroy_ctfd, register_user, login_as_user, + simulate_user_activity, + gen_challenge, gen_user, - simulate_user_activity + gen_solve, + gen_award, + gen_fail, ) +from freezegun import freeze_time def test_api_users_get_public(): @@ -543,6 +548,49 @@ def test_api_user_get_solves(): destroy_ctfd(app) +def test_api_user_get_solves_after_freze_time(): + """Can a user get /api/v1/users//solves after freeze time""" + app = create_ctfd(user_mode="users") + with app.app_context(): + register_user(app, name="user1", email="user1@ctfd.io") + register_user(app, name="user2", email="user2@ctfd.io") + + # Friday, October 6, 2017 12:00:00 AM GMT-04:00 DST + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + chal = gen_challenge(app.db) + chal_id = chal.id + gen_solve(app.db, user_id=2, challenge_id=chal_id) + chal2 = gen_challenge(app.db) + chal2_id = chal2.id + + with freeze_time("2017-10-8"): + chal2 = gen_solve(app.db, user_id=2, challenge_id=chal2_id) + + # There should now be two solves assigned to the same user. + assert Solves.query.count() == 2 + + # User 2 should have 2 solves when seen by themselves + client = login_as_user(app, name="user1") + r = client.get('/api/v1/users/me/solves') + data = r.get_json()['data'] + assert len(data) == 2 + + # User 2 should have 1 solve when seen by another user + client = login_as_user(app, name="user2") + r = client.get('/api/v1/users/2/solves') + data = r.get_json()['data'] + assert len(data) == 1 + + # Admins should see all solves for the user + admin = login_as_user(app, name="admin") + + r = admin.get('/api/v1/users/2/solves') + data = r.get_json()['data'] + assert len(data) == 2 + destroy_ctfd(app) + + def test_api_user_get_me_fails_not_logged_in(): """Can a user get /api/v1/users/me/fails if not logged in""" app = create_ctfd() @@ -575,6 +623,46 @@ def test_api_user_get_fails(): destroy_ctfd(app) +def test_api_user_get_fails_after_freze_time(): + """Can a user get /api/v1/users//fails after freeze time""" + app = create_ctfd(user_mode="users") + with app.app_context(): + register_user(app, name="user1", email="user1@ctfd.io") + register_user(app, name="user2", email="user2@ctfd.io") + + # Friday, October 6, 2017 12:00:00 AM GMT-04:00 DST + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + chal = gen_challenge(app.db) + chal_id = chal.id + chal2 = gen_challenge(app.db) + chal2_id = chal2.id + gen_fail(app.db, user_id=2, challenge_id=chal_id) + + with freeze_time("2017-10-8"): + chal2 = gen_fail(app.db, user_id=2, challenge_id=chal2_id) + + # There should now be two fails assigned to the same user. + assert Fails.query.count() == 2 + + # User 2 should have 2 fail when seen by themselves + client = login_as_user(app, name="user1") + r = client.get('/api/v1/users/me/fails') + assert r.get_json()['meta']['count'] == 2 + + # User 2 should have 1 fail when seen by another user + client = login_as_user(app, name="user2") + r = client.get('/api/v1/users/2/fails') + assert r.get_json()['meta']['count'] == 1 + + # Admins should see all fails for the user + admin = login_as_user(app, name="admin") + + r = admin.get('/api/v1/users/2/fails') + assert r.get_json()['meta']['count'] == 2 + destroy_ctfd(app) + + def test_api_user_get_me_awards_not_logged_in(): """Can a user get /api/v1/users/me/awards if not logged in""" app = create_ctfd() @@ -607,6 +695,45 @@ def test_api_user_get_awards(): destroy_ctfd(app) +def test_api_user_get_awards_after_freze_time(): + """Can a user get /api/v1/users//awards after freeze time""" + app = create_ctfd(user_mode="users") + with app.app_context(): + register_user(app, name="user1", email="user1@ctfd.io") + register_user(app, name="user2", email="user2@ctfd.io") + + # Friday, October 6, 2017 12:00:00 AM GMT-04:00 DST + set_config('freeze', '1507262400') + with freeze_time("2017-10-4"): + gen_award(app.db, user_id=2) + + with freeze_time("2017-10-8"): + gen_award(app.db, user_id=2) + + # There should now be two awards assigned to the same user. + assert Awards.query.count() == 2 + + # User 2 should have 2 awards when seen by themselves + client = login_as_user(app, name="user1") + r = client.get('/api/v1/users/me/awards') + data = r.get_json()['data'] + assert len(data) == 2 + + # User 2 should have 1 award when seen by another user + client = login_as_user(app, name="user2") + r = client.get('/api/v1/users/2/awards') + data = r.get_json()['data'] + assert len(data) == 1 + + # Admins should see all awards for the user + admin = login_as_user(app, name="admin") + + r = admin.get('/api/v1/users/2/awards') + data = r.get_json()['data'] + assert len(data) == 2 + destroy_ctfd(app) + + def test_api_accessing_hidden_users(): """Hidden users should not be visible to normal users, only to admins""" app = create_ctfd()