From 8917ffb1e73ac8ed0fc825113593e5e5ca9b4573 Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Thu, 31 Jul 2014 20:33:04 +0100 Subject: [PATCH] Fix some security concerns regrding inpersonation in federation code. --- mediagoblin/federation/views.py | 20 ++++++++- mediagoblin/tests/test_api.py | 80 +++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/mediagoblin/federation/views.py b/mediagoblin/federation/views.py index a6912166..d3ded448 100644 --- 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 88d86053..a003e66d 100644 --- 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 """ -- 2.25.1