Repository: CTFd/CTFd
Pull request #2628 (2024-10-10 15:24:39)

Allow users & teams to set their password if there is no password set
* Allow users & teams to set their password if there is no password set


Patch:
diff --git a/CTFd/schemas/teams.py b/CTFd/schemas/teams.py
index 14dc8b38..fe53db89 100644
--- a/CTFd/schemas/teams.py
+++ b/CTFd/schemas/teams.py
@@ -144,6 +144,9 @@ class TeamSchema(ma.ModelSchema):
                     field_names=["captain_id"],
                 )
 
+            if current_team.password is None:
+                return
+
             if password and (bool(confirm) is False):
                 raise ValidationError(
                     "Please confirm your current password", field_names=["confirm"]
diff --git a/CTFd/schemas/users.py b/CTFd/schemas/users.py
index cd9fdd6a..bb095145 100644
--- a/CTFd/schemas/users.py
+++ b/CTFd/schemas/users.py
@@ -167,17 +167,15 @@ class UserSchema(ma.ModelSchema):
         if is_admin():
             pass
         else:
+            # If the user has no password set, allow them to set their password
+            if target_user.password is None:
+                return
+
             if password and (bool(confirm) is False):
                 raise ValidationError(
                     "Please confirm your current password", field_names=["confirm"]
                 )
 
-            if target_user.password is None:
-                # Prevent password from being set but allow other data through
-                data.pop("password", None)
-                data.pop("confirm", None)
-                return
-
             if password and confirm:
                 test = verify_password(
                     plaintext=confirm, ciphertext=target_user.password


Test patch:
diff --git a/tests/api/v1/teams/test_teams.py b/tests/api/v1/teams/test_teams.py
index fe9e2614..411f446a 100644
--- a/tests/api/v1/teams/test_teams.py
+++ b/tests/api/v1/teams/test_teams.py
@@ -1,6 +1,8 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
+from CTFd.models import Teams, Users, db
+from CTFd.utils.crypto import verify_password
 from tests.helpers import (
     create_ctfd,
     destroy_ctfd,
@@ -26,3 +28,65 @@ def test_api_can_query_by_team_emails():
             assert r.get_json()["data"][0]["id"] == 1
             assert r.get_json()["data"][0]["name"] == "team_name"
     destroy_ctfd(app)
+
+
+def test_api_team_can_update_password_if_none_not_if_set():
+    app = create_ctfd(user_mode="teams")
+    with app.app_context():
+        # Create a user with a null password. Use raw SQL to bypass SQLAlchemy validates
+        gen_team(app.db, name="testteam", email="team@examplectf.com")
+        db.session.execute("UPDATE teams SET password=NULL WHERE name='testteam'")
+        team = Teams.query.filter_by(id=1).first()
+        db.session.commit()
+        assert team.password is None
+
+        # Login and test that we are authed
+        captain = Users.query.filter_by(id=2).first()
+        normal_user = Users.query.filter_by(id=3).first()
+        with login_as_user(app, captain.name) as client:
+            r = client.get("/api/v1/teams/me", json=True)
+            assert r.get_json()["data"]["id"] == team.id
+            assert r.status_code == 200
+
+            # Patch the team's password from NULL
+            team = Teams.query.filter_by(name="testteam").first()
+            assert team.password is None
+            data = {"password": "12345", "confirm": "password"}
+            r = client.patch("/api/v1/teams/me", json=data)
+            assert r.status_code == 200
+
+            # Verify password is now set
+            team = Teams.query.filter_by(name="testteam").first()
+            assert verify_password(plaintext="12345", ciphertext=team.password)
+
+            # Verify that password cannot be changed without valid password
+            data = {"password": "noset", "confirm": "noset"}
+            r = client.patch("/api/v1/teams/me", json=data)
+            resp = r.get_json()
+            assert resp["errors"]["confirm"] == ["Your previous password is incorrect"]
+            assert r.status_code == 400
+
+            # Verify that a normal user cannot change the team password
+            with login_as_user(app, normal_user.name) as client:
+                # Try changing the password for the team
+                data = {"password": "newpassword", "confirm": "12345"}
+                r = client.patch("/api/v1/teams/me", json=data)
+                assert r.status_code == 403
+
+                # Verify that team password has not changed
+                team = Teams.query.filter_by(name="testteam").first()
+                assert verify_password(plaintext="12345", ciphertext=team.password)
+
+            # Create a new team
+            new_team = gen_team(app.db, name="newteam", email="newteam@examplectf.com")
+            new_captain = Users.query.filter_by(id=new_team.captain_id).first()
+
+            # Verify that the captain from the new team cannot change the password of the original team
+            with login_as_user(app, new_captain.name) as client:
+                data = {"password": "newpassword", "confirm": "12345"}
+                r = client.patch("/api/v1/teams/1", json=data)
+                assert r.status_code == 403
+
+                # Verify that old test team password has not changed
+                team = Teams.query.filter_by(name="testteam").first()
+                assert verify_password(plaintext="12345", ciphertext=team.password)
diff --git a/tests/api/v1/users/test_users.py b/tests/api/v1/users/test_users.py
index bbbe4279..708000bc 100644
--- a/tests/api/v1/users/test_users.py
+++ b/tests/api/v1/users/test_users.py
@@ -1,6 +1,8 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
+from CTFd.models import Users, db
+from CTFd.utils.crypto import verify_password
 from tests.helpers import create_ctfd, destroy_ctfd, login_as_user, register_user
 
 
@@ -51,3 +53,61 @@ def test_api_can_query_by_user_emails():
             assert r.status_code == 200
             assert r.get_json()["data"][0]["id"] == 2
     destroy_ctfd(app)
+
+
+def test_api_user_can_update_password_if_none_not_if_set():
+    """Can a user set their password if they do not currently have a password"""
+    app = create_ctfd()
+    with app.app_context():
+        # Create a user with a null password. Use raw SQL to bypass SQLAlchemy validates
+        register_user(app, name="testuser", email="user@examplectf.com")
+        db.session.execute("UPDATE users SET password=NULL WHERE name='testuser'")
+        user = Users.query.filter_by(name="testuser").first()
+        db.session.commit()
+        assert user.password is None
+
+        with app.test_client() as client:
+            # Login as user
+            with client.session_transaction() as sess:
+                sess["id"] = user.id
+            r = client.get("/api/v1/users/me", json=True)
+            assert r.status_code == 200
+
+            # Test that user can change password
+            user = Users.query.filter_by(name="testuser").first()
+            assert user.password is None
+            data = {"password": "12345", "confirm": "password"}
+            r = client.patch("/api/v1/users/me", json=data)
+            assert r.status_code == 200
+
+            # Verify password is now set
+            user = Users.query.filter_by(name="testuser").first()
+            assert verify_password(plaintext="12345", ciphertext=user.password)
+
+            # Verify that password cannot be changed
+            data = {"password": "noset", "confirm": "password"}
+            r = client.patch("/api/v1/users/me", json=data)
+            resp = r.get_json()
+            assert resp["errors"]["confirm"] == ["Your previous password is incorrect"]
+            assert r.status_code == 400
+
+            # Verify a regular user cannot patch another user
+            register_user(
+                app,
+                name="testuser2",
+                email="user2@examplectf.com",
+                password="testinguser",
+            )
+            testuser = Users.query.filter_by(name="testuser2").first()
+            assert verify_password(
+                plaintext="testinguser", ciphertext=testuser.password
+            )
+            data = {"password": "password", "confirm": "password"}
+            r = client.patch("/api/v1/users/3", json=data)
+            assert r.status_code == 403
+            testuser = Users.query.filter_by(name="testuser2").first()
+            assert verify_password(
+                plaintext="testinguser", ciphertext=testuser.password
+            )
+
+    destroy_ctfd(app)