Raise more precise exception on keyword mapping errors

Currently mapping rules cannot be built in a way where a remote
keyword such as 'any_one_of' or 'not_any_of' is used for direct
mapping ('{0}', '{1}' etc in local rules). However, there is also
no good way of informing user of faulty mapping rules. The original
code will raise an unhelpful IndexError.

This patch:

- introduces new exception class (resulting in HTTP 500 error code)
- changes the logic of mapping rules evaluation, that new exception
  class is being raised under appropriate circumstances.

Change-Id: I0f9e7a6949ff8bfbd147e2d3cac59fd5197934f1
Closes-Bug: #1401057
This commit is contained in:
Marek Denis 2015-04-21 18:32:34 +02:00 committed by David Stanek
parent 71436b3ca2
commit c8682888bc
5 changed files with 91 additions and 33 deletions

View File

@ -437,6 +437,12 @@ class MetadataFileError(UnexpectedError):
debug_message_format = _("Error while reading metadata file, %(reason)s")
class DirectMappingError(UnexpectedError):
message_format = _("Local section in mapping %(mapping_id)s refers to a "
"remote match that doesn't exist "
"(e.g. {0} in a local section).")
class AssignmentTypeCalculationError(UnexpectedError):
debug_message_format = _(
'Unexpected combination of grant attributes - '

View File

@ -94,7 +94,7 @@ class Manager(manager.Manager):
def evaluate(self, idp_id, protocol_id, assertion_data):
mapping = self.get_mapping_from_idp_and_protocol(idp_id, protocol_id)
rules = mapping['rules']
rule_processor = utils.RuleProcessor(rules)
rule_processor = utils.RuleProcessor(mapping['id'], rules)
mapped_properties = rule_processor.process(assertion_data)
return mapped_properties, mapping['id']

View File

@ -379,16 +379,19 @@ class RuleProcessor(object):
BLACKLIST = 'blacklist'
WHITELIST = 'whitelist'
def __init__(self, rules):
def __init__(self, mapping_id, rules):
"""Initialize RuleProcessor.
Example rules can be found at:
:class:`keystone.tests.mapping_fixtures`
:param mapping_id: id for the mapping
:type mapping_id: string
:param rules: rules from a mapping
:type rules: dict
"""
self.mapping_id = mapping_id
self.rules = rules
def process(self, assertion_data):
@ -614,6 +617,9 @@ class RuleProcessor(object):
{'user': {'name': 'Bob Thompson', 'email': 'bob@example.org'}}
:raises keystone.exception.DirectMappingError: when referring to a
remote match from a local section of a rule
"""
LOG.debug('direct_maps: %s', direct_maps)
LOG.debug('local: %s', local)
@ -622,7 +628,12 @@ class RuleProcessor(object):
if isinstance(v, dict):
new_value = self._update_local_mapping(v, direct_maps)
else:
new_value = v.format(*direct_maps)
try:
new_value = v.format(*direct_maps)
except IndexError:
raise exception.DirectMappingError(
mapping_id=self.mapping_id)
new[k] = new_value
return new

View File

@ -19,6 +19,9 @@ from keystone.tests import unit
from keystone.tests.unit import mapping_fixtures
FAKE_MAPPING_ID = uuid.uuid4().hex
class MappingRuleEngineTests(unit.BaseTestCase):
"""A class for testing the mapping rule engine."""
@ -52,7 +55,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_LARGE
assertion = mapping_fixtures.ADMIN_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
fn = assertion.get('FirstName')
@ -75,7 +78,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_LARGE
assertion = mapping_fixtures.BAD_TESTER_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertValidMappedUserObject(mapped_properties)
@ -93,7 +96,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_TESTER_REGEX
assertion = mapping_fixtures.TESTER_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
self.assertValidMappedUserObject(values)
@ -115,7 +118,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_SMALL
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
self.assertValidMappedUserObject(values)
@ -136,7 +139,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_LARGE
assertion = mapping_fixtures.CUSTOMER_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
self.assertValidMappedUserObject(values)
@ -157,7 +160,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_SMALL
assertion = mapping_fixtures.EMPLOYEE_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
self.assertValidMappedUserObject(values)
@ -179,7 +182,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_DEVELOPER_REGEX
assertion = mapping_fixtures.DEVELOPER_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
self.assertValidMappedUserObject(values)
@ -201,7 +204,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_DEVELOPER_REGEX
assertion = mapping_fixtures.BAD_DEVELOPER_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertValidMappedUserObject(mapped_properties)
@ -216,7 +219,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_LARGE
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
values = rp.process(assertion)
user_name = assertion.get('UserName')
@ -260,13 +263,27 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_SMALL
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CONTRACTOR_MALFORMED_ASSERTION
mapped_properties = rp.process(assertion)
self.assertValidMappedUserObject(mapped_properties)
self.assertIsNone(mapped_properties['user'].get('name'))
self.assertListEqual(list(), mapped_properties['group_ids'])
def test_using_remote_direct_mapping_that_doesnt_exist_fails(self):
"""Test for the correct error when referring to a bad remote match.
The remote match must exist in a rule when a local section refers to
a remote matching using the format (e.g. {0} in a local section).
"""
mapping = mapping_fixtures.MAPPING_DIRECT_MAPPING_THROUGH_KEYWORD
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CUSTOMER_ASSERTION
self.assertRaises(exception.DirectMappingError,
rp.process,
assertion)
def test_rule_engine_returns_group_names(self):
"""Check whether RuleProcessor returns group names with their domains.
@ -276,7 +293,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUP_NAMES
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.EMPLOYEE_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -310,7 +327,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST
assertion = mapping_fixtures.EMPLOYEE_ASSERTION_MULTIPLE_GROUPS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -346,7 +363,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_BLACKLIST
assertion = mapping_fixtures.EMPLOYEE_ASSERTION_MULTIPLE_GROUPS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -374,7 +391,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_BLACKLIST_MULTIPLES
assertion = mapping_fixtures.EMPLOYEE_ASSERTION_MULTIPLE_GROUPS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -400,7 +417,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_MISSING_DOMAIN
assertion = mapping_fixtures.EMPLOYEE_ASSERTION_MULTIPLE_GROUPS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
self.assertRaises(exception.ValidationError, rp.process, assertion)
def test_rule_engine_blacklist_direct_group_mapping_missing_domain(self):
@ -411,7 +428,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_BLACKLIST_MISSING_DOMAIN
assertion = mapping_fixtures.EMPLOYEE_ASSERTION_MULTIPLE_GROUPS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
self.assertRaises(exception.ValidationError, rp.process, assertion)
def test_rule_engine_no_groups_allowed(self):
@ -424,7 +441,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST
assertion = mapping_fixtures.EMPLOYEE_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
self.assertListEqual(mapped_properties['group_names'], [])
@ -439,7 +456,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.EMPLOYEE_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -458,7 +475,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -476,7 +493,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER_LOCAL_DOMAIN
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -485,7 +502,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
def test_local_user_local_domain(self):
"""Test that local users can have non-service domains assigned."""
mapping = mapping_fixtures.MAPPING_LOCAL_USER_LOCAL_DOMAIN
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -507,7 +524,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_USER_IDS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -530,7 +547,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_USER_IDS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.EMPLOYEE_ASSERTION
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
@ -554,7 +571,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_USER_IDS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.ADMIN_ASSERTION
mapped_properties = rp.process(assertion)
context = {'environment': {}}
@ -589,7 +606,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
(mapping_fixtures.EMPLOYEE_ASSERTION, 'tbo')]
for assertion, exp_user_name in testcases:
mapping = mapping_fixtures.MAPPING_USER_IDS
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
context = {'environment': {}}
self.assertIsNotNone(mapped_properties)
@ -601,7 +618,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
def test_whitelist_pass_through(self):
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = mapping_fixtures.DEVELOPER_ASSERTION
mapped_properties = rp.process(assertion)
self.assertValidMappedUserObject(mapped_properties)
@ -613,7 +630,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
def test_type_not_in_assertion(self):
"""Test that if the remote "type" is not in the assertion it fails."""
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
assertion = {uuid.uuid4().hex: uuid.uuid4().hex}
mapped_properties = rp.process(assertion)
self.assertValidMappedUserObject(mapped_properties)
@ -629,7 +646,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_IDS_WHITELIST
assertion = mapping_fixtures.GROUP_IDS_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
self.assertEqual('opilotte', mapped_properties['user']['name'])
@ -645,7 +662,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_IDS_BLACKLIST
assertion = mapping_fixtures.GROUP_IDS_ASSERTION
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
self.assertEqual('opilotte', mapped_properties['user']['name'])
@ -662,7 +679,7 @@ class MappingRuleEngineTests(unit.BaseTestCase):
"""
mapping = mapping_fixtures.MAPPING_GROUPS_IDS_WHITELIST
assertion = mapping_fixtures.GROUP_IDS_ASSERTION_ONLY_ONE_GROUP
rp = mapping_utils.RuleProcessor(mapping['rules'])
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
mapped_properties = rp.process(assertion)
self.assertIsNotNone(mapped_properties)
self.assertEqual('opilotte', mapped_properties['user']['name'])

View File

@ -463,6 +463,30 @@ MAPPING_TESTER_REGEX = {
]
}
MAPPING_DIRECT_MAPPING_THROUGH_KEYWORD = {
"rules": [
{
"local": [
{
"user": "{0}"
},
{
"group": TESTER_GROUP_ID
}
],
"remote": [
{
"type": "UserName",
"any_one_of": [
"bwilliams"
]
}
]
}
]
}
MAPPING_DEVELOPER_REGEX = {
"rules": [
{