Relax the requirement for mappings to result in group memberships
Now that we're able to grant authorization to federated users using concrete role assignments, we can drop the requirement for the mapping engine to result in any authorization (via group membership) at all. Closes-Bug: #1601929 Change-Id: Ie144e20deb4a0bb987182de5c9231a14f0aa2bc8
This commit is contained in:
parent
0061419170
commit
7ba5370198
|
@ -89,7 +89,7 @@ def handle_scoped_token(request, auth_payload, auth_context, token_ref,
|
|||
try:
|
||||
mapping = federation_api.get_mapping_from_idp_and_protocol(
|
||||
identity_provider, protocol)
|
||||
utils.validate_groups(group_ids, mapping['id'], identity_api)
|
||||
utils.validate_mapped_group_ids(group_ids, mapping['id'], identity_api)
|
||||
|
||||
except Exception:
|
||||
# NOTE(topol): Diaper defense to catch any exception, so we can
|
||||
|
@ -159,7 +159,6 @@ def handle_unscoped_token(request, auth_payload, auth_context,
|
|||
display_name)
|
||||
user_id = user['id']
|
||||
group_ids = mapped_properties['group_ids']
|
||||
utils.validate_groups_cardinality(group_ids, mapping_id)
|
||||
build_ephemeral_user_context(auth_context, user,
|
||||
mapped_properties,
|
||||
identity_provider, protocol)
|
||||
|
@ -208,9 +207,7 @@ def apply_mapping_filter(identity_provider, protocol, assertion,
|
|||
# ``mapping_id`` was used as well as idenity_api and resource_api
|
||||
# objects.
|
||||
group_ids = mapped_properties['group_ids']
|
||||
utils.validate_groups_in_backend(group_ids,
|
||||
mapping_id,
|
||||
identity_api)
|
||||
utils.validate_mapped_group_ids(group_ids, mapping_id, identity_api)
|
||||
group_ids.extend(
|
||||
utils.transform_to_group_ids(
|
||||
mapped_properties['group_names'], mapping_id,
|
||||
|
|
|
@ -131,9 +131,9 @@ class TokenlessAuthHelper(object):
|
|||
if user_type == utils.UserType.EPHEMERAL:
|
||||
user_ref = {'type': utils.UserType.EPHEMERAL}
|
||||
group_ids = mapped_properties['group_ids']
|
||||
utils.validate_groups_in_backend(group_ids,
|
||||
mapping_id,
|
||||
self.identity_api)
|
||||
utils.validate_mapped_group_ids(group_ids,
|
||||
mapping_id,
|
||||
self.identity_api)
|
||||
group_ids.extend(
|
||||
utils.transform_to_group_ids(
|
||||
mapped_properties['group_names'], mapping_id,
|
||||
|
|
|
@ -254,11 +254,6 @@ class AuthPluginException(Unauthorized):
|
|||
self.authentication = {}
|
||||
|
||||
|
||||
class MissingGroups(Unauthorized):
|
||||
message_format = _("Unable to find valid groups while using "
|
||||
"mapping %(mapping_id)s")
|
||||
|
||||
|
||||
class UserDisabled(Unauthorized):
|
||||
message_format = _("The account is disabled for user: %(user_id)s")
|
||||
|
||||
|
|
|
@ -247,19 +247,6 @@ def validate_expiration(token_ref):
|
|||
raise exception.Unauthorized(_('Federation token is expired'))
|
||||
|
||||
|
||||
def validate_groups_cardinality(group_ids, mapping_id):
|
||||
"""Check if groups list is non-empty.
|
||||
|
||||
:param group_ids: list of group ids
|
||||
:type group_ids: list of str
|
||||
|
||||
:raises keystone.exception.MissingGroups: if ``group_ids`` cardinality is 0
|
||||
|
||||
"""
|
||||
if not group_ids:
|
||||
raise exception.MissingGroups(mapping_id=mapping_id)
|
||||
|
||||
|
||||
def get_remote_id_parameter(protocol):
|
||||
# NOTE(marco-fargetta): Since we support any protocol ID, we attempt to
|
||||
# retrieve the remote_id_attribute of the protocol ID. If it's not
|
||||
|
@ -306,7 +293,7 @@ def validate_idp(idp, protocol, assertion):
|
|||
raise exception.Forbidden(msg)
|
||||
|
||||
|
||||
def validate_groups_in_backend(group_ids, mapping_id, identity_api):
|
||||
def validate_mapped_group_ids(group_ids, mapping_id, identity_api):
|
||||
"""Iterate over group ids and make sure they are present in the backend.
|
||||
|
||||
This call is not transactional.
|
||||
|
@ -332,32 +319,6 @@ def validate_groups_in_backend(group_ids, mapping_id, identity_api):
|
|||
group_id=group_id, mapping_id=mapping_id)
|
||||
|
||||
|
||||
def validate_groups(group_ids, mapping_id, identity_api):
|
||||
"""Check group ids cardinality and check their existence in the backend.
|
||||
|
||||
This call is not transactional.
|
||||
:param group_ids: IDs of the groups to be checked
|
||||
:type group_ids: list of str
|
||||
|
||||
:param mapping_id: id of the mapping used for this operation
|
||||
:type mapping_id: str
|
||||
|
||||
:param identity_api: Identity Manager object used for communication with
|
||||
backend
|
||||
:type identity_api: identity.Manager
|
||||
|
||||
:raises keystone.exception.MappedGroupNotFound: If the group returned by
|
||||
mapping was not found in the backend.
|
||||
:raises keystone.exception.MissingGroups: If ``group_ids`` cardinality
|
||||
is 0.
|
||||
|
||||
"""
|
||||
# TODO(rderose): remove cardinality check, as federated users can now
|
||||
# receive direct role assignments
|
||||
validate_groups_cardinality(group_ids, mapping_id)
|
||||
validate_groups_in_backend(group_ids, mapping_id, identity_api)
|
||||
|
||||
|
||||
# TODO(marek-denis): Optimize this function, so the number of calls to the
|
||||
# backend are minimized.
|
||||
def transform_to_group_ids(group_names, mapping_id,
|
||||
|
@ -422,8 +383,8 @@ def transform_to_group_ids(group_names, mapping_id,
|
|||
group['name'], resolve_domain(group['domain']))
|
||||
yield group_dict['id']
|
||||
except exception.GroupNotFound:
|
||||
LOG.debug('Skip mapping group %s; has no entry in the backend',
|
||||
group['name'])
|
||||
raise exception.MappedGroupNotFound(group_id=group['name'],
|
||||
mapping_id=mapping_id)
|
||||
|
||||
|
||||
def get_assertion_params_from_env(request):
|
||||
|
|
|
@ -1522,6 +1522,12 @@ ANOTHER_LOCAL_USER_ASSERTION = {
|
|||
'Position': 'DirectorGeneral'
|
||||
}
|
||||
|
||||
USER_NO_GROUPS_ASSERTION = {
|
||||
'Email': 'nogroupsuser1@example.org',
|
||||
'UserName': 'nogroupsuser1',
|
||||
'orgPersonType': 'NoGroupsOrg'
|
||||
}
|
||||
|
||||
UNMATCHED_GROUP_ASSERTION = {
|
||||
'REMOTE_USER': 'Any Momoose',
|
||||
'REMOTE_USER_GROUPS': 'EXISTS;NO_EXISTS'
|
||||
|
|
|
@ -691,6 +691,31 @@ class FederatedSetupMixin(object):
|
|||
]
|
||||
}
|
||||
]
|
||||
},
|
||||
# rules for users with no groups
|
||||
{
|
||||
"local": [
|
||||
{
|
||||
'user': {
|
||||
'name': '{0}',
|
||||
'id': '{1}'
|
||||
}
|
||||
}
|
||||
],
|
||||
"remote": [
|
||||
{
|
||||
'type': 'UserName',
|
||||
},
|
||||
{
|
||||
'type': 'Email',
|
||||
},
|
||||
{
|
||||
'type': 'orgPersonType',
|
||||
'any_one_of': [
|
||||
'NoGroupsOrg'
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
|
@ -1668,7 +1693,7 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
|
|||
self.assertEqual(ref_groups, token_groups)
|
||||
|
||||
def test_issue_unscoped_tokens_nonexisting_group(self):
|
||||
self.assertRaises(exception.MissingGroups,
|
||||
self.assertRaises(exception.MappedGroupNotFound,
|
||||
self._issue_unscoped_token,
|
||||
assertion='ANOTHER_TESTER_ASSERTION')
|
||||
|
||||
|
@ -1745,9 +1770,11 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
|
|||
self.assertIsNotNone(r.headers.get('X-Subject-Token'))
|
||||
|
||||
def test_issue_unscoped_token_no_groups(self):
|
||||
self.assertRaises(exception.Unauthorized,
|
||||
self._issue_unscoped_token,
|
||||
assertion='BAD_TESTER_ASSERTION')
|
||||
r = self._issue_unscoped_token(assertion='USER_NO_GROUPS_ASSERTION')
|
||||
self.assertIsNotNone(r.headers.get('X-Subject-Token'))
|
||||
token_resp = r.json_body
|
||||
token_groups = token_resp['token']['user']['OS-FEDERATION']['groups']
|
||||
self.assertEqual(0, len(token_groups))
|
||||
|
||||
def test_issue_unscoped_token_malformed_environment(self):
|
||||
"""Test whether non string objects are filtered out.
|
||||
|
@ -2314,9 +2341,8 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
|
|||
- Create group ``EXISTS``
|
||||
- Set mapping rules for existing IdP with an empty whitelist
|
||||
that whould discard any values from the assertion
|
||||
- Try issuing unscoped token, expect server to raise
|
||||
``exception.MissingGroups`` as no groups were matched and ephemeral
|
||||
user does not have any group assigned.
|
||||
- Try issuing unscoped token, no groups were matched and that the
|
||||
federated user does not have any group assigned.
|
||||
|
||||
"""
|
||||
domain_id = self.domainA['id']
|
||||
|
@ -2357,10 +2383,9 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
|
|||
]
|
||||
}
|
||||
self.federation_api.update_mapping(self.mapping['id'], rules)
|
||||
|
||||
self.assertRaises(exception.MissingGroups,
|
||||
self._issue_unscoped_token,
|
||||
assertion='UNMATCHED_GROUP_ASSERTION')
|
||||
r = self._issue_unscoped_token(assertion='UNMATCHED_GROUP_ASSERTION')
|
||||
assigned_groups = r.json['token']['user']['OS-FEDERATION']['groups']
|
||||
self.assertEqual(len(assigned_groups), 0)
|
||||
|
||||
def test_not_setting_whitelist_accepts_all_values(self):
|
||||
"""Test that not setting whitelist passes.
|
||||
|
|
Loading…
Reference in New Issue