dev_ldap: Make userPassword a multi-value attribute.

`fakeldap` assumes every attribute to be a multi-value attribute
while making comparison in `_comapare_s()` and so while making
comparisons for password it gives a false positive. The result
of this was that it was possible to login in the dev environment
using LDAP using a substring of the password. For example, if the
LDAP password is `ldapuser1` even entering `u` would log you in.
This commit is contained in:
Harshit Bansal 2019-01-16 07:36:27 +00:00 committed by Tim Abbott
parent 3c4e3ddcdb
commit 356c5bfb0e
5 changed files with 42 additions and 28 deletions

View File

@ -25,7 +25,7 @@ def generate_dev_ldap_dir(mode: str, num_users: int=8) -> Dict[str, Dict[str, An
email_username = email.split('@')[0]
ldap_dir['uid=' + email + ',ou=users,dc=zulip,dc=com'] = {
'cn': [name[0], ],
'userPassword': email_username,
'userPassword': [email_username, ],
'thumbnailPhoto': [profile_images[i % len(profile_images)], ],
'userAccountControl': [LDAP_USER_ACCOUNT_CONTROL_NORMAL, ],
}
@ -34,7 +34,7 @@ def generate_dev_ldap_dir(mode: str, num_users: int=8) -> Dict[str, Dict[str, An
email_username = email.split('@')[0]
ldap_dir['uid=' + email_username + ',ou=users,dc=zulip,dc=com'] = {
'cn': [name[0], ],
'userPassword': email_username,
'userPassword': [email_username, ],
'jpegPhoto': [profile_images[i % len(profile_images)], ],
}
elif mode == 'c':
@ -42,7 +42,7 @@ def generate_dev_ldap_dir(mode: str, num_users: int=8) -> Dict[str, Dict[str, An
email_username = email.split('@')[0]
ldap_dir['uid=' + email_username + ',ou=users,dc=zulip,dc=com'] = {
'cn': [name[0], ],
'userPassword': email_username + '_test',
'userPassword': [email_username + '_test', ],
'email': email,
}

View File

@ -1428,7 +1428,7 @@ class FetchAPIKeyTest(ZulipTestCase):
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -1681,7 +1681,7 @@ class TestTwoFactor(ZulipTestCase):
full_name = 'New LDAP fullname'
mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': [full_name],
'sn': ['shortname'],
}
@ -2162,11 +2162,25 @@ class TestLDAP(ZulipLDAPTestCase):
self.assertTrue(regex.match(key))
self.assertCountEqual(list(value.keys()), ['cn', 'userPassword', 'email'])
@override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',))
def test_dev_ldap_fail_login(self) -> None:
# Tests that login with a substring of password fails. We had a bug in
# dev LDAP environment that allowed login via password substrings.
self.mock_ldap.directory = generate_dev_ldap_dir('B', 8)
with self.settings(
LDAP_APPEND_DOMAIN='zulip.com',
AUTH_LDAP_BIND_PASSWORD='',
AUTH_LDAP_USER_DN_TEMPLATE='uid=%(user)s,ou=users,dc=zulip,dc=com'):
user_profile = self.backend.authenticate('ldapuser1', 'dapu',
realm=get_realm('zulip'))
assert(user_profile is None)
@override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',))
def test_login_success(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2183,7 +2197,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_success_with_username(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2200,7 +2214,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_success_with_email_attr(self) -> None:
self.mock_ldap.directory = {
'uid=letham,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'email': ['hamlet@zulip.com'],
}
}
@ -2217,7 +2231,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_failure_due_to_wrong_password(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2231,7 +2245,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_failure_due_to_nonexistent_user(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2373,7 +2387,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_failure_due_to_wrong_subdomain(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2388,7 +2402,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_failure_due_to_invalid_subdomain(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2403,7 +2417,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_success_with_valid_subdomain(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
with self.settings(
@ -2419,7 +2433,7 @@ class TestLDAP(ZulipLDAPTestCase):
def test_login_failure_due_to_deactivated_user(self) -> None:
self.mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing'
'userPassword': ['testing', ]
}
}
user_profile = self.example_user("hamlet")
@ -2442,7 +2456,7 @@ class TestLDAP(ZulipLDAPTestCase):
self.mock_ldap.directory = {
'uid=nonexisting,ou=users,dc=acme,dc=com': {
'cn': ['NonExisting', ],
'userPassword': 'testing',
'userPassword': ['testing', ],
'thumbnailPhoto': [open(os.path.join(settings.STATIC_ROOT, "images/team/tim.png"), "rb").read()],
}
}
@ -2468,7 +2482,7 @@ class TestLDAP(ZulipLDAPTestCase):
'uid=nonexisting,ou=users,dc=acme,dc=com': {
'fn': ['Non', ],
'ln': ['Existing', ],
'userPassword': 'testing',
'userPassword': ['testing', ],
}
}
with self.settings(

View File

@ -68,7 +68,7 @@ class TestFollowupEmails(ZulipTestCase):
mock_ldap.directory = {
"uid=newuser@zulip.com,ou=users,dc=zulip,dc=com": {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['full_name'],
'sn': ['shortname'],
}
@ -100,7 +100,7 @@ class TestFollowupEmails(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['full_name'],
'sn': ['shortname'],
}
@ -133,7 +133,7 @@ class TestFollowupEmails(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['full_name'],
'sn': ['shortname'],
'email': ['newuser_email@zulip.com'],

View File

@ -199,7 +199,7 @@ class ChangeSettingsTest(ZulipTestCase):
mock_ldap.directory = {
'uid=hamlet,ou=users,dc=zulip,dc=com': {
'userPassword': 'ldappassword',
'userPassword': ['ldappassword', ],
'fn': ['New LDAP fullname']
}
}

View File

@ -2409,7 +2409,7 @@ class UserSignUpTest(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['New LDAP fullname']
}
}
@ -2464,7 +2464,7 @@ class UserSignUpTest(ZulipTestCase):
# Test the TypeError exception handler
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': None # This will raise TypeError
}
}
@ -2494,7 +2494,7 @@ class UserSignUpTest(ZulipTestCase):
full_name = 'New LDAP fullname'
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': [full_name],
'sn': ['shortname'],
}
@ -2566,7 +2566,7 @@ class UserSignUpTest(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['First'],
'ln': ['Last'],
}
@ -2636,7 +2636,7 @@ class UserSignUpTest(ZulipTestCase):
full_name = 'New LDAP fullname'
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': [full_name],
'sn': ['shortname'],
}
@ -2672,7 +2672,7 @@ class UserSignUpTest(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['New LDAP fullname'],
'sn': ['New LDAP shortname'],
}
@ -2730,7 +2730,7 @@ class UserSignUpTest(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['New LDAP fullname'],
'sn': ['New LDAP shortname'],
}
@ -2845,7 +2845,7 @@ class UserSignUpTest(ZulipTestCase):
mock_ldap.directory = {
'uid=newuser,ou=users,dc=zulip,dc=com': {
'userPassword': 'testing',
'userPassword': ['testing', ],
'fn': ['New User Name']
}
}