From fa7316722e302ebe4985e82245ae6565d138dee9 Mon Sep 17 00:00:00 2001
From: Kevin Chung <kchung@ctfd.io>
Date: Tue, 9 Feb 2021 04:03:04 -0500
Subject: [PATCH] Fix exception occuring on Admin demotion (#1799)

* Fix an exception that occurred when demoting an Admin user
* Fix the response from the above request from returning a list instead of a dict
* Closes #1794
---
 CTFd/api/v1/users.py             | 10 ++++++----
 tests/api/v1/test_users.py       |  2 +-
 tests/api/v1/users/test_users.py | 22 +++++++++++++++++++++-
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/CTFd/api/v1/users.py b/CTFd/api/v1/users.py
index deace1c1..a1c6dd6e 100644
--- a/CTFd/api/v1/users.py
+++ b/CTFd/api/v1/users.py
@@ -233,16 +233,18 @@ class UserPublic(Resource):
         if response.errors:
             return {"success": False, "errors": response.errors}, 400
 
-        db.session.commit()
-
+        # This generates the response first before actually changing the type
+        # This avoids an error during User type changes where we change
+        # the polymorphic identity resulting in an ObjectDeletedError
+        # https://github.com/CTFd/CTFd/issues/1794
         response = schema.dump(response.data)
-
+        db.session.commit()
         db.session.close()
 
         clear_user_session(user_id=user_id)
         clear_standings()
 
-        return {"success": True, "data": response}
+        return {"success": True, "data": response.data}
 
     @admins_only
     @users_namespace.doc(
diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py
index da2a8bc5..8660465b 100644
--- a/tests/api/v1/test_users.py
+++ b/tests/api/v1/test_users.py
@@ -308,7 +308,7 @@ def test_api_user_patch_admin():
                 },
             )
             assert r.status_code == 200
-            user_data = r.get_json()["data"][0]
+            user_data = r.get_json()["data"]
             assert user_data["country"] == "US"
             assert user_data["verified"] is True
     destroy_ctfd(app)
diff --git a/tests/api/v1/users/test_users.py b/tests/api/v1/users/test_users.py
index 07d1b8b3..2ca65aae 100644
--- a/tests/api/v1/users/test_users.py
+++ b/tests/api/v1/users/test_users.py
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
-from tests.helpers import create_ctfd, destroy_ctfd, login_as_user
+from tests.helpers import create_ctfd, destroy_ctfd, login_as_user, register_user
 
 
 def test_api_self_ban():
@@ -15,3 +15,23 @@ def test_api_self_ban():
             assert resp["success"] == False
             assert resp["errors"] == {"id": "You cannot ban yourself"}
     destroy_ctfd(app)
+
+
+def test_api_modify_user_type():
+    """Can a user patch /api/v1/users/<user_id> to promote a user to admin and demote them to user"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app)
+        with login_as_user(app, "admin") as client:
+            r = client.patch("/api/v1/users/2", json={"type": "admin"})
+            assert r.status_code == 200
+            user_data = r.get_json()["data"]
+            assert user_data["name"] == "user"
+            assert user_data["type"] == "admin"
+
+            r = client.patch("/api/v1/users/2", json={"type": "user"})
+            assert r.status_code == 200
+            user_data = r.get_json()["data"]
+            assert user_data["name"] == "user"
+            assert user_data["type"] == "user"
+    destroy_ctfd(app)