From acd5d30797bf5c3a02efe1fa09b98feafc7bab2f Mon Sep 17 00:00:00 2001 From: Aaron-DH Date: Wed, 24 Feb 2016 11:51:24 +0800 Subject: [PATCH] Fix api access with public acl routes Route path in public acl routes should be access without authentication even if authentication is enabled(enable_authentication = True) Closes-Bug: #1549071 Change-Id: Ie0a2684464adfa5ae5dd64f17d8ad399f7202604 --- magnum/api/config.py | 3 +- magnum/api/middleware/auth_token.py | 2 +- magnum/tests/unit/api/base.py | 43 +++++---- .../tests/unit/api/controllers/test_root.py | 95 +++++++++++++++---- 4 files changed, 105 insertions(+), 38 deletions(-) diff --git a/magnum/api/config.py b/magnum/api/config.py index 576dea968a..bb1ef88a21 100644 --- a/magnum/api/config.py +++ b/magnum/api/config.py @@ -25,7 +25,8 @@ app = { hooks.NoExceptionTracebackHook(), ], 'acl_public_routes': [ - '/' + '/', + '/v1', ], } diff --git a/magnum/api/middleware/auth_token.py b/magnum/api/middleware/auth_token.py index 13726eb5b3..40ec9abf91 100644 --- a/magnum/api/middleware/auth_token.py +++ b/magnum/api/middleware/auth_token.py @@ -32,7 +32,7 @@ class AuthTokenMiddleware(auth_token.AuthProtocol): def __init__(self, app, conf, public_api_routes=None): if public_api_routes is None: public_api_routes = [] - route_pattern_tpl = '%s\.json?$' + route_pattern_tpl = '%s(\.json)?$' try: self.public_api_routes = [re.compile(route_pattern_tpl % route_tpl) diff --git a/magnum/tests/unit/api/base.py b/magnum/tests/unit/api/base.py index 32c9895a1b..0f72e8d298 100644 --- a/magnum/tests/unit/api/base.py +++ b/magnum/tests/unit/api/base.py @@ -45,6 +45,25 @@ class FunctionalTest(base.DbTestCase): group='keystone_authtoken') cfg.CONF.set_override("admin_user", "admin", group='keystone_authtoken') + + # Determine where we are so we can set up paths in the config + root_dir = self.get_path() + self.config = { + 'app': { + 'root': 'magnum.api.controllers.root.RootController', + 'modules': ['magnum.api'], + 'static_root': '%s/public' % root_dir, + 'template_path': '%s/api/templates' % root_dir, + 'enable_acl': False, + 'acl_public_routes': ['/', '/v1'], + 'hooks': [ + hooks.ContextHook(), + hooks.RPCHook(), + hooks.NoExceptionTracebackHook(), + ], + }, + } + self.app = self._make_app() def reset_pecan(): @@ -64,27 +83,13 @@ class FunctionalTest(base.DbTestCase): for attr in attrs: verify_method(attr, response) - def _make_app(self, enable_acl=False): - # Determine where we are so we can set up paths in the config - root_dir = self.get_path() + def _make_app(self, config=None, enable_acl=False): + if not config: + config = self.config - self.config = { - 'app': { - 'root': 'magnum.api.controllers.root.RootController', - 'modules': ['magnum.api'], - 'static_root': '%s/public' % root_dir, - 'template_path': '%s/api/templates' % root_dir, - 'enable_acl': enable_acl, - 'acl_public_routes': ['/', '/v1'], - 'hooks': [ - hooks.ContextHook(), - hooks.RPCHook(), - hooks.NoExceptionTracebackHook(), - ], - }, - } + config["app"]["enable_acl"] = enable_acl - return pecan.testing.load_test_app(self.config) + return pecan.testing.load_test_app(config) def _request_json(self, path, params, expect_errors=False, headers=None, method="post", extra_environ=None, status=None, diff --git a/magnum/tests/unit/api/controllers/test_root.py b/magnum/tests/unit/api/controllers/test_root.py index d1a1aa86b2..9d4480745b 100644 --- a/magnum/tests/unit/api/controllers/test_root.py +++ b/magnum/tests/unit/api/controllers/test_root.py @@ -10,7 +10,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy + import mock +from oslo_config import cfg from webob import exc as webob_exc from magnum.api.controllers import v1 as v1_api @@ -19,23 +22,21 @@ from magnum.tests.unit.api import base as api_base class TestRootController(api_base.FunctionalTest): - def test_version(self): - expected = {u'default_version': - {u'id': u'v1', u'links': - [{u'href': u'http://localhost/v1/', u'rel': u'self'}]}, - u'description': u'Magnum is an OpenStack project which ' - 'aims to provide container management.', - u'name': u'OpenStack Magnum API', - u'versions': [{u'id': u'v1', - u'links': - [{u'href': u'http://localhost/v1/', - u'rel': u'self'}]}]} + def setUp(self): + super(TestRootController, self).setUp() + self.root_expected = { + u'default_version': + {u'id': u'v1', u'links': + [{u'href': u'http://localhost/v1/', u'rel': u'self'}]}, + u'description': u'Magnum is an OpenStack project which ' + 'aims to provide container management.', + u'name': u'OpenStack Magnum API', + u'versions': [{u'id': u'v1', + u'links': + [{u'href': u'http://localhost/v1/', + u'rel': u'self'}]}]} - response = self.app.get('/') - self.assertEqual(expected, response.json) - - def test_v1_controller(self): - expected = { + self.v1_expected = { u'media_types': [{u'base': u'application/json', u'type': u'application/vnd.openstack.magnum.v1+json'}], @@ -83,13 +84,73 @@ class TestRootController(api_base.FunctionalTest): {u'href': u'http://localhost/mservices/', u'rel': u'bookmark'}]} + def test_version(self): + response = self.app.get('/') + self.assertEqual(self.root_expected, response.json) + + def test_v1_controller(self): response = self.app.get('/v1/') - self.assertEqual(expected, response.json) + self.assertEqual(self.v1_expected, response.json) def test_get_not_found(self): response = self.app.get('/a/bogus/url', expect_errors=True) assert response.status_int == 404 + def test_acl_access_with_all(self): + cfg.CONF.set_override("enable_authentication", True) + + config = copy.deepcopy(self.config) + # Both / and /v1 and access without auth + config["app"]["acl_public_routes"] = ['/', '/v1'] + app = self._make_app(config=config) + + response = app.get('/') + self.assertEqual(self.root_expected, response.json) + + response = app.get('/v1/') + self.assertEqual(self.v1_expected, response.json) + + def test_acl_access_with_root(self): + cfg.CONF.set_override("enable_authentication", True) + + config = copy.deepcopy(self.config) + # Only / can access without auth + config["app"]["acl_public_routes"] = ['/'] + app = self._make_app(config=config) + + response = app.get('/') + self.assertEqual(self.root_expected, response.json) + + response = app.get('/v1/', expect_errors=True) + self.assertEqual(401, response.status_int) + + def test_acl_access_with_v1(self): + cfg.CONF.set_override("enable_authentication", True) + + config = copy.deepcopy(self.config) + # Only /v1 can access without auth + config["app"]["acl_public_routes"] = ['/v1'] + app = self._make_app(config=config) + + response = app.get('/', expect_errors=True) + self.assertEqual(401, response.status_int) + + response = app.get('/v1/') + self.assertEqual(self.v1_expected, response.json) + + def test_acl_with_neither(self): + cfg.CONF.set_override("enable_authentication", True) + + config = copy.deepcopy(self.config) + config["app"]["acl_public_routes"] = [] + app = self._make_app(config=config) + + response = app.get('/', expect_errors=True) + self.assertEqual(401, response.status_int) + + response = app.get('/v1/', expect_errors=True) + self.assertEqual(401, response.status_int) + class TestV1Routing(api_base.FunctionalTest):