From 4ea73581bf8afdaec29603f7b1574172b1c1489f Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Mon, 11 Feb 2019 22:17:02 -0800 Subject: [PATCH 01/10] reenabled LDAP privileged binds --- ocflib/account/creation.py | 9 ++++----- ocflib/infra/ldap.py | 21 ++++++++++++++------- ocflib/vhost/web.py | 3 +-- tests/account/creation_test.py | 1 - 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/ocflib/account/creation.py b/ocflib/account/creation.py index 5f4f585c..92f25df3 100644 --- a/ocflib/account/creation.py +++ b/ocflib/account/creation.py @@ -263,12 +263,11 @@ def validate_calnet_uid(uid): if not attrs: raise ValidationError("CalNet UID can't be found in university LDAP.") - # TODO: Uncomment when we get privileged LDAP bind. # check if user is eligible for an account - # affiliations = attrs['berkeleyEduAffiliations'] - # if not eligible_for_account(affiliations): - # raise ValidationWarning( - # 'Affiliate type not eligible for account: ' + str(affiliations)) + affiliations = attrs['berkeleyEduAffiliations'] + if not eligible_for_account(affiliations): + raise ValidationWarning( + 'Affiliate type not eligible for account: ' + str(affiliations)) def eligible_for_account(affiliations): diff --git a/ocflib/infra/ldap.py b/ocflib/infra/ldap.py index b96c2bbe..c1830fba 100644 --- a/ocflib/infra/ldap.py +++ b/ocflib/infra/ldap.py @@ -21,11 +21,11 @@ UCB_LDAP = 'ldap.berkeley.edu' UCB_LDAP_URL = 'ldaps://' + UCB_LDAP UCB_LDAP_PEOPLE = 'ou=People,dc=Berkeley,dc=EDU' - +UCB_LDAP_DN = 'uid=ocf,ou=applications,dc=berkeley,dc=edu' @contextmanager -def ldap_connection(host): - """Context manager that provides an ldap3 Connection. +def ldap_connection(host, dn=None, password=None): + """Context manager that provides an ldap3 Connection. Also supports optional arguments for a privileged bind. Example usage: @@ -37,10 +37,14 @@ def ldap_connection(host): :param host: server hostname """ + server = ldap3.Server(host, use_ssl=True) - with ldap3.Connection(server) as connection: - yield connection - + if(dn is None or password is None): + with ldap3.Connection(server) as connection: + yield connection + else: + with ldap3.Connection(server, dn, password) as connection: + yield connection def ldap_ocf(): """Context manager that provides an ldap3 Connection to OCF's LDAP server. @@ -61,7 +65,10 @@ def ldap_ucb(): with ldap_ucb() as c: c.search(UCB_LDAP_PEOPLE, '(uid=ckuehl)', attributes=['uidNumber']) """ - return ldap_connection(UCB_LDAP) + with open('/etc/ucbldap.passwd', 'r') as passwordFile: + password = passwordFile.read() + return ldap_connection(UCB_LDAP, UCB_LDAP_DN, password) + def _format_attr(key, values): diff --git a/ocflib/vhost/web.py b/ocflib/vhost/web.py index 5e1a5f57..d512153a 100644 --- a/ocflib/vhost/web.py +++ b/ocflib/vhost/web.py @@ -87,8 +87,7 @@ def eligible_for_vhost(user): return True elif 'calnetUid' in attrs: attrs_ucb = user_attrs_ucb(attrs['calnetUid']) - # TODO: Uncomment when we get a privileged LDAP bind. - if attrs_ucb: # and 'EMPLOYEE-TYPE-ACADEMIC' in attrs_ucb['berkeleyEduAffiliations']: + if attrs_ucb and 'EMPLOYEE-TYPE-ACADEMIC' in attrs_ucb['berkeleyEduAffiliations']: return True return False diff --git a/tests/account/creation_test.py b/tests/account/creation_test.py index a40b55f0..3023d659 100644 --- a/tests/account/creation_test.py +++ b/tests/account/creation_test.py @@ -276,7 +276,6 @@ def test_validate_calnet_uid_error(self, bad_uid): def test_validate_calnet_uid_success(self, mock_valid_calnet_uid): validate_calnet_uid(9999999999999) - @pytest.mark.skip(reason='Checking for affiliations temp. patched out (ocflib PR 140)') def test_validate_calnet_affiliations_failure(self, mock_invalid_calnet_uid): with pytest.raises(ValidationWarning): validate_calnet_uid(9999999999999) From cf922618fc9280ab0423d00f7412849b5eca4d71 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Mon, 11 Feb 2019 22:19:41 -0800 Subject: [PATCH 02/10] minor style fixes --- ocflib/infra/ldap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocflib/infra/ldap.py b/ocflib/infra/ldap.py index c1830fba..5f987956 100644 --- a/ocflib/infra/ldap.py +++ b/ocflib/infra/ldap.py @@ -39,7 +39,7 @@ def ldap_connection(host, dn=None, password=None): """ server = ldap3.Server(host, use_ssl=True) - if(dn is None or password is None): + if dn is None or password is None: with ldap3.Connection(server) as connection: yield connection else: From f7ad601118be0e5d39deb77a989ec84546f56483 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 12 Feb 2019 19:56:12 -0800 Subject: [PATCH 03/10] allow both unprivileged/privileged binds and dependent changes --- ocflib/account/creation.py | 6 +++++- ocflib/account/search.py | 10 +++++----- ocflib/infra/ldap.py | 37 +++++++++++++++++++++---------------- ocflib/vhost/web.py | 6 +++++- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/ocflib/account/creation.py b/ocflib/account/creation.py index 92f25df3..54a6a437 100644 --- a/ocflib/account/creation.py +++ b/ocflib/account/creation.py @@ -22,6 +22,8 @@ from ocflib.infra.ldap import create_ldap_entry from ocflib.infra.ldap import ldap_ocf from ocflib.infra.ldap import OCF_LDAP_PEOPLE +from ocflib.infra.ldap import UCB_LDAP_DN +from ocflib.infra.ldap import read_ucb_password from ocflib.misc.mail import jinja_mail_env from ocflib.misc.mail import send_mail from ocflib.misc.validators import valid_email @@ -257,8 +259,10 @@ def validate_calnet_uid(uid): if existing_accounts: raise ValidationError( 'CalNet UID already has account: ' + str(existing_accounts)) + + ldap_password = read_ucb_password() - attrs = search.user_attrs_ucb(uid) + attrs = search.user_attrs_ucb(uid, UCB_LDAP_DN, ldap_password) if not attrs: raise ValidationError("CalNet UID can't be found in university LDAP.") diff --git a/ocflib/account/search.py b/ocflib/account/search.py index 50161d9e..92833fcf 100644 --- a/ocflib/account/search.py +++ b/ocflib/account/search.py @@ -32,7 +32,7 @@ def users_by_callink_oid(callink_oid): return users_by_filter('(callinkOid={})'.format(callink_oid)) -def user_attrs(uid, connection=ldap.ldap_ocf, base=OCF_LDAP_PEOPLE): +def user_attrs(uid, connection=ldap.ldap_ocf, base=OCF_LDAP_PEOPLE, dn=None, password=None): """Returns a dictionary of LDAP attributes for a given LDAP UID. The returned dictionary looks like: @@ -44,16 +44,16 @@ def user_attrs(uid, connection=ldap.ldap_ocf, base=OCF_LDAP_PEOPLE): Returns None if no account exists with uid=user_account. """ - with connection() as c: + with connection(dn, password) as c: c.search(base, '(uid={})'.format(uid), attributes=ldap3.ALL_ATTRIBUTES) if len(c.response) > 0: return c.response[0]['attributes'] -def user_attrs_ucb(uid): - return user_attrs(uid, connection=ldap.ldap_ucb, - base=UCB_LDAP_PEOPLE) +def user_attrs_ucb(uid, dn=None, password=None): + return user_attrs(uid, connection=ldap.ldap_ucb, + base=UCB_LDAP_PEOPLE, dn=dn, password=password) def user_exists(account): diff --git a/ocflib/infra/ldap.py b/ocflib/infra/ldap.py index 5f987956..92b428f5 100644 --- a/ocflib/infra/ldap.py +++ b/ocflib/infra/ldap.py @@ -22,10 +22,11 @@ UCB_LDAP_URL = 'ldaps://' + UCB_LDAP UCB_LDAP_PEOPLE = 'ou=People,dc=Berkeley,dc=EDU' UCB_LDAP_DN = 'uid=ocf,ou=applications,dc=berkeley,dc=edu' +UCB_LDAP_PASSWORD_PATH = '' @contextmanager def ldap_connection(host, dn=None, password=None): - """Context manager that provides an ldap3 Connection. Also supports optional arguments for a privileged bind. + """Context manager that provides an ldap3 Connection. Also supports optional credentials for a privileged bind. Example usage: @@ -39,35 +40,29 @@ def ldap_connection(host, dn=None, password=None): """ server = ldap3.Server(host, use_ssl=True) - if dn is None or password is None: - with ldap3.Connection(server) as connection: - yield connection - else: - with ldap3.Connection(server, dn, password) as connection: - yield connection + with ldap3.Connection(server, dn, password) as connection: + yield connection -def ldap_ocf(): - """Context manager that provides an ldap3 Connection to OCF's LDAP server. +def ldap_ocf(dn=None, password=None): + """Context manager that provides an ldap3 Connection to OCF's LDAP server. Accepts optional DN and password. Example usage: with ldap_ocf() as c: c.search(OCF_LDAP_PEOPLE, '(uid=ckuehl)', attributes=['uidNumber']) """ - return ldap_connection(OCF_LDAP) - + return ldap_connection(OCF_LDAP, dn, password) -def ldap_ucb(): - """Context manager that provides an ldap3 Connection to the campus LDAP. +def ldap_ucb(dn=None, password=None): + """Context manager that provides an ldap3 Connection to the campus LDAP. Accepts optional DN and password. Example usage: with ldap_ucb() as c: c.search(UCB_LDAP_PEOPLE, '(uid=ckuehl)', attributes=['uidNumber']) """ - with open('/etc/ucbldap.passwd', 'r') as passwordFile: - password = passwordFile.read() - return ldap_connection(UCB_LDAP, UCB_LDAP_DN, password) + return ldap_connection(UCB_LDAP, dn, password) + @@ -233,3 +228,13 @@ def format_timestamp(timestamp): if timestamp.tzinfo is None or timestamp.tzinfo.utcoffset(timestamp) is None: raise ValueError('Timestamp has no timezone info') return timestamp.strftime('%Y%m%d%H%M%S%z') + +def read_ucb_password(): + """Returns a string of the current campus LDAP privileged bind password + found in UCB_LDAP_PASSWORD_PATH + + :return: A string of the campus LDAP bind password + """ + + with open('/etc/ucbldap.passwd', 'r') as passwordFile: + return passwordFile.read() \ No newline at end of file diff --git a/ocflib/vhost/web.py b/ocflib/vhost/web.py index d512153a..3ec5b228 100644 --- a/ocflib/vhost/web.py +++ b/ocflib/vhost/web.py @@ -4,6 +4,8 @@ from ocflib.account.search import user_attrs from ocflib.account.search import user_attrs_ucb +from ocflib.infra.ldap import UCB_LDAP_DN +from ocflib.infra.ldap import read_ucb_password VHOST_DB_PATH = '/home/s/st/staff/vhost/vhost.conf' VHOST_DB_URL = 'https://www.ocf.berkeley.edu/~staff/vhost.conf' @@ -86,7 +88,9 @@ def eligible_for_vhost(user): if 'callinkOid' in attrs: return True elif 'calnetUid' in attrs: - attrs_ucb = user_attrs_ucb(attrs['calnetUid']) + ldap_password = read_ucb_password() + attrs_ucb = user_attrs_ucb(attrs['calnetUid'], UCB_LDAP_DN, ldap_password) + if attrs_ucb and 'EMPLOYEE-TYPE-ACADEMIC' in attrs_ucb['berkeleyEduAffiliations']: return True From af23ec0ac6ba5e2da05f36f39c743e77db659d94 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 12 Feb 2019 19:58:04 -0800 Subject: [PATCH 04/10] minor style fixes --- ocflib/account/creation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ocflib/account/creation.py b/ocflib/account/creation.py index 54a6a437..c0601e1c 100644 --- a/ocflib/account/creation.py +++ b/ocflib/account/creation.py @@ -261,7 +261,6 @@ def validate_calnet_uid(uid): 'CalNet UID already has account: ' + str(existing_accounts)) ldap_password = read_ucb_password() - attrs = search.user_attrs_ucb(uid, UCB_LDAP_DN, ldap_password) if not attrs: From ac4e65ab6e1dbb86888d74ca3484256129d77272 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 00:08:01 -0800 Subject: [PATCH 05/10] modified tests to mock read_ucb_password --- tests/account/creation_test.py | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/tests/account/creation_test.py b/tests/account/creation_test.py index 3023d659..2151f3bf 100644 --- a/tests/account/creation_test.py +++ b/tests/account/creation_test.py @@ -265,13 +265,13 @@ def test_long_names(self, _, __): class TestAccountEligibility: - @pytest.mark.parametrize('bad_uid', [ - 1101587, # good uid, but already has account - 9999999999, # fake uid, not in university ldap - ]) - def test_validate_calnet_uid_error(self, bad_uid): + def test_validate_calnet_uid_error(self, mock_existing_calnet_uid): + with pytest.raises(ValidationError): + validate_calnet_uid(1101587) + + def test_validate_nonexistent_calnet_uid(self, mock_nonexistent_calnet_uid): with pytest.raises(ValidationError): - validate_calnet_uid(bad_uid) + validate_calnet_uid(9999999999) def test_validate_calnet_uid_success(self, mock_valid_calnet_uid): validate_calnet_uid(9999999999999) @@ -421,13 +421,28 @@ def fake_credentials(mock_rsa_key): redis_uri='redis://create', ) +@pytest.yield_fixture +def mock_existing_calnet_uid(): + with mock.patch( + 'ocflib.account.search.users_by_calnet_uid', + return_value=["not empty"] + ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): + yield + +@pytest.yield_fixture +def mock_nonexistent_calnet_uid(): + with mock.patch( + 'ocflib.account.search.users_by_calnet_uid', + return_value=None + ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): + yield @pytest.yield_fixture def mock_valid_calnet_uid(): with mock.patch( 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-TYPE-REGISTERED']} - ): + ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): yield @@ -436,7 +451,7 @@ def mock_invalid_calnet_uid(): with mock.patch( 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-STATUS-EXPIRED']}, - ): + ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): yield From 74b1ca1fcd69770bd910ae8ec833b35b3bcb26bd Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 00:51:31 -0800 Subject: [PATCH 06/10] fixed tests again --- tests/account/creation_test.py | 24 ++++++++++++++++-------- tests/vhost/web_test.py | 28 +++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/tests/account/creation_test.py b/tests/account/creation_test.py index 2151f3bf..8dea5af4 100644 --- a/tests/account/creation_test.py +++ b/tests/account/creation_test.py @@ -426,24 +426,31 @@ def mock_existing_calnet_uid(): with mock.patch( 'ocflib.account.search.users_by_calnet_uid', return_value=["not empty"] - ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): - yield + ): + with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): + yield @pytest.yield_fixture def mock_nonexistent_calnet_uid(): with mock.patch( 'ocflib.account.search.users_by_calnet_uid', return_value=None - ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): - yield + ): + with mock.patch( + 'ocflib.account.search.user_attrs_ucb', + return_value=None + ): + with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): + yield @pytest.yield_fixture def mock_valid_calnet_uid(): with mock.patch( 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-TYPE-REGISTERED']} - ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): - yield + ): + with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): + yield @pytest.yield_fixture @@ -451,8 +458,9 @@ def mock_invalid_calnet_uid(): with mock.patch( 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-STATUS-EXPIRED']}, - ), mock.patch('ocflib.infra.ldap.read_ucb_password', return_value=""): - yield + ): + with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): + yield class TestValidateRequest: diff --git a/tests/vhost/web_test.py b/tests/vhost/web_test.py index 2f649e30..de28ab20 100644 --- a/tests/vhost/web_test.py +++ b/tests/vhost/web_test.py @@ -52,6 +52,28 @@ def mock_get_vhosts_db(): ): yield +@pytest.yield_fixture +def mock_ucb_attrs_allowed(): + with mock.patch( + 'ocflib.account.search.user_attrs_ucb', + return_value={'callinkOid': ['0']} + ): + with mock.patch('ocflib.vhost.web.read_ucb_password', + return_value="" + ): + yield + +@pytest.yield_fixturee +def mock_ucb_attrs_disallowed(): + with mock.patch( + 'ocflib.account.search.user_attrs_ucb', + return_value=None + ): + with mock.patch('ocflib.vhost.web.read_ucb_password', + return_value="" + ): + yield + class TestVirtualHosts: @@ -85,9 +107,13 @@ def test_has_vhost(self, user, should_have_vhost, mock_get_vhosts_db): assert has_vhost(user) == should_have_vhost @pytest.mark.parametrize('user,should_be_eligible', [ - ('mattmcal', False), ('ggroup', True), ('bh', True), ]) + @pytest.mark.usefixtures('mock_ucb_attrs_eligible') def test_eligible_for_vhost(self, user, should_be_eligible): assert eligible_for_vhost(user) == should_be_eligible + + @pytest.mark.usefixtures('mock_ucb_attrs_disallowed') + def test_not_eligible_for_vhost(self, user, should_be_eligible): + assert eligible_for_vhost('mattmcal') == False From 2cf8287a64009e015a19b53d0420b261d6539fc4 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 01:12:45 -0800 Subject: [PATCH 07/10] fixed using constant for string and more tests --- ocflib/infra/ldap.py | 4 ++-- tests/vhost/web_test.py | 43 +++++++++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/ocflib/infra/ldap.py b/ocflib/infra/ldap.py index 92b428f5..3267445c 100644 --- a/ocflib/infra/ldap.py +++ b/ocflib/infra/ldap.py @@ -22,7 +22,7 @@ UCB_LDAP_URL = 'ldaps://' + UCB_LDAP UCB_LDAP_PEOPLE = 'ou=People,dc=Berkeley,dc=EDU' UCB_LDAP_DN = 'uid=ocf,ou=applications,dc=berkeley,dc=edu' -UCB_LDAP_PASSWORD_PATH = '' +UCB_LDAP_PASSWORD_PATH = '/etc/ucbldap.passwd' @contextmanager def ldap_connection(host, dn=None, password=None): @@ -236,5 +236,5 @@ def read_ucb_password(): :return: A string of the campus LDAP bind password """ - with open('/etc/ucbldap.passwd', 'r') as passwordFile: + with open(UCB_LDAP_PASSWORD_PATH, 'r') as passwordFile: return passwordFile.read() \ No newline at end of file diff --git a/tests/vhost/web_test.py b/tests/vhost/web_test.py index de28ab20..3e80c948 100644 --- a/tests/vhost/web_test.py +++ b/tests/vhost/web_test.py @@ -53,9 +53,9 @@ def mock_get_vhosts_db(): yield @pytest.yield_fixture -def mock_ucb_attrs_allowed(): +def mock_group_ucb_attrs(): with mock.patch( - 'ocflib.account.search.user_attrs_ucb', + 'ocflib.vhost.web.user_attrs', return_value={'callinkOid': ['0']} ): with mock.patch('ocflib.vhost.web.read_ucb_password', @@ -63,10 +63,25 @@ def mock_ucb_attrs_allowed(): ): yield -@pytest.yield_fixturee -def mock_ucb_attrs_disallowed(): +@pytest.yield_fixture +def mock_staff_ucb_attrs(): with mock.patch( - 'ocflib.account.search.user_attrs_ucb', + 'ocflib.vhost.web.user_attrs_ucb', + return_value={'berkeleyEduAffiliations': ['EMPLOYEE-TYPE-ACADEMIC']} + ): + with mock.patch( + 'ocflib.vhost.web.user_attrs', + return_value={'calnetUid': ['0']} + ): + with mock.patch('ocflib.vhost.web.read_ucb_password', + return_value="" + ): + yield + +@pytest.yield_fixture +def mock_ucb_attrs_uneligible(): + with mock.patch( + 'ocflib.vhost.web.user_attrs_ucb', return_value=None ): with mock.patch('ocflib.vhost.web.read_ucb_password', @@ -106,14 +121,14 @@ def test_proper_parse(self, mock_get_vhosts_db): def test_has_vhost(self, user, should_have_vhost, mock_get_vhosts_db): assert has_vhost(user) == should_have_vhost - @pytest.mark.parametrize('user,should_be_eligible', [ - ('ggroup', True), - ('bh', True), - ]) - @pytest.mark.usefixtures('mock_ucb_attrs_eligible') - def test_eligible_for_vhost(self, user, should_be_eligible): - assert eligible_for_vhost(user) == should_be_eligible + @pytest.mark.usefixtures('mock_group_ucb_attrs') + def test_groups_eligible_for_vhost(self): + assert eligible_for_vhost('ggroups') == True + + @pytest.mark.usefixtures('mock_staff_ucb_attrs') + def test_staff_eligible_for_vhost(self): + assert eligible_for_vhost('bh') == True - @pytest.mark.usefixtures('mock_ucb_attrs_disallowed') - def test_not_eligible_for_vhost(self, user, should_be_eligible): + @pytest.mark.usefixtures('mock_ucb_attrs_uneligible') + def test_not_eligible_for_vhost(self): assert eligible_for_vhost('mattmcal') == False From 02610d2656851c56b83cda3ac58a4c996cc0451b Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 01:34:41 -0800 Subject: [PATCH 08/10] refactored to keep password in file, fixed tests --- ocflib/account/creation.py | 7 ++---- ocflib/account/search.py | 6 ++--- ocflib/infra/ldap.py | 25 +++++++++++++++++---- ocflib/vhost/web.py | 5 +---- tests/account/creation_test.py | 19 ++++++++-------- tests/vhost/web_test.py | 40 +++++++++++++++------------------- 6 files changed, 53 insertions(+), 49 deletions(-) diff --git a/ocflib/account/creation.py b/ocflib/account/creation.py index c0601e1c..92f25df3 100644 --- a/ocflib/account/creation.py +++ b/ocflib/account/creation.py @@ -22,8 +22,6 @@ from ocflib.infra.ldap import create_ldap_entry from ocflib.infra.ldap import ldap_ocf from ocflib.infra.ldap import OCF_LDAP_PEOPLE -from ocflib.infra.ldap import UCB_LDAP_DN -from ocflib.infra.ldap import read_ucb_password from ocflib.misc.mail import jinja_mail_env from ocflib.misc.mail import send_mail from ocflib.misc.validators import valid_email @@ -259,9 +257,8 @@ def validate_calnet_uid(uid): if existing_accounts: raise ValidationError( 'CalNet UID already has account: ' + str(existing_accounts)) - - ldap_password = read_ucb_password() - attrs = search.user_attrs_ucb(uid, UCB_LDAP_DN, ldap_password) + + attrs = search.user_attrs_ucb(uid) if not attrs: raise ValidationError("CalNet UID can't be found in university LDAP.") diff --git a/ocflib/account/search.py b/ocflib/account/search.py index 92833fcf..a983516a 100644 --- a/ocflib/account/search.py +++ b/ocflib/account/search.py @@ -51,9 +51,9 @@ def user_attrs(uid, connection=ldap.ldap_ocf, base=OCF_LDAP_PEOPLE, dn=None, pas return c.response[0]['attributes'] -def user_attrs_ucb(uid, dn=None, password=None): - return user_attrs(uid, connection=ldap.ldap_ucb, - base=UCB_LDAP_PEOPLE, dn=dn, password=password) +def user_attrs_ucb(uid): + return user_attrs(uid, connection=ldap.ldap_ucb_privileged, + base=UCB_LDAP_PEOPLE) def user_exists(account): diff --git a/ocflib/infra/ldap.py b/ocflib/infra/ldap.py index 3267445c..3d1d10f1 100644 --- a/ocflib/infra/ldap.py +++ b/ocflib/infra/ldap.py @@ -24,6 +24,7 @@ UCB_LDAP_DN = 'uid=ocf,ou=applications,dc=berkeley,dc=edu' UCB_LDAP_PASSWORD_PATH = '/etc/ucbldap.passwd' + @contextmanager def ldap_connection(host, dn=None, password=None): """Context manager that provides an ldap3 Connection. Also supports optional credentials for a privileged bind. @@ -38,11 +39,12 @@ def ldap_connection(host, dn=None, password=None): :param host: server hostname """ - + server = ldap3.Server(host, use_ssl=True) with ldap3.Connection(server, dn, password) as connection: yield connection + def ldap_ocf(dn=None, password=None): """Context manager that provides an ldap3 Connection to OCF's LDAP server. Accepts optional DN and password. @@ -53,6 +55,7 @@ def ldap_ocf(dn=None, password=None): """ return ldap_connection(OCF_LDAP, dn, password) + def ldap_ucb(dn=None, password=None): """Context manager that provides an ldap3 Connection to the campus LDAP. Accepts optional DN and password. @@ -63,7 +66,20 @@ def ldap_ucb(dn=None, password=None): """ return ldap_connection(UCB_LDAP, dn, password) - + +def ldap_ucb_privileged(dn=None, password=None): + """Context manager that provides a privileged ldap3 Connection to the campus LDAP. + + Note that this method will ignore all dn and password arguments, + which are being kept for compatibility with user_attrs(). + + Example usage: + + with ldap_ucb_privileged() as c: + c.search(UCB_LDAP_PEOPLE, '(uid=ckuehl)', attributes=['uidNumber']) + """ + password = _read_ucb_password() + return ldap_ucb(UCB_LDAP_DN, password) def _format_attr(key, values): @@ -229,7 +245,8 @@ def format_timestamp(timestamp): raise ValueError('Timestamp has no timezone info') return timestamp.strftime('%Y%m%d%H%M%S%z') -def read_ucb_password(): + +def _read_ucb_password(): """Returns a string of the current campus LDAP privileged bind password found in UCB_LDAP_PASSWORD_PATH @@ -237,4 +254,4 @@ def read_ucb_password(): """ with open(UCB_LDAP_PASSWORD_PATH, 'r') as passwordFile: - return passwordFile.read() \ No newline at end of file + return passwordFile.read() diff --git a/ocflib/vhost/web.py b/ocflib/vhost/web.py index 3ec5b228..fe65954e 100644 --- a/ocflib/vhost/web.py +++ b/ocflib/vhost/web.py @@ -4,8 +4,6 @@ from ocflib.account.search import user_attrs from ocflib.account.search import user_attrs_ucb -from ocflib.infra.ldap import UCB_LDAP_DN -from ocflib.infra.ldap import read_ucb_password VHOST_DB_PATH = '/home/s/st/staff/vhost/vhost.conf' VHOST_DB_URL = 'https://www.ocf.berkeley.edu/~staff/vhost.conf' @@ -88,8 +86,7 @@ def eligible_for_vhost(user): if 'callinkOid' in attrs: return True elif 'calnetUid' in attrs: - ldap_password = read_ucb_password() - attrs_ucb = user_attrs_ucb(attrs['calnetUid'], UCB_LDAP_DN, ldap_password) + attrs_ucb = user_attrs_ucb(attrs['calnetUid']) if attrs_ucb and 'EMPLOYEE-TYPE-ACADEMIC' in attrs_ucb['berkeleyEduAffiliations']: return True diff --git a/tests/account/creation_test.py b/tests/account/creation_test.py index 8dea5af4..c0c0390b 100644 --- a/tests/account/creation_test.py +++ b/tests/account/creation_test.py @@ -421,14 +421,15 @@ def fake_credentials(mock_rsa_key): redis_uri='redis://create', ) + @pytest.yield_fixture def mock_existing_calnet_uid(): with mock.patch( 'ocflib.account.search.users_by_calnet_uid', - return_value=["not empty"] + return_value=['not empty'] ): - with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): - yield + yield + @pytest.yield_fixture def mock_nonexistent_calnet_uid(): @@ -440,17 +441,16 @@ def mock_nonexistent_calnet_uid(): 'ocflib.account.search.user_attrs_ucb', return_value=None ): - with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): - yield + yield + @pytest.yield_fixture def mock_valid_calnet_uid(): with mock.patch( 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-TYPE-REGISTERED']} - ): - with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): - yield + ): + yield @pytest.yield_fixture @@ -459,8 +459,7 @@ def mock_invalid_calnet_uid(): 'ocflib.account.search.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['STUDENT-STATUS-EXPIRED']}, ): - with mock.patch('ocflib.account.creation.read_ucb_password', return_value=""): - yield + yield class TestValidateRequest: diff --git a/tests/vhost/web_test.py b/tests/vhost/web_test.py index 3e80c948..b4583757 100644 --- a/tests/vhost/web_test.py +++ b/tests/vhost/web_test.py @@ -52,42 +52,36 @@ def mock_get_vhosts_db(): ): yield + @pytest.yield_fixture -def mock_group_ucb_attrs(): +def mock_group_user_attrs(): with mock.patch( 'ocflib.vhost.web.user_attrs', return_value={'callinkOid': ['0']} - ): - with mock.patch('ocflib.vhost.web.read_ucb_password', - return_value="" - ): - yield + ): + yield + @pytest.yield_fixture def mock_staff_ucb_attrs(): with mock.patch( 'ocflib.vhost.web.user_attrs_ucb', return_value={'berkeleyEduAffiliations': ['EMPLOYEE-TYPE-ACADEMIC']} - ): + ): with mock.patch( 'ocflib.vhost.web.user_attrs', return_value={'calnetUid': ['0']} - ): - with mock.patch('ocflib.vhost.web.read_ucb_password', - return_value="" - ): - yield + ): + yield + @pytest.yield_fixture -def mock_ucb_attrs_uneligible(): +def mock_user_attrs_uneligible(): with mock.patch( 'ocflib.vhost.web.user_attrs_ucb', return_value=None - ): - with mock.patch('ocflib.vhost.web.read_ucb_password', - return_value="" - ): - yield + ): + yield class TestVirtualHosts: @@ -121,14 +115,14 @@ def test_proper_parse(self, mock_get_vhosts_db): def test_has_vhost(self, user, should_have_vhost, mock_get_vhosts_db): assert has_vhost(user) == should_have_vhost - @pytest.mark.usefixtures('mock_group_ucb_attrs') + @pytest.mark.usefixtures('mock_group_user_attrs') def test_groups_eligible_for_vhost(self): - assert eligible_for_vhost('ggroups') == True + assert eligible_for_vhost('ggroups') @pytest.mark.usefixtures('mock_staff_ucb_attrs') def test_staff_eligible_for_vhost(self): - assert eligible_for_vhost('bh') == True + assert eligible_for_vhost('bh') - @pytest.mark.usefixtures('mock_ucb_attrs_uneligible') + @pytest.mark.usefixtures('mock_user_attrs_uneligible') def test_not_eligible_for_vhost(self): - assert eligible_for_vhost('mattmcal') == False + assert not eligible_for_vhost('mattmcal') From c5aea5bf86f13c590cb40c9966eb697223a23481 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 14:33:43 -0800 Subject: [PATCH 09/10] moved untestable to manual, refactor to make pw access transparent --- tests-manual/account/search | 9 +++++++++ tests/account/search_test.py | 13 ------------- tests/ucb/directory_test.py | 7 ------- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/tests-manual/account/search b/tests-manual/account/search index eb2f0a0f..858c4369 100755 --- a/tests-manual/account/search +++ b/tests-manual/account/search @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import ocflib.account.search as search +import ocflib.ucb.directory as directory if __name__ == '__main__': print('Users with CalNet UID 1034192: {}'.format( @@ -19,3 +20,11 @@ if __name__ == '__main__': for user in ('ckuehl', 'ggroup'): print('User {} is group: {}'.format(user, search.user_is_group(user))) + + user = search.user_attrs_ucb(1101587) # jvperrin's uid + print('User with uid {} has uid {}'.format(user['uid'], str(1101587))) + + print('{} should be in user[\'objectClass\']: {}'.format('person', 'person' in user['objectClass'])) + + print('Real query with invalid uid 9999999 should be empty: {}'.format( + (None or '').lower() == (directory.name_by_calnet_uid(9999999) or '').lower())) diff --git a/tests/account/search_test.py b/tests/account/search_test.py index 4f9f9eae..de17dee1 100644 --- a/tests/account/search_test.py +++ b/tests/account/search_test.py @@ -2,14 +2,12 @@ from ldap3.core.exceptions import LDAPAttributeError from ocflib.account.search import user_attrs -from ocflib.account.search import user_attrs_ucb from ocflib.account.search import user_exists from ocflib.account.search import user_is_group from ocflib.account.search import user_is_sorried from ocflib.account.search import users_by_callink_oid from ocflib.account.search import users_by_calnet_uid from ocflib.account.search import users_by_filter -from tests.conftest import TEST_PERSON_CALNET_UID class TestUsersByFilter: @@ -61,17 +59,6 @@ def test_nonexistent_user(self): assert user_attrs('doesnotexist') is None -class TestUserAttrsUCB: - - def test_existing_user(self, test_uid=TEST_PERSON_CALNET_UID): - user = user_attrs_ucb(test_uid) - assert user['uid'] == [str(test_uid)] - assert 'person' in user['objectClass'] - - def test_nonexistent_user(self): - assert user_attrs_ucb(9999999999) is None - - @pytest.mark.parametrize('user,exists', [ ('ckuehl', True), ('bpreview', True), diff --git a/tests/ucb/directory_test.py b/tests/ucb/directory_test.py index 5162026c..ba03d368 100644 --- a/tests/ucb/directory_test.py +++ b/tests/ucb/directory_test.py @@ -23,13 +23,6 @@ def test_name_by_calnet_uid(self, attrs, expected): ): assert name_by_calnet_uid(0) == expected - @pytest.mark.parametrize('uid,expected', [ - (TEST_PERSON_CALNET_UID, TEST_PERSON_NAME), - (9999999, None), - ]) - def test_name_by_calnet_uid_real_query(self, uid, expected): - assert (name_by_calnet_uid(uid) or '').lower() == (expected or '').lower() - class TestCalNetUIDsByName: From f87eb6fb666c6f91f104e95731094123c0459234 Mon Sep 17 00:00:00 2001 From: "Ja (Thanakul) Wattanawong" Date: Tue, 19 Feb 2019 14:37:18 -0800 Subject: [PATCH 10/10] added note explaining failure case for manual test --- tests-manual/account/search | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests-manual/account/search b/tests-manual/account/search index 858c4369..94d12071 100755 --- a/tests-manual/account/search +++ b/tests-manual/account/search @@ -21,6 +21,9 @@ if __name__ == '__main__': for user in ('ckuehl', 'ggroup'): print('User {} is group: {}'.format(user, search.user_is_group(user))) + """ Note: the following tests will fail if the password file in + UCB_LDAP_PASSWORD_PATH does not exist or is invalid + """ user = search.user_attrs_ucb(1101587) # jvperrin's uid print('User with uid {} has uid {}'.format(user['uid'], str(1101587)))