diff --git a/keystone/auth/plugins/mapped.py b/keystone/auth/plugins/mapped.py index 612c73bd47..e9716201a8 100644 --- a/keystone/auth/plugins/mapped.py +++ b/keystone/auth/plugins/mapped.py @@ -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 = ( diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index 18cbf0f281..f97356ec39 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -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 diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py index 9a45b7681d..2af2a7a0e3 100644 --- a/keystone/tests/unit/contrib/federation/test_utils.py +++ b/keystone/tests/unit/contrib/federation/test_utils.py @@ -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 diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 7dc9067b9c..d6e786e59d 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -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={