Repository: CTFd/CTFd
Pull request #943 (2019-04-08 03:41:55)

Properly load schemas specified as strings
* Properly load schemas specified by their key string
* Add test for UserSchema 
* Prevent users without teams from interacting with challenges if the CTF is in Team Mode

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

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

```diff
@@            Coverage Diff             @@
##           master     #943      +/-   ##
==========================================
+ Coverage   87.36%   87.37%   +0.01%     
==========================================
  Files          91       91              
  Lines        4647     4659      +12     
==========================================
+ Hits         4060     4071      +11     
- Misses        587      588       +1
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/943?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/schemas/teams.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3RlYW1zLnB5) | `78.75% <100%> (+0.26%)` | :arrow_up: |
| [CTFd/schemas/flags.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL2ZsYWdzLnB5) | `77.77% <33.33%> (+1.3%)` | :arrow_up: |
| [CTFd/schemas/pages.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3BhZ2VzLnB5) | `81.81% <33.33%> (+0.86%)` | :arrow_up: |
| [CTFd/schemas/unlocks.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3VubG9ja3MucHk=) | `78.94% <33.33%> (+1.16%)` | :arrow_up: |
| [CTFd/schemas/files.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL2ZpbGVzLnB5) | `77.77% <33.33%> (+1.3%)` | :arrow_up: |
| [CTFd/schemas/config.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL2NvbmZpZy5weQ==) | `78.94% <33.33%> (+1.16%)` | :arrow_up: |
| [CTFd/schemas/notifications.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL25vdGlmaWNhdGlvbnMucHk=) | `77.77% <33.33%> (+1.3%)` | :arrow_up: |
| [CTFd/schemas/tags.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3RhZ3MucHk=) | `89.47% <66.66%> (+0.58%)` | :arrow_up: |
| [CTFd/schemas/submissions.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3N1Ym1pc3Npb25zLnB5) | `90% <66.66%> (+0.52%)` | :arrow_up: |
| [CTFd/schemas/users.py](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3VzZXJzLnB5) | `97.75% <66.66%> (-1.12%)` | :arrow_down: |
| ... and [2 more](https://codecov.io/gh/CTFd/CTFd/pull/943/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/CTFd/CTFd/pull/943?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/943?src=pr&el=footer). Last update [7c60c69...80d2d1d](https://codecov.io/gh/CTFd/CTFd/pull/943?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py
index d9eb6f2b..d58ce6be 100644
--- a/CTFd/api/v1/challenges.py
+++ b/CTFd/api/v1/challenges.py
@@ -66,6 +66,10 @@ class ChallengeList(Resource):
                 .order_by(Solves.challenge_id.asc())\
                 .all()
             solve_ids = set([value for value, in solve_ids])
+
+            # TODO: Convert this into a re-useable decorator
+            if config.is_teams_mode() and get_current_team() is None:
+                abort(403)
         else:
             solve_ids = set()
 
@@ -210,6 +214,10 @@ class Challenge(Resource):
             unlocked_hints = set([u.target for u in HintUnlocks.query.filter_by(
                 type='hints', account_id=user.account_id)])
 
+            # TODO: Convert this into a re-useable decorator
+            if config.is_teams_mode() and get_current_team() is None:
+                abort(403)
+
         for hint in Hints.query.filter_by(challenge_id=chal.id).all():
             if hint.id in unlocked_hints or ctf_ended():
                 hints.append({'id': hint.id, 'cost': hint.cost,
@@ -309,6 +317,10 @@ class ChallengeAttempt(Resource):
         user = get_current_user()
         team = get_current_team()
 
+        # TODO: Convert this into a re-useable decorator
+        if config.is_teams_mode() and team is None:
+            abort(403)
+
         fails = Fails.query.filter_by(
             account_id=user.account_id,
             challenge_id=challenge_id
diff --git a/CTFd/schemas/awards.py b/CTFd/schemas/awards.py
index ff3962d0..9b213356 100644
--- a/CTFd/schemas/awards.py
+++ b/CTFd/schemas/awards.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Awards
+from CTFd.utils import string_types
 
 
 class AwardSchema(ma.ModelSchema):
@@ -43,9 +44,9 @@ class AwardSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(AwardSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/config.py b/CTFd/schemas/config.py
index b227e1fc..84fe4168 100644
--- a/CTFd/schemas/config.py
+++ b/CTFd/schemas/config.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Configs
+from CTFd.utils import string_types
 
 
 class ConfigSchema(ma.ModelSchema):
@@ -21,9 +22,9 @@ class ConfigSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(ConfigSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/files.py b/CTFd/schemas/files.py
index fa50e142..191eb53c 100644
--- a/CTFd/schemas/files.py
+++ b/CTFd/schemas/files.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Files, ChallengeFiles, PageFiles
+from CTFd.utils import string_types
 
 
 class FileSchema(ma.ModelSchema):
@@ -13,9 +14,9 @@ class FileSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(FileSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/flags.py b/CTFd/schemas/flags.py
index e3c44a15..1e661042 100644
--- a/CTFd/schemas/flags.py
+++ b/CTFd/schemas/flags.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Flags
+from CTFd.utils import string_types
 
 
 class FlagSchema(ma.ModelSchema):
@@ -13,9 +14,9 @@ class FlagSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(FlagSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/hints.py b/CTFd/schemas/hints.py
index 4ab8a098..08748461 100644
--- a/CTFd/schemas/hints.py
+++ b/CTFd/schemas/hints.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Hints
+from CTFd.utils import string_types
 
 
 class HintSchema(ma.ModelSchema):
@@ -37,9 +38,9 @@ class HintSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(HintSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/notifications.py b/CTFd/schemas/notifications.py
index 62400271..bcb48039 100644
--- a/CTFd/schemas/notifications.py
+++ b/CTFd/schemas/notifications.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Notifications
+from CTFd.utils import string_types
 
 
 class NotificationSchema(ma.ModelSchema):
@@ -13,9 +14,9 @@ class NotificationSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(NotificationSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/pages.py b/CTFd/schemas/pages.py
index 21a7077d..6b5ff3a3 100644
--- a/CTFd/schemas/pages.py
+++ b/CTFd/schemas/pages.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError, pre_load
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Pages
+from CTFd.utils import string_types
 
 
 class PageSchema(ma.ModelSchema):
@@ -19,9 +20,9 @@ class PageSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(PageSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/submissions.py b/CTFd/schemas/submissions.py
index 7e0a391e..5a482056 100644
--- a/CTFd/schemas/submissions.py
+++ b/CTFd/schemas/submissions.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load, validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.schemas.challenges import ChallengeSchema
 from CTFd.models import ma, Submissions
+from CTFd.utils import string_types
 
 
 class SubmissionSchema(ma.ModelSchema):
@@ -38,9 +39,9 @@ class SubmissionSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(SubmissionSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/tags.py b/CTFd/schemas/tags.py
index 8ce17706..94fac88d 100644
--- a/CTFd/schemas/tags.py
+++ b/CTFd/schemas/tags.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Tags
+from CTFd.utils import string_types
 
 
 class TagSchema(ma.ModelSchema):
@@ -24,9 +25,9 @@ class TagSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(TagSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/teams.py b/CTFd/schemas/teams.py
index 5aaed8e0..c4cb0e71 100644
--- a/CTFd/schemas/teams.py
+++ b/CTFd/schemas/teams.py
@@ -9,6 +9,7 @@ from CTFd.utils.user import is_admin, get_current_team
 from CTFd.utils.countries import lookup_country_code
 from CTFd.utils.user import is_admin, get_current_team
 from CTFd.utils.crypto import verify_password, hash_password
+from CTFd.utils import string_types
 
 
 class TeamSchema(ma.ModelSchema):
@@ -162,9 +163,9 @@ class TeamSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(TeamSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/unlocks.py b/CTFd/schemas/unlocks.py
index 4faeba7b..9b540509 100644
--- a/CTFd/schemas/unlocks.py
+++ b/CTFd/schemas/unlocks.py
@@ -3,6 +3,7 @@ from marshmallow import fields, post_load
 from marshmallow import validate, ValidationError
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Unlocks
+from CTFd.utils import string_types
 
 
 class UnlockSchema(ma.ModelSchema):
@@ -30,9 +31,9 @@ class UnlockSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(UnlockSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/schemas/users.py b/CTFd/schemas/users.py
index 9b71ef29..48ec97b3 100644
--- a/CTFd/schemas/users.py
+++ b/CTFd/schemas/users.py
@@ -11,6 +11,7 @@ from CTFd.utils.user import is_admin, get_current_user
 from CTFd.utils.countries import lookup_country_code
 from CTFd.utils.crypto import verify_password, hash_password
 from CTFd.utils.email import check_email_is_whitelisted
+from CTFd.utils import string_types
 
 
 class UserSchema(ma.ModelSchema):
@@ -182,9 +183,9 @@ class UserSchema(ma.ModelSchema):
 
     def __init__(self, view=None, *args, **kwargs):
         if view:
-            if type(view) == str:
+            if isinstance(view, string_types):
                 kwargs['only'] = self.views[view]
-            elif type(view) == list:
+            elif isinstance(view, list):
                 kwargs['only'] = view
 
         super(UserSchema, self).__init__(*args, **kwargs)
diff --git a/CTFd/utils/config/__init__.py b/CTFd/utils/config/__init__.py
index c98cd68f..3bf594c9 100644
--- a/CTFd/utils/config/__init__.py
+++ b/CTFd/utils/config/__init__.py
@@ -3,6 +3,7 @@ from CTFd.models import Configs, Users, Teams
 from CTFd.cache import cache
 from CTFd.utils import get_config
 from CTFd.utils.user import authed
+from CTFd.utils.modes import USERS_MODE, TEAMS_MODE
 import time
 import os
 
@@ -16,6 +17,14 @@ def user_mode():
     return get_config('user_mode')
 
 
+def is_users_mode():
+    return user_mode() == USERS_MODE
+
+
+def is_teams_mode():
+    return user_mode() == TEAMS_MODE
+
+
 def ctf_logo():
     return get_config('ctf_logo')
 


Test patch:
diff --git a/tests/api/v1/test_teams.py b/tests/api/v1/test_teams.py
index 7de02cd9..41c8dc10 100644
--- a/tests/api/v1/test_teams.py
+++ b/tests/api/v1/test_teams.py
@@ -480,3 +480,37 @@ def test_api_accessing_hidden_banned_users():
             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)
+
+
+def test_api_user_without_team_challenge_interaction():
+    """Can a user interact with challenges without having joined a team?"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        register_user(app)
+        gen_challenge(app.db)
+        gen_flag(app.db, 1)
+
+        with login_as_user(app) as client:
+            assert client.get('/api/v1/challenges').status_code == 403
+            assert client.get('/api/v1/challenges/1').status_code == 403
+            assert client.post('/api/v1/challenges/attempt', json={
+                "challenge_id": 1,
+                "submission": "wrong_flag"
+            }).status_code == 403
+
+        # Create a user with a team
+        user = gen_user(app.db, email='user_name@ctfd.io')
+        team = gen_team(app.db)
+        team.members.append(user)
+        user.team_id = team.id
+        app.db.session.commit()
+
+        # Test if user with team can interact with challenges
+        with login_as_user(app, name="user_name") as client:
+            assert client.get('/api/v1/challenges').status_code == 200
+            assert client.get('/api/v1/challenges/1').status_code == 200
+            assert client.post('/api/v1/challenges/attempt', json={
+                "challenge_id": 1,
+                "submission": "flag"
+            }).status_code == 200
+    destroy_ctfd(app)
diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py
index 3d7a8676..16e4427b 100644
--- a/tests/api/v1/test_users.py
+++ b/tests/api/v1/test_users.py
@@ -3,6 +3,7 @@
 
 from CTFd.utils import set_config
 from CTFd.utils.crypto import verify_password
+from CTFd.schemas.users import UserSchema
 from tests.helpers import *
 
 
@@ -674,3 +675,22 @@ def test_api_user_send_email():
             assert r.status_code == 200
 
     destroy_ctfd(app)
+
+
+def test_api_user_get_schema():
+    """Can a user get /api/v1/users/<user_id> doesn't return unnecessary data"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app, name="user1", email="user1@ctfd.io")  # ID 2
+        register_user(app, name="user2", email="user2@ctfd.io")  # ID 3
+
+        with app.test_client() as client:
+            r = client.get('/api/v1/users/3')
+            data = r.get_json()['data']
+            assert sorted(data.keys()) == sorted(UserSchema.views['user'] + ['score', 'place'])
+
+        with login_as_user(app, name="user1") as client:
+            r = client.get('/api/v1/users/3')
+            data = r.get_json()['data']
+            assert sorted(data.keys()) == sorted(UserSchema.views['user'] + ['score', 'place'])
+    destroy_ctfd(app)