Merge "Fix role assignment cache for federated users"

This commit is contained in:
Zuul 2025-12-09 11:54:49 +00:00 committed by Gerrit Code Review
commit 64a06477d1
5 changed files with 670 additions and 10 deletions

View file

@ -15,20 +15,24 @@
"""Main entry point into the Assignment service."""
import copy
import datetime
import itertools
from oslo_log import log
from oslo_utils import timeutils
from keystone.common import cache
from keystone.common import driver_hints
from keystone.common import manager
from keystone.common import provider_api
from keystone.common.resource_options import options as ro_opt
import keystone.conf
from keystone import exception
from keystone.i18n import _
from keystone.models import revoke_model
from keystone import notifications
import keystone.conf
CONF = keystone.conf.CONF
LOG = log.getLogger(__name__)
PROVIDERS = provider_api.ProviderAPIs
@ -1293,6 +1297,32 @@ class Manager(manager.Manager):
self.delete_system_grant_for_user(user_id, assignment['id'])
COMPUTED_ASSIGNMENTS_REGION.invalidate()
def invalidate_user_cache_on_group_change(self, user_id):
"""Invalidate user cache when group membership changes."""
LOG.debug(
'Revoking tokens for federated user %(user_id)s due to group '
'membership changes in the identity provider',
{'user_id': user_id},
)
COMPUTED_ASSIGNMENTS_REGION.invalidate()
# Create revocation event with -1 second to avoid race condition
# where a new token issued at the same second as the revocation
# event would be incorrectly revoked. This ensures only tokens
# issued before the group membership change are revoked.
issued_before = timeutils.utcnow().replace(
microsecond=0
) - datetime.timedelta(seconds=1)
PROVIDERS.revoke_api.revoke(
revoke_model.RevokeEvent(
user_id=user_id, issued_before=issued_before
)
)
reason = f'User {user_id} group membership changed'
notifications.invalidate_token_cache_notification(reason)
def check_system_grant_for_user(self, user_id, role_id):
"""Check if a user has a specific role on the system.

View file

@ -34,12 +34,13 @@ from keystone.common import driver_hints
from keystone.common import manager
from keystone.common import provider_api
from keystone.common.validation import validators
import keystone.conf
from keystone import exception
from keystone.i18n import _
from keystone.identity.mapping_backends import mapping
from keystone import notifications
import keystone.conf
CONF = keystone.conf.CONF
LOG = log.getLogger(__name__)
@ -1760,18 +1761,55 @@ class Manager(manager.Manager):
:returns: dictionary of the mapped User entity
"""
user_already_existed = True
try:
PROVIDERS.shadow_users_api.get_federated_user(
idp_id, protocol_id, user['id']
)
except exception.UserNotFound:
user_already_existed = False
user_dict = self._shadow_federated_user(idp_id, protocol_id, user)
# Note(knikolla): The shadowing operation can be cached,
# however we need to update the expiring group memberships.
if group_ids:
for group_id in group_ids:
LOG.info(
"Adding user [%s] to group [%s].", user_dict, group_id
)
PROVIDERS.shadow_users_api.add_user_to_group_expires(
user_dict['id'], group_id
)
if group_ids is None:
group_ids = []
membership_changed = False
for group_id in group_ids:
LOG.info("Adding user [%s] to group [%s].", user_dict, group_id)
if PROVIDERS.shadow_users_api.add_user_to_group_expires(
user_dict['id'], group_id
):
membership_changed = True
removed_group_ids = (
PROVIDERS.shadow_users_api.cleanup_stale_group_memberships(
user_dict['id'], idp_id, group_ids
)
)
if removed_group_ids:
LOG.debug(
'User %s was removed from groups %s, marking as membership changed',
user_dict['id'],
removed_group_ids,
)
membership_changed = True
if membership_changed and user_already_existed:
LOG.debug(
'Group membership changed for federated user %s, '
'revoking tokens',
user_dict['id'],
)
PROVIDERS.assignment_api.invalidate_user_cache_on_group_change(
user_dict['id']
)
return user_dict

View file

@ -251,8 +251,11 @@ class ShadowUsers(base.ShadowUsersDriverBase):
membership = query.first()
if membership:
# Existing membership, just refresh TTL
membership.last_verified = timeutils.utcnow()
return False # No change, just renewal
else:
# New membership - this is a change
session.add(
model.ExpiringUserGroupMembership(
user_id=user_id,
@ -261,3 +264,23 @@ class ShadowUsers(base.ShadowUsersDriverBase):
last_verified=timeutils.utcnow(),
)
)
return True # New membership added
def cleanup_stale_group_memberships(
self, user_id, idp_id, current_group_ids
):
"""Remove expiring group memberships that are no longer in the IdP assertion."""
with sql.session_for_write() as session:
query = session.query(model.ExpiringUserGroupMembership)
query = query.filter_by(user_id=user_id, idp_id=idp_id)
existing_memberships = query.all()
current_group_set = set(current_group_ids)
removed_group_ids = []
for membership in existing_memberships:
if membership.group_id not in current_group_set:
removed_group_ids.append(membership.group_id)
session.delete(membership)
return removed_group_ids

View file

@ -0,0 +1,221 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from unittest import mock
import uuid
from keystone.common import provider_api
from keystone.tests import unit
from keystone.tests.unit import default_fixtures
from keystone.tests.unit.ksfixtures import database
PROVIDERS = provider_api.ProviderAPIs
class FederatedGroupCacheTest(unit.TestCase):
"""Test federated group membership cache invalidation."""
def setUp(self):
# Mock LDAP to avoid environment-specific errors
ldap_patcher = mock.patch('ldap.initialize')
self.addCleanup(ldap_patcher.stop)
ldap_patcher.start()
# Mock LDAP options that are checked during setup
with mock.patch('ldap.get_option', return_value=None):
with mock.patch('ldap.set_option'):
super().setUp()
self.useFixture(database.Database())
self.load_backends()
# Create domain
PROVIDERS.resource_api.create_domain(
default_fixtures.ROOT_DOMAIN['id'], default_fixtures.ROOT_DOMAIN
)
# Create IDP, mapping, and protocol
self.idp = {
'id': uuid.uuid4().hex,
'enabled': True,
'description': uuid.uuid4().hex,
}
self.mapping = {'id': uuid.uuid4().hex}
self.protocol = {
'id': uuid.uuid4().hex,
'idp_id': self.idp['id'],
'mapping_id': self.mapping['id'],
}
PROVIDERS.federation_api.create_idp(self.idp['id'], self.idp)
PROVIDERS.federation_api.create_mapping(
self.mapping['id'], self.mapping
)
PROVIDERS.federation_api.create_protocol(
self.idp['id'], self.protocol['id'], self.protocol
)
self.domain_id = PROVIDERS.federation_api.get_idp(self.idp['id'])[
'domain_id'
]
# Create a federated user
self.federated_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': uuid.uuid4().hex,
}
self.user = PROVIDERS.shadow_users_api.create_federated_user(
self.domain_id, self.federated_user
)
# Create groups
self.group1 = unit.new_group_ref(domain_id=self.domain_id)
self.group1 = PROVIDERS.identity_api.create_group(self.group1)
self.group2 = unit.new_group_ref(domain_id=self.domain_id)
self.group2 = PROVIDERS.identity_api.create_group(self.group2)
def test_group_membership_returns_true_for_new_membership(self):
"""Test that adding a new group membership returns True."""
result = PROVIDERS.shadow_users_api.add_user_to_group_expires(
self.user['id'], self.group1['id']
)
self.assertTrue(result)
def test_group_membership_returns_false_for_renewal(self):
"""Test that renewing an existing group membership returns False."""
PROVIDERS.shadow_users_api.add_user_to_group_expires(
self.user['id'], self.group1['id']
)
result = PROVIDERS.shadow_users_api.add_user_to_group_expires(
self.user['id'], self.group1['id']
)
self.assertFalse(result)
def test_new_group_membership_triggers_token_revocation(self):
"""Test that new group membership triggers token revocation."""
with mock.patch.object(
PROVIDERS.assignment_api, 'invalidate_user_cache_on_group_change'
) as mock_revoke:
user_dict = {
'id': self.federated_user['unique_id'],
'name': self.federated_user['display_name'],
'domain': {'id': self.domain_id},
}
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
mock_revoke.assert_called_once_with(self.user['id'])
def test_membership_renewal_does_not_trigger_revocation(self):
"""Test that pure membership renewal doesn't trigger token revocation."""
with mock.patch.object(
PROVIDERS.shadow_users_api,
'add_user_to_group_expires',
return_value=False,
):
with mock.patch.object(
PROVIDERS.shadow_users_api,
'cleanup_stale_group_memberships',
return_value=False,
):
with mock.patch.object(
PROVIDERS.assignment_api,
'invalidate_user_cache_on_group_change',
) as mock_revoke:
user_dict = {
'id': self.federated_user['unique_id'],
'name': self.federated_user['display_name'],
'domain': {'id': self.domain_id},
}
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
mock_revoke.assert_not_called()
def test_adding_additional_group_triggers_revocation(self):
"""Test that adding an additional group triggers token revocation."""
PROVIDERS.shadow_users_api.add_user_to_group_expires(
self.user['id'], self.group1['id']
)
with mock.patch.object(
PROVIDERS.assignment_api, 'invalidate_user_cache_on_group_change'
) as mock_revoke:
user_dict = {
'id': self.federated_user['unique_id'],
'name': self.federated_user['display_name'],
'domain': {'id': self.domain_id},
}
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id'], self.group2['id']],
)
mock_revoke.assert_called_once_with(self.user['id'])
def test_empty_group_list_does_not_trigger_revocation(self):
"""Test that shadowing with no groups doesn't trigger revocation."""
with mock.patch.object(
PROVIDERS.assignment_api, 'invalidate_user_cache_on_group_change'
) as mock_revoke:
user_dict = {
'id': self.federated_user['unique_id'],
'name': self.federated_user['display_name'],
'domain': {'id': self.domain_id},
}
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'], self.protocol['id'], user_dict, group_ids=[]
)
mock_revoke.assert_not_called()
@mock.patch('keystone.notifications.invalidate_token_cache_notification')
def test_token_revocation_persists_event_and_invalidates_cache(
self, mock_cache_notification
):
"""Test that token revocation creates revoke event and invalidates cache."""
with mock.patch.object(PROVIDERS.revoke_api, 'revoke') as mock_revoke:
PROVIDERS.assignment_api.invalidate_user_cache_on_group_change(
self.user['id']
)
# Verify revoke was called with a RevokeEvent
mock_revoke.assert_called_once()
revoke_event = mock_revoke.call_args[0][0]
# Check that the revoke event has the correct user_id
self.assertEqual(revoke_event.user_id, self.user['id'])
# Check that issued_before is set (indicating -1 second logic)
self.assertIsNotNone(revoke_event.issued_before)
# Verify cache invalidation notification was called
mock_cache_notification.assert_called_once()
call_args = mock_cache_notification.call_args[0][0]
self.assertIn(self.user['id'], call_args)
self.assertIn('group membership changed', call_args)

View file

@ -0,0 +1,348 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""Integration tests for federated group membership cache invalidation."""
from unittest import mock
import uuid
from keystone.common import provider_api
from keystone.tests import unit
from keystone.tests.unit import default_fixtures
from keystone.tests.unit.ksfixtures import database
PROVIDERS = provider_api.ProviderAPIs
class FederatedGroupCacheIntegrationTest(unit.TestCase):
"""Integration test for federated group membership cache invalidation."""
def setUp(self):
# Mock LDAP to avoid environment-specific errors
ldap_patcher = mock.patch('ldap.initialize')
self.addCleanup(ldap_patcher.stop)
ldap_patcher.start()
# Mock LDAP options that are checked during setup
with mock.patch('ldap.get_option', return_value=None):
with mock.patch('ldap.set_option'):
super().setUp()
self.useFixture(database.Database())
self.load_backends()
# Create domain
PROVIDERS.resource_api.create_domain(
default_fixtures.ROOT_DOMAIN['id'], default_fixtures.ROOT_DOMAIN
)
# Create IDP, mapping, and protocol
self.idp = {
'id': uuid.uuid4().hex,
'enabled': True,
'description': 'Test Identity Provider',
}
self.mapping = {'id': uuid.uuid4().hex, 'rules': []}
self.protocol = {
'id': uuid.uuid4().hex,
'idp_id': self.idp['id'],
'mapping_id': self.mapping['id'],
}
PROVIDERS.federation_api.create_idp(self.idp['id'], self.idp)
PROVIDERS.federation_api.create_mapping(
self.mapping['id'], self.mapping
)
PROVIDERS.federation_api.create_protocol(
self.idp['id'], self.protocol['id'], self.protocol
)
self.domain_id = PROVIDERS.federation_api.get_idp(self.idp['id'])[
'domain_id'
]
# Create groups
self.group1 = unit.new_group_ref(
domain_id=self.domain_id, name='group1'
)
self.group1 = PROVIDERS.identity_api.create_group(self.group1)
self.group2 = unit.new_group_ref(
domain_id=self.domain_id, name='group2'
)
self.group2 = PROVIDERS.identity_api.create_group(self.group2)
self.group3 = unit.new_group_ref(
domain_id=self.domain_id, name='group3'
)
self.group3 = PROVIDERS.identity_api.create_group(self.group3)
def test_complete_user_shadowing_workflow(self):
"""Test complete workflow of user shadowing with cache invalidation."""
federated_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': 'Test User',
}
user_dict = {
'id': federated_user['unique_id'],
'name': federated_user['display_name'],
'domain': {'id': self.domain_id},
}
with mock.patch.object(
PROVIDERS.assignment_api, 'invalidate_user_cache_on_group_change'
) as mock_revoke:
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
self.assertEqual(0, mock_revoke.call_count)
mock_revoke.reset_mock()
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id'], self.group2['id']],
)
self.assertGreaterEqual(mock_revoke.call_count, 1)
mock_revoke.reset_mock()
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group2['id'], self.group3['id']],
)
self.assertEqual(1, mock_revoke.call_count)
def test_multiple_users_concurrent_operations(self):
"""Test cache invalidation with multiple concurrent user operations."""
users = []
for i in range(3):
fed_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': f'User {i}',
}
user_dict = {
'id': fed_user['unique_id'],
'name': fed_user['display_name'],
'domain': {'id': self.domain_id},
}
users.append((fed_user, user_dict))
invalidation_counts = {}
original_revoke = (
PROVIDERS.assignment_api.invalidate_user_cache_on_group_change
)
def track_revocation(user_id):
invalidation_counts[user_id] = (
invalidation_counts.get(user_id, 0) + 1
)
return original_revoke(user_id)
with mock.patch.object(
PROVIDERS.assignment_api,
'invalidate_user_cache_on_group_change',
side_effect=track_revocation,
):
shadowed_users = []
for fed_user, user_dict in users:
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
shadowed_users.append(user)
self.assertEqual(0, len(invalidation_counts))
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
users[1][1],
group_ids=[self.group1['id'], self.group2['id']],
)
self.assertEqual(1, len(invalidation_counts))
self.assertEqual(1, invalidation_counts[shadowed_users[1]['id']])
def test_expired_membership_removal_triggers_revocation(self):
"""Test that expired membership removal triggers cache invalidation."""
federated_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': 'Expiry Test User',
}
user_dict = {
'id': federated_user['unique_id'],
'name': federated_user['display_name'],
'domain': {'id': self.domain_id},
}
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
with mock.patch.object(
PROVIDERS.shadow_users_api,
'cleanup_stale_group_memberships',
return_value=True,
):
with mock.patch.object(
PROVIDERS.assignment_api,
'invalidate_user_cache_on_group_change',
) as mock_revoke:
PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
mock_revoke.assert_called_once_with(user['id'])
def test_role_assignments_reflect_group_changes(self):
"""Test that role assignments reflect group membership changes."""
project1 = unit.new_project_ref(domain_id=self.domain_id)
project1 = PROVIDERS.resource_api.create_project(
project1['id'], project1
)
project2 = unit.new_project_ref(domain_id=self.domain_id)
project2 = PROVIDERS.resource_api.create_project(
project2['id'], project2
)
role_admin = unit.new_role_ref(name='admin')
role_admin = PROVIDERS.role_api.create_role(
role_admin['id'], role_admin
)
role_member = unit.new_role_ref(name='member')
role_member = PROVIDERS.role_api.create_role(
role_member['id'], role_member
)
PROVIDERS.assignment_api.create_grant(
role_admin['id'],
group_id=self.group1['id'],
project_id=project1['id'],
)
PROVIDERS.assignment_api.create_grant(
role_member['id'],
group_id=self.group2['id'],
project_id=project2['id'],
)
federated_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': 'Role Test User',
}
user_dict = {
'id': federated_user['unique_id'],
'name': federated_user['display_name'],
'domain': {'id': self.domain_id},
}
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
PROVIDERS.identity_api.add_user_to_group(user['id'], self.group1['id'])
roles_proj1 = PROVIDERS.assignment_api.list_role_assignments(
user_id=user['id'], project_id=project1['id'], effective=True
)
self.assertEqual(1, len(roles_proj1))
self.assertEqual(role_admin['id'], roles_proj1[0]['role_id'])
PROVIDERS.identity_api.remove_user_from_group(
user['id'], self.group1['id']
)
PROVIDERS.identity_api.add_user_to_group(user['id'], self.group2['id'])
PROVIDERS.assignment_api.invalidate_user_cache_on_group_change(
user['id']
)
roles_proj2 = PROVIDERS.assignment_api.list_role_assignments(
user_id=user['id'], project_id=project2['id'], effective=True
)
self.assertEqual(1, len(roles_proj2))
self.assertEqual(role_member['id'], roles_proj2[0]['role_id'])
def test_token_revocation_event_persisted(self):
"""Test that revoke event is created on group change."""
federated_user = {
'idp_id': self.idp['id'],
'protocol_id': self.protocol['id'],
'unique_id': uuid.uuid4().hex,
'display_name': 'Revocation Test User',
}
user_dict = {
'id': federated_user['unique_id'],
'name': federated_user['display_name'],
'domain': {'id': self.domain_id},
}
# Initial shadowing - no revocation expected
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id']],
)
# Shadow again with additional group - revocation expected
with mock.patch.object(PROVIDERS.revoke_api, 'revoke') as mock_revoke:
user = PROVIDERS.identity_api.shadow_federated_user(
self.idp['id'],
self.protocol['id'],
user_dict,
group_ids=[self.group1['id'], self.group2['id']],
)
# Verify revoke was called with a RevokeEvent
mock_revoke.assert_called_once()
revoke_event = mock_revoke.call_args[0][0]
# Check that the revoke event has the correct user_id
self.assertEqual(revoke_event.user_id, user['id'])
# Check that issued_before is set (indicating -1 second logic)
self.assertIsNotNone(revoke_event.issued_before)