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 <[email protected]> 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):
