Repository: CTFd/CTFd
Pull request #777 (2018-12-03 11:11:18)

Fix admin cannot modify verified status in Edit User
The Edit User page has a form field to modify the user's Verified, Hidden and Banned status. Both the Hidden and Banned status can be modified by an admin, however the Verified status cannot be modified. This appears to be a bug.

![Edit User](https://user-images.githubusercontent.com/31173491/49369815-e8b72480-f724-11e8-8367-6a3c3119ebd8.png)
  
There are two potential solutions for this bug:
- Remove the Verified field from the form
- Allow admin to modify `verified` field in `UserSchema`

In this pull request I propose that admins should be allowed access to modify the `verified` field in `UserSchema`. 

This change has two advantages:
- It makes the existing Edit User form work as intended. 
- It allows admins to manually verify users, which enables contests to employ user verification without setting up a mail server.




codecov[bot]: # [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/777?src=pr&el=h1) Report
> Merging [#777](https://codecov.io/gh/CTFd/CTFd/pull/777?src=pr&el=desc) into [master](https://codecov.io/gh/CTFd/CTFd/commit/809e4df4719fa63841ff3f28175f9a09da31963a?src=pr&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master     #777      +/-   ##
==========================================
- Coverage   83.79%   83.61%   -0.19%     
==========================================
  Files          91       91              
  Lines        5160     4503     -657     
==========================================
- Hits         4324     3765     -559     
+ Misses        836      738      -98
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/777?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/schemas/users.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3VzZXJzLnB5) | `86.11% <ø> (-2.78%)` | :arrow_down: |
| [CTFd/schemas/teams.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC9zY2hlbWFzL3RlYW1zLnB5) | `75% <0%> (-6.06%)` | :arrow_down: |
| [CTFd/utils/config/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy9jb25maWcvX19pbml0X18ucHk=) | `82.69% <0%> (-5.15%)` | :arrow_down: |
| [CTFd/utils/exports/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy9leHBvcnRzL19faW5pdF9fLnB5) | `79.83% <0%> (-5.01%)` | :arrow_down: |
| [CTFd/utils/uploads/uploaders.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy91cGxvYWRzL3VwbG9hZGVycy5weQ==) | `52.74% <0%> (-3.86%)` | :arrow_down: |
| [CTFd/auth.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC9hdXRoLnB5) | `65.04% <0%> (-2.71%)` | :arrow_down: |
| [CTFd/utils/dates/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy9kYXRlcy9fX2luaXRfXy5weQ==) | `87.5% <0%> (-2.5%)` | :arrow_down: |
| [CTFd/models/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC9tb2RlbHMvX19pbml0X18ucHk=) | `87.85% <0%> (-1.99%)` | :arrow_down: |
| [CTFd/admin/teams.py](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree#diff-Q1RGZC9hZG1pbi90ZWFtcy5weQ==) | `46.15% <0%> (-1.61%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/CTFd/CTFd/pull/777/diff?src=pr&el=tree-more) | |

------

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

ramadistra: Hi @ColdHeat , I've modified the admin patch user test to exercise the new functionality.

Patch:
diff --git a/.codecov.yml b/.codecov.yml
new file mode 100644
index 00000000..35dff8bb
--- /dev/null
+++ b/.codecov.yml
@@ -0,0 +1,9 @@
+coverage:
+  status:
+    project:
+      default:
+        # Fail the status if coverage drops by >= 1%
+        threshold: 1
+    patch:
+      default:
+        threshold: 1
diff --git a/CTFd/schemas/users.py b/CTFd/schemas/users.py
index bddaf1fb..8a22eb59 100644
--- a/CTFd/schemas/users.py
+++ b/CTFd/schemas/users.py
@@ -152,7 +152,8 @@ class UserSchema(ma.ModelSchema):
             'id',
             'oauth_id',
             'password',
-            'type'
+            'type',
+            'verified'
         ]
     }
 


Test patch:
diff --git a/tests/api/v1/test_users.py b/tests/api/v1/test_users.py
index 1691b893..b17cfde6 100644
--- a/tests/api/v1/test_users.py
+++ b/tests/api/v1/test_users.py
@@ -92,6 +92,32 @@ def test_api_users_post_admin():
     destroy_ctfd(app)
 
 
+def test_api_users_post_admin_with_attributes():
+    """Can a user post /api/v1/users with user settings"""
+    app = create_ctfd()
+    with app.app_context():
+        with login_as_user(app, 'admin') as client:
+            # Create user
+            r = client.post('/api/v1/users', json={
+                "name": "user",
+                "email": "user@user.com",
+                "password": "password",
+                "banned": True,
+                "hidden": True,
+                "verified": True
+            })
+            assert r.status_code == 200
+
+            # Make sure password was hashed properly
+            user = Users.query.filter_by(email='user@user.com').first()
+            assert user
+            assert verify_password('password', user.password)
+            assert user.banned
+            assert user.hidden
+            assert user.verified
+    destroy_ctfd(app)
+
+
 def test_api_team_get_public():
     """Can a user get /api/v1/team/<user_id> if users are public"""
     app = create_ctfd()
@@ -168,10 +194,13 @@ def test_api_user_patch_admin():
                 "name": "user",
                 "email": "user@ctfd.io",
                 "password": "password",
-                "country": "US"
+                "country": "US",
+                "verified": True
             })
             assert r.status_code == 200
-            assert r.get_json()['data'][0]['country'] == 'US'
+            user_data = r.get_json()['data'][0]
+            assert user_data['country'] == 'US'
+            assert user_data['verified'] is True
     destroy_ctfd(app)