diff --git a/keystone/tests/unit/test_v3_trust.py b/keystone/tests/unit/test_v3_trust.py index f5d1ebe333..0a363654ad 100644 --- a/keystone/tests/unit/test_v3_trust.py +++ b/keystone/tests/unit/test_v3_trust.py @@ -19,7 +19,6 @@ import keystone.conf from keystone import exception from keystone.tests import unit from keystone.tests.unit import test_v3 -from keystone.tests.unit import utils as test_utils CONF = keystone.conf.CONF @@ -297,8 +296,6 @@ class TestTrustOperations(test_v3.RestfulTestCase): self.post('/OS-TRUST/trusts', body={'trust': ref}, expected_status=http_client.NOT_FOUND) - @test_utils.wip('Waiting on validation to be added from fixing bug' - '1734244') def test_create_trust_with_extra_attributes_fails(self): ref = unit.new_trust_ref(trustor_user_id=self.user_id, trustee_user_id=self.trustee_user_id, @@ -306,8 +303,6 @@ class TestTrustOperations(test_v3.RestfulTestCase): role_ids=[self.role_id]) ref['roles'].append({'fake_key': 'fake_value'}) - # Should return 400 Bad Request because `fake_key` is an extra - # attribute that keystone doesn't associate with trusts. self.post('/OS-TRUST/trusts', body={'trust': ref}, expected_status=http_client.BAD_REQUEST) diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 56c980cea9..28d2e853a3 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -1483,7 +1483,9 @@ class EndpointGroupValidationTestCase(unit.BaseTestCase): class TrustValidationTestCase(unit.BaseTestCase): """Test for V3 Trust API validation.""" - _valid_roles = ['member', uuid.uuid4().hex, str(uuid.uuid4())] + _valid_roles = [{'name': 'member'}, + {'id': uuid.uuid4().hex}, + {'id': str(uuid.uuid4())}] _invalid_roles = [False, True, 123, None] def setUp(self): @@ -1505,7 +1507,8 @@ class TrustValidationTestCase(unit.BaseTestCase): 'trustee_user_id': uuid.uuid4().hex, 'impersonation': False, 'project_id': uuid.uuid4().hex, - 'roles': [uuid.uuid4().hex, uuid.uuid4().hex], + 'roles': [{'id': uuid.uuid4().hex}, + {'id': uuid.uuid4().hex}], 'expires_at': 'some timestamp', 'remaining_uses': 2} self.create_trust_validator.validate(request_to_validate) @@ -1540,7 +1543,8 @@ class TrustValidationTestCase(unit.BaseTestCase): 'trustee_user_id': uuid.uuid4().hex, 'impersonation': False, 'project_id': uuid.uuid4().hex, - 'roles': [uuid.uuid4().hex, uuid.uuid4().hex], + 'roles': [{'id': uuid.uuid4().hex}, + {'id': uuid.uuid4().hex}], 'expires_at': 'some timestamp', 'remaining_uses': 2, 'extra': 'something extra!'} diff --git a/keystone/trust/controllers.py b/keystone/trust/controllers.py index ecd5cbb8e0..dac848c1f2 100644 --- a/keystone/trust/controllers.py +++ b/keystone/trust/controllers.py @@ -79,23 +79,23 @@ class TrustV3(controller.V3Controller): 'previous': None} def _normalize_role_list(self, trust_roles): - roles = [{'id': role['id']} for role in trust_roles if 'id' in role] - names = [role['name'] for role in trust_roles if 'id' not in role] - if len(names): - # Long way - for name in names: + roles = [] + for role in trust_roles: + if role.get('id'): + roles.append({'id': role['id']}) + else: hints = driver_hints.Hints() - hints.add_filter("name", name, case_sensitive=True) + hints.add_filter("name", role['name'], case_sensitive=True) found_roles = self.role_api.list_roles(hints) if not found_roles: raise exception.RoleNotFound( - _("Role %s is not defined") % name + _("Role %s is not defined") % role['name'] ) elif len(found_roles) == 1: roles.append({'id': found_roles[0]['id']}) else: raise exception.AmbiguityError(resource='role', - name=name) + name=role['name']) return roles def _find_redelegated_trust(self, request): diff --git a/keystone/trust/schema.py b/keystone/trust/schema.py index 673b786b26..b8b941c058 100644 --- a/keystone/trust/schema.py +++ b/keystone/trust/schema.py @@ -13,6 +13,19 @@ from keystone.common import validation from keystone.common.validation import parameter_types +_role_properties = { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'id': parameter_types.id_string, + 'name': parameter_types.id_string + }, + 'minProperties': 1, + 'maxProperties': 1, + 'additionalProperties': False + } +} _trust_properties = { # NOTE(lbragstad): These are set as external_id_string because they have @@ -36,9 +49,7 @@ _trust_properties = { 'type': ['integer', 'null'], 'minimum': 0 }, - # TODO(lbragstad): Need to find a better way to do this. We should be - # checking that a role is a list of IDs and/or names. - 'roles': validation.add_array_type(parameter_types.id_string) + 'roles': _role_properties } trust_create = { diff --git a/releasenotes/notes/bug-1734244-1b4ea83baa72566d.yaml b/releasenotes/notes/bug-1734244-1b4ea83baa72566d.yaml new file mode 100644 index 0000000000..12d8136bdb --- /dev/null +++ b/releasenotes/notes/bug-1734244-1b4ea83baa72566d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + [`bug 1734244 `_] + Return a 400 status code instead of a 500 when creating a trust with + extra attributes in the roles parameter.