Fix snag that happens when PUTting a user document without roles key.

pillar/api/users/hooks.py

@@ -26,6 +26,8 @@ def before_replacing_user(request, lookup):
 
     # Make sure that the replacement has a valid auth field.
     put_data = request.get_json()
+    if put_data is None:
+        raise wz_exceptions.BadRequest('No JSON data received')
 
     # We should get a ref to the cached JSON, and not a copy. This will allow us to
     # modify the cached JSON so that Eve sees our modifications.
@@ -57,7 +59,7 @@ def before_replacing_user(request, lookup):
             del put_data[db_key]
 
     # Regular users should always have an email address
-    if 'service' not in put_data['roles']:
+    if 'service' not in put_data.get('roles', ()):
         if not put_data.get('email'):
             raise wz_exceptions.UnprocessableEntity('email field must be given')
 

pillar/tests/__init__.py

@@ -256,22 +256,22 @@ class AbstractPillarTest(TestMinimal):
             users = self.app.data.driver.db['users']
             assert isinstance(users, pymongo.collection.Collection)
 
-            result = users.insert_one({
+            user = {'_id': ObjectId(user_id),
-                '_id': ObjectId(user_id),
+                    '_updated': datetime.datetime(2016, 4, 15, 13, 15, 11, tzinfo=tz_util.utc),
-                '_updated': datetime.datetime(2016, 4, 15, 13, 15, 11, tzinfo=tz_util.utc),
+                    '_created': datetime.datetime(2016, 4, 15, 13, 15, 11, tzinfo=tz_util.utc),
-                '_created': datetime.datetime(2016, 4, 15, 13, 15, 11, tzinfo=tz_util.utc),
+                    '_etag': 'unittest-%s' % uuid.uuid4().hex,
-                '_etag': 'unittest-%s' % uuid.uuid4().hex,
+                    'username': make_unique_username('tester'),
-                'username': make_unique_username('tester'),
+                    'groups': groups or [],
-                'groups': groups or [],
+                    'settings': {'email_communications': 1},
-                'roles': list(roles),
+                    'auth': [{'token': '',
-                'settings': {'email_communications': 1},
+                              'user_id': str(ctd.BLENDER_ID_TEST_USERID),
-                'auth': [{'token': '',
+                              'provider': 'blender-id'}],
-                          'user_id': str(ctd.BLENDER_ID_TEST_USERID),
+                    'full_name': 'คนรักของผัดไทย',
-                          'provider': 'blender-id'}],
+                    'email': email}
-                'full_name': 'คนรักของผัดไทย',
+            if roles:
-                'email': email
+                user['roles'] = list(roles)
-            })
 
+            result = users.insert_one(user)
             user_id = result.inserted_id
 
         if token:

tests/test_api/test_users.py

@@ -86,3 +86,19 @@ class UsersTest(AbstractPillarTest):
 
         db_user = self.fetch_user_from_db(user_id)
         self.assertEqual([], db_user['groups'])
+
+    def test_replace_user_without_roles(self):
+        from pillar.api.utils import remove_private_keys
+
+        self.enter_app_context()
+
+        user_id = bson.ObjectId(24 * '1')
+        self.create_user(user_id, roles=(), token='token')
+
+        user_doc = self.get(f'/api/users/{user_id}', auth_token='token').get_json()
+        self.assertNotIn('roles', user_doc)
+
+        self.put(f'/api/users/{user_id}',
+                 auth_token='token',
+                 json=remove_private_keys(user_doc),
+                 etag=user_doc['_etag'])