Mapping which yield no identities should result in ValidationError
Currently mapping produce a bogus "blind" default identity when no rules match the incoming attributes. This is unnecessary and downright dangerous. There's absolutely no use case for the "blind" identity. Furthermore, consumers of mapped properties assumed that the "blind" identity is legit. This lead to expected failures such as KeyError when they try to reference the required identity attributes such as user['name']. We should raise ValidationError if the rules yield no valid identity. This patch also removed the tests where the bogus "blind" identity is expected. Change-Id: I117621673ffc0b4f8e2c48721329daa3b6090327 Closes-Bug: 1557238
This commit is contained in:
guang-yee
parent
196c4ad3f9
commit
e5dcb3b4b6
|
|
@ -135,9 +135,14 @@ def handle_unscoped_token(context, auth_payload, auth_context,
|
|||
user_id = None
|
||||
|
||||
try:
|
||||
mapped_properties, mapping_id = apply_mapping_filter(
|
||||
identity_provider, protocol, assertion, resource_api,
|
||||
federation_api, identity_api)
|
||||
try:
|
||||
mapped_properties, mapping_id = apply_mapping_filter(
|
||||
identity_provider, protocol, assertion, resource_api,
|
||||
federation_api, identity_api)
|
||||
except exception.ValidationError as e:
|
||||
# if mapping is either invalid or yield no valid identity,
|
||||
# it is considered a failed authentication
|
||||
raise exception.Unauthorized(e)
|
||||
|
||||
if is_ephemeral_user(mapped_properties):
|
||||
unique_id, display_name = (
|
||||
|
|
|
|||
|
|
@ -599,6 +599,15 @@ class RuleProcessor(object):
|
|||
group_names = list()
|
||||
groups_by_domain = dict()
|
||||
|
||||
# if mapping yield no valid identity values, we should bail right away
|
||||
# instead of continuing on with a normalized bogus user
|
||||
if not identity_values:
|
||||
msg = _("Could not map any federated user properties to identity "
|
||||
"values. Check debug logs or the mapping used for "
|
||||
"additional details.")
|
||||
LOG.warning(msg)
|
||||
raise exception.ValidationError(msg)
|
||||
|
||||
for identity_value in identity_values:
|
||||
if 'user' in identity_value:
|
||||
# if a mapping outputs more than one user name, log it
|
||||
|
|
|
|||
|
|
@ -77,17 +77,15 @@ class MappingRuleEngineTests(unit.BaseTestCase):
|
|||
This will not match since the email in the assertion will fail
|
||||
the regex test. It is set to match any @example.com address.
|
||||
But the incoming value is set to eviltester@example.org.
|
||||
RuleProcessor should return list of empty group_ids.
|
||||
RuleProcessor should raise ValidationError.
|
||||
|
||||
"""
|
||||
mapping = mapping_fixtures.MAPPING_LARGE
|
||||
assertion = mapping_fixtures.BAD_TESTER_ASSERTION
|
||||
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
|
||||
mapped_properties = rp.process(assertion)
|
||||
|
||||
self.assertValidMappedUserObject(mapped_properties)
|
||||
self.assertIsNone(mapped_properties['user'].get('name'))
|
||||
self.assertListEqual(list(), mapped_properties['group_ids'])
|
||||
self.assertRaises(exception.ValidationError,
|
||||
rp.process,
|
||||
assertion)
|
||||
|
||||
def test_rule_engine_regex_many_groups(self):
|
||||
"""Should return group CONTRACTOR_GROUP_ID.
|
||||
|
|
@ -203,17 +201,15 @@ class MappingRuleEngineTests(unit.BaseTestCase):
|
|||
The email in the assertion will fail the regex test.
|
||||
It is set to reject any @example.org address, but the
|
||||
incoming value is set to evildeveloper@example.org.
|
||||
RuleProcessor should return list of empty group_ids.
|
||||
RuleProcessor should yield ValidationError.
|
||||
|
||||
"""
|
||||
mapping = mapping_fixtures.MAPPING_DEVELOPER_REGEX
|
||||
assertion = mapping_fixtures.BAD_DEVELOPER_ASSERTION
|
||||
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
|
||||
mapped_properties = rp.process(assertion)
|
||||
|
||||
self.assertValidMappedUserObject(mapped_properties)
|
||||
self.assertIsNone(mapped_properties['user'].get('name'))
|
||||
self.assertListEqual(list(), mapped_properties['group_ids'])
|
||||
self.assertRaises(exception.ValidationError,
|
||||
rp.process,
|
||||
assertion)
|
||||
|
||||
def _rule_engine_regex_match_and_many_groups(self, assertion):
|
||||
"""Should return group DEVELOPER_GROUP_ID and TESTER_GROUP_ID.
|
||||
|
|
@ -263,16 +259,15 @@ class MappingRuleEngineTests(unit.BaseTestCase):
|
|||
|
||||
Expect RuleProcessor to discard non string object, which
|
||||
is required for a correct rule match. RuleProcessor will result with
|
||||
empty list of groups.
|
||||
ValidationError.
|
||||
|
||||
"""
|
||||
mapping = mapping_fixtures.MAPPING_SMALL
|
||||
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'])
|
||||
self.assertRaises(exception.ValidationError,
|
||||
rp.process,
|
||||
assertion)
|
||||
|
||||
def test_using_remote_direct_mapping_that_doesnt_exist_fails(self):
|
||||
"""Test for the correct error when referring to a bad remote match.
|
||||
|
|
@ -466,28 +461,6 @@ class MappingRuleEngineTests(unit.BaseTestCase):
|
|||
self.assertIsNotNone(mapped_properties)
|
||||
self.assertValidMappedUserObject(mapped_properties)
|
||||
|
||||
def test_create_user_object_with_bad_mapping(self):
|
||||
"""Test if user object is created even with bad mapping.
|
||||
|
||||
User objects will be created by mapping engine always as long as there
|
||||
is corresponding local rule. This test shows, that even with assertion
|
||||
where no group names nor ids are matched, but there is 'blind' rule for
|
||||
mapping user, such object will be created.
|
||||
|
||||
In this test MAPPING_EHPEMERAL_USER expects UserName set to jsmith
|
||||
whereas value from assertion is 'tbo'.
|
||||
|
||||
"""
|
||||
mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER
|
||||
rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
|
||||
assertion = mapping_fixtures.CONTRACTOR_ASSERTION
|
||||
mapped_properties = rp.process(assertion)
|
||||
self.assertIsNotNone(mapped_properties)
|
||||
self.assertValidMappedUserObject(mapped_properties)
|
||||
|
||||
self.assertNotIn('id', mapped_properties['user'])
|
||||
self.assertNotIn('name', mapped_properties['user'])
|
||||
|
||||
def test_set_ephemeral_domain_to_ephemeral_users(self):
|
||||
"""Test auto assigning service domain to ephemeral users.
|
||||
|
||||
|
|
@ -645,11 +618,9 @@ class MappingRuleEngineTests(unit.BaseTestCase):
|
|||
mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
|
||||
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)
|
||||
|
||||
self.assertNotIn('id', mapped_properties['user'])
|
||||
self.assertNotIn('name', mapped_properties['user'])
|
||||
self.assertRaises(exception.ValidationError,
|
||||
rp.process,
|
||||
assertion)
|
||||
|
||||
def test_rule_engine_group_ids_mapping_whitelist(self):
|
||||
"""Test mapping engine when group_ids is explicitly set
|
||||
|
|
|
|||
|
|
@ -1698,7 +1698,7 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
|
|||
def test_issue_unscoped_token_with_remote_unavailable(self):
|
||||
self.config_fixture.config(group='federation',
|
||||
remote_id_attribute=self.REMOTE_ID_ATTR)
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.assertRaises(exception.Unauthorized,
|
||||
self._issue_unscoped_token,
|
||||
idp=self.IDP_WITH_REMOTE,
|
||||
environment={
|
||||
|
|
|
|||
Loading…
Reference in New Issue