ldap: Tweak AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL behavior.

The original behavior of this setting was to disable LDAP
authentication for any realms not configured to use it. This was an
arbitrary choice, and its only value was to potentially help catch
typos for users who are lazy about testing their configuration.

Since it makes it a very inconvenient to potentially host multiple
organizations with different LDAP configurations, remove that
behavior.
This commit is contained in:
Mateusz Mandera 2023-10-08 22:53:54 +02:00 committed by Tim Abbott
parent cc934429fe
commit 1800b2c797
4 changed files with 22 additions and 14 deletions

View File

@ -159,6 +159,12 @@ _Released 2023-11-16_
various certain other special patterns are now forbidden. In the various certain other special patterns are now forbidden. In the
unlikely event that existing user groups have names matching these unlikely event that existing user groups have names matching these
patterns, they will be automatically renamed on upgrade. patterns, they will be automatically renamed on upgrade.
- The behavior of the `AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL` has
subtly changed. Previously, using this setting at all would block
LDAP authentication in organizations that are configured to use LDAP
authentication but not explicitly configured with advanced access
controls. This behavior was removed to simplify hosting multiple
organizations with different LDAP configuration preferences.
## Zulip Server 7.x series ## Zulip Server 7.x series

View File

@ -435,9 +435,9 @@ organization's subdomain. The corresponding value is a list of
`attribute: value` pair sets such that a user is permitted to access `attribute: value` pair sets such that a user is permitted to access
the organization if and only if the `attribute: value` pairs in at the organization if and only if the `attribute: value` pairs in at
least one of these sets match the user's LDAP attributes. If this least one of these sets match the user's LDAP attributes. If this
setting is enabled, organizations not explicitly configured in this setting is enabled, organizations not explicitly configured in it
setting will not be accessible via ldap authentication at all. will not be affected - they'll allow normal LDAP login, unless restricted
by other settings.
This is better illustrated with an example: This is better illustrated with an example:
``` ```
@ -457,8 +457,8 @@ AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL = {
This means that the organization `"zulip"` will be accessible via ldap This means that the organization `"zulip"` will be accessible via ldap
authentication only for users whose ldap attributes either contain authentication only for users whose ldap attributes either contain
both `department: main` `employeeType: staff` or just `office: both `department: main` `employeeType: staff` or just `office:
Dallas`. LDAP authentication will always fail for all other Dallas`. LDAP authentication will proceed normally for all other
organizations in this configuration. organizations.
:::{warning} :::{warning}
Restricting access using these mechanisms only affects authentication via LDAP, Restricting access using these mechanisms only affects authentication via LDAP,

View File

@ -4985,7 +4985,8 @@ class FetchAPIKeyTest(ZulipTestCase):
self.assert_json_success(result) self.assert_json_success(result)
self.remove_ldap_user_attr("hamlet", "department") self.remove_ldap_user_attr("hamlet", "department")
# Test wrong configuration # Test a realm that's not configured in the setting. Such a realm should not be affected,
# and just allow normal ldap login.
with override_settings( with override_settings(
AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL={"not_zulip": [{"department": "zulip"}]} AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL={"not_zulip": [{"department": "zulip"}]}
): ):
@ -4993,7 +4994,7 @@ class FetchAPIKeyTest(ZulipTestCase):
"/api/v1/fetch_api_key", "/api/v1/fetch_api_key",
dict(username="hamlet", password=self.ldap_password("hamlet")), dict(username="hamlet", password=self.ldap_password("hamlet")),
) )
self.assert_json_error(result, "Your username or password is incorrect", 401) self.assert_json_success(result)
def test_inactive_user(self) -> None: def test_inactive_user(self) -> None:
do_deactivate_user(self.user_profile, acting_user=None) do_deactivate_user(self.user_profile, acting_user=None)

View File

@ -843,17 +843,18 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend):
# If neither setting is configured, allow access. # If neither setting is configured, allow access.
if realm_access_control is None: if realm_access_control is None:
return False return False
if realm.subdomain not in realm_access_control:
# If a realm is not configured in this setting, it shouldn't
# be affected by it - therefore, allow access.
return False
# With settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL, we # With settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL, we
# allow access if and only if one of the entries for the # allow access if and only if one of the entries for the
# target subdomain matches the user's LDAP attributes. # target subdomain matches the user's LDAP attributes.
if not (
realm.subdomain in realm_access_control # Make sure the format of the setting makes sense.
and isinstance(realm_access_control[realm.subdomain], list) assert isinstance(realm_access_control[realm.subdomain], list)
and len(realm_access_control[realm.subdomain]) > 0 assert len(realm_access_control[realm.subdomain]) > 0
):
# If configuration is wrong, do not allow access
return True
# Go through every "or" check # Go through every "or" check
for attribute_group in realm_access_control[realm.subdomain]: for attribute_group in realm_access_control[realm.subdomain]: