From f7a8a053f3a923a5e211c9e71c1abdb573555159 Mon Sep 17 00:00:00 2001 From: Gyorgy Szombathelyi Date: Fri, 6 May 2016 12:39:16 +0200 Subject: [PATCH] Enhance federation group mapping validation A group must be reffered either with an ID, or the name _and_ the domain. Change the JSON validation schema to check this. Closes-Bug: #1657978 Change-Id: I213876e30fc0521195848479278080bdac8387de (cherry picked from commit a9d79e098732445efcd58a6b03148fe6c62e044a) --- keystone/federation/utils.py | 58 +++++++------- .../unit/contrib/federation/test_utils.py | 21 +++++- keystone/tests/unit/mapping_fixtures.py | 75 +++++++++++++++++++ 3 files changed, 124 insertions(+), 30 deletions(-) diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index 1d215a6864..1e3d536f82 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -61,12 +61,7 @@ MAPPING_SCHEMA = { "name": {"type": "string"}, "email": {"type": "string"}, "domain": { - "type": "object", - "properties": { - "id": {"type": "string"}, - "name": {"type": "string"} - }, - "additionalProperties": False, + "$ref": "#/definitions/domain" }, "type": { "type": "string", @@ -78,19 +73,10 @@ MAPPING_SCHEMA = { }, "group": { "type": "object", - "properties": { - "id": {"type": "string"}, - "name": {"type": "string"}, - "domain": { - "type": "object", - "properties": { - "id": {"type": "string"}, - "name": {"type": "string"} - }, - "additionalProperties": False, - }, - }, - "additionalProperties": False, + "oneOf": [ + {"$ref": "#/definitions/group_by_id"}, + {"$ref": "#/definitions/group_by_name"} + ] }, "groups": { "type": "string" @@ -98,14 +84,7 @@ MAPPING_SCHEMA = { "group_ids": { "type": "string" }, - "domain": { - "type": "object", - "properties": { - "id": {"type": "string"}, - "name": {"type": "string"} - }, - "additionalProperties": False - } + "domain": {"$ref": "#/definitions/domain"}, } } }, @@ -195,6 +174,31 @@ MAPPING_SCHEMA = { "type": "array" } } + }, + "domain": { + "type": "object", + "properties": { + "id": {"type": "string"}, + "name": {"type": "string"} + }, + "additionalProperties": False + }, + "group_by_id": { + "type": "object", + "properties": { + "id": {"type": "string"} + }, + "additionalProperties": False, + "required": ["id"] + }, + "group_by_name": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "domain": {"$ref": "#/definitions/domain"} + }, + "additionalProperties": False, + "required": ["name", "domain"] } } } diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py index 4feb3905f0..eb9b01cc92 100644 --- a/keystone/tests/unit/contrib/federation/test_utils.py +++ b/keystone/tests/unit/contrib/federation/test_utils.py @@ -21,7 +21,6 @@ from keystone import exception from keystone.federation import utils as mapping_utils from keystone.tests import unit from keystone.tests.unit import mapping_fixtures -from keystone.tests.unit import utils as test_utils FAKE_MAPPING_ID = uuid.uuid4().hex @@ -630,14 +629,30 @@ class MappingRuleEngineTests(unit.BaseTestCase): mapping = mapping_fixtures.MAPPING_GROUP_NAMES mapping_utils.validate_mapping_structure(mapping) - @test_utils.wip('waiting for fix the validator ' - 'to choke on group name without domain') + def test_mapping_validation_bad_domain(self): + mapping = mapping_fixtures.MAPPING_BAD_DOMAIN + self.assertRaises(exception.ValidationError, + mapping_utils.validate_mapping_structure, + mapping) + + def test_mapping_validation_bad_group(self): + mapping = mapping_fixtures.MAPPING_BAD_GROUP + self.assertRaises(exception.ValidationError, + mapping_utils.validate_mapping_structure, + mapping) + def test_mapping_validation_with_group_name_without_domain(self): mapping = mapping_fixtures.MAPPING_GROUP_NAME_WITHOUT_DOMAIN self.assertRaises(exception.ValidationError, mapping_utils.validate_mapping_structure, mapping) + def test_mapping_validation_with_group_id_and_domain(self): + mapping = mapping_fixtures.MAPPING_GROUP_ID_WITH_DOMAIN + self.assertRaises(exception.ValidationError, + mapping_utils.validate_mapping_structure, + mapping) + def test_mapping_validation_no_local(self): mapping = mapping_fixtures.MAPPING_MISSING_LOCAL self.assertRaises(exception.ValidationError, diff --git a/keystone/tests/unit/mapping_fixtures.py b/keystone/tests/unit/mapping_fixtures.py index 1e3d06d282..e4a836fb74 100644 --- a/keystone/tests/unit/mapping_fixtures.py +++ b/keystone/tests/unit/mapping_fixtures.py @@ -607,6 +607,81 @@ MAPPING_GROUP_NAME_WITHOUT_DOMAIN = { ] } +MAPPING_GROUP_ID_WITH_DOMAIN = { + + "rules": [ + { + "local": [ + { + "group": { + "id": EMPLOYEE_GROUP_ID, + "domain": { + "id": DEVELOPER_GROUP_DOMAIN_ID + } + } + } + ], + "remote": [ + { + "type": "orgPersonType", + "any_one_of": [ + "Employee" + ], + } + ] + }, + ] +} + +MAPPING_BAD_GROUP = { + + "rules": [ + { + "local": [ + { + "group": { + } + } + ], + "remote": [ + { + "type": "orgPersonType", + "any_one_of": [ + "Employee" + ], + } + ] + }, + ] +} + +MAPPING_BAD_DOMAIN = { + + "rules": [ + { + "local": [ + { + "group": { + "id": EMPLOYEE_GROUP_ID, + "domain": { + "id": DEVELOPER_GROUP_DOMAIN_ID, + "badkey": "badvalue" + } + } + } + ], + "remote": [ + { + "type": "orgPersonType", + "any_one_of": [ + "Employee" + ], + } + ] + }, + ] +} + MAPPING_EPHEMERAL_USER = { "rules": [ {