Merge pull request #797 from scitran/db-enforce-https

Enforce https for avatars

bin/database.py

@@ -19,7 +19,7 @@ from api.jobs.jobs import Job
 from api.jobs import gears
 from api.types import Origin
 
-CURRENT_DATABASE_VERSION = 28 # An int that is bumped when a new schema change is made
+CURRENT_DATABASE_VERSION = 29 # An int that is bumped when a new schema change is made
 
 def get_db_version():
 
@@ -960,6 +960,7 @@ def upgrade_to_26_closure(job):
     if gear.get('gear', {}).get('name', None) is None:
         logging.info('No gear found for job ' + str(job['_id']))
         return True
+
     # This logic WILL NOT WORK in parallel mode
 
     gear_name = gear['gear']['name']
@@ -1024,6 +1025,36 @@ def upgrade_to_28():
         config.db.sessions.update({'_id': session['_id']}, session)
 
 
+
+
+def upgrade_to_29_closure(user):
+
+    avatars = user['avatars']
+    if avatars.get('custom') and not 'https:' in avatars['custom']:
+        if user['avatar'] == user['avatars']['custom']:
+            if(user['avatars'].get('provider') == None):
+                config.db.users.update_one({'_id': user['_id']},
+                    {'$unset': {'avatar': ""}})
+            else:    
+                config.db.users.update_one({'_id': user['_id']},
+                    {'$set': {'avatar': user['avatars'].get('provider')}}
+                )
+        logging.info('Deleting custom ...')
+        config.db.users.update_one({'_id': user['_id']},
+            {'$unset': {"avatars.custom": ""}}
+        )
+    return True
+
+
+def upgrade_to_29():
+    """
+    Enforces HTTPS urls for user avatars
+    """
+
+    users = config.db.users.find({})
+    process_cursor(users, upgrade_to_29_closure)
+
+
 def upgrade_schema():
     """
     Upgrades db to the current schema version

raml/schemas/definitions/avatars.json

@@ -5,7 +5,7 @@
   "properties": {
     "gravatar":         {"type": ["string", "null"], "format": "uri" },
     "provider":         {"type": ["string", "null"], "format": "uri" },
-    "custom":           {"type": ["string", "null"], "format": "uri" }
+    "custom":           {"type": ["string", "null"], "pattern": "^https:", "format": "uri" }
   },
   "additionalProperties": false
 }

test/integration_tests/python/test_users.py

@@ -67,6 +67,28 @@ def test_users(as_root, as_user, as_public):
     r = as_root.delete('/users/' + new_user_id)
     assert r.ok
 
+    # Test HTTPS enforcement on avatar urls
+    new_user_id = 'new@user.com'
+    r = as_root.post('/users', json={
+        '_id': new_user_id,
+        'firstname': 'New',
+        'lastname': 'User',
+    })
+    assert r.ok
+    r = as_root.get('/users/' + new_user_id)
+    assert r.ok
+
+    r = as_root.put('/users/' + new_user_id, json={'avatar': 'https://lh3.googleusercontent.com/-XdUIqdMkCWA/AAAAAAAAAAI/AAAAAAAAAAA/4252rscbv5M/photo.jpg'})
+    r = as_root.get('/users/' + new_user_id)
+    assert r.json()['avatar'] == 'https://lh3.googleusercontent.com/-XdUIqdMkCWA/AAAAAAAAAAI/AAAAAAAAAAA/4252rscbv5M/photo.jpg'
+
+    r = as_root.put('/users/' + new_user_id, json={'avatar': 'http://media.nomadicmatt.com/maldivestop001.jpg', 'avatars': {'custom': 'http://media.nomadicmatt.com/maldivestop001.jpg', 'provider': 'https://lh3.googleusercontent.com/-XdUIqdMkCWA/AAAAAAAAAAI/AAAAAAAAAAA/4252rscbv5M/photo.jpg'}})
+    assert r.status_code == 400
+    r = as_root.get('/users/' + new_user_id)
+    assert r.json()['avatar'] != 'http://media.nomadicmatt.com/maldivestop001.jpg'
+
+    r = as_root.delete('/users/' + new_user_id)
+    assert r.ok
 
 def test_generate_api_key(data_builder, as_public):
     # Try to generate new api key w/o logging in