diff --git a/docs/production/authentication-methods.md b/docs/production/authentication-methods.md index cfdf1c3759..3a127e338e 100644 --- a/docs/production/authentication-methods.md +++ b/docs/production/authentication-methods.md @@ -304,14 +304,27 @@ department: www ... ``` +More complex access control rules are possible via the +`AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL` setting. Note that +`org_membership` takes precedence over +`AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL`: + +1. If `org_membership` is set and allows access, access will be granted +2. If `org_membership` is not set or does not allow access, + `AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL` will control access. + +This contains a map keyed by the organization's subdomain. The +organization list with multiple maps, that contain a map with an attribute, and a required +value for that attribute. If for any of the attribute maps, all user's +LDAP attributes match what is configured, access is granted. + ```eval_rst .. warning:: - Restricting access using this mechanism only affects authentication via LDAP, + Restricting access using these mechanisms only affects authentication via LDAP, and won't prevent users from accessing the organization using any other authentication backends that are enabled for the organization. ``` - ### Troubleshooting Most issues with LDAP authentication are caused by misconfigurations of diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index c3d5a38f9e..06244ad053 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1165,6 +1165,15 @@ Output: self.mock_ldap.directory[dn][attr_name] = [data] + def remove_ldap_user_attr(self, username: str, attr_name: str) -> None: + """ + Method for removing the value of an attribute of a user entry in the mock + directory. This changes the attribute only for the specific test function + that calls this method, and is isolated from other tests. + """ + dn = f"uid={username},ou=users,dc=zulip,dc=com" + self.mock_ldap.directory[dn].pop(attr_name, None) + def ldap_username(self, username: str) -> str: """ Maps Zulip username to the name of the corresponding LDAP user diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index b401d5dbed..0baa458fbd 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -3812,6 +3812,86 @@ class FetchAPIKeyTest(ZulipTestCase): ) self.assert_json_success(result) + @override_settings( + AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",), + AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "org_membership": "department"}, + AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL={ + "zulip": [{"test1": "test", "test2": "testing"}, {"test1": "test2"}], + "anotherRealm": [{"test2": "test2"}], + }, + ) + def test_ldap_auth_email_auth_advanced_organization_restriction(self) -> None: + self.init_default_ldap_database() + + # The first user has no attribute set + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_error(result, "Your username or password is incorrect.", 403) + + self.change_ldap_user_attr("hamlet", "test2", "testing") + # Check with only one set + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_error(result, "Your username or password is incorrect.", 403) + + self.change_ldap_user_attr("hamlet", "test1", "test") + # Setting org_membership to not cause django_ldap_auth to warn, when synchronising + self.change_ldap_user_attr("hamlet", "department", "wrongDepartment") + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_success(result) + self.remove_ldap_user_attr("hamlet", "test2") + self.remove_ldap_user_attr("hamlet", "test1") + + # Using the OR value + self.change_ldap_user_attr("hamlet", "test1", "test2") + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_success(result) + + # Testing without org_membership + with override_settings(AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn"}): + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_success(result) + + # Setting test1 to wrong value + self.change_ldap_user_attr("hamlet", "test1", "invalid") + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_error(result, "Your username or password is incorrect.", 403) + + # Override access with `org_membership` + self.change_ldap_user_attr("hamlet", "department", "zulip") + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_success(result) + self.remove_ldap_user_attr("hamlet", "department") + + # Test wrong configuration + with override_settings( + AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL={"not_zulip": [{"department": "zulip"}]} + ): + result = self.client_post( + "/api/v1/fetch_api_key", + dict(username=self.example_email("hamlet"), password=self.ldap_password("hamlet")), + ) + self.assert_json_error(result, "Your username or password is incorrect.", 403) + def test_inactive_user(self) -> None: do_deactivate_user(self.user_profile, acting_user=None) result = self.client_post( diff --git a/zproject/backends.py b/zproject/backends.py index 741bb9b49f..be442fbd4a 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -632,12 +632,46 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): return ldap_disabled def is_account_realm_access_forbidden(self, ldap_user: _LDAPUser, realm: Realm) -> bool: - if "org_membership" not in settings.AUTH_LDAP_USER_ATTR_MAP: + # org_membership takes priority over AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL. + if "org_membership" in settings.AUTH_LDAP_USER_ATTR_MAP: + org_membership_attr = settings.AUTH_LDAP_USER_ATTR_MAP["org_membership"] + allowed_orgs: List[str] = ldap_user.attrs.get(org_membership_attr, []) + if is_subdomain_in_allowed_subdomains_list(realm.subdomain, allowed_orgs): + return False + # If Advanced is not configured, forbid access + if settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL is None: + return True + + # If neither setting is configured, allow access. + if settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL is None: return False - org_membership_attr = settings.AUTH_LDAP_USER_ATTR_MAP["org_membership"] - allowed_orgs: List[str] = ldap_user.attrs.get(org_membership_attr, []) - return not is_subdomain_in_allowed_subdomains_list(realm.subdomain, allowed_orgs) + # With settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL, we + # allow access if and only if one of the entries for the + # target subdomain matches the user's LDAP attributes. + realm_access_control = settings.AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL + if not ( + isinstance(realm_access_control, dict) + and realm.subdomain in realm_access_control + and isinstance(realm_access_control[realm.subdomain], list) + and len(realm_access_control[realm.subdomain]) > 0 + ): + # If configuration is wrong, do not allow access + return True + + # Go through every "or" check + for attribute_group in realm_access_control[realm.subdomain]: + access = True + for attribute in attribute_group: + if not ( + attribute in ldap_user.attrs + and attribute_group[attribute] in ldap_user.attrs[attribute] + ): + access = False + if access: + return False + + return True @classmethod def get_mapped_name(cls, ldap_user: _LDAPUser) -> str: diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 6581979cf7..43e35b8690 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -63,6 +63,7 @@ AUTH_LDAP_ALWAYS_UPDATE_USER = False # Detailed docs in zproject/dev_settings.py. FAKE_LDAP_MODE: Optional[str] = None FAKE_LDAP_NUM_USERS = 8 +AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL = None # Social auth; we support providing values for some of these # settings in zulip-secrets.conf instead of settings.py in development. diff --git a/zproject/prod_settings_template.py b/zproject/prod_settings_template.py index 9546554af4..bba0858475 100644 --- a/zproject/prod_settings_template.py +++ b/zproject/prod_settings_template.py @@ -247,6 +247,16 @@ AUTH_LDAP_USER_ATTR_MAP = { ## False. # LDAP_DEACTIVATE_NON_MATCHING_USERS = True +## See: https://zulip.readthedocs.io/en/latest/production/authentication-methods.html#restricting-ldap-user-access-to-specific-organizations +# AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL = { +# "zulip": +# [ # OR +# { # AND +# "department": "main", +# "employeeType": "staff" +# } +# ] +# } ######## ## Google OAuth.