From 60b52c1248ddd5e682838d9e8ba853789940c284 Mon Sep 17 00:00:00 2001 From: Henry Nash Date: Sun, 13 Dec 2015 00:15:34 +0000 Subject: [PATCH] Add support for strict url safe option on new projects and domains Building on the earlier patch that provdided the 'new' url name restriction, this patch adds the 'strict' open that prevents authenticating to projects and domains with unsafe names. A release note and config documentation is also added that covers both this and the earlier patch. Partially Implements: blueprint url-safe-naming Change-Id: Ie69025e7759bae1067e05d9190bede192a5e6830 --- doc/source/configuration.rst | 33 +++++ keystone/auth/controllers.py | 8 ++ keystone/common/config.py | 12 +- keystone/tests/unit/test_auth.py | 30 +++++ keystone/tests/unit/test_v3_auth.py | 116 ++++++++++++++++++ keystone/tests/unit/test_v3_resource.py | 68 +++++----- keystone/token/controllers.py | 4 + .../bp-url-safe-naming-ad90d6a659f5bf3c.yaml | 7 ++ 8 files changed, 242 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/bp-url-safe-naming-ad90d6a659f5bf3c.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 31a79e50d8..fdafdab6c0 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -1246,6 +1246,39 @@ If a response to ``list_{entity}`` call has been truncated, then the response status code will still be 200 (OK), but the ``truncated`` attribute in the collection will be set to ``true``. + +Url safe naming of projects and domains +--------------------------------------- + +In the future, keystone may offer the ability to identify a project in a +hierarchy via a url style of naming from the root of the hierarchy (for example +specifying 'projectA/projectB/projectC' as the project name in an +authentication request). In order to prepare for this, keystone supports the +optional ability to ensure both projects and domains are named without +including any of the reserverd characters specified in section 2.2 of +`rfc3986 `_. + +The safety of the names of projects and domains can be controlled via two +configuration options: + +.. code-block:: ini + + [resource] + project_name_url_safe = off + domain_name_url_safe = off + +When set to ``off`` (which is the default), no checking is done on the url +safeness of names. When set to ``new``, an attempt to create a new project or +domain with an unsafe name (or update the name of a project or domain to be +unsafe) will cause a status code of 400 (Bad Request) to be returned. Setting +the configuration option to ``strict`` will, in addition to preventing the +creation and updating of entities with unsafe names, cause an authentication +attempt which specifies a project or domain name that is unsafe to return a +status code of 401 (Unauthorized). + +It is recommended that installations take the steps necessary to where they +can run with both options set to ``strict`` as soon as is practical. + Sample Configuration Files -------------------------- diff --git a/keystone/auth/controllers.py b/keystone/auth/controllers.py index d356607da6..fb2c9f1ba4 100644 --- a/keystone/auth/controllers.py +++ b/keystone/auth/controllers.py @@ -177,6 +177,10 @@ class AuthInfo(object): target='domain') try: if domain_name: + if (CONF.resource.domain_name_url_safe == 'strict' and + utils.is_not_url_safe(domain_name)): + msg = _('Domain name cannot contain reserved characters.') + raise exception.Unauthorized(message=msg) domain_ref = self.resource_api.get_domain_by_name( domain_name) else: @@ -196,6 +200,10 @@ class AuthInfo(object): target='project') try: if project_name: + if (CONF.resource.project_name_url_safe == 'strict' and + utils.is_not_url_safe(project_name)): + msg = _('Project name cannot contain reserved characters.') + raise exception.Unauthorized(message=msg) if 'domain' not in project_info: raise exception.ValidationError(attribute='domain', target='project') diff --git a/keystone/common/config.py b/keystone/common/config.py index ad0e8c313a..44526d6e1c 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -398,17 +398,21 @@ FILE_OPTIONS = { 'this project will contain the key/value ' '`is_admin_project=true`. Defaults to None.'), cfg.StrOpt('project_name_url_safe', - choices=['off', 'new'], default='off', + choices=['off', 'new', 'strict'], default='off', help='Whether the names of projects are restricted from ' 'containing url reserved characters. If set to new, ' 'attempts to create or update a project with a url ' - 'unsafe name will return an error.'), + 'unsafe name will return an error. In addition, if ' + 'set to strict, attempts to scope a token using ' + 'an unsafe project name will return an error.'), cfg.StrOpt('domain_name_url_safe', - choices=['off', 'new'], default='off', + choices=['off', 'new', 'strict'], default='off', help='Whether the names of domains are restricted from ' 'containing url reserved characters. If set to new, ' 'attempts to create or update a domain with a url ' - 'unsafe name will return an error.'), + 'unsafe name will return an error. In addition, if ' + 'set to strict, attempts to scope a token using a ' + 'domain name which is unsafe will return an error.'), ], 'domain_config': [ cfg.StrOpt('driver', diff --git a/keystone/tests/unit/test_auth.py b/keystone/tests/unit/test_auth.py index ed7113555f..7603c6b1d2 100644 --- a/keystone/tests/unit/test_auth.py +++ b/keystone/tests/unit/test_auth.py @@ -223,6 +223,36 @@ class AuthBadRequests(AuthTest): self.controller.authenticate, {}, body_dict) + def test_authenticate_fails_if_project_unsafe(self): + """Verify authenticate to a project with unsafe name fails.""" + # Start with url name restrictions off, so we can create the unsafe + # named project + self.config_fixture.config(group='resource', + project_name_url_safe='off') + unsafe_name = 'i am not / safe' + project = unit.new_project_ref(domain_id=DEFAULT_DOMAIN_ID, + name=unsafe_name) + self.resource_api.create_project(project['id'], project) + self.assignment_api.add_role_to_user_and_project( + self.user_foo['id'], project['id'], self.role_member['id']) + no_context = {} + + body_dict = _build_user_auth( + username=self.user_foo['name'], + password=self.user_foo['password'], + tenant_name=project['name']) + + # Since name url restriction is off, we should be able to autenticate + self.controller.authenticate(no_context, body_dict) + + # Set the name url restriction to strict and we should fail to + # authenticate + self.config_fixture.config(group='resource', + project_name_url_safe='strict') + self.assertRaises(exception.Unauthorized, + self.controller.authenticate, + no_context, body_dict) + class AuthWithToken(AuthTest): def test_unscoped_token(self): diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py index 29718b0cee..e468454b30 100644 --- a/keystone/tests/unit/test_v3_auth.py +++ b/keystone/tests/unit/test_v3_auth.py @@ -2735,6 +2735,122 @@ class TestAuth(test_v3.RestfulTestCase): self.v3_create_token(auth_data, expected_status=http_client.UNAUTHORIZED) + def test_authenticate_fails_if_project_unsafe(self): + """Verify authenticate to a project with unsafe name fails.""" + # Start with url name restrictions off, so we can create the unsafe + # named project + self.config_fixture.config(group='resource', + project_name_url_safe='off') + unsafe_name = 'i am not / safe' + project = unit.new_project_ref(domain_id=test_v3.DEFAULT_DOMAIN_ID, + name=unsafe_name) + self.resource_api.create_project(project['id'], project) + role_member = unit.new_role_ref() + self.role_api.create_role(role_member['id'], role_member) + self.assignment_api.add_role_to_user_and_project( + self.user['id'], project['id'], role_member['id']) + + auth_data = self.build_authentication_request( + user_id=self.user['id'], + password=self.user['password'], + project_name=project['name'], + project_domain_id=test_v3.DEFAULT_DOMAIN_ID) + + # Since name url restriction is off, we should be able to autenticate + self.v3_create_token(auth_data) + + # Set the name url restriction to new, which should still allow us to + # authenticate + self.config_fixture.config(group='resource', + project_name_url_safe='new') + self.v3_create_token(auth_data) + + # Set the name url restriction to strict and we should fail to + # authenticate + self.config_fixture.config(group='resource', + project_name_url_safe='strict') + self.v3_create_token(auth_data, + expected_status=http_client.UNAUTHORIZED) + + def test_authenticate_fails_if_domain_unsafe(self): + """Verify authenticate to a domain with unsafe name fails.""" + # Start with url name restrictions off, so we can create the unsafe + # named domain + self.config_fixture.config(group='resource', + domain_name_url_safe='off') + unsafe_name = 'i am not / safe' + domain = unit.new_domain_ref(name=unsafe_name) + self.resource_api.create_domain(domain['id'], domain) + role_member = unit.new_role_ref() + self.role_api.create_role(role_member['id'], role_member) + self.assignment_api.create_grant( + role_member['id'], + user_id=self.user['id'], + domain_id=domain['id']) + + auth_data = self.build_authentication_request( + user_id=self.user['id'], + password=self.user['password'], + domain_name=domain['name']) + + # Since name url restriction is off, we should be able to autenticate + self.v3_create_token(auth_data) + + # Set the name url restriction to new, which should still allow us to + # authenticate + self.config_fixture.config(group='resource', + project_name_url_safe='new') + self.v3_create_token(auth_data) + + # Set the name url restriction to strict and we should fail to + # authenticate + self.config_fixture.config(group='resource', + domain_name_url_safe='strict') + self.v3_create_token(auth_data, + expected_status=http_client.UNAUTHORIZED) + + def test_authenticate_fails_to_project_if_domain_unsafe(self): + """Verify authenticate to a project using unsafe domain name fails.""" + # Start with url name restrictions off, so we can create the unsafe + # named domain + self.config_fixture.config(group='resource', + domain_name_url_safe='off') + unsafe_name = 'i am not / safe' + domain = unit.new_domain_ref(name=unsafe_name) + self.resource_api.create_domain(domain['id'], domain) + # Add a (safely named) project to that domain + project = unit.new_project_ref(domain_id=domain['id']) + self.resource_api.create_project(project['id'], project) + role_member = unit.new_role_ref() + self.role_api.create_role(role_member['id'], role_member) + self.assignment_api.create_grant( + role_member['id'], + user_id=self.user['id'], + project_id=project['id']) + + # An auth request via project ID, but specifying domain by name + auth_data = self.build_authentication_request( + user_id=self.user['id'], + password=self.user['password'], + project_name=project['name'], + project_domain_name=domain['name']) + + # Since name url restriction is off, we should be able to autenticate + self.v3_create_token(auth_data) + + # Set the name url restriction to new, which should still allow us to + # authenticate + self.config_fixture.config(group='resource', + project_name_url_safe='new') + self.v3_create_token(auth_data) + + # Set the name url restriction to strict and we should fail to + # authenticate + self.config_fixture.config(group='resource', + domain_name_url_safe='strict') + self.v3_create_token(auth_data, + expected_status=http_client.UNAUTHORIZED) + class TestAuthJSONExternal(test_v3.RestfulTestCase): content_type = 'json' diff --git a/keystone/tests/unit/test_v3_resource.py b/keystone/tests/unit/test_v3_resource.py index 3ef09025df..21b460c07a 100644 --- a/keystone/tests/unit/test_v3_resource.py +++ b/keystone/tests/unit/test_v3_resource.py @@ -73,13 +73,14 @@ class ResourceTestCase(test_v3.RestfulTestCase, '/domains', body={'domain': ref}) - self.config_fixture.config(group='resource', - domain_name_url_safe='new') - ref = unit.new_domain_ref(name=unsafe_name) - self.post( - '/domains', - body={'domain': ref}, - expected_status=http_client.BAD_REQUEST) + for config_setting in ['new', 'strict']: + self.config_fixture.config(group='resource', + domain_name_url_safe=config_setting) + ref = unit.new_domain_ref(name=unsafe_name) + self.post( + '/domains', + body={'domain': ref}, + expected_status=http_client.BAD_REQUEST) def test_create_domain_unsafe_default(self): """Check default for unsafe names for``POST /domains ``.""" @@ -126,14 +127,15 @@ class ResourceTestCase(test_v3.RestfulTestCase, body={'domain': ref}) unsafe_name = 'i am still not / safe' - self.config_fixture.config(group='resource', - domain_name_url_safe='new') - ref = unit.new_domain_ref(name=unsafe_name) - del ref['id'] - self.patch('/domains/%(domain_id)s' % { - 'domain_id': self.domain_id}, - body={'domain': ref}, - expected_status=http_client.BAD_REQUEST) + for config_setting in ['new', 'strict']: + self.config_fixture.config(group='resource', + domain_name_url_safe=config_setting) + ref = unit.new_domain_ref(name=unsafe_name) + del ref['id'] + self.patch('/domains/%(domain_id)s' % { + 'domain_id': self.domain_id}, + body={'domain': ref}, + expected_status=http_client.BAD_REQUEST) def test_update_domain_unsafe_default(self): """Check default for unsafe names for``POST /domains ``.""" @@ -545,13 +547,14 @@ class ResourceTestCase(test_v3.RestfulTestCase, '/projects', body={'project': ref}) - self.config_fixture.config(group='resource', - project_name_url_safe='new') - ref = unit.new_project_ref(name=unsafe_name) - self.post( - '/projects', - body={'project': ref}, - expected_status=http_client.BAD_REQUEST) + for config_setting in ['new', 'strict']: + self.config_fixture.config(group='resource', + project_name_url_safe=config_setting) + ref = unit.new_project_ref(name=unsafe_name) + self.post( + '/projects', + body={'project': ref}, + expected_status=http_client.BAD_REQUEST) def test_create_project_unsafe_default(self): """Check default for unsafe names for``POST /projects ``.""" @@ -1056,16 +1059,17 @@ class ResourceTestCase(test_v3.RestfulTestCase, body={'project': ref}) unsafe_name = 'i am still not / safe' - self.config_fixture.config(group='resource', - project_name_url_safe='new') - ref = unit.new_project_ref(name=unsafe_name, - domain_id=self.domain_id) - del ref['id'] - self.patch( - '/projects/%(project_id)s' % { - 'project_id': self.project_id}, - body={'project': ref}, - expected_status=http_client.BAD_REQUEST) + for config_setting in ['new', 'strict']: + self.config_fixture.config(group='resource', + project_name_url_safe=config_setting) + ref = unit.new_project_ref(name=unsafe_name, + domain_id=self.domain_id) + del ref['id'] + self.patch( + '/projects/%(project_id)s' % { + 'project_id': self.project_id}, + body={'project': ref}, + expected_status=http_client.BAD_REQUEST) def test_update_project_unsafe_default(self): """Check default for unsafe names for``POST /projects ``.""" diff --git a/keystone/token/controllers.py b/keystone/token/controllers.py index 59b56b129a..86d037784b 100644 --- a/keystone/token/controllers.py +++ b/keystone/token/controllers.py @@ -367,6 +367,10 @@ class Auth(controller.V2Controller): size=CONF.max_param_size) if tenant_name: + if (CONF.resource.project_name_url_safe == 'strict' and + utils.is_not_url_safe(tenant_name)): + msg = _('Tenant name cannot contain reserved characters.') + raise exception.Unauthorized(message=msg) try: tenant_ref = self.resource_api.get_project_by_name( tenant_name, CONF.identity.default_domain_id) diff --git a/releasenotes/notes/bp-url-safe-naming-ad90d6a659f5bf3c.yaml b/releasenotes/notes/bp-url-safe-naming-ad90d6a659f5bf3c.yaml new file mode 100644 index 0000000000..1c81d8663c --- /dev/null +++ b/releasenotes/notes/bp-url-safe-naming-ad90d6a659f5bf3c.yaml @@ -0,0 +1,7 @@ +--- +features: + - > + [`blueprint url-safe-naming `_] + The names of projects and domains can optionally be ensured to be url safe, + to support the future ability to specify projects using hierarchical + naming.