Consider account configs when user patches their own account (#836)

* Consider account configs when user patches their own account
* Add test for name changing
* Add test to ensure that users changing emails are marked unconfirmed
* Only allow users to change to emails in whitelisted domains
* Simplify assertion for error check

CTFd/auth.py

@@ -140,18 +140,13 @@ def register():
         valid_email = validators.validate_email(request.form['email'])
         team_name_email_check = validators.validate_email(name)
 
-        local_id, _, domain = email_address.partition('@')
-
-        domain_whitelist = get_config('domain_whitelist')
-
         if not valid_email:
             errors.append("Please enter a valid email address")
-        if domain_whitelist:
-            domain_whitelist = [d.strip() for d in domain_whitelist.split(',')]
-            if domain not in domain_whitelist:
+        if email.check_email_is_whitelisted(email_address) is False:
                 errors.append(
                     "Only email addresses under {domains} may register".format(
-                        domains=', '.join(domain_whitelist))
+                        domains=get_config('domain_whitelist')
+                    )
                 )
         if names:
             errors.append('That team name is already taken')

CTFd/cache/__init__.py

@@ -4,8 +4,8 @@ cache = Cache()
 
 
 def clear_config():
-    from CTFd.utils import get_config, get_app_config
-    cache.delete_memoized(get_config)
+    from CTFd.utils import _get_config, get_app_config
+    cache.delete_memoized(_get_config)
     cache.delete_memoized(get_app_config)
 
 

CTFd/schemas/teams.py

@@ -4,6 +4,7 @@ from marshmallow import validate, ValidationError, pre_load
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Teams
 from CTFd.utils.validators import validate_country_code
+from CTFd.utils import get_config
 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
@@ -69,6 +70,10 @@ class TeamSchema(ma.ModelSchema):
             if data['name'] == current_team.name:
                 return data
             else:
+                name_changes = get_config('name_changes', default=True)
+                if bool(name_changes) is False:
+                    raise ValidationError('Name changes are disabled', field_names=['name'])
+
                 if existing_team:
                     raise ValidationError('Team name has already been taken', field_names=['name'])
 

CTFd/schemas/users.py

@@ -5,10 +5,12 @@ from marshmallow import validate, ValidationError, pre_load
 from marshmallow.decorators import validates_schema
 from marshmallow_sqlalchemy import field_for
 from CTFd.models import ma, Users
+from CTFd.utils import get_config
 from CTFd.utils.validators import unique_email, validate_country_code
 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
 
 
 class UserSchema(ma.ModelSchema):
@@ -23,7 +25,7 @@ class UserSchema(ma.ModelSchema):
         'name',
         required=True,
         validate=[
-            validate.Length(min=1, max=128, error='Team names must not be empty')
+            validate.Length(min=1, max=128, error='User names must not be empty')
         ]
     )
     email = field_for(
@@ -74,6 +76,9 @@ class UserSchema(ma.ModelSchema):
             if name == current_user.name:
                 return data
             else:
+                name_changes = get_config('name_changes', default=True)
+                if bool(name_changes) is False:
+                    raise ValidationError('Name changes are disabled', field_names=['name'])
                 if existing_user:
                     raise ValidationError('User name has already been taken', field_names=['name'])
 
@@ -96,6 +101,15 @@ class UserSchema(ma.ModelSchema):
             else:
                 if existing_user:
                     raise ValidationError('Email address has already been used', field_names=['email'])
+                if check_email_is_whitelisted(email) is False:
+                    raise ValidationError(
+                        "Only email addresses under {domains} may register".format(
+                            domains=get_config('domain_whitelist')
+                        ),
+                        field_names=['email']
+                    )
+                if get_config('verify_emails'):
+                    current_user.verified = False
 
     @pre_load
     def validate_password_confirmation(self, data):

CTFd/utils/__init__.py

@@ -30,14 +30,13 @@ else:
 markdown = mistune.Markdown()
 
 
-@cache.memoize()
-def get_app_config(key):
-    value = app.config.get(key)
+def get_app_config(key, default=None):
+    value = app.config.get(key, default)
     return value
 
 
 @cache.memoize()
-def get_config(key):
+def _get_config(key):
     config = Configs.query.filter_by(key=key).first()
     if config and config.value:
         value = config.value
@@ -52,6 +51,14 @@ def get_config(key):
                 return value
 
 
+def get_config(key, default=None):
+    value = _get_config(key)
+    if value is None:
+        return default
+    else:
+        return value
+
+
 def set_config(key, value):
     config = Configs.query.filter_by(key=key).first()
     if config:
@@ -60,5 +67,5 @@ def set_config(key, value):
         config = Configs(key=key, value=value)
         db.session.add(config)
     db.session.commit()
-    cache.delete_memoized(get_config, key)
+    cache.delete_memoized(_get_config, key)
     return config

CTFd/utils/email/__init__.py

@@ -41,3 +41,13 @@ def verify_email_address(addr):
 
 def check_email_format(email):
     return bool(re.match(EMAIL_REGEX, email))
+
+
+def check_email_is_whitelisted(email_address):
+    local_id, _, domain = email_address.partition('@')
+    domain_whitelist = get_config('domain_whitelist')
+    if domain_whitelist:
+        domain_whitelist = [d.strip() for d in domain_whitelist.split(',')]
+        if domain not in domain_whitelist:
+            return False
+    return True

tests/api/v1/test_users.py

@@ -264,16 +264,121 @@ def test_api_user_patch_me_logged_in():
     with app.app_context():
         register_user(app)
         with login_as_user(app) as client:
-            r = client.patch('/api/v1/users/me', json={"name": "user",
-                                                       "email": "user@ctfd.io",
-                                                       "password": "password",
-                                                       "confirm": "password",
-                                                       "country": "US"})
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "name": "user",
+                    "email": "user@ctfd.io",
+                    "password": "password",
+                    "confirm": "password",
+                    "country": "US"
+                }
+            )
             assert r.status_code == 200
             assert r.get_json()['data']['country'] == 'US'
     destroy_ctfd(app)
 
 
+def test_api_user_change_name():
+    """Can a user change their name via the API"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app)
+        with login_as_user(app) as client:
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "name": "user2",
+                }
+            )
+            assert r.status_code == 200
+            resp = r.get_json()
+            assert resp['data']['name'] == 'user2'
+            assert resp['success'] is True
+
+            set_config('name_changes', False)
+
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "name": "new_name",
+                }
+            )
+            assert r.status_code == 400
+            resp = r.get_json()
+            assert 'name' in resp['errors']
+            assert resp['success'] is False
+
+            set_config('name_changes', True)
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "name": "new_name",
+                }
+            )
+            assert r.status_code == 200
+            resp = r.get_json()
+            assert resp['data']['name'] == 'new_name'
+            assert resp['success'] is True
+    destroy_ctfd(app)
+
+
+def test_api_user_change_verify_email():
+    """Test that users are marked unconfirmed if they change their email and verify_emails is turned on"""
+    app = create_ctfd()
+    with app.app_context():
+        set_config('verify_emails', True)
+        register_user(app)
+        user = Users.query.filter_by(id=2).first()
+        user.verified = True
+        app.db.session.commit()
+        with login_as_user(app) as client:
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "email": "new_email@email.com",
+                }
+            )
+            assert r.status_code == 200
+            resp = r.get_json()
+            assert resp['data']['email'] == "new_email@email.com"
+            assert resp['success'] is True
+            user = Users.query.filter_by(id=2).first()
+            assert user.verified is False
+    destroy_ctfd(app)
+
+
+def test_api_user_change_email_under_whitelist():
+    """Test that users can only change emails to ones in the whitelist"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app)
+        set_config('domain_whitelist', 'whitelisted.com, whitelisted.org, whitelisted.net')
+        with login_as_user(app) as client:
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "email": "new_email@email.com",
+                }
+            )
+            assert r.status_code == 400
+            resp = r.get_json()
+            assert resp['errors']['email']
+            assert resp['success'] is False
+
+            r = client.patch(
+                '/api/v1/users/me',
+                json={
+                    "email": "new_email@whitelisted.com",
+                }
+            )
+            assert r.status_code == 200
+            resp = r.get_json()
+            assert resp['data']['email'] == "new_email@whitelisted.com"
+            assert resp['success'] is True
+    destroy_ctfd(app)
+
+
 def test_api_user_get_me_solves_not_logged_in():
     """Can a user get /api/v1/users/me/solves if not logged in"""
     app = create_ctfd()