diff --git a/python/nav/web/auth/__init__.py b/python/nav/web/auth/__init__.py index 3835eff229..ac0806a870 100644 --- a/python/nav/web/auth/__init__.py +++ b/python/nav/web/auth/__init__.py @@ -49,68 +49,35 @@ def authenticate(username, password): Returns account object if user was authenticated, else None. """ # FIXME Log stuff? - auth = False - account = None # Try to find the account in the database. If it's not found we can try # LDAP. try: account = Account.objects.get(login__iexact=username) except Account.DoesNotExist: - if ldap.available: - user = ldap.authenticate(username, password) - # If we authenticated, store the user in database. - if user: - account = Account( - login=user.username, name=user.get_real_name(), ext_sync='ldap' - ) - account.set_password(password) - account.save() - _handle_ldap_admin_status(user, account) - # We're authenticated now - auth = True - - if account and account.locked: + # account autocreated if username is authenticated + account = ldap.authenticate(username, password) + return account + + if account.locked: _logger.info("Locked user %s tried to log in", account.login) + return None - if ( - account - and account.ext_sync == 'ldap' - and ldap.available - and not auth - and not account.locked - ): + if account.ext_sync == 'ldap' and ldap.available: try: - auth = ldap.authenticate(username, password) + ldap_user = ldap.get_ldap_user(username, password) except ldap.NoAnswerError: - # Fallback to stored password if ldap is unavailable - auth = False + pass else: - if auth: - account.set_password(password) - account.save() - _handle_ldap_admin_status(auth, account) - else: - return + if ldap_user: + account = ldap.update_ldap_user(ldap_user, account, password) + return account + return None + # Fallback to stored password if ldap is unavailable - if account and not auth: - auth = account.check_password(password) - - if auth and account: + if account.check_password(password): return account - else: - return None - - -def _handle_ldap_admin_status(ldap_user, nav_account): - is_admin = ldap_user.is_admin() - # Only modify admin status if an entitlement is configured in webfront.conf - if is_admin is not None: - admin_group = AccountGroup.objects.get(id=AccountGroup.ADMIN_GROUP) - if is_admin: - nav_account.groups.add(admin_group) - else: - nav_account.groups.remove(admin_group) + return None def get_login_url(request): diff --git a/python/nav/web/auth/ldap.py b/python/nav/web/auth/ldap.py index 344608cd05..a3271c2d7b 100644 --- a/python/nav/web/auth/ldap.py +++ b/python/nav/web/auth/ldap.py @@ -24,6 +24,7 @@ import nav.errors from nav.config import NAVConfigParser +from nav.models.profiles import Account, AccountGroup _logger = logging.getLogger(__name__) @@ -121,11 +122,66 @@ def open_ldap(): return lconn -def authenticate(login, password): +def authenticate(username, password): """ - Attempt to authenticate the login name with password against the - configured LDAP server. If the user is authenticated, required - group memberships are also verified. + Authenticate the username and password against the configured LDAP server. + + Required group memberships are also verified. + + Returns an authenticated Account with updated groups, or None. + """ + if not available: + return None + ldap_user = get_ldap_user(username, password) + try: + account = Account.objects.get(login__iexact=username, ext_sync='ldap') + except Account.DoesNotExist: + if ldap_user: + account = autocreate_ldap_user(ldap_user, password) + return account + if account.locked: + _logger.info("Locked user %s tried to log in", account.login) + return None + if account.check_password(password): + account = update_ldap_user(ldap_user, account, password) + return account + return None + + +def autocreate_ldap_user(ldap_user, password): + account = Account( + login=ldap_user.username, + name=ldap_user.get_real_name(), + ext_sync='ldap', + ) + account = update_ldap_user(ldap_user, account, password) + return account + + +def update_ldap_user(ldap_user, account, password): + account.set_password(password) + account.save() + _handle_ldap_admin_status(ldap_user, account) + return account + + +def _handle_ldap_admin_status(ldap_user, nav_account): + is_admin = ldap_user.is_admin() + # Only modify admin status if an entitlement is configured in webfront.conf + if is_admin is not None: + admin_group = AccountGroup.objects.get(id=AccountGroup.ADMIN_GROUP) + if is_admin: + nav_account.groups.add(admin_group) + else: + nav_account.groups.remove(admin_group) + + +def get_ldap_user(login, password): + """ + Fetch an LDAPUser from an ldap server if login and password matches. + + Returns an autenticated LDAPUser of a specific group or with specific + entitlements, or False. """ lconn = open_ldap() server = _config.get('ldap', 'server') diff --git a/tests/unittests/general/webfront_test.py b/tests/unittests/general/webfront_test.py index 325a87842e..4d516db900 100644 --- a/tests/unittests/general/webfront_test.py +++ b/tests/unittests/general/webfront_test.py @@ -2,9 +2,6 @@ from mock import patch, MagicMock, Mock from django.test import RequestFactory -import pytest - -import nav.web.auth.ldap from nav.web import auth from nav.web.auth import remote_user from nav.web.auth.utils import ACCOUNT_ID_VAR @@ -23,12 +20,12 @@ def test_authenticate_should_return_account_when_ldap_says_yes(self): ldap_user = Mock() ldap_user.is_admin.return_value = None # mock to avoid database access with patch("nav.web.auth.ldap.available", new=True): - with patch("nav.web.auth.ldap.authenticate", return_value=ldap_user): + with patch("nav.web.auth.ldap.get_ldap_user", return_value=ldap_user): assert auth.authenticate('knight', 'shrubbery') == LDAP_ACCOUNT def test_authenticate_should_return_false_when_ldap_says_no(self): with patch("nav.web.auth.ldap.available", new=True): - with patch("nav.web.auth.ldap.authenticate", return_value=False): + with patch("nav.web.auth.ldap.get_ldap_user", return_value=False): assert not auth.authenticate('knight', 'shrubbery') def test_authenticate_should_fallback_when_ldap_is_disabled(self): @@ -168,173 +165,3 @@ def test_remote_user_set(self, fake_session): assert ( request.session.get(ACCOUNT_ID_VAR, None) == REMOTE_USER_ACCOUNT.id ) - - -class TestLdapUser(object): - @patch.dict( - "nav.web.auth.ldap._config._sections", - { - 'ldap': { - '__name__': 'ldap', - 'basedn': 'empty', - 'manager': 'empty', - 'manager_password': 'empty', - 'uid_attr': 'sAMAccountName', - 'encoding': 'utf-8', - }, - }, - ) - def test_search_result_with_referrals_should_be_considered_empty(self): - """LP#1207737""" - conn = Mock( - **{ - 'search_s.return_value': [ - (None, "restaurant"), - (None, "at the end of the universe"), - ] - } - ) - u = nav.web.auth.ldap.LDAPUser("zaphod", conn) - with pytest.raises(nav.web.auth.ldap.UserNotFound): - u.search_dn() - - @patch.dict( - "nav.web.auth.ldap._config._sections", - { - 'ldap': { - '__name__': 'ldap', - 'basedn': 'empty', - 'lookupmethod': 'direct', - 'uid_attr': 'uid', - 'encoding': 'utf-8', - 'suffix': '', - } - }, - ) - def test_non_ascii_password_should_work(self): - """LP#1213818""" - conn = Mock( - **{ - 'simple_bind_s.side_effect': lambda x, y: ( - str(x), - str(y), - ), - } - ) - u = nav.web.auth.ldap.LDAPUser(u"zaphod", conn) - u.bind(u"æøå") - - @patch.dict( - "nav.web.auth.ldap._config._sections", - { - 'ldap': { - '__name__': 'ldap', - 'basedn': 'cn=users,dc=example,dc=org', - 'lookupmethod': 'direct', - 'uid_attr': 'uid', - 'encoding': 'utf-8', - 'group_search': '(member=%%s)', - }, - }, - ) - def test_is_group_member_for_non_ascii_user_should_not_raise(self): - """LP#1301794""" - - def fake_search(base, scope, filtr): - str(base) - str(filtr) - return [] - - conn = Mock( - **{ - 'search_s.side_effect': fake_search, - } - ) - u = nav.web.auth.ldap.LDAPUser(u"Ægir", conn) - u.is_group_member('cn=noc-operators,cn=groups,dc=example,dc=com') - - -@patch.dict( - "nav.web.auth.ldap._config._sections", - { - 'ldap': { - '__name__': 'ldap', - 'basedn': 'cn=users,dc=example,dc=org', - 'lookupmethod': 'direct', - 'uid_attr': 'uid', - 'encoding': 'utf-8', - 'require_entitlement': 'president', - 'admin_entitlement': 'boss', - 'entitlement_attribute': 'eduPersonEntitlement', - }, - }, -) -class TestLdapEntitlements(object): - def test_required_entitlement_should_be_verified(self, user_zaphod): - u = nav.web.auth.ldap.LDAPUser("zaphod", user_zaphod) - assert u.has_entitlement('president') - - def test_missing_entitlement_should_not_be_verified(self, user_marvin): - u = nav.web.auth.ldap.LDAPUser("marvin", user_marvin) - assert not u.has_entitlement('president') - - def test_admin_entitlement_should_be_verified(self, user_zaphod): - u = nav.web.auth.ldap.LDAPUser("zaphod", user_zaphod) - assert u.is_admin() - - def test_missing_admin_entitlement_should_be_verified(self, user_marvin): - u = nav.web.auth.ldap.LDAPUser("marvin", user_marvin) - assert not u.is_admin() - - -@patch.dict( - "nav.web.auth.ldap._config._sections", - { - 'ldap': { - '__name__': 'ldap', - 'basedn': 'cn=users,dc=example,dc=org', - 'lookupmethod': 'direct', - 'uid_attr': 'uid', - 'encoding': 'utf-8', - 'require_entitlement': 'president', - 'admin_entitlement': '', - 'entitlement_attribute': 'eduPersonEntitlement', - }, - }, -) -def test_no_admin_entitlement_option_should_make_no_admin_decision(user_zaphod): - u = nav.web.auth.ldap.LDAPUser("zaphod", user_zaphod) - assert u.is_admin() is None - - -# -# Pytest fixtures -# - - -@pytest.fixture -def user_zaphod(): - return Mock( - **{ - 'search_s.return_value': [ - ( - u'uid=zaphod,cn=users,dc=example,dc=org', - {u'eduPersonEntitlement': [b'president', b'boss']}, - ) - ] - } - ) - - -@pytest.fixture -def user_marvin(): - return Mock( - **{ - 'search_s.return_value': [ - ( - u'uid=marvin,cn=users,dc=example,dc=org', - {u'eduPersonEntitlement': [b'paranoid']}, - ) - ] - } - ) diff --git a/tests/unittests/web/auth/__init__.py b/tests/unittests/web/auth/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unittests/web/auth/ldap_test.py b/tests/unittests/web/auth/ldap_test.py new file mode 100644 index 0000000000..5f42abbce8 --- /dev/null +++ b/tests/unittests/web/auth/ldap_test.py @@ -0,0 +1,305 @@ +from mock import Mock, MagicMock, patch + +import pytest + +from nav.config import NAVConfigParser +from nav.models.profiles import Account +from nav.web import auth +from nav.web.auth import ldap + + +LOCKED_ACCOUNT = auth.Account( + login='galahad', + ext_sync='ldap', + password='shrubbery', + locked=True, +) +ACTIVE_ACCOUNT = auth.Account( + login='arthur', + ext_sync='ldap', + password='shrubbery', + locked=False, +) + + +class LdapTestConfig(NAVConfigParser): + DEFAULT_CONFIG_FILES = [] + DEFAULT_CONFIG = u""" +[ldap] +basedn=cn=people,dc=example,dc=org +uid_attr=samAccountName +name_attr=cn +manager= +manager_password= +encoding=utf-8 +""" + + +@patch('nav.web.auth.ldap._config', LdapTestConfig()) +def test_ldapuser_search_dn_decode_regression(): + """Verifies that LDAPUser.search_dn() returns user's DN untouched""" + connection = Mock() + connection.search_s.return_value = [ + ( + u'CN=Zaphod Beeblebr\xf6x,CN=people,DC=example,DC=org', + { + u'cn': b'Zaphod Beeblebr\xc3\xb6x', + u'displayName': b'Zaphod Beeblebr\xc3\xb6x', + }, + ) + ] + + user = ldap.LDAPUser('zaphod', connection) + dn, uid = user.search_dn() + assert dn == u'CN=Zaphod Beeblebr\xf6x,CN=people,DC=example,DC=org' + + +class LdapOpenTestConfig(NAVConfigParser): + DEFAULT_CONFIG_FILES = [] + DEFAULT_CONFIG = u""" +[ldap] +server=ldap.example.org +port=636 +encryption=tls +timeout=3 +debug=true +""" + + +@patch('nav.web.auth.ldap._config', LdapOpenTestConfig()) +def test_open_ldap_should_run_without_error(): + with patch('ldap.initialize') as initialize: + assert ldap.open_ldap() + + +class LdapOpenTestInvalidEncryptionConfig(NAVConfigParser): + DEFAULT_CONFIG_FILES = [] + DEFAULT_CONFIG = u""" +[ldap] +server=ldap.example.org +port=636 +encryption=invalid +timeout=3 +debug=true +""" + + +@patch('nav.web.auth.ldap._config', LdapOpenTestInvalidEncryptionConfig()) +def test_when_encryption_setting_is_invalid_open_ldap_should_run_without_encryption(): + with patch('ldap.initialize') as initialize: + assert ldap.open_ldap() + + +class TestLdapUser(object): + @patch.dict( + "nav.web.auth.ldap._config._sections", + { + 'ldap': { + '__name__': 'ldap', + 'basedn': 'empty', + 'manager': 'empty', + 'manager_password': 'empty', + 'uid_attr': 'sAMAccountName', + 'encoding': 'utf-8', + }, + }, + ) + def test_search_result_with_referrals_should_be_considered_empty(self): + """LP#1207737""" + conn = Mock( + **{ + 'search_s.return_value': [ + (None, "restaurant"), + (None, "at the end of the universe"), + ] + } + ) + u = ldap.LDAPUser("zaphod", conn) + with pytest.raises(ldap.UserNotFound): + u.search_dn() + + @patch.dict( + "nav.web.auth.ldap._config._sections", + { + 'ldap': { + '__name__': 'ldap', + 'basedn': 'empty', + 'lookupmethod': 'direct', + 'uid_attr': 'uid', + 'encoding': 'utf-8', + 'suffix': '', + } + }, + ) + def test_non_ascii_password_should_work(self): + """LP#1213818""" + conn = Mock( + **{ + 'simple_bind_s.side_effect': lambda x, y: ( + str(x), + str(y), + ), + } + ) + u = ldap.LDAPUser(u"zaphod", conn) + u.bind(u"æøå") + + @patch.dict( + "nav.web.auth.ldap._config._sections", + { + 'ldap': { + '__name__': 'ldap', + 'basedn': 'cn=users,dc=example,dc=org', + 'lookupmethod': 'direct', + 'uid_attr': 'uid', + 'encoding': 'utf-8', + 'group_search': '(member=%%s)', + }, + }, + ) + def test_is_group_member_for_non_ascii_user_should_not_raise(self): + """LP#1301794""" + + def fake_search(base, scope, filtr): + str(base) + str(filtr) + return [] + + conn = Mock( + **{ + 'search_s.side_effect': fake_search, + } + ) + u = ldap.LDAPUser(u"Ægir", conn) + u.is_group_member('cn=noc-operators,cn=groups,dc=example,dc=com') + + +@patch.dict( + "nav.web.auth.ldap._config._sections", + { + 'ldap': { + '__name__': 'ldap', + 'basedn': 'cn=users,dc=example,dc=org', + 'lookupmethod': 'direct', + 'uid_attr': 'uid', + 'encoding': 'utf-8', + 'require_entitlement': 'president', + 'admin_entitlement': 'boss', + 'entitlement_attribute': 'eduPersonEntitlement', + }, + }, +) +class TestLdapEntitlements(object): + def test_required_entitlement_should_be_verified(self, user_zaphod): + u = ldap.LDAPUser("zaphod", user_zaphod) + assert u.has_entitlement('president') + + def test_missing_entitlement_should_not_be_verified(self, user_marvin): + u = ldap.LDAPUser("marvin", user_marvin) + assert not u.has_entitlement('president') + + def test_admin_entitlement_should_be_verified(self, user_zaphod): + u = ldap.LDAPUser("zaphod", user_zaphod) + assert u.is_admin() + + def test_missing_admin_entitlement_should_be_verified(self, user_marvin): + u = ldap.LDAPUser("marvin", user_marvin) + assert not u.is_admin() + + +@patch.dict( + "nav.web.auth.ldap._config._sections", + { + 'ldap': { + '__name__': 'ldap', + 'basedn': 'cn=users,dc=example,dc=org', + 'lookupmethod': 'direct', + 'uid_attr': 'uid', + 'encoding': 'utf-8', + 'require_entitlement': 'president', + 'admin_entitlement': '', + 'entitlement_attribute': 'eduPersonEntitlement', + }, + }, +) +def test_no_admin_entitlement_option_should_make_no_admin_decision(user_zaphod): + u = ldap.LDAPUser("zaphod", user_zaphod) + assert u.is_admin() is None + + +class TestLdapAuthenticate: + @patch("nav.web.auth.ldap.available", new=False) + def test_if_ldap_not_available_return_None(self, *_): + result = ldap.authenticate('foo', 'bar') + assert result is None + + @patch("nav.web.auth.ldap.available", new=True) + @patch("nav.web.auth.ldap.get_ldap_user", return_value=False) + @patch( + "nav.web.auth.Account.objects.get", new=MagicMock(return_value=LOCKED_ACCOUNT) + ) + def test_locked_accounts_return_None(self, *_): + result = ldap.authenticate('foo', 'bar') + assert result is None + + @patch("nav.web.auth.ldap.available", new=True) + @patch("nav.web.auth.ldap.get_ldap_user", return_value=False) + @patch( + "nav.web.auth.Account.objects.get", new=MagicMock(return_value=ACTIVE_ACCOUNT) + ) + @patch("nav.web.auth.Account.check_password", return_value=False) + def test_active_account_with_wrong_password_return_None(self, *_): + result = ldap.authenticate('foo', 'ni!') + assert result is None + + @patch("nav.web.auth.ldap.available", new=True) + @patch("nav.web.auth.ldap.get_ldap_user", return_value=False) + @patch( + "nav.web.auth.Account.objects.get", new=MagicMock(return_value=ACTIVE_ACCOUNT) + ) + @patch("nav.web.auth.Account.check_password", return_value=True) + @patch("nav.web.auth.ldap.update_ldap_user", return_value=ACTIVE_ACCOUNT) + def test_active_account_with_correct_password_return_account(self, *_): + result = ldap.authenticate('foo', 'ni!') + assert result == ACTIVE_ACCOUNT + + @patch("nav.web.auth.ldap.available", new=True) + @patch("nav.web.auth.ldap.get_ldap_user", return_value=True) + @patch("nav.web.auth.Account.objects.get", side_effect=Account.DoesNotExist()) + @patch("nav.web.auth.ldap.autocreate_ldap_user", return_value=ACTIVE_ACCOUNT) + def test_nonexisting_accounts_are_created(self, *_): + result = ldap.authenticate('foo', 'ni!') + assert result == ACTIVE_ACCOUNT + + +# +# Pytest fixtures +# + + +@pytest.fixture +def user_zaphod(): + return Mock( + **{ + 'search_s.return_value': [ + ( + u'uid=zaphod,cn=users,dc=example,dc=org', + {u'eduPersonEntitlement': [b'president', b'boss']}, + ) + ] + } + ) + + +@pytest.fixture +def user_marvin(): + return Mock( + **{ + 'search_s.return_value': [ + ( + u'uid=marvin,cn=users,dc=example,dc=org', + {u'eduPersonEntitlement': [b'paranoid']}, + ) + ] + } + ) diff --git a/tests/unittests/web/ldapauth_test.py b/tests/unittests/web/ldapauth_test.py deleted file mode 100644 index 2655f1166d..0000000000 --- a/tests/unittests/web/ldapauth_test.py +++ /dev/null @@ -1,71 +0,0 @@ -from nav.config import NAVConfigParser -from nav.web.auth.ldap import LDAPUser, open_ldap -from mock import Mock, patch - - -class LdapTestConfig(NAVConfigParser): - DEFAULT_CONFIG_FILES = [] - DEFAULT_CONFIG = u""" -[ldap] -basedn=cn=people,dc=example,dc=org -uid_attr=samAccountName -name_attr=cn -manager= -manager_password= -encoding=utf-8 -""" - - -@patch('nav.web.auth.ldap._config', LdapTestConfig()) -def test_ldapuser_search_dn_decode_regression(): - """Verifies that LDAPUser.search_dn() returns user's DN untouched""" - connection = Mock() - connection.search_s.return_value = [ - ( - u'CN=Zaphod Beeblebr\xf6x,CN=people,DC=example,DC=org', - { - u'cn': b'Zaphod Beeblebr\xc3\xb6x', - u'displayName': b'Zaphod Beeblebr\xc3\xb6x', - }, - ) - ] - - user = LDAPUser('zaphod', connection) - dn, uid = user.search_dn() - assert dn == u'CN=Zaphod Beeblebr\xf6x,CN=people,DC=example,DC=org' - - -class LdapOpenTestConfig(NAVConfigParser): - DEFAULT_CONFIG_FILES = [] - DEFAULT_CONFIG = u""" -[ldap] -server=ldap.example.org -port=636 -encryption=tls -timeout=3 -debug=true -""" - - -@patch('nav.web.auth.ldap._config', LdapOpenTestConfig()) -def test_open_ldap_should_run_without_error(): - with patch('ldap.initialize') as initialize: - assert open_ldap() - - -class LdapOpenTestInvalidEncryptionConfig(NAVConfigParser): - DEFAULT_CONFIG_FILES = [] - DEFAULT_CONFIG = u""" -[ldap] -server=ldap.example.org -port=636 -encryption=invalid -timeout=3 -debug=true -""" - - -@patch('nav.web.auth.ldap._config', LdapOpenTestInvalidEncryptionConfig()) -def test_when_encryption_setting_is_invalid_open_ldap_should_run_without_encryption(): - with patch('ldap.initialize') as initialize: - assert open_ldap()