diff mbox

[1/2] bundle: Fix use of basic auth for bundle mboxes

Message ID 20170525073805.30513-1-andrew.donnellan@au1.ibm.com
State Accepted
Headers show

Commit Message

Andrew Donnellan May 25, 2017, 7:38 a.m. UTC
Commit 0b4f508a8438 ("views: Allow use of basic auth for bundle mboxes")
added support for using Django REST Framework's BasicAuthentication to
authenticate when accessing the bundle-mbox view.

To check the user's credentials, we call
BasicAuthentication.authenticate(), however, we don't check whether
the returned user is actually the bundle owner. This means that any user
can access any private bundle if they authenticate using basic
authentication.

Additionally, if invalid credentials are provided via a basic
authentication header, BasicAuthentication.authenticate() will throw an
AuthenticationFailed exception. We currently don't catch this, resulting in
an exception page being displayed rather than a 404.

Add a new helper, rest_auth(), that takes a request and returns a user.
Call this in bundle_mbox() and save the result into request.user before we
check whether request.user is actually the bundle owner.

Found by code inspection.

Fixes: 0b4f508a8438 ("views: Allow use of basic auth for bundle mboxes")
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

Given this is a (very, very, very minor) security issue, this should land
in the 2.0 release.
---
 patchwork/views/bundle.py | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
diff mbox

Patch

diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
index dc9b74b..387b7c6 100644
--- a/patchwork/views/bundle.py
+++ b/patchwork/views/bundle.py
@@ -37,9 +37,19 @@  from patchwork.views.utils import bundle_to_mbox
 
 if settings.ENABLE_REST_API:
     from rest_framework.authentication import BasicAuthentication  # noqa
-    basic_auth = BasicAuthentication()
-else:
-    basic_auth = None
+    from rest_framework.exceptions import AuthenticationFailed
+
+
+def rest_auth(request):
+    if not settings.ENABLE_REST_API:
+        return request.user
+    try:
+        auth_result = BasicAuthentication().authenticate(request)
+        if auth_result:
+            return auth_result[0]
+    except AuthenticationFailed:
+        pass
+    return request.user
 
 
 @login_required
@@ -140,8 +150,8 @@  def bundle_mbox(request, username, bundlename):
     bundle = get_object_or_404(Bundle, owner__username=username,
                                name=bundlename)
 
-    if not (request.user == bundle.owner or bundle.public or
-            (basic_auth and basic_auth.authenticate(request))):
+    request.user = rest_auth(request)
+    if not (request.user == bundle.owner or bundle.public):
         return HttpResponseNotFound()
 
     response = HttpResponse(content_type='text/plain')