Repository: CTFd/CTFd
Pull request #932 (2019-04-04 23:27:27)

Properly hide users/teams if they are set to banned/hidden
* 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. 

codecov[bot]: # [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=h1) Report
> Merging [#932](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=desc) into [master](https://codecov.io/gh/CTFd/CTFd/commit/268ed85f609b7dd76651aa1a63692b68ec829227?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/CTFd/CTFd/pull/932/graphs/tree.svg?width=650&token=IfPqkWkt9A&height=150&src=pr)](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #932      +/-   ##
==========================================
+ Coverage   87.34%   87.36%   +0.02%     
==========================================
  Files          91       91              
  Lines        4631     4647      +16     
==========================================
+ Hits         4045     4060      +15     
- Misses        586      587       +1
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/api/v1/teams.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC9hcGkvdjEvdGVhbXMucHk=) | `92.15% <100%> (+1.12%)` | :arrow_up: |
| [CTFd/api/v1/users.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC9hcGkvdjEvdXNlcnMucHk=) | `94.64% <100%> (+0.89%)` | :arrow_up: |
| [CTFd/teams.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC90ZWFtcy5weQ==) | `97.72% <100%> (ø)` | :arrow_up: |
| [CTFd/users.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC91c2Vycy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [CTFd/utils/updates/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy91cGRhdGVzL19faW5pdF9fLnB5) | `81.57% <0%> (-5.27%)` | :arrow_down: |
| [CTFd/utils/sessions/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/932/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy9zZXNzaW9ucy9fX2luaXRfXy5weQ==) | `92.42% <0%> (-1.52%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=footer). Last update [268ed85...8159693](https://codecov.io/gh/CTFd/CTFd/pull/932?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
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)


Test patch:
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/<user_id>/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()