diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index e62aa88ad8..80bd1f29a1 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -41,8 +41,8 @@ from zerver.lib.test_classes import ( ZulipTestCase, ) from zerver.models import \ - get_realm, email_to_username, UserProfile, \ - PreregistrationUser, Realm, get_user, MultiuseInvite + get_realm, email_to_username, CustomProfileField, CustomProfileFieldValue, \ + UserProfile, PreregistrationUser, Realm, get_user, MultiuseInvite from zerver.signals import JUST_CREATED_THRESHOLD from confirmation.models import Confirmation, create_confirmation_link @@ -2587,6 +2587,105 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase): hamlet = self.example_user('hamlet') self.assertFalse(hamlet.is_active) + def test_update_custom_profile_field(self) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'phoneNumber': ['123456789', ], + 'birthDate': ['1900-09-08', ], + } + } + + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'custom_profile_field__phone_number': 'phoneNumber', + 'custom_profile_field__birthday': 'birthDate'}): + self.perform_ldap_sync(self.example_user('hamlet')) + hamlet = self.example_user('hamlet') + test_data = [ + { + 'field_name': 'Phone number', + 'expected_value': '123456789', + }, + { + 'field_name': 'Birthday', + 'expected_value': '1900-09-08', + }, + ] + for test_case in test_data: + field = CustomProfileField.objects.get(realm=hamlet.realm, name=test_case['field_name']) + field_value = CustomProfileFieldValue.objects.get(user_profile=hamlet, field=field).value + self.assertEqual(field_value, test_case['expected_value']) + + def test_update_non_existent_profile_field(self) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'phoneNumber': ['123456789', ], + } + } + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'custom_profile_field__non_existent': 'phoneNumber'}): + with self.assertRaisesRegex(ZulipLDAPException, 'Custom profile field with name non_existent not found'): + self.perform_ldap_sync(self.example_user('hamlet')) + + def test_update_custom_profile_field_invalid_data(self) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'birthDate': ['123456789', ], + } + } + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'custom_profile_field__birthday': 'birthDate'}): + with self.assertRaisesRegex(ZulipLDAPException, 'Invalid data for birthday field'): + self.perform_ldap_sync(self.example_user('hamlet')) + + def test_update_custom_profile_field_no_mapping(self) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'birthDate': ['1990-01-01', ], + } + } + hamlet = self.example_user('hamlet') + no_op_field = CustomProfileField.objects.get(realm=hamlet.realm, name='Phone number') + expected_value = CustomProfileFieldValue.objects.get(user_profile=hamlet, field=no_op_field).value + + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'custom_profile_field__birthday': 'birthDate'}): + self.perform_ldap_sync(self.example_user('hamlet')) + + actual_value = CustomProfileFieldValue.objects.get(user_profile=hamlet, field=no_op_field).value + self.assertEqual(actual_value, expected_value) + + def test_update_custom_profile_field_no_update(self) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'phoneNumber': ['new-number', ], + 'birthDate': ['1990-01-01', ], + } + } + hamlet = self.example_user('hamlet') + phone_number_field = CustomProfileField.objects.get(realm=hamlet.realm, name='Phone number') + birthday_field = CustomProfileField.objects.get(realm=hamlet.realm, name='Birthday') + phone_number_field_value = CustomProfileFieldValue.objects.get(user_profile=hamlet, + field=phone_number_field) + phone_number_field_value.value = 'new-number' + phone_number_field_value.save(update_fields=['value']) + expected_call_args = [hamlet, [ + { + 'id': birthday_field.id, + 'value': '1990-01-01', + }, + ]] + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'custom_profile_field__birthday': 'birthDate', + 'custom_profile_field__phone_number': 'phoneNumber'}): + with mock.patch('zproject.backends.do_update_user_custom_profile_data') as f: + self.perform_ldap_sync(self.example_user('hamlet')) + f.assert_called_once_with(*expected_call_args) + class TestZulipAuthMixin(ZulipTestCase): def test_get_user(self) -> None: backend = ZulipAuthMixin() diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 32a2573edf..431863ef02 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -27,9 +27,9 @@ from zerver.views.invite import get_invitee_emails_set from zerver.views.development.registration import confirmation_key from zerver.models import ( - get_realm, get_user, get_stream_recipient, get_realm_stream, - PreregistrationUser, Realm, Recipient, Message, - ScheduledEmail, UserProfile, UserMessage, + get_realm, get_user, get_realm_stream, get_stream_recipient, + CustomProfileField, CustomProfileFieldValue, PreregistrationUser, + Realm, Recipient, Message, ScheduledEmail, UserProfile, UserMessage, Stream, Subscription, flush_per_request_caches ) from zerver.lib.actions import ( @@ -2606,13 +2606,20 @@ class UserSignUpTest(InviteUserBase): email = "newuser@zulip.com" subdomain = "zulip" - ldap_user_attr_map = {'full_name': 'fn', 'short_name': 'sn'} + ldap_user_attr_map = { + 'full_name': 'fn', + 'short_name': 'sn', + 'custom_profile_field__phone_number': 'phoneNumber', + 'custom_profile_field__birthday': 'birthDate', + } full_name = 'New LDAP fullname' mock_directory = { 'uid=newuser,ou=users,dc=zulip,dc=com': { 'userPassword': ['testing', ], 'fn': [full_name], 'sn': ['shortname'], + 'phoneNumber': ['a-new-number', ], + 'birthDate': ['1990-12-19', ], } } init_fakeldap(mock_directory) @@ -2630,6 +2637,17 @@ class UserSignUpTest(InviteUserBase): user_profile = UserProfile.objects.get(email=email) # Name comes from form which was set by LDAP. self.assertEqual(user_profile.full_name, full_name) + self.assertEqual(user_profile.short_name, 'shortname') + + # Test custom profile fields are properly synced. + birthday_field = CustomProfileField.objects.get(realm=user_profile.realm, name='Birthday') + phone_number_field = CustomProfileField.objects.get(realm=user_profile.realm, name='Phone number') + birthday_field_value = CustomProfileFieldValue.objects.get(user_profile=user_profile, + field=birthday_field) + phone_number_field_value = CustomProfileFieldValue.objects.get(user_profile=user_profile, + field=phone_number_field) + self.assertEqual(birthday_field_value.value, '1990-12-19') + self.assertEqual(phone_number_field_value.value, 'a-new-number') @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend', 'zproject.backends.ZulipDummyBackend')) diff --git a/zproject/backends.py b/zproject/backends.py index 817a1dca4b..14ebd35351 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -1,5 +1,5 @@ import logging -from typing import Any, Dict, List, Set, Tuple, Optional +from typing import Any, Dict, List, Optional, Set, Tuple, Union from django_auth_ldap.backend import LDAPBackend, _LDAPUser import django.contrib.auth @@ -16,13 +16,14 @@ from social_core.backends.base import BaseAuth from social_core.backends.oauth import BaseOAuth2 from social_core.exceptions import AuthFailed, SocialAuthBaseException -from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate_user +from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate_user, \ + do_update_user_custom_profile_data from zerver.lib.dev_ldap_directory import init_fakeldap from zerver.lib.request import JsonableError -from zerver.lib.users import check_full_name -from zerver.models import PreregistrationUser, UserProfile, Realm, get_default_stream_groups, \ - get_user_profile_by_id, remote_user_to_email, email_to_username, get_realm, \ - get_user_by_delivery_email +from zerver.lib.users import check_full_name, validate_user_custom_profile_field +from zerver.models import CustomProfileField, PreregistrationUser, UserProfile, Realm, \ + custom_profile_fields_for_realm, get_default_stream_groups, get_user_profile_by_id, \ + remote_user_to_email, email_to_username, get_realm, get_user_by_delivery_email def pad_method_dict(method_dict: Dict[str, bool]) -> Dict[str, bool]: """Pads an authentication methods dict to contain all auth backends @@ -339,11 +340,50 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): raise ZulipLDAPException(e.msg) do_change_full_name(user_profile, full_name, None) + def sync_custom_profile_fields_from_ldap(self, user_profile: UserProfile, + ldap_user: _LDAPUser) -> None: + values_by_var_name = {} # type: Dict[str, Union[int, str, List[int]]] + for attr, ldap_attr in settings.AUTH_LDAP_USER_ATTR_MAP.items(): + if not attr.startswith('custom_profile_field__'): + continue + var_name = attr.split('custom_profile_field__')[1] + value = ldap_user.attrs[ldap_attr][0] + values_by_var_name[var_name] = value + + fields_by_var_name = {} # type: Dict[str, CustomProfileField] + custom_profile_fields = custom_profile_fields_for_realm(user_profile.realm.id) + for field in custom_profile_fields: + var_name = '_'.join(field.name.lower().split(' ')) + fields_by_var_name[var_name] = field + + existing_values = {} + for data in user_profile.profile_data: + var_name = '_'.join(data['name'].lower().split(' ')) # type: ignore # data field values can also be int + existing_values[var_name] = data['value'] + + profile_data = [] # type: List[Dict[str, Union[int, str, List[int]]]] + for var_name, value in values_by_var_name.items(): + try: + field = fields_by_var_name[var_name] + except KeyError: + raise ZulipLDAPException('Custom profile field with name %s not found.' % (var_name,)) + if existing_values.get(var_name) == value: + continue + result = validate_user_custom_profile_field(user_profile.realm.id, field, value) + if result is not None: + raise ZulipLDAPException('Invalid data for %s field: %s' % (var_name, result)) + profile_data.append({ + 'id': field.id, + 'value': value, + }) + do_update_user_custom_profile_data(user_profile, profile_data) + def get_or_build_user(self, username: str, ldap_user: _LDAPUser) -> Tuple[UserProfile, bool]: (user, built) = super().get_or_build_user(username, ldap_user) self.sync_avatar_from_ldap(user, ldap_user) self.sync_full_name_from_ldap(user, ldap_user) + self.sync_custom_profile_fields_from_ldap(user, ldap_user) if 'userAccountControl' in settings.AUTH_LDAP_USER_ATTR_MAP: user_disabled_in_ldap = self.is_account_control_disabled_user(ldap_user) if user_disabled_in_ldap and user.is_active: @@ -439,6 +479,7 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase): user_profile = do_create_user(username, None, self._realm, full_name, short_name, **opts) self.sync_avatar_from_ldap(user_profile, ldap_user) + self.sync_custom_profile_fields_from_ldap(user_profile, ldap_user) return user_profile, True