mirror of https://github.com/zulip/zulip.git
ldap: Support alternative attrs to userAccountControl.
Fixes #17456. The main tricky part has to do with what values the attribute should have. LDAP defines a Boolean as Boolean = "TRUE" / "FALSE" so ideally we'd always see exactly those values. However, although the issue is now marked as resolved, the discussion in https://pagure.io/freeipa/issue/1259 shows how this may not always be respected - meaning it makes sense for us to be more liberal in interpreting these values.
This commit is contained in:
parent
b32450f98e
commit
8ad7520180
|
@ -212,17 +212,23 @@ corresponding LDAP attribute is `linkedinProfile` then you just need
|
||||||
to add `'custom_profile_field__linkedin_profile': 'linkedinProfile'`
|
to add `'custom_profile_field__linkedin_profile': 'linkedinProfile'`
|
||||||
to the `AUTH_LDAP_USER_ATTR_MAP`.
|
to the `AUTH_LDAP_USER_ATTR_MAP`.
|
||||||
|
|
||||||
#### Automatically deactivating users with Active Directory
|
#### Automatically deactivating users
|
||||||
|
|
||||||
Zulip supports synchronizing the
|
Zulip supports synchronizing the
|
||||||
disabled/deactivated status of users from Active Directory. You can
|
disabled/deactivated status of users. If you're using Active Directory,
|
||||||
configure this by uncommenting the sample line
|
you can configure this by uncommenting the sample line
|
||||||
`"userAccountControl": "userAccountControl",` in
|
`"userAccountControl": "userAccountControl",` in
|
||||||
`AUTH_LDAP_USER_ATTR_MAP` (and restarting the Zulip server). Zulip
|
`AUTH_LDAP_USER_ATTR_MAP` (and restarting the Zulip server). Zulip
|
||||||
will then treat users that are disabled via the "Disable Account"
|
will then treat users that are disabled via the "Disable Account"
|
||||||
feature in Active Directory as deactivated in Zulip.
|
feature in Active Directory as deactivated in Zulip.
|
||||||
|
|
||||||
Users disabled in active directory will be immediately unable to log in
|
If you're using a different LDAP server which uses a boolean attribute
|
||||||
|
which is `TRUE` or `YES` for users that should be deactivated and `FALSE`
|
||||||
|
or `NO` otherwise. You can configure a mapping for `deactivated` in
|
||||||
|
`AUTH_LDAP_USER_ATTR_MAP`. For example, `"deactivated": "nsAccountLock",` is a correct mapping for a
|
||||||
|
[FreeIPA](https://www.freeipa.org/) LDAP database.
|
||||||
|
|
||||||
|
Disabled users will be immediately unable to log in
|
||||||
to Zulip, since Zulip queries the LDAP/Active Directory server on
|
to Zulip, since Zulip queries the LDAP/Active Directory server on
|
||||||
every login attempt. The user will be fully deactivated the next time
|
every login attempt. The user will be fully deactivated the next time
|
||||||
your `manage.py sync_ldap_user_data` cron job runs (at which point
|
your `manage.py sync_ldap_user_data` cron job runs (at which point
|
||||||
|
|
|
@ -5877,7 +5877,7 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
|
||||||
["WARNING:django_auth_ldap:Name too short! while authenticating hamlet"],
|
["WARNING:django_auth_ldap:Name too short! while authenticating hamlet"],
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_deactivate_user(self) -> None:
|
def test_deactivate_user_with_useraccountcontrol_attr(self) -> None:
|
||||||
self.change_ldap_user_attr("hamlet", "userAccountControl", "2")
|
self.change_ldap_user_attr("hamlet", "userAccountControl", "2")
|
||||||
|
|
||||||
with self.settings(
|
with self.settings(
|
||||||
|
@ -5893,6 +5893,50 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_deactivate_reactivate_user_with_deactivated_attr(self) -> None:
|
||||||
|
self.change_ldap_user_attr("hamlet", "someCustomAttr", "TRUE")
|
||||||
|
|
||||||
|
with self.settings(
|
||||||
|
AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "deactivated": "someCustomAttr"}
|
||||||
|
), self.assertLogs("zulip.ldap") as info_logs:
|
||||||
|
self.perform_ldap_sync(self.example_user("hamlet"))
|
||||||
|
hamlet = self.example_user("hamlet")
|
||||||
|
self.assertFalse(hamlet.is_active)
|
||||||
|
self.assertEqual(
|
||||||
|
info_logs.output,
|
||||||
|
[
|
||||||
|
"INFO:zulip.ldap:Deactivating user hamlet@zulip.com because they are disabled in LDAP."
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
self.change_ldap_user_attr("hamlet", "someCustomAttr", "FALSE")
|
||||||
|
with self.settings(
|
||||||
|
AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "deactivated": "someCustomAttr"}
|
||||||
|
), self.assertLogs("zulip.ldap") as info_logs:
|
||||||
|
self.perform_ldap_sync(self.example_user("hamlet"))
|
||||||
|
hamlet.refresh_from_db()
|
||||||
|
self.assertTrue(hamlet.is_active)
|
||||||
|
self.assertEqual(
|
||||||
|
info_logs.output,
|
||||||
|
[
|
||||||
|
"INFO:zulip.ldap:Reactivating user hamlet@zulip.com because they are not disabled in LDAP."
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
self.change_ldap_user_attr("hamlet", "someCustomAttr", "YESSS")
|
||||||
|
with self.settings(
|
||||||
|
AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "deactivated": "someCustomAttr"}
|
||||||
|
), self.assertLogs("django_auth_ldap") as ldap_logs, self.assertRaises(AssertionError):
|
||||||
|
self.perform_ldap_sync(self.example_user("hamlet"))
|
||||||
|
hamlet.refresh_from_db()
|
||||||
|
self.assertTrue(hamlet.is_active)
|
||||||
|
self.assertEqual(
|
||||||
|
ldap_logs.output,
|
||||||
|
[
|
||||||
|
"WARNING:django_auth_ldap:Invalid value 'YESSS' in the LDAP attribute mapped to deactivated while authenticating hamlet"
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
@mock.patch("zproject.backends.ZulipLDAPAuthBackendBase.sync_full_name_from_ldap")
|
@mock.patch("zproject.backends.ZulipLDAPAuthBackendBase.sync_full_name_from_ldap")
|
||||||
def test_dont_sync_disabled_ldap_user(self, fake_sync: mock.MagicMock) -> None:
|
def test_dont_sync_disabled_ldap_user(self, fake_sync: mock.MagicMock) -> None:
|
||||||
self.change_ldap_user_attr("hamlet", "userAccountControl", "2")
|
self.change_ldap_user_attr("hamlet", "userAccountControl", "2")
|
||||||
|
|
|
@ -477,6 +477,23 @@ def check_ldap_config() -> None:
|
||||||
# Email search needs to be configured in this case.
|
# Email search needs to be configured in this case.
|
||||||
assert settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH
|
assert settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH
|
||||||
|
|
||||||
|
# These two are alternatives approaches to deactivating users based on an ldap attribute
|
||||||
|
# and thus don't make sense to have enabled together.
|
||||||
|
assert not (
|
||||||
|
settings.AUTH_LDAP_USER_ATTR_MAP.get("userAccountControl")
|
||||||
|
and settings.AUTH_LDAP_USER_ATTR_MAP.get("deactivated")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def ldap_should_sync_active_status() -> bool:
|
||||||
|
if "userAccountControl" in settings.AUTH_LDAP_USER_ATTR_MAP:
|
||||||
|
return True
|
||||||
|
|
||||||
|
if "deactivated" in settings.AUTH_LDAP_USER_ATTR_MAP:
|
||||||
|
return True
|
||||||
|
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
def find_ldap_users_by_email(email: str) -> List[_LDAPUser]:
|
def find_ldap_users_by_email(email: str) -> List[_LDAPUser]:
|
||||||
"""
|
"""
|
||||||
|
@ -715,15 +732,30 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend):
|
||||||
else:
|
else:
|
||||||
logging.warning("Could not parse %s field for user %s", avatar_attr_name, user.id)
|
logging.warning("Could not parse %s field for user %s", avatar_attr_name, user.id)
|
||||||
|
|
||||||
def is_account_control_disabled_user(self, ldap_user: _LDAPUser) -> bool:
|
def is_user_disabled_in_ldap(self, ldap_user: _LDAPUser) -> bool:
|
||||||
"""Implements the userAccountControl check for whether a user has been
|
"""Implements checks for whether a user has been
|
||||||
disabled in an Active Directory server being integrated with
|
disabled in the LDAP server being integrated with
|
||||||
Zulip via LDAP."""
|
Zulip."""
|
||||||
|
if "userAccountControl" in settings.AUTH_LDAP_USER_ATTR_MAP:
|
||||||
account_control_value = ldap_user.attrs[
|
account_control_value = ldap_user.attrs[
|
||||||
settings.AUTH_LDAP_USER_ATTR_MAP["userAccountControl"]
|
settings.AUTH_LDAP_USER_ATTR_MAP["userAccountControl"]
|
||||||
][0]
|
][0]
|
||||||
ldap_disabled = bool(int(account_control_value) & LDAP_USER_ACCOUNT_CONTROL_DISABLED_MASK)
|
return bool(int(account_control_value) & LDAP_USER_ACCOUNT_CONTROL_DISABLED_MASK)
|
||||||
return ldap_disabled
|
|
||||||
|
assert "deactivated" in settings.AUTH_LDAP_USER_ATTR_MAP
|
||||||
|
attr_value = ldap_user.attrs[settings.AUTH_LDAP_USER_ATTR_MAP["deactivated"]][0]
|
||||||
|
|
||||||
|
# In the LDAP specification, a Boolean attribute should be
|
||||||
|
# *exactly* either "TRUE" or "FALSE". However,
|
||||||
|
# https://www.freeipa.org/page/V4/User_Life-Cycle_Management suggests
|
||||||
|
# that FreeIPA at least documents using Yes/No for booleans.
|
||||||
|
true_values = ["TRUE", "YES"]
|
||||||
|
false_values = ["FALSE", "NO"]
|
||||||
|
attr_value_upper = attr_value.upper()
|
||||||
|
assert (
|
||||||
|
attr_value_upper in true_values or attr_value_upper in false_values
|
||||||
|
), f"Invalid value '{attr_value}' in the LDAP attribute mapped to deactivated"
|
||||||
|
return attr_value_upper in true_values
|
||||||
|
|
||||||
def is_account_realm_access_forbidden(self, ldap_user: _LDAPUser, realm: Realm) -> bool:
|
def is_account_realm_access_forbidden(self, ldap_user: _LDAPUser, realm: Realm) -> bool:
|
||||||
# org_membership takes priority over AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL.
|
# org_membership takes priority over AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL.
|
||||||
|
@ -881,8 +913,8 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase):
|
||||||
if self.is_account_realm_access_forbidden(ldap_user, self._realm):
|
if self.is_account_realm_access_forbidden(ldap_user, self._realm):
|
||||||
raise ZulipLDAPException("User not allowed to access realm")
|
raise ZulipLDAPException("User not allowed to access realm")
|
||||||
|
|
||||||
if "userAccountControl" in settings.AUTH_LDAP_USER_ATTR_MAP: # nocoverage
|
if ldap_should_sync_active_status(): # nocoverage
|
||||||
ldap_disabled = self.is_account_control_disabled_user(ldap_user)
|
ldap_disabled = self.is_user_disabled_in_ldap(ldap_user)
|
||||||
if ldap_disabled:
|
if ldap_disabled:
|
||||||
# Treat disabled users as deactivated in Zulip.
|
# Treat disabled users as deactivated in Zulip.
|
||||||
return_data["inactive_user"] = True
|
return_data["inactive_user"] = True
|
||||||
|
@ -1012,8 +1044,8 @@ class ZulipLDAPUserPopulator(ZulipLDAPAuthBackendBase):
|
||||||
user = get_user_by_delivery_email(username, ldap_user.realm)
|
user = get_user_by_delivery_email(username, ldap_user.realm)
|
||||||
built = False
|
built = False
|
||||||
# Synchronise the UserProfile with its LDAP attributes:
|
# Synchronise the UserProfile with its LDAP attributes:
|
||||||
if "userAccountControl" in settings.AUTH_LDAP_USER_ATTR_MAP:
|
if ldap_should_sync_active_status():
|
||||||
user_disabled_in_ldap = self.is_account_control_disabled_user(ldap_user)
|
user_disabled_in_ldap = self.is_user_disabled_in_ldap(ldap_user)
|
||||||
if user_disabled_in_ldap:
|
if user_disabled_in_ldap:
|
||||||
if user.is_active:
|
if user.is_active:
|
||||||
ldap_logger.info(
|
ldap_logger.info(
|
||||||
|
|
|
@ -239,6 +239,9 @@ AUTH_LDAP_USER_ATTR_MAP = {
|
||||||
## who are disabled in LDAP/Active Directory (and reactivate users who are not).
|
## who are disabled in LDAP/Active Directory (and reactivate users who are not).
|
||||||
## See docs for usage details and precise semantics.
|
## See docs for usage details and precise semantics.
|
||||||
# "userAccountControl": "userAccountControl",
|
# "userAccountControl": "userAccountControl",
|
||||||
|
## Alternatively, you can map "deactivated" to a boolean attribute
|
||||||
|
## that is "TRUE" for deactivated users and "FALSE" otherwise.
|
||||||
|
# "deactivated": "nsAccountLock",
|
||||||
## Restrict access to organizations using an LDAP attribute.
|
## Restrict access to organizations using an LDAP attribute.
|
||||||
## See https://zulip.readthedocs.io/en/latest/production/authentication-methods.html#restricting-ldap-user-access-to-specific-organizations
|
## See https://zulip.readthedocs.io/en/latest/production/authentication-methods.html#restricting-ldap-user-access-to-specific-organizations
|
||||||
# "org_membership": "department",
|
# "org_membership": "department",
|
||||||
|
|
Loading…
Reference in New Issue