From 7c60c697ee38a500b4584fbac14e2e424e48e4d4 Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Thu, 4 Apr 2019 22:44:18 -0400 Subject: [PATCH] Properly hide users/teams if they are set to banned/hidden (#932) * Properly hide users/teams if they are set to hidden/banned * This should be in the API and in the main user panel. This should not affect admins. * Update tests to reflect this behavior. --- CTFd/api/v1/teams.py | 12 +++++++++ CTFd/api/v1/users.py | 14 +++++++++- CTFd/teams.py | 2 +- CTFd/users.py | 2 +- tests/api/v1/test_teams.py | 44 ++++++++++++++++++++++++++++++ tests/api/v1/test_users.py | 48 +++++++++++++++++++++++++++++++++ tests/teams/test_teams.py | 21 +++++++++++++++ tests/users/test_submissions.py | 21 ++++++++------- tests/users/test_teams.py | 0 tests/users/test_users.py | 29 ++++++++++++++++++++ 10 files changed, 181 insertions(+), 12 deletions(-) delete mode 100644 tests/users/test_teams.py diff --git a/CTFd/api/v1/teams.py b/CTFd/api/v1/teams.py index ff16f748..e7683d7d 100644 --- a/CTFd/api/v1/teams.py +++ b/CTFd/api/v1/teams.py @@ -80,6 +80,9 @@ class TeamPublic(Resource): def get(self, team_id): team = Teams.query.filter_by(id=team_id).first_or_404() + if (team.banned or team.hidden) and is_admin() is False: + abort(404) + view = TeamSchema.views.get(session.get('type', 'user')) schema = TeamSchema(view=view) response = schema.dump(team) @@ -196,6 +199,9 @@ class TeamSolves(Resource): abort(404) team = Teams.query.filter_by(id=team_id).first_or_404() + if (team.banned or team.hidden) and is_admin() is False: + abort(404) + solves = team.get_solves( admin=is_admin() ) @@ -230,6 +236,9 @@ class TeamFails(Resource): abort(404) team = Teams.query.filter_by(id=team_id).first_or_404() + if (team.banned or team.hidden) and is_admin() is False: + abort(404) + fails = team.get_fails( admin=is_admin() ) @@ -274,6 +283,9 @@ class TeamAwards(Resource): abort(404) team = Teams.query.filter_by(id=team_id).first_or_404() + if (team.banned or team.hidden) and is_admin() is False: + abort(404) + awards = team.get_awards( admin=is_admin() ) diff --git a/CTFd/api/v1/users.py b/CTFd/api/v1/users.py index 5e37b9a8..052b79d6 100644 --- a/CTFd/api/v1/users.py +++ b/CTFd/api/v1/users.py @@ -32,7 +32,7 @@ users_namespace = Namespace('users', description="Endpoint to retrieve Users") class UserList(Resource): @check_account_visibility def get(self): - users = Users.query.filter_by(banned=False) + users = Users.query.filter_by(banned=False, hidden=False) response = UserSchema(view='user', many=True).dump(users) if response.errors: @@ -78,6 +78,9 @@ class UserPublic(Resource): def get(self, user_id): user = Users.query.filter_by(id=user_id).first_or_404() + if (user.banned or user.hidden) and is_admin() is False: + abort(404) + response = UserSchema( view=session.get('type', 'user') ).dump(user) @@ -192,6 +195,9 @@ class UserSolves(Resource): abort(404) user = Users.query.filter_by(id=user_id).first_or_404() + if (user.banned or user.hidden) and is_admin() is False: + abort(404) + solves = user.get_solves( admin=is_admin() ) @@ -226,6 +232,9 @@ class UserFails(Resource): abort(404) user = Users.query.filter_by(id=user_id).first_or_404() + if (user.banned or user.hidden) and is_admin() is False: + abort(404) + fails = user.get_fails( admin=is_admin() ) @@ -266,6 +275,9 @@ class UserAwards(Resource): abort(404) user = Users.query.filter_by(id=user_id).first_or_404() + if (user.banned or user.hidden) and is_admin() is False: + abort(404) + awards = user.get_awards( admin=is_admin() ) diff --git a/CTFd/teams.py b/CTFd/teams.py index 2dd2555c..2e4a99f8 100644 --- a/CTFd/teams.py +++ b/CTFd/teams.py @@ -128,7 +128,7 @@ def private(): @require_team_mode def public(team_id): errors = get_errors() - team = Teams.query.filter_by(id=team_id).first_or_404() + team = Teams.query.filter_by(id=team_id, banned=False, hidden=False).first_or_404() solves = team.get_solves() awards = team.get_awards() diff --git a/CTFd/users.py b/CTFd/users.py index c1038118..2f7a2dac 100644 --- a/CTFd/users.py +++ b/CTFd/users.py @@ -58,5 +58,5 @@ def private(): @check_account_visibility @check_score_visibility def public(user_id): - user = Users.query.filter_by(id=user_id).first_or_404() + user = Users.query.filter_by(id=user_id, banned=False, hidden=False).first_or_404() return render_template('users/user.html', user=user) diff --git a/tests/api/v1/test_teams.py b/tests/api/v1/test_teams.py index b558f6b2..7de02cd9 100644 --- a/tests/api/v1/test_teams.py +++ b/tests/api/v1/test_teams.py @@ -436,3 +436,47 @@ def test_api_team_get_awards(): print(r.get_json()) assert r.status_code == 200 destroy_ctfd(app) + + +def test_api_accessing_hidden_banned_users(): + """Hidden/Banned users should not be visible to normal users, only to admins""" + app = create_ctfd(user_mode="teams") + with app.app_context(): + register_user(app) + register_user(app, name="user2", email="user2@ctfd.io") + register_user(app, name="visible_user", email="visible_user@ctfd.io") + + user = Users.query.filter_by(id=2).first() + team = gen_team(app.db, name='hidden_team', email="hidden_team@ctfd.io", hidden=True) + team.members.append(user) + user.team_id = team.id + app.db.session.commit() + + user = Users.query.filter_by(id=3).first() + team = gen_team(app.db, name='banned_team', email="banned_team@ctfd.io", banned=True) + team.members.append(user) + user.team_id = team.id + app.db.session.commit() + + with login_as_user(app, name="visible_user") as client: + assert client.get('/api/v1/teams/1').status_code == 404 + assert client.get('/api/v1/teams/1/solves').status_code == 404 + assert client.get('/api/v1/teams/1/fails').status_code == 404 + assert client.get('/api/v1/teams/1/awards').status_code == 404 + + assert client.get('/api/v1/teams/2').status_code == 404 + assert client.get('/api/v1/teams/2/solves').status_code == 404 + assert client.get('/api/v1/teams/2/fails').status_code == 404 + assert client.get('/api/v1/teams/2/awards').status_code == 404 + + with login_as_user(app, name="admin") as client: + assert client.get('/api/v1/teams/1').status_code == 200 + assert client.get('/api/v1/teams/1/solves').status_code == 200 + assert client.get('/api/v1/teams/1/fails').status_code == 200 + assert client.get('/api/v1/teams/1/awards').status_code == 200 + + assert client.get('/api/v1/teams/2').status_code == 200 + assert client.get('/api/v1/teams/2/solves').status_code == 200 + assert client.get('/api/v1/teams/2/fails').status_code == 200 + assert client.get('/api/v1/teams/2/awards').status_code == 200 + destroy_ctfd(app) diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py index 4108bdac..3d7a8676 100644 --- a/tests/api/v1/test_users.py +++ b/tests/api/v1/test_users.py @@ -571,6 +571,54 @@ def test_api_user_get_awards(): destroy_ctfd(app) +def test_api_accessing_hidden_users(): + """Hidden users should not be visible to normal users, only to admins""" + app = create_ctfd() + with app.app_context(): + register_user(app, name="visible_user", email="visible_user@ctfd.io") + register_user(app, name="hidden_user", email="hidden_user@ctfd.io") # ID 3 + user = Users.query.filter_by(name="hidden_user").first() + user.hidden = True + app.db.session.commit() + + with login_as_user(app, name="visible_user") as client: + assert client.get('/api/v1/users/3').status_code == 404 + assert client.get('/api/v1/users/3/solves').status_code == 404 + assert client.get('/api/v1/users/3/fails').status_code == 404 + assert client.get('/api/v1/users/3/awards').status_code == 404 + + with login_as_user(app, name="admin") as client: + assert client.get('/api/v1/users/3').status_code == 200 + assert client.get('/api/v1/users/3/solves').status_code == 200 + assert client.get('/api/v1/users/3/fails').status_code == 200 + assert client.get('/api/v1/users/3/awards').status_code == 200 + destroy_ctfd(app) + + +def test_api_accessing_banned_users(): + """Banned users should not be visible to normal users, only to admins""" + app = create_ctfd() + with app.app_context(): + register_user(app, name="visible_user", email="visible_user@ctfd.io") + register_user(app, name="banned_user", email="banned_user@ctfd.io") # ID 3 + user = Users.query.filter_by(name="banned_user").first() + user.banned = True + app.db.session.commit() + + with login_as_user(app, name="visible_user") as client: + assert client.get('/api/v1/users/3').status_code == 404 + assert client.get('/api/v1/users/3/solves').status_code == 404 + assert client.get('/api/v1/users/3/fails').status_code == 404 + assert client.get('/api/v1/users/3/awards').status_code == 404 + + with login_as_user(app, name="admin") as client: + assert client.get('/api/v1/users/3').status_code == 200 + assert client.get('/api/v1/users/3/solves').status_code == 200 + assert client.get('/api/v1/users/3/fails').status_code == 200 + assert client.get('/api/v1/users/3/awards').status_code == 200 + destroy_ctfd(app) + + def test_api_user_send_email(): """Can an admin post /api/v1/users//email""" app = create_ctfd() diff --git a/tests/teams/test_teams.py b/tests/teams/test_teams.py index a2ed5915..df9db3d7 100644 --- a/tests/teams/test_teams.py +++ b/tests/teams/test_teams.py @@ -22,6 +22,27 @@ def test_teams_get(): destroy_ctfd(app) +def test_accessing_hidden_teams(): + """Hidden teams should not give any data from /teams or /api/v1/teams""" + app = create_ctfd(user_mode="teams") + with app.app_context(): + register_user(app) + register_user(app, name="visible_user", email="visible_user@ctfd.io") + with login_as_user(app, name="visible_user") as client: + user = Users.query.filter_by(id=2).first() + team = gen_team(app.db, name='visible_team', hidden=True) + team.members.append(user) + user.team_id = team.id + app.db.session.commit() + + assert client.get('/teams/1').status_code == 404 + assert client.get('/api/v1/teams/1').status_code == 404 + assert client.get('/api/v1/teams/1/solves').status_code == 404 + assert client.get('/api/v1/teams/1/fails').status_code == 404 + assert client.get('/api/v1/teams/1/awards').status_code == 404 + destroy_ctfd(app) + + def test_hidden_teams_visibility(): """Hidden teams should not show up on /teams or /api/v1/teams or /api/v1/scoreboard""" app = create_ctfd(user_mode="teams") diff --git a/tests/users/test_submissions.py b/tests/users/test_submissions.py index e3bb16e7..d9ae91ad 100644 --- a/tests/users/test_submissions.py +++ b/tests/users/test_submissions.py @@ -29,9 +29,10 @@ def test_user_get_another_public_solves(): """Can a registered user load public solves page of another user""" app = create_ctfd() with app.app_context(): - register_user(app) - client = login_as_user(app) - r = client.get('/api/v1/users/1/solves') + register_user(app, name='user1', email='user1@ctfd.io') # ID 2 + register_user(app, name='user2', email='user2@ctfd.io') # ID 3 + client = login_as_user(app, name='user2') + r = client.get('/api/v1/users/2/solves') assert r.status_code == 200 destroy_ctfd(app) @@ -62,9 +63,10 @@ def test_user_get_another_public_fails(): """Can a registered user load public fails page of another user""" app = create_ctfd() with app.app_context(): - register_user(app) - client = login_as_user(app) - r = client.get('/api/v1/users/1/fails') + register_user(app, name='user1', email='user1@ctfd.io') # ID 2 + register_user(app, name='user2', email='user2@ctfd.io') # ID 3 + client = login_as_user(app, name="user2") + r = client.get('/api/v1/users/2/fails') assert r.status_code == 200 destroy_ctfd(app) @@ -84,9 +86,10 @@ def test_user_get_another_public_team_page(): """Can a registered user load the public profile of another user (/users/1)""" app = create_ctfd() with app.app_context(): - register_user(app) - client = login_as_user(app) - r = client.get('/users/1') + register_user(app, name='user1', email='user1@ctfd.io') # ID 2 + register_user(app, name='user2', email='user2@ctfd.io') # ID 3 + client = login_as_user(app, name='user2') + r = client.get('/users/2') assert r.status_code == 200 destroy_ctfd(app) diff --git a/tests/users/test_teams.py b/tests/users/test_teams.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/users/test_users.py b/tests/users/test_users.py index acbfac94..45a7706c 100644 --- a/tests/users/test_users.py +++ b/tests/users/test_users.py @@ -4,6 +4,35 @@ from tests.helpers import * +def test_accessing_hidden_users(): + """Hidden users should not give any data from /users or /api/v1/users""" + app = create_ctfd() + with app.app_context(): + register_user(app, name="visible_user", email="visible_user@ctfd.io") # ID 2 + register_user(app, name="hidden_user", email="hidden_user@ctfd.io") # ID 3 + register_user(app, name="banned_user", email="banned_user@ctfd.io") # ID 4 + user = Users.query.filter_by(name="hidden_user").first() + user.hidden = True + app.db.session.commit() + user = Users.query.filter_by(name="banned_user").first() + user.banned = True + app.db.session.commit() + + with login_as_user(app, name="visible_user") as client: + assert client.get('/users/3').status_code == 404 + assert client.get('/api/v1/users/3').status_code == 404 + assert client.get('/api/v1/users/3/solves').status_code == 404 + assert client.get('/api/v1/users/3/fails').status_code == 404 + assert client.get('/api/v1/users/3/awards').status_code == 404 + + assert client.get('/users/4').status_code == 404 + assert client.get('/api/v1/users/4').status_code == 404 + assert client.get('/api/v1/users/4/solves').status_code == 404 + assert client.get('/api/v1/users/4/fails').status_code == 404 + assert client.get('/api/v1/users/4/awards').status_code == 404 + destroy_ctfd(app) + + def test_hidden_user_visibility(): """Hidden users should not show up on /users or /api/v1/users or /api/v1/scoreboard""" app = create_ctfd()