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 d783ab26b9e31dc8827bd111999bddeccd13cf95 Author: Dave Brondsema <[email protected]> AuthorDate: Wed Aug 3 17:58:38 2022 -0400 [#8450] do oauth check for all /rest/ paths, and simplify normal request auth methods --- Allura/allura/controllers/basetest_project_root.py | 13 +++---- Allura/allura/controllers/rest.py | 13 +++---- Allura/allura/controllers/root.py | 7 ++-- Allura/allura/lib/base.py | 40 ---------------------- Allura/allura/tests/functional/test_auth.py | 2 +- Allura/allura/tests/functional/test_rest.py | 6 ++++ 6 files changed, 21 insertions(+), 60 deletions(-) diff --git a/Allura/allura/controllers/basetest_project_root.py b/Allura/allura/controllers/basetest_project_root.py index 7d382dea7..428b24b79 100644 --- a/Allura/allura/controllers/basetest_project_root.py +++ b/Allura/allura/controllers/basetest_project_root.py @@ -23,9 +23,9 @@ from tg import tmpl_context as c from tg import request from webob import exc from tg import expose +from tg import TGController from paste.deploy.converters import asbool -from allura.lib.base import WsgiDispatchController from allura.lib.security import require, require_authenticated, require_access, has_access from allura.lib import helpers as h from allura.lib import plugin @@ -39,7 +39,7 @@ __all__ = ['RootController'] log = logging.getLogger(__name__) -class BasetestProjectRootController(WsgiDispatchController, ProjectController): +class BasetestProjectRootController(TGController, ProjectController): '''Root controller for testing -- it behaves just like the RootController plus acts as shorthand for the ProjectController at /p/test/ plus all tools are mounted, on-demand, at the mount point that is the same as their entry point name. @@ -71,9 +71,6 @@ class BasetestProjectRootController(WsgiDispatchController, ProjectController): super().__init__() - def _setup_request(self): - pass - @expose() def _lookup(self, name, *remainder): # first try a neighborhood path through the real root controller @@ -116,9 +113,7 @@ class BasetestProjectRootController(WsgiDispatchController, ProjectController): c.project = M.Project.query.get( shortname='test', neighborhood_id=self.p_nbhd._id) auth = plugin.AuthenticationProvider.get(request) - if asbool(environ.get('disable_auth_magic')): - c.user = auth.authenticate_request() - else: + if not asbool(environ.get('disable_auth_magic')): user = auth.by_username(environ.get('username', 'test-admin')) if not user: user = M.User.anonymous() @@ -126,7 +121,7 @@ class BasetestProjectRootController(WsgiDispatchController, ProjectController): # save and persist, so that a creation time is set environ['beaker.session'].save() environ['beaker.session'].persist() - c.user = auth.authenticate_request() + c.user = auth.authenticate_request() return super()._perform_call(context) diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py index 310ac5c3f..c36b45451 100644 --- a/Allura/allura/controllers/rest.py +++ b/Allura/allura/controllers/rest.py @@ -49,6 +49,12 @@ class RestController: def __init__(self): self.oauth = OAuthNegotiator() + def _check_security(self): + if not request.path.startswith('/rest/oauth/'): # everything but OAuthNegotiator + c.api_token = self._authenticate_request() + if c.api_token: + c.user = c.api_token.user + def _authenticate_request(self): 'Based on request.params or oauth, authenticate the request' headers_auth = 'Authorization' in request.headers @@ -92,11 +98,9 @@ class RestController: @expose('json:') def notification(self, cookie='', url='', tool_name='', **kw): - c.api_token = self._authenticate_request() - user = c.api_token.user if c.api_token else c.user r = g.theme._get_site_notification( url=url, - user=user, + user=c.user, tool_name=tool_name, site_notification_cookie_value=cookie ) @@ -106,9 +110,6 @@ class RestController: @expose() def _lookup(self, name, *remainder): - c.api_token = self._authenticate_request() - if c.api_token: - c.user = c.api_token.user neighborhood = M.Neighborhood.query.get(url_prefix='/' + name + '/') if not neighborhood: raise exc.HTTPNotFound(name) diff --git a/Allura/allura/controllers/root.py b/Allura/allura/controllers/root.py index 72dacc756..c626ddf4e 100644 --- a/Allura/allura/controllers/root.py +++ b/Allura/allura/controllers/root.py @@ -23,11 +23,10 @@ from tg import expose, request, config, session, redirect, flash from tg.decorators import with_trailing_slash from tg import tmpl_context as c from tg import response -from paste.deploy.converters import asbool +from tg import TGController from webob import exc from allura.app import SitemapEntry -from allura.lib.base import WsgiDispatchController from allura.lib import plugin from allura.controllers.error import ErrorController from allura.controllers.project import NeighborhoodController @@ -52,7 +51,7 @@ class W: project_summary = plw.ProjectSummary() -class RootController(WsgiDispatchController): +class RootController(TGController): """ The root controller for the allura application. @@ -96,7 +95,7 @@ class RootController(WsgiDispatchController): n = M.Neighborhood.query.get(url_prefix=url_prefix) return n - def _setup_request(self): + def _check_security(self): c.project = c.app = None c.user = plugin.AuthenticationProvider.get(request).authenticate_request() assert c.user is not None, ('c.user should always be at least User.anonymous(). ' diff --git a/Allura/allura/lib/base.py b/Allura/allura/lib/base.py deleted file mode 100644 index 4dbde9c34..000000000 --- a/Allura/allura/lib/base.py +++ /dev/null @@ -1,40 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""The base Controller API.""" -from tg import TGController - -__all__ = ['WsgiDispatchController'] - - -class WsgiDispatchController(TGController): - - """ - Base class for the controllers in the application. - - Your web application should have one of these. The root of - your application is used to compute URLs used by your app. - - """ - - def _setup_request(self): - '''Responsible for setting all the values we need to be set on tg.tmpl_context''' - raise NotImplementedError('_setup_request') - - def _perform_call(self, context): - self._setup_request() - return super()._perform_call(context) diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py index 99a16a574..e555cf8de 100644 --- a/Allura/allura/tests/functional/test_auth.py +++ b/Allura/allura/tests/functional/test_auth.py @@ -1857,7 +1857,7 @@ class TestOAuth(TestController): oauth_client.request('http://localhost/rest/p/test/', 'GET') oa2url = oa2_req.call_args[0][1] oa2url = oa2url.replace('http://localhost', '') - print(oa2url) + # print(oa2url) oa2kwargs = oa2_req.call_args[1] self.app.get(oa2url, headers=oa2kwargs['headers'], status=200) self.app.get(oa2url.replace('oauth_signature=', 'removed='), headers=oa2kwargs['headers'], status=401) diff --git a/Allura/allura/tests/functional/test_rest.py b/Allura/allura/tests/functional/test_rest.py index f61a93bae..94b009fd5 100644 --- a/Allura/allura/tests/functional/test_rest.py +++ b/Allura/allura/tests/functional/test_rest.py @@ -46,6 +46,7 @@ class TestRestHome(TestRestApiBase): request.headers = {} request.params = {'access_token': 'foo'} request.scheme = 'https' + request.path = '/rest/p/test/wiki' self._patch_token(OAuthAccessToken) access_token = OAuthAccessToken.query.get.return_value access_token.is_bearer = False @@ -58,6 +59,7 @@ class TestRestHome(TestRestApiBase): request.headers = {} request.params = {'access_token': 'foo'} request.scheme = 'https' + request.path = '/rest/p/test/wiki' self._patch_token(OAuthAccessToken) OAuthAccessToken.query.get.return_value = None r = self.api_post('/rest/p/test/wiki', access_token='foo', status=401) @@ -87,6 +89,7 @@ class TestRestHome(TestRestApiBase): request.headers = {} request.params = {'access_token': access_token.api_key} request.scheme = 'https' + request.path = '/rest/p/test/wiki' r = self.api_post('/rest/p/test/wiki', access_token='foo') assert_equal(r.status_int, 200) @@ -97,6 +100,7 @@ class TestRestHome(TestRestApiBase): 'Authorization': 'Bearer foo' } request.scheme = 'https' + request.path = '/rest/p/test/wiki' self._patch_token(OAuthAccessToken) access_token = OAuthAccessToken.query.get.return_value access_token.is_bearer = False @@ -110,6 +114,7 @@ class TestRestHome(TestRestApiBase): 'Authorization': 'Bearer foo' } request.scheme = 'https' + request.path = '/rest/p/test/wiki' self._patch_token(OAuthAccessToken) OAuthAccessToken.query.get.return_value = None r = self.api_post('/rest/p/test/wiki', access_token='foo', status=401) @@ -142,6 +147,7 @@ class TestRestHome(TestRestApiBase): 'Authorization': f'Bearer {token}' } request.scheme = 'https' + request.path = '/rest/p/test/wiki' r = self.api_post('/rest/p/test/wiki', access_token='foo', status=200) # reverse proxy situation request.scheme = 'http'
