Improved Team Handling (#1713)

* Prevent team joining while already on a team
* Return 403 instead of 200 for team join/create errors
* Allow team captains whose teams haven't done anything to disband their team
* Closes #1588

CTFd/api/v1/teams.py

@@ -306,6 +306,62 @@ class TeamPrivate(Resource):
 
         return {"success": True, "data": response.data}
 
+    @authed_only
+    @require_team
+    @teams_namespace.doc(
+        description="Endpoint to disband your current team. Can only be used if the team has performed no actions in the CTF.",
+        responses={200: ("Success", "APISimpleSuccessResponse")},
+    )
+    def delete(self):
+        team = get_current_team()
+        if team.captain_id != session["id"]:
+            return (
+                {
+                    "success": False,
+                    "errors": {"": ["Only team captains can disband their team"]},
+                },
+                403,
+            )
+
+        # The team must not have performed any actions in the CTF
+        performed_actions = any(
+            [
+                team.solves != [],
+                team.fails != [],
+                team.awards != [],
+                Submissions.query.filter_by(team_id=team.id).all() != [],
+                Unlocks.query.filter_by(team_id=team.id).all() != [],
+            ]
+        )
+
+        if performed_actions:
+            return (
+                {
+                    "success": False,
+                    "errors": {
+                        "": [
+                            "You cannot disband your team as it has participated in the event. "
+                            "Please contact an admin to disband your team or remove a member."
+                        ]
+                    },
+                },
+                403,
+            )
+
+        for member in team.members:
+            member.team_id = None
+            clear_user_session(user_id=member.id)
+
+        db.session.delete(team)
+        db.session.commit()
+
+        clear_team_session(team_id=team.id)
+        clear_standings()
+
+        db.session.close()
+
+        return {"success": True}
+
 
 @teams_namespace.route("/<team_id>/members")
 @teams_namespace.param("team_id", "Team ID")

CTFd/teams.py

@@ -11,7 +11,7 @@ from CTFd.utils.decorators.visibility import (
     check_score_visibility,
 )
 from CTFd.utils.helpers import get_errors, get_infos
-from CTFd.utils.user import get_current_user
+from CTFd.utils.user import get_current_user, get_current_user_attrs
 
 teams = Blueprint("teams", __name__)
 
@@ -57,6 +57,11 @@ def listing():
 def join():
     infos = get_infos()
     errors = get_errors()
+
+    user = get_current_user_attrs()
+    if user.team_id:
+        errors.append("You are already in a team. You cannot join another.")
+
     if request.method == "GET":
         team_size_limit = get_config("team_size", default=0)
         if team_size_limit:
@@ -74,6 +79,12 @@ def join():
 
         team = Teams.query.filter_by(name=teamname).first()
 
+        if errors:
+            return (
+                render_template("teams/join_team.html", infos=infos, errors=errors),
+                403,
+            )
+
         if team and verify_password(passphrase, team.password):
             team_size_limit = get_config("team_size", default=0)
             if team_size_limit and len(team.members) >= team_size_limit:
@@ -109,6 +120,11 @@ def join():
 def new():
     infos = get_infos()
     errors = get_errors()
+
+    user = get_current_user_attrs()
+    if user.team_id:
+        errors.append("You are already in a team. You cannot join another.")
+
     if request.method == "GET":
         team_size_limit = get_config("team_size", default=0)
         if team_size_limit:
@@ -118,12 +134,11 @@ def new():
                     limit=team_size_limit, plural=plural
                 )
             )
-
         return render_template("teams/new_team.html", infos=infos, errors=errors)
+
     elif request.method == "POST":
         teamname = request.form.get("name", "").strip()
         passphrase = request.form.get("password", "").strip()
-        errors = get_errors()
 
         website = request.form.get("website")
         affiliation = request.form.get("affiliation")
@@ -177,7 +192,7 @@ def new():
             errors.append("Please provide a shorter affiliation")
 
         if errors:
-            return render_template("teams/new_team.html", errors=errors)
+            return render_template("teams/new_team.html", errors=errors), 403
 
         team = Teams(name=teamname, password=passphrase, captain_id=user.id)
 

CTFd/themes/core/assets/js/pages/teams/private.js

@@ -3,7 +3,7 @@ import "../../utils";
 import CTFd from "../../CTFd";
 import "bootstrap/js/dist/modal";
 import $ from "jquery";
-import { ezBadge } from "../../ezq";
+import { ezBadge, ezQuery, ezAlert } from "../../ezq";
 
 $(() => {
   if (window.team_captain) {
@@ -14,6 +14,32 @@ $(() => {
     $(".edit-captain").click(function() {
       $("#team-captain-modal").modal();
     });
+
+    $(".disband-team").click(function() {
+      ezQuery({
+        title: "Disband Team",
+        body: "Are you sure you want to disband your team?",
+        success: function() {
+          CTFd.fetch("/api/v1/teams/me", {
+            method: "DELETE"
+          })
+            .then(function(response) {
+              return response.json();
+            })
+            .then(function(response) {
+              if (response.success) {
+                window.location.reload();
+              } else {
+                ezAlert({
+                  title: "Error",
+                  body: response.errors[""].join(" "),
+                  button: "Got it!"
+                });
+              }
+            });
+        }
+      });
+    });
   }
 
   let form = $("#team-info-form");

CTFd/themes/core/templates/teams/private.html

@@ -160,6 +160,11 @@
 							<i class="btn-fa fas fa-user-tag fa-2x px-2" data-toggle="tooltip" data-placement="top"
 							   title="Choose Captain"></i>
 						</a>
+
+						<a class="disband-team">
+							<i class="btn-fa fas fa-trash-alt fa-2x px-2" data-toggle="tooltip" data-placement="top"
+								title="Disband Team"></i>
+						</a>
 					{% else %}
 						<i class="btn-fa fas fa-cogs fa-2x px-2 fa-disabled"
 						   data-toggle="tooltip"
@@ -171,6 +176,12 @@
 							   data-placement="top"
 							   title="Only team captains can choose a new captain"></i>
 						</a>
+						<a class="disband-team">
+							<i class="btn-fa fas fa-trash-alt fa-2x px-2 fa-disabled"
+								data-toggle="tooltip"
+								data-placement="top"
+								title="Only team captains can disband the team"></i>
+						</a>
 					{% endif %}
 				</a>
 				{% if team.website and (team.website.startswith('http://') or team.website.startswith('https://')) %}

tests/api/v1/test_teams.py

@@ -695,6 +695,79 @@ def test_api_team_patch_password():
             )
 
 
+def test_api_team_captain_disbanding():
+    """Test that only team captains can disband teams"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        user = gen_user(app.db, name="user")
+        team = gen_team(app.db)
+        team.members.append(user)
+        user.team_id = team.id
+        team.captain_id = 2
+        user2 = gen_user(app.db, name="user2", email="user2@ctfd.io")
+        team.members.append(user2)
+        app.db.session.commit()
+        with login_as_user(app, name="user2") as client:
+            r = client.delete("/api/v1/teams/me", json="")
+            assert r.status_code == 403
+            assert r.get_json() == {
+                "success": False,
+                "errors": {"": ["Only team captains can disband their team"]},
+            }
+        with login_as_user(app) as client:
+            r = client.delete("/api/v1/teams/me", json="")
+            assert r.status_code == 200
+            assert r.get_json() == {
+                "success": True,
+            }
+    destroy_ctfd(app)
+
+
+def test_api_team_captain_disbanding_only_inactive_teams():
+    """Test that only teams that haven't conducted any actions can be disbanded"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        user = gen_user(app.db, name="user")
+        team = gen_team(app.db)
+        team.members.append(user)
+        user.team_id = team.id
+        team.captain_id = 2
+        user2 = gen_user(app.db, name="user2", email="user2@ctfd.io")
+        team.members.append(user2)
+        app.db.session.commit()
+
+        gen_challenge(app.db)
+        gen_flag(app.db, 1)
+        gen_solve(app.db, user_id=3, team_id=1, challenge_id=1)
+
+        with login_as_user(app) as client:
+            r = client.delete("/api/v1/teams/me", json="")
+            assert r.status_code == 403
+            assert r.get_json() == {
+                "success": False,
+                "errors": {
+                    "": [
+                        "You cannot disband your team as it has participated in the event. "
+                        "Please contact an admin to disband your team or remove a member."
+                    ]
+                },
+            }
+
+        user = gen_user(app.db, name="user3", email="user3@ctfd.io")
+        team = gen_team(app.db, name="team2", email="team2@ctfd.io")
+        print(user.id)
+        team.members.append(user)
+        user.team_id = team.id
+        team.captain_id = user.id
+        app.db.session.commit()
+        with login_as_user(app, name="user3") as client:
+            r = client.delete("/api/v1/teams/me", json="")
+            print(r.get_json())
+            assert r.status_code == 200
+            assert r.get_json() == {"success": True}
+    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")

tests/teams/test_auth.py

@@ -58,10 +58,47 @@ def test_teams_join_post():
                 }
             r = client.post("/teams/join", data=data)
             assert r.status_code == 302
+
+            # Cannot join a team with an incorrect password
             incorrect_data = data
             incorrect_data["password"] = ""
             r = client.post("/teams/join", data=incorrect_data)
+            assert r.status_code == 403
+    destroy_ctfd(app)
+
+
+def test_teams_join_when_already_on_team():
+    """Test that a user cannot join another team"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        gen_user(app.db, name="user")
+        gen_team(app.db, email="team1@ctfd.io", name="team1")
+        gen_team(app.db, email="team2@ctfd.io", name="team2")
+        with login_as_user(app) as client:
+            r = client.get("/teams/join")
             assert r.status_code == 200
+            with client.session_transaction() as sess:
+                data = {
+                    "name": "team1",
+                    "password": "password",
+                    "nonce": sess.get("nonce"),
+                }
+            r = client.post("/teams/join", data=data)
+            assert r.status_code == 302
+
+            # Try to join another team while on a team
+            r = client.get("/teams/join")
+            assert r.status_code == 200
+            with client.session_transaction() as sess:
+                data = {
+                    "name": "team2",
+                    "password": "password",
+                    "nonce": sess.get("nonce"),
+                }
+            r = client.post("/teams/join", data=data)
+            assert r.status_code == 403
+            user = Users.query.filter_by(name="user").first()
+            assert user.team.name == "team1"
     destroy_ctfd(app)
 
 
@@ -104,3 +141,72 @@ def test_team_join_ratelimited():
                 assert r.status_code == 429
                 assert Users.query.filter_by(id=2).first().team_id is None
     destroy_ctfd(app)
+
+
+def test_teams_new_get():
+    """Can a user get /teams/new"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        register_user(app)
+        with login_as_user(app) as client:
+            r = client.get("/teams/new")
+            assert r.status_code == 200
+    destroy_ctfd(app)
+
+
+def test_teams_new_post():
+    """Can a user post /teams/new"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        gen_user(app.db, name="user")
+        with login_as_user(app) as client:
+            with client.session_transaction() as sess:
+                data = {
+                    "name": "team",
+                    "password": "password",
+                    "nonce": sess.get("nonce"),
+                }
+            r = client.post("/teams/new", data=data)
+            assert r.status_code == 302
+
+            # You can't create a team with a duplicate name
+            r = client.post("/teams/new", data=data)
+            assert r.status_code == 403
+
+            # You can't create a team with an empty name
+            incorrect_data = data
+            incorrect_data["name"] = ""
+            r = client.post("/teams/new", data=incorrect_data)
+            assert r.status_code == 403
+    destroy_ctfd(app)
+
+
+def test_teams_new_post_when_already_on_team():
+    """Test that a user cannot create a new team while on a team"""
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        gen_user(app.db, name="user")
+        with login_as_user(app) as client:
+            with client.session_transaction() as sess:
+                data = {
+                    "name": "team1",
+                    "password": "password",
+                    "nonce": sess.get("nonce"),
+                }
+            r = client.post("/teams/new", data=data)
+            assert r.status_code == 302
+
+            # Try to create another team while on a team
+            r = client.get("/teams/new")
+            assert r.status_code == 200
+            with client.session_transaction() as sess:
+                data = {
+                    "name": "team2",
+                    "password": "password",
+                    "nonce": sess.get("nonce"),
+                }
+            r = client.post("/teams/join", data=data)
+            assert r.status_code == 403
+            user = Users.query.filter_by(name="user").first()
+            assert user.team.name == "team1"
+    destroy_ctfd(app)

tests/teams/test_teams.py

@@ -116,40 +116,6 @@ def test_teams_get_user_mode():
     destroy_ctfd(app)
 
 
-def test_teams_new_get():
-    """Can a user get /teams/new"""
-    app = create_ctfd(user_mode="teams")
-    with app.app_context():
-        register_user(app)
-        with login_as_user(app) as client:
-            r = client.get("/teams/new")
-            assert r.status_code == 200
-    destroy_ctfd(app)
-
-
-def test_teams_new_post():
-    """Can a user post /teams/new"""
-    app = create_ctfd(user_mode="teams")
-    with app.app_context():
-        gen_user(app.db, name="user")
-        with login_as_user(app) as client:
-            with client.session_transaction() as sess:
-                data = {
-                    "name": "team",
-                    "password": "password",
-                    "nonce": sess.get("nonce"),
-                }
-            r = client.post("/teams/new", data=data)
-            assert r.status_code == 302
-            r = client.post("/teams/new", data=data)
-            assert r.status_code == 200
-            incorrect_data = data
-            incorrect_data["name"] = ""
-            r = client.post("/teams/new", data=incorrect_data)
-            assert r.status_code == 200
-    destroy_ctfd(app)
-
-
 def test_team_get():
     """Can a user get /team"""
     app = create_ctfd(user_mode="teams")