Fix some security concerns regrding inpersonation in federation code.

diff --git a/mediagoblin/federation/views.py b/mediagoblin/federation/views.py
index a6912166e843eebc5fe980943fd7c6be6c2df717..d3ded448403794e64e4d5494ba08a2f7ac19db63 100644 (file)
--- a/mediagoblin/federation/views.py
+++ b/mediagoblin/federation/views.py
@@ -73,8 +73,16 @@ def uploads(request):
     if requested_user is None:
         return json_error("No such 'user' with id '{0}'".format(user), 404)
 
-    request.user = requested_user[0]
+    requested_user = requested_user[0]
     if request.method == "POST":
+        # Ensure that the user is only able to upload to their own
+        # upload endpoint.
+        if requested_user.id != request.user.id:
+            return json_error(
+                "Not able to post to another users feed.",
+                status=403
+            )
+        
         # Wrap the data in the werkzeug file wrapper
         if "Content-Type" not in request.headers:
             return json_error(
@@ -107,12 +115,20 @@ def feed(request):
     if requested_user is None:
         return json_error("No such 'user' with id '{0}'".format(user), 404)
 
-    request.user = requested_user[0]
+    requested_user = requested_user[0]
     if request.data:
         data = json.loads(request.data)
     else:
         data = {"verb": None, "object": {}}
 
+    # We need to check that the user they're posting to is
+    # the person that they are.
+    if request.method in ["POST", "PUT"] and requested_user.id != request.user.id:
+        return json_error(
+            "Not able to post to another users feed.",
+            status=403
+        )
+
     if request.method == "POST" and data["verb"] == "post":
         obj = data.get("object", None)
         if obj is None:

diff --git a/mediagoblin/tests/test_api.py b/mediagoblin/tests/test_api.py
index 88d8605360dd222d9e32496ddb39de70c6e5e13e..a003e66d1fdbbd603b9c598df1084825dd9bc243 100644 (file)
--- a/mediagoblin/tests/test_api.py
+++ b/mediagoblin/tests/test_api.py
@@ -35,6 +35,10 @@ class TestAPI(object):
         self.db = mg_globals.database
 
         self.user = fixture_add_user(privileges=[u'active', u'uploader', u'commenter'])
+        self.other_user = fixture_add_user(
+            username="otheruser",
+            privileges=[u'active', u'uploader', u'commenter']
+        )
 
     def _activity_to_feed(self, test_app, activity, headers=None):
         """ Posts an activity to the user's feed """
@@ -115,6 +119,51 @@ class TestAPI(object):
         # Check that we got the response we're expecting
         response, _ = self._post_image_to_feed(test_app, image)
         assert response.status_code == 200
+    
+    def test_unable_to_upload_as_someone_else(self, test_app):
+        """ Test that can't upload as someoen else """
+        data = open(GOOD_JPG, "rb").read()
+        headers = {
+            "Content-Type": "image/jpeg",
+            "Content-Length": str(len(data))
+        }
+        
+        with mock.patch("mediagoblin.decorators.oauth_required",
+                        new_callable=self.mocked_oauth_required):
+            
+            # Will be self.user trying to upload as self.other_user
+            with pytest.raises(AppError) as excinfo:
+                test_app.post(
+                    "/api/user/{0}/uploads".format(self.other_user.username),
+                    data,
+                    headers=headers
+                )
+            
+            assert "403 FORBIDDEN" in excinfo.value.message
+    
+    def test_unable_to_post_feed_as_someone_else(self, test_app):
+        """ Tests that can't post an image to someone else's feed """
+        response, data = self._upload_image(test_app, GOOD_JPG)
+        
+        activity = {
+            "verb": "post",
+            "object": data
+        }
+        
+        headers = {
+            "Content-Type": "application/json",
+        }
+        
+        with mock.patch("mediagoblin.decorators.oauth_required",
+                    new_callable=self.mocked_oauth_required):
+            with pytest.raises(AppError) as excinfo:
+                test_app.post(
+                    "/api/user/{0}/feed".format(self.other_user.username),
+                    json.dumps(activity),
+                    headers=headers
+                )
+            
+            assert "403 FORBIDDEN" in excinfo.value.message
 
     def test_upload_image_with_filename(self, test_app):
         """ Tests that you can upload an image with filename and description """
@@ -243,6 +292,37 @@ class TestAPI(object):
         # Test that the response is what we should be given
         assert comment.id == comment_data["object"]["id"]
         assert comment.content == comment_data["object"]["content"]
+    
+    def test_unable_to_post_comment_as_someone_else(self, test_app):
+        """ Tests that you're unable to post a comment as someone else. """
+        # Upload some media to comment on
+        response, data = self._upload_image(test_app, GOOD_JPG)
+        response, data = self._post_image_to_feed(test_app, data)
+        
+        activity = {
+            "verb": "post",
+            "object": {
+                "objectType": "comment",
+                "content": "comment commenty comment ^_^",
+                "inReplyTo": data["object"],
+            }
+        }
+        
+        headers = {
+            "Content-Type": "application/json",
+        }
+        
+        with mock.patch("mediagoblin.decorators.oauth_required",
+                        new_callable=self.mocked_oauth_required):
+            with pytest.raises(AppError) as excinfo:
+                test_app.post(
+                    "/api/user/{0}/feed".format(self.other_user.username),
+                    json.dumps(activity),
+                    headers=headers
+                )
+                
+            assert "403 FORBIDDEN" in excinfo.value.message
+
 
     def test_profile(self, test_app):
         """ Tests profile endpoint """