allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kentontay...@apache.org
Subject [allura] 01/02: Better handling of certain bad requests
Date Mon, 08 Mar 2021 20:32:46 GMT
This is an automated email from the ASF dual-hosted git repository.

kentontaylor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 53ca34290964419322e79f29354a1cc7f96aabbb
Author: Dave Brondsema <dbrondsema@slashdotmedia.com>
AuthorDate: Mon Mar 8 13:03:21 2021 -0500

    Better handling of certain bad requests
---
 Allura/allura/controllers/repository.py                |  8 ++++++--
 Allura/allura/controllers/rest.py                      |  9 +++++++--
 Allura/allura/lib/custom_middleware.py                 |  5 ++---
 Allura/allura/tests/functional/test_auth.py            | 11 ++++++++---
 ForgeGit/forgegit/tests/functional/test_controllers.py |  6 ++++++
 5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/Allura/allura/controllers/repository.py b/Allura/allura/controllers/repository.py
index 96c7294..2b34847 100644
--- a/Allura/allura/controllers/repository.py
+++ b/Allura/allura/controllers/repository.py
@@ -602,7 +602,9 @@ class RefsController(object):
         self.BranchBrowserClass = BranchBrowserClass
 
     @expose()
-    def _lookup(self, ref, *remainder):
+    def _lookup(self, ref=None, *remainder):
+        if ref is None:
+            raise exc.HTTPNotFound
         EOR = c.app.END_OF_REF_ESCAPE
         if EOR in remainder:
             i = remainder.index(EOR)
@@ -614,7 +616,9 @@ class RefsController(object):
 class CommitsController(object):
 
     @expose()
-    def _lookup(self, ci, *remainder):
+    def _lookup(self, ci=None, *remainder):
+        if ci is None:
+            raise exc.HTTPNotFound
         ci = unquote(ci)
         EOR = c.app.END_OF_REF_ESCAPE
         if EOR in remainder:
diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index db7c4a1..dbd9817 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -158,6 +158,12 @@ class OAuthNegotiator(object):
             parameters=dict(request.params),
             query_string=request.query_string
         )
+        if 'oauth_consumer_key' not in req:
+            log.error('Missing consumer token')
+            return None
+        if 'oauth_token' not in req:
+            log.error('Missing access token')
+            raise exc.HTTPUnauthorized
         consumer_token = M.OAuthConsumerToken.query.get(api_key=req['oauth_consumer_key'])
         access_token = M.OAuthAccessToken.query.get(api_key=req['oauth_token'])
         if consumer_token is None:
@@ -183,8 +189,7 @@ class OAuthNegotiator(object):
             parameters=dict(request.params),
             query_string=request.query_string
         )
-        consumer_token = M.OAuthConsumerToken.query.get(
-            api_key=req['oauth_consumer_key'])
+        consumer_token = M.OAuthConsumerToken.query.get(api_key=req.get('oauth_consumer_key'))
         if consumer_token is None:
             log.error('Invalid consumer token')
             raise exc.HTTPUnauthorized
diff --git a/Allura/allura/lib/custom_middleware.py b/Allura/allura/lib/custom_middleware.py
index 6b17600..25b31bf 100644
--- a/Allura/allura/lib/custom_middleware.py
+++ b/Allura/allura/lib/custom_middleware.py
@@ -289,12 +289,11 @@ class SetRequestHostFromConfig(object):
         #   'HTTP_X_FORWARDED_PROTO' == 'https'
         req = Request(environ)
         try:
-            req.params  # check for malformed unicode, this is the first middleware that
might trip over it.
+            req.params  # check for malformed unicode or POSTs, this is the first middleware
that might trip over it.
             resp = self.app
-        except UnicodeError:
+        except (UnicodeError, ValueError):
             resp = exc.HTTPBadRequest()
 
-
         return resp(environ, start_response)
 
 
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index 0fa9a09..2b07312 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -1892,14 +1892,19 @@ class TestOAuth(TestController):
 
     @mock.patch('allura.controllers.rest.oauth.Server')
     @mock.patch('allura.controllers.rest.oauth.Request')
-    def test_request_token_no_consumer_token(self, Request, Server):
-        Request.from_request.return_value = {
-            'oauth_consumer_key': 'api_key'}
+    def test_request_token_no_consumer_token_matching(self, Request, Server):
+        Request.from_request.return_value = {'oauth_consumer_key': 'api_key'}
         self.app.post('/rest/oauth/request_token',
                       params={'key': 'value'}, status=401)
 
     @mock.patch('allura.controllers.rest.oauth.Server')
     @mock.patch('allura.controllers.rest.oauth.Request')
+    def test_request_token_no_consumer_token_given(self, Request, Server):
+        Request.from_request.return_value = {}
+        self.app.post('/rest/oauth/request_token', params={'key': 'value'}, status=401)
+
+    @mock.patch('allura.controllers.rest.oauth.Server')
+    @mock.patch('allura.controllers.rest.oauth.Request')
     def test_request_token_invalid(self, Request, Server):
         Server().verify_request.side_effect = oauth2.Error('test_request_token_invalid')
         M.OAuthConsumerToken.consumer = mock.Mock()
diff --git a/ForgeGit/forgegit/tests/functional/test_controllers.py b/ForgeGit/forgegit/tests/functional/test_controllers.py
index c08993d..d6b0285 100644
--- a/ForgeGit/forgegit/tests/functional/test_controllers.py
+++ b/ForgeGit/forgegit/tests/functional/test_controllers.py
@@ -209,6 +209,9 @@ class TestRootController(_TestCase):
         assert 'revision="1e146e67985dcd71c74de79613719bef7bddca4a"' not in r
         assert 'url_commit="/p/test/src-git/ci/1e146e67985dcd71c74de79613719bef7bddca4a/">'
not in r
 
+    def test_ci_missing(self):
+        r = self.app.get('/src-git/ci/', status=404)
+
     def test_tags(self):
         self.app.get('/src-git/ref/master~/tags/')
 
@@ -1097,6 +1100,9 @@ class TestGitBranch(TestController):
         assert r.request.url.endswith('src-git/ci/releases/v1.1.1/~/tree/'), r.request.url
         r.mustcontain('on a branch')  # commit for this tag
 
+    def test_ref_url_missing(self):
+        self.app.get('/src-git/ref/', status=404)
+
 
 class TestIncludeMacro(_TestCase):
     def setUp(self):


Mime
View raw message