From e4fd1c47ddb0ee69f877b2ff8e24dcbb50ef5bec Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Fri, 7 Dec 2018 23:37:30 -0500 Subject: [PATCH] Fix downloading files as an anonymous user. (#792) * Fix downloading files as an anonymous user. * Fix viewing challenges anonymously if they have empty requirements. Closes #789 * Allow anonymous users to see see challenges with empty requirements or anonymized challenges --- CTFd/api/v1/challenges.py | 42 ++++++++++++++++++++------------------- CTFd/views.py | 3 ++- tests/users/test_views.py | 41 +++++++++++++++++++++++++++----------- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py index 6d75ebfa..0a6f5348 100644 --- a/CTFd/api/v1/challenges.py +++ b/CTFd/api/v1/challenges.py @@ -27,7 +27,7 @@ from CTFd.utils.decorators.visibility import ( ) from CTFd.cache import cache, clear_standings from CTFd.utils.scores import get_standings -from CTFd.utils.config.visibility import scores_visible, accounts_visible +from CTFd.utils.config.visibility import scores_visible, accounts_visible, challenges_visible from CTFd.utils.user import get_current_user, is_admin, authed from CTFd.utils.modes import get_model from CTFd.schemas.tags import TagSchema @@ -72,10 +72,10 @@ class ChallengeList(Resource): response = [] tag_schema = TagSchema(view='user', many=True) for challenge in challenges: - requirements = challenge.requirements - if requirements: - prereqs = set(requirements.get('prerequisites', [])) - anonymize = requirements.get('anonymize') + if challenge.requirements: + requirements = challenge.requirements.get('prerequisites', []) + anonymize = challenge.requirements.get('anonymize') + prereqs = set(requirements) if solve_ids >= prereqs: pass else: @@ -160,19 +160,22 @@ class Challenge(Resource): chal_class = get_chal_class(chal.type) - requirements = chal.requirements - if requirements: - if current_user.authed(): + if chal.requirements: + requirements = chal.requirements.get('prerequisites', []) + anonymize = chal.requirements.get('anonymize') + if challenges_visible(): user = get_current_user() - solve_ids = Solves.query \ - .with_entities(Solves.challenge_id) \ - .filter_by(account_id=user.account_id) \ - .order_by(Solves.challenge_id.asc()) \ - .all() + if user: + solve_ids = Solves.query \ + .with_entities(Solves.challenge_id) \ + .filter_by(account_id=user.account_id) \ + .order_by(Solves.challenge_id.asc()) \ + .all() + else: + # We need to handle the case where a user is viewing challenges anonymously + solve_ids = [] solve_ids = set([value for value, in solve_ids]) - - prereqs = set(requirements.get('prerequisites', [])) - anonymize = requirements.get('anonymize') + prereqs = set(requirements) if solve_ids >= prereqs or is_admin(): pass else: @@ -313,16 +316,15 @@ class ChallengeAttempt(Resource): if challenge.state == 'locked': abort(403) - requirements = challenge.requirements - if requirements: + if challenge.requirements: + requirements = challenge.requirements.get('prerequisites', []) solve_ids = Solves.query \ .with_entities(Solves.challenge_id) \ .filter_by(account_id=user.account_id) \ .order_by(Solves.challenge_id.asc()) \ .all() solve_ids = set([solve_id for solve_id, in solve_ids]) - - prereqs = set(requirements.get('prerequisites', [])) + prereqs = set(requirements) if solve_ids >= prereqs: pass else: diff --git a/CTFd/views.py b/CTFd/views.py index 2a6c392b..e3b8e186 100644 --- a/CTFd/views.py +++ b/CTFd/views.py @@ -9,6 +9,7 @@ from CTFd.utils.user import authed, get_current_user from CTFd.utils import config from CTFd.utils.uploads import get_uploader from CTFd.utils.config.pages import get_page +from CTFd.utils.config.visibility import challenges_visible from CTFd.utils.security.auth import login_user from CTFd.utils.security.csrf import generate_nonce from CTFd.utils import user as current_user @@ -188,7 +189,7 @@ def files(path): """ f = Files.query.filter_by(location=path).first_or_404() if f.type == 'challenge': - if current_user.authed(): + if challenges_visible(): if current_user.is_admin() is False: if not ctftime(): abort(403) diff --git a/tests/users/test_views.py b/tests/users/test_views.py index ebea642e..6a49509f 100644 --- a/tests/users/test_views.py +++ b/tests/users/test_views.py @@ -171,14 +171,23 @@ def test_user_can_access_files(): f = gen_file(app.db, location=model_path, challenge_id=chal_id) url = url_for('views.files', path=model_path) - # Unauthed user should return 403 + # Unauthed user should be able to see challenges if challenges are public + set_config('challenge_visibility', 'public') + with app.test_client() as client: + r = client.get(url) + + assert r.status_code == 200 + assert r.get_data(as_text=True) == 'testing file load' + + # Unauthed user should be able to see challenges if challenges are private + set_config('challenge_visibility', 'private') with app.test_client() as client: r = client.get(url) assert r.status_code == 403 assert r.get_data(as_text=True) != 'testing file load' - # Authed user should be able to see the files + # Authed user should be able to see files if challenges are private register_user(app) client = login_as_user(app) r = client.get(url) @@ -187,18 +196,26 @@ def test_user_can_access_files(): with freeze_time("2017-10-7"): set_config('end', '1507262400') # Friday, October 6, 2017 12:00:00 AM GMT-04:00 DST + for v in ('public', 'private'): + set_config('challenge_visibility', v) - # Authed users shouldn't be able to see files if the CTF hasn't started - client = login_as_user(app) - r = client.get(url) - assert r.status_code == 403 - assert r.get_data(as_text=True) != 'testing file load' + # Unauthed users shouldn't be able to see files if the CTF hasn't started + client = app.test_client() + r = client.get(url) + assert r.status_code == 403 + assert r.get_data(as_text=True) != 'testing file load' - # Admins should be able to see files if the CTF hasn't started - admin = login_as_user(app, "admin") - r = admin.get(url) - assert r.status_code == 200 - assert r.get_data(as_text=True) == 'testing file load' + # Authed users shouldn't be able to see files if the CTF hasn't started + client = login_as_user(app) + r = client.get(url) + assert r.status_code == 403 + assert r.get_data(as_text=True) != 'testing file load' + + # Admins should be able to see files if the CTF hasn't started + admin = login_as_user(app, "admin") + r = admin.get(url) + assert r.status_code == 200 + assert r.get_data(as_text=True) == 'testing file load' finally: rmdir(directory) destroy_ctfd(app)