From 5567bedfe18422b9900b4f27521ff6725aa24d26 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Wed, 6 Apr 2022 22:16:52 +0200 Subject: [PATCH] ldap: use guid attributes as global external id (#63646) --- src/authentic2/backends/ldap_backend.py | 170 ++++++++++++++---- .../migrations/0038_add_external_guid.py | 36 ++++ src/authentic2/models.py | 18 +- tests/test_ldap.py | 77 +++++--- 4 files changed, 237 insertions(+), 64 deletions(-) create mode 100644 src/authentic2/migrations/0038_add_external_guid.py diff --git a/src/authentic2/backends/ldap_backend.py b/src/authentic2/backends/ldap_backend.py index 9e1a6aa6..42e2719d 100644 --- a/src/authentic2/backends/ldap_backend.py +++ b/src/authentic2/backends/ldap_backend.py @@ -24,6 +24,7 @@ import socket import ssl import time import urllib.parse +import uuid import ldap import ldap.modlist @@ -34,6 +35,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.core.cache import cache from django.core.exceptions import ImproperlyConfigured +from django.db.models import Q from django.db.transaction import atomic from django.utils.encoding import force_bytes, force_text from django.utils.translation import ngettext @@ -71,6 +73,8 @@ CA_BUNDLE_PATHS = [ '/var/lib/ca-certificates/ca-bundle.pem', # OpenSuse ] +USUAL_GUID_ATTRIBUTES = ['entryuuid', 'objectguid', 'nsuniqueid'] + class UserCreationError(Exception): pass @@ -123,10 +127,22 @@ class LDAPObject(NativeLDAPObject): @to_list def _convert_results_to_unicode(self, result_list): for dn, attrs in result_list: - if dn is not None: - # tuple is a real entry with a DN not a search reference - attrs = {attribute: filter_non_unicode_values(attrs[attribute]) for attribute in attrs} - yield dn, attrs + if dn is None: + continue + new_attrs = {} + for attribute in attrs: + values = attrs[attribute] + # specialize for GUID attributes + if attribute in USUAL_GUID_ATTRIBUTES and len(values[0]) == 16: + try: + values = [str(uuid.UUID(bytes=values[0]))] + except ValueError: + values = [] + values = filter_non_unicode_values(values) + if not values: + continue + new_attrs[attribute] = values + yield dn, new_attrs def modify_s(self, dn, modlist): new_modlist = [] @@ -482,7 +498,7 @@ class LDAPBackend: # generated username are unique 'update_username': False, # lookup existing user with an external id build with attributes - 'lookups': ('external_id', 'username', 'email'), + 'lookups': ('guid', 'external_id', 'username', 'email'), 'external_id_tuples': ( ('uid',), ('dn:noquote',), @@ -745,7 +761,8 @@ class LDAPBackend: try: return self._return_user(authz_id, password, conn, block) except UserCreationError as e: - messages.error(request, str(e)) + if request: + messages.error(request, str(e)) return None except ldap.CONNECT_ERROR: log.error( @@ -1095,6 +1112,8 @@ class LDAPBackend: for key in at_mapping: if at_mapping[key] != 'dn': attributes.add(at_mapping[key]) + # add usual GUID attributes + attributes.update(USUAL_GUID_ATTRIBUTES) return list({attribute.lower() for attribute in attributes}) @classmethod @@ -1350,6 +1369,7 @@ class LDAPBackend: return None def _lookup_by_external_id(self, block, attributes): + realm = block['realm'] for eid_tuple in map_text(block['external_id_tuples']): external_id = self.build_external_id(eid_tuple, attributes) if not external_id: @@ -1359,7 +1379,7 @@ class LDAPBackend: LDAPUser.objects.prefetch_related('groups') .filter( userexternalid__external_id__iexact=external_id, - userexternalid__source=force_text(block['realm']), + userexternalid__source=realm, ) .order_by('-last_login') ) @@ -1373,12 +1393,51 @@ class LDAPBackend: len(users), ) for other in users[1:]: - for r in other.roles.all(): - user.roles.add(r) + user.roles.add(*other.roles.all()) other.delete() return user return None + def _lookup_by_external_guid(self, block, attribute, guid): + if not guid: + return None + log.debug('ldap: lookup using guid %s=%s', attribute, guid) + try: + return LDAPUser.objects.get( + userexternalid__source=block['realm'], userexternalid__external_guid=guid + ) + except LDAPUser.DoesNotExist: + return None + + # entryuuid is encoded as the string representation of the UUID, as an octet string + @classmethod + def _decode_entryuuid_guid(cls, value): + try: + return uuid.UUID(value) + except (ValueError, UnicodeDecodeError): + return None + + # objectid is encoded as the byte representation of the UUID + _decode_objectguid_guid = _decode_entryuuid_guid + + # nsuniqueid is encoded as the byte representation of the UUID + _decode_nsuniqueid_guid = _decode_objectguid_guid + + def _lookup_by_guid(self, block, attributes): + attribute, guid = self._get_guid(attributes) + return self._lookup_by_external_guid(block=block, attribute=attribute, guid=guid) + + @classmethod + def _get_guid(cls, attributes): + for attribute in USUAL_GUID_ATTRIBUTES: + if attribute not in attributes: + continue + value = attributes[attribute][0] + guid = getattr(cls, f'_decode_{attribute}_guid')(value) + if guid: + return attribute, guid + return None, None + def _lookup_existing_user(self, username, block, attributes): user = None ou = self._get_target_ou(block) @@ -1393,10 +1452,13 @@ class LDAPBackend: user = self._lookup_by_external_id(block=block, attributes=attributes) elif lookup_type == 'email' and attributes: user = self._lookup_by_email(ou=ou, block=block, attributes=attributes) + elif lookup_type == 'guid' and attributes: + user = self._lookup_by_guid(block=block, attributes=attributes) if user: return user def update_user_identifiers(self, user, username, block, attributes): + realm = block['realm'] # if username has changed and we propagate those changes, update it if block['update_username']: if user.username != username: @@ -1406,36 +1468,63 @@ class LDAPBackend: log_msg = 'updating username from %r to %r' log.debug(log_msg, old_username, user.username) # if external_id lookup is used, update it + userexternalid = None + use_guid = False + use_external_id = False + guid = None + external_id = None + if 'guid' in block['lookups']: + use_guid = True + dummy, guid = self._get_guid(attributes) + + if guid and user.pk: + attribute, guid = self._get_guid(attributes) + if guid: + try: + userexternalid = UserExternalId.objects.get(user=user, external_guid=guid, source=realm) + except UserExternalId.DoesNotExist: + pass + if ( 'external_id' in block['lookups'] and block.get('external_id_tuples') and block['external_id_tuples'][0] ): - if not user.pk: - user.save() - user._changed = False + use_external_id = True external_id = self.build_external_id(map_text(block['external_id_tuples'][0]), attributes) - if external_id: - new, dummy = UserExternalId.objects.get_or_create( - user=user, external_id=external_id, source=force_text(block['realm']) - ) - if block['clean_external_id_on_update']: - UserExternalId.objects.exclude(id=new.id).filter( - user=user, source=force_text(block['realm']) - ).delete() - elif user._created: - log.error( - 'ldap: unable to build an external_id (%r) with attributes: %r', - block['external_id_tuples'][0], - attributes, + + if external_id and user.pk: + try: + userexternalid = UserExternalId.objects.get( + user=user, external_id__iexact=external_id, source=realm ) - raise UserCreationError(_('LDAP configuration si broken, please contact your administrator')) - else: + except UserExternalId.DoesNotExist: + pass + + if userexternalid: + changed = False + if userexternalid.external_guid != guid: + userexternalid.external_guid = guid + changed = True + if userexternalid.external_id != external_id: + userexternalid.external_id = external_id + changed = True + if changed: + userexternalid.save() + elif use_guid or use_external_id: + if not guid and not external_id: log.error( - 'ldap: unable to update an external_id (%r) with attributes: %r', - block['external_id_tuples'][0], + 'ldap: unable to build an user_external_id (%r) with attributes: %r', + block['external_id_tuples'], attributes, ) + raise UserCreationError(_('LDAP configuration is broken, please contact your administrator')) + if not user.pk: + user._changed = False + user.save() + UserExternalId.objects.create( + user=user, source=realm, external_id=external_id, external_guid=guid + ) def _record_failure_for_user(self, request, reason, user_id, block, conn, attributes=None): user = None @@ -1589,10 +1678,16 @@ class LDAPBackend: 'external_id', flat=True ) ) + guids = set( + UserExternalId.objects.filter(user__is_active=True, source=block['realm']).values_list( + 'external_guid', flat=True + ) + ) basedn = force_text(block.get('user_basedn') or block['basedn']) - attribute_names = [ - a[0] for a in cls.attribute_name_from_external_id_tuple(block['external_id_tuples']) - ] + attribute_names = list( + {a[0] for a in cls.attribute_name_from_external_id_tuple(block['external_id_tuples'])} + | set(USUAL_GUID_ATTRIBUTES) + ) user_filter = cls.get_sync_ldap_user_filter(block) results = cls.paged_search( conn, basedn, ldap.SCOPE_SUBTREE, user_filter, attrlist=attribute_names @@ -1600,6 +1695,9 @@ class LDAPBackend: for dn, attrs in results: data = attrs.copy() data['dn'] = dn + dummy, guid = cls._get_guid(data) + if guid and guid in guids: + guids.discard(guid) for eid_tuple in map_text(block['external_id_tuples']): backend = cls() external_id = backend.build_external_id(eid_tuple, data) @@ -1608,8 +1706,10 @@ class LDAPBackend: eids.remove(external_id) except ValueError: pass - for eid in UserExternalId.objects.filter( - external_id__in=eids, user__is_active=True, source=block['realm'] + for eid in UserExternalId.objects.filter(user__is_active=True, source=block['realm']).filter( + Q(external_id__in=eids, external_guid__isnull=True) + | Q(external_guid__in=guids, external_id__isnull=True) + | Q(external_guid__in=guids, external_id__in=eids) ): if eid.user.is_active: eid.user.mark_as_inactive(reason=LDAP_DEACTIVATION_REASON_NOT_PRESENT) @@ -1896,6 +1996,8 @@ class LDAPBackendPasswordLost(LDAPBackend): return for user_external_id in user.userexternalid_set.all(): external_id = user_external_id.external_id + if not external_id: + continue for block in config: if user_external_id.source != force_text(block['realm']): continue diff --git a/src/authentic2/migrations/0038_add_external_guid.py b/src/authentic2/migrations/0038_add_external_guid.py new file mode 100644 index 00000000..5491599e --- /dev/null +++ b/src/authentic2/migrations/0038_add_external_guid.py @@ -0,0 +1,36 @@ +# Generated by Django 2.2.27 on 2022-04-06 21:11 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('authentic2', '0037_auto_20220331_1513'), + ] + + operations = [ + migrations.AddField( + model_name='userexternalid', + name='external_guid', + field=models.UUIDField(null=True, verbose_name='External GUID'), + ), + migrations.AlterField( + model_name='userexternalid', + name='external_id', + field=models.CharField(max_length=256, null=True, verbose_name='external id'), + ), + migrations.AlterUniqueTogether( + name='userexternalid', + unique_together={('source', 'external_guid'), ('source', 'external_id')}, + ), + migrations.AddConstraint( + model_name='userexternalid', + constraint=models.CheckConstraint( + check=models.Q( + ('external_id__isnull', False), ('external_guid__isnull', False), _connector='OR' + ), + name='at_least_one_id', + ), + ), + ] diff --git a/src/authentic2/models.py b/src/authentic2/models.py index ee6ab70c..d14d52c1 100644 --- a/src/authentic2/models.py +++ b/src/authentic2/models.py @@ -49,16 +49,20 @@ from .utils.misc import ServiceAccessDenied class UserExternalId(models.Model): user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'), on_delete=models.CASCADE) source = models.CharField(max_length=256, verbose_name=_('source')) - external_id = models.CharField(max_length=256, verbose_name=_('external id')) + external_id = models.CharField(max_length=256, verbose_name=_('external id'), null=True) + external_guid = models.UUIDField(verbose_name=_('External GUID'), null=True) created = models.DateTimeField(auto_now_add=True, verbose_name=_('creation date')) updated = models.DateTimeField(auto_now=True, verbose_name=_('last update date')) def __str__(self): - return f'{self.user} is {self.external_id} on {self.source}' + return f'{self.user} is {self.external_id or self.external_guid} on {self.source}' def __repr__(self): - return '