Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5356 closed defect (fixed)

Can't GET another user's outbox

Reported by: ayleph Owned by:
Priority: major Milestone: 0.9.0
Component: programming Keywords: api, pump
Cc: tsyesika Parent Tickets:

Description

When a pump client tries to GET the outbox of another user, the API always returns the outbox of the requesting user. The below patch addresses this issue.

From c5f40d03a2ae6dd5f5c8ea67e441d4711e052c35 Mon Sep 17 00:00:00 2001
From: ayleph <ayleph@thisshitistemp.com>
Date: Sat, 31 Oct 2015 04:18:44 -0400
Subject: [PATCH 2/2] Allow API client to GET another user's outbox

---
 mediagoblin/api/views.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mediagoblin/api/views.py b/mediagoblin/api/views.py
index 671c3b3..dcd04cd 100644
--- a/mediagoblin/api/views.py
+++ b/mediagoblin/api/views.py
@@ -565,9 +565,9 @@ def feed_endpoint(request, outbox=None):
 
     # Create outbox
     if outbox is None:
-        outbox = Activity.query.filter_by(actor=request.user.id)
+        outbox = Activity.query.filter_by(actor=requested_user.id)
     else:
-        outbox = outbox.filter_by(actor=request.user.id)
+        outbox = outbox.filter_by(actor=requested_user.id)
 
     # We want the newest things at the top (issue: #1055)
     outbox = outbox.order_by(Activity.published.desc())
-- 
2.6.2

I think this change is safe to make because:

  • There is already code to check that the requesting user and the requested user match for PUT/POST requests.
421                 # Check that the person trying to update the comment is
422                 # the author of the comment.
423                 if image.actor != request.user.id:
424                     return json_error(
425                         "Only uploader of image is able to update image.",
426                         status=403
427                     )
  • There is code which throws an error if a request other than PUT, POST, or GET is made.
548     elif request.method != "GET":
549         return json_error(
550             "Unsupported HTTP method {0}".format(request.method),
551             status=501
552         

So theoretically only a GET request should be able to pass through with the requested user not the same as the requesting user.

Change History (5)

comment:1 by ayleph, 8 years ago

Status: newreview

comment:2 by ayleph, 8 years ago

Milestone: 0.8.1

comment:3 by Christopher Allan Webber, 8 years ago

Okay, we should also have tsyesika review this one when she gets back next week.

comment:4 by Jessica Tallon, 8 years ago

Resolution: fixed
Status: reviewclosed

It's correct that changing this is perfectly fine as by this stage we're just doing a read get request. This has no impact on writing methods which do indeed check that no one else can read this, this is confirmed my the unit tests which ensure only the user can POST/PUT/DELETE on their own feed.

I have added a unit test in addition to reviewing and signing off on Ayleph's patch. This has been fixed as of 34f1d8a (code fix) and 0d053bf (the unit test), thanks Ayleph for your patch.

comment:5 by Christopher Allan Webber, 8 years ago

Milestone: 0.8.20.9.0

All 0.8.2 tickets are being rolled over to 0.9.0

Note: See TracTickets for help on using tickets.