diff --git a/CHANGELOG.md b/CHANGELOG.md index 79918630..2933b6f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# UNRELEASED + +**General** + +- Fix an issue where admins couldn't see challenges which had requirements in the add requirements interface + # 3.3.0 / 2020-03-26 **General** diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py index bf8a5d5b..69bcc1b3 100644 --- a/CTFd/api/v1/challenges.py +++ b/CTFd/api/v1/challenges.py @@ -120,12 +120,20 @@ def _build_solves_query(extra_filters=(), admin_view=False): .group_by(Solves.challenge_id) ) # Also gather the user's solve items which can be different from above query - # Even if we are a hidden user, we should see that we have solved the challenge - # But as a hidden user we are not included in the count - solve_ids = ( - Solves.query.with_entities(Solves.challenge_id).filter(user_solved_cond).all() - ) - solve_ids = {value for value, in solve_ids} + # For example, even if we are a hidden user, we should see that we have solved a challenge + # however as a hidden user we are not included in the count of the above query + if admin_view: + # If we're an admin we should show all challenges as solved to break through any requirements + challenges = Challenges.query.all() + solve_ids = {challenge.id for challenge in challenges} + else: + # If not an admin we calculate solves as normal + solve_ids = ( + Solves.query.with_entities(Solves.challenge_id) + .filter(user_solved_cond) + .all() + ) + solve_ids = {value for value, in solve_ids} return solves_q, solve_ids @@ -441,7 +449,7 @@ class Challenge(Resource): response = chal_class.read(challenge=chal) solves_q, user_solves = _build_solves_query( - admin_view=is_admin(), extra_filters=(Solves.challenge_id == chal.id,) + extra_filters=(Solves.challenge_id == chal.id,) ) # If there are no solves for this challenge ID then we have 0 rows maybe_row = solves_q.first() diff --git a/tests/api/v1/challenges/requirements/test_requirements.py b/tests/api/v1/challenges/requirements/test_requirements.py new file mode 100644 index 00000000..b9b83f30 --- /dev/null +++ b/tests/api/v1/challenges/requirements/test_requirements.py @@ -0,0 +1,171 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +from CTFd.models import Users +from CTFd.utils import set_config +from tests.helpers import ( + create_ctfd, + destroy_ctfd, + gen_challenge, + gen_solve, + login_as_user, + register_user, +) + + +def test_api_challenges_admins_can_bypass_requirements(): + """Test that admins can bypass requirements checks with admin capabilities and view-admin""" + app = create_ctfd() + with app.app_context(): + # Create challenges + prereq_id = gen_challenge(app.db).id + chal_obj = gen_challenge(app.db) + chal_obj.requirements = {"prerequisites": [prereq_id]} + + register_user(app) + # Confirm that regular users cannot see prerequisites + with login_as_user(app) as client: + # Locked challenges aren't shown + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 1 + assert data[0]["id"] == 1 + + # Not even with tricks + r = client.get("/api/v1/challenges?view=admin") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 1 + assert data[0]["id"] == 1 + + # Not even with forced browsing + r = client.get("/api/v1/challenges/2") + assert r.status_code == 403 + + # Confirm that admins + with login_as_user(app, name="admin") as admin: + # Admins see as regular users + r = admin.get("/api/v1/challenges") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 1 + assert data[0]["id"] == 1 + + # Now admins can see all challenges + r = admin.get("/api/v1/challenges?view=admin") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 2 + assert data[0]["id"] == 1 + assert data[1]["id"] == 2 + + # Admins can force browse to challenges + r = admin.get("/api/v1/challenges/2") + assert r.status_code == 200 + assert r.get_json()["data"] + destroy_ctfd(app) + + +def test_api_challenges_challenge_with_requirements(): + """Does the challenge list API show challenges with requirements met?""" + app = create_ctfd() + with app.app_context(): + prereq_id = gen_challenge(app.db).id + chal_obj = gen_challenge(app.db) + chal_obj.requirements = {"prerequisites": [prereq_id]} + chal_id = chal_obj.id + # Create a new user which will solve the prerequisite + register_user(app) + # Confirm that only the prerequisite challenge is listed initially + with login_as_user(app) as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + (chal_data,) = r.get_json()["data"] + assert chal_data["id"] == prereq_id + # Generate a solve and then confirm the second challenge is visible + gen_solve(app.db, user_id=2, challenge_id=prereq_id) + with login_as_user(app) as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 2 + chal_ids = {c["id"] for c in r.get_json()["data"]} + assert chal_ids == {prereq_id, chal_id} + destroy_ctfd(app) + + +def test_api_challenges_challenge_with_requirements_hidden_user(): + """Does the challenge list API show gated challenges to a hidden user?""" + app = create_ctfd() + with app.app_context(): + prereq_id = gen_challenge(app.db).id + chal_obj = gen_challenge(app.db) + chal_obj.requirements = {"prerequisites": [prereq_id]} + chal_id = chal_obj.id + # Create a new user which will solve the prerequisite and hide them + register_user(app) + Users.query.get(2).hidden = True + app.db.session.commit() + # Confirm that only the prerequisite challenge is listed initially + with login_as_user(app) as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + (chal_data,) = r.get_json()["data"] + assert chal_data["id"] == prereq_id + # Generate a solve and then confirm the second challenge is visible + gen_solve(app.db, user_id=2, challenge_id=prereq_id) + with login_as_user(app) as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + data = r.get_json()["data"] + assert len(data) == 2 + chal_ids = {c["id"] for c in r.get_json()["data"]} + assert chal_ids == {prereq_id, chal_id} + destroy_ctfd(app) + + +def test_api_challenges_challenge_with_requirements_banned_user(): + """Does the challenge list API show gated challenges to a banned user?""" + app = create_ctfd() + with app.app_context(): + prereq_id = gen_challenge(app.db).id + chal_obj = gen_challenge(app.db) + chal_obj.requirements = {"prerequisites": [prereq_id]} + # Create a new user which will solve the prerequisite and ban them + register_user(app) + Users.query.get(2).banned = True + app.db.session.commit() + # Generate a solve just in case and confirm the API 403s + gen_solve(app.db, user_id=2, challenge_id=prereq_id) + with login_as_user(app) as client: + assert client.get("/api/v1/challenges").status_code == 403 + destroy_ctfd(app) + + +def test_api_challenges_challenge_with_requirements_no_user(): + """Does the challenge list API show gated challenges to the public?""" + app = create_ctfd() + with app.app_context(): + set_config("challenge_visibility", "public") + prereq_id = gen_challenge(app.db).id + chal_obj = gen_challenge(app.db) + chal_obj.requirements = {"prerequisites": [prereq_id]} + # Create a new user which will solve the prerequisite + register_user(app) + # Confirm that only the prerequisite challenge is listed publicly + with app.test_client() as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + initial_data = r.get_json()["data"] + (chal_data,) = initial_data + assert chal_data["id"] == prereq_id + # Fix up the solve count for later comparison with `initial_data` + chal_data["solves"] += 1 + # Generate a solve and then confirm the response is unchanged + gen_solve(app.db, user_id=2, challenge_id=prereq_id) + with app.test_client() as client: + r = client.get("/api/v1/challenges") + assert r.status_code == 200 + assert r.get_json()["data"] == initial_data + destroy_ctfd(app) diff --git a/tests/api/v1/test_challenges.py b/tests/api/v1/test_challenges.py index a952a09b..b17571f5 100644 --- a/tests/api/v1/test_challenges.py +++ b/tests/api/v1/test_challenges.py @@ -405,110 +405,6 @@ def test_api_challenges_get_solve_count_banned_user(): destroy_ctfd(app) -def test_api_challenges_challenge_with_requirements(): - """Does the challenge list API show challenges with requirements met?""" - app = create_ctfd() - with app.app_context(): - prereq_id = gen_challenge(app.db).id - chal_obj = gen_challenge(app.db) - chal_obj.requirements = {"prerequisites": [prereq_id]} - chal_id = chal_obj.id - # Create a new user which will solve the prerequisite - register_user(app) - # Confirm that only the prerequisite challenge is listed initially - with login_as_user(app) as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - (chal_data,) = r.get_json()["data"] - assert chal_data["id"] == prereq_id - # Generate a solve and then confirm the second challenge is visible - gen_solve(app.db, user_id=2, challenge_id=prereq_id) - with login_as_user(app) as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - data = r.get_json()["data"] - assert len(data) == 2 - chal_ids = {c["id"] for c in r.get_json()["data"]} - assert chal_ids == {prereq_id, chal_id} - destroy_ctfd(app) - - -def test_api_challenges_challenge_with_requirements_hidden_user(): - """Does the challenge list API show gated challenges to a hidden user?""" - app = create_ctfd() - with app.app_context(): - prereq_id = gen_challenge(app.db).id - chal_obj = gen_challenge(app.db) - chal_obj.requirements = {"prerequisites": [prereq_id]} - chal_id = chal_obj.id - # Create a new user which will solve the prerequisite and hide them - register_user(app) - Users.query.get(2).hidden = True - app.db.session.commit() - # Confirm that only the prerequisite challenge is listed initially - with login_as_user(app) as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - (chal_data,) = r.get_json()["data"] - assert chal_data["id"] == prereq_id - # Generate a solve and then confirm the second challenge is visible - gen_solve(app.db, user_id=2, challenge_id=prereq_id) - with login_as_user(app) as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - data = r.get_json()["data"] - assert len(data) == 2 - chal_ids = {c["id"] for c in r.get_json()["data"]} - assert chal_ids == {prereq_id, chal_id} - destroy_ctfd(app) - - -def test_api_challenges_challenge_with_requirements_banned_user(): - """Does the challenge list API show gated challenges to a banned user?""" - app = create_ctfd() - with app.app_context(): - prereq_id = gen_challenge(app.db).id - chal_obj = gen_challenge(app.db) - chal_obj.requirements = {"prerequisites": [prereq_id]} - # Create a new user which will solve the prerequisite and ban them - register_user(app) - Users.query.get(2).banned = True - app.db.session.commit() - # Generate a solve just in case and confirm the API 403s - gen_solve(app.db, user_id=2, challenge_id=prereq_id) - with login_as_user(app) as client: - assert client.get("/api/v1/challenges").status_code == 403 - destroy_ctfd(app) - - -def test_api_challenges_challenge_with_requirements_no_user(): - """Does the challenge list API show gated challenges to the public?""" - app = create_ctfd() - with app.app_context(): - set_config("challenge_visibility", "public") - prereq_id = gen_challenge(app.db).id - chal_obj = gen_challenge(app.db) - chal_obj.requirements = {"prerequisites": [prereq_id]} - # Create a new user which will solve the prerequisite - register_user(app) - # Confirm that only the prerequisite challenge is listed publicly - with app.test_client() as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - initial_data = r.get_json()["data"] - (chal_data,) = initial_data - assert chal_data["id"] == prereq_id - # Fix up the solve count for later comparison with `initial_data` - chal_data["solves"] += 1 - # Generate a solve and then confirm the response is unchanged - gen_solve(app.db, user_id=2, challenge_id=prereq_id) - with app.test_client() as client: - r = client.get("/api/v1/challenges") - assert r.status_code == 200 - assert r.get_json()["data"] == initial_data - destroy_ctfd(app) - - def test_api_challenges_post_admin(): """Can a user post /api/v1/challenges if admin""" app = create_ctfd()