diff options
-rw-r--r-- | changes/login_error_undistinguishable | 2 | ||||
-rw-r--r-- | src/leap/bitmask/crypto/srpauth.py | 20 | ||||
-rw-r--r-- | src/leap/bitmask/crypto/tests/test_srpauth.py | 11 | ||||
-rw-r--r-- | src/leap/bitmask/gui/preferenceswindow.py | 5 | ||||
-rw-r--r-- | src/leap/bitmask/provider/providerbootstrapper.py | 4 | ||||
-rw-r--r-- | src/leap/bitmask/provider/tests/test_providerbootstrapper.py | 50 | ||||
-rw-r--r-- | src/leap/bitmask/services/__init__.py | 4 | ||||
-rw-r--r-- | src/leap/bitmask/services/eip/tests/test_eipbootstrapper.py | 4 |
8 files changed, 43 insertions, 57 deletions
diff --git a/changes/login_error_undistinguishable b/changes/login_error_undistinguishable new file mode 100644 index 00000000..5391f3fc --- /dev/null +++ b/changes/login_error_undistinguishable @@ -0,0 +1,2 @@ + o Do not distinguish between different possible authentication + errors. Fixes #3859.
\ No newline at end of file diff --git a/src/leap/bitmask/crypto/srpauth.py b/src/leap/bitmask/crypto/srpauth.py index bf85f75c..9c08d353 100644 --- a/src/leap/bitmask/crypto/srpauth.py +++ b/src/leap/bitmask/crypto/srpauth.py @@ -52,13 +52,6 @@ class SRPAuthConnectionError(SRPAuthenticationError): pass -class SRPAuthUnknownUser(SRPAuthenticationError): - """ - Exception raised when trying to authenticate an unknown user - """ - pass - - class SRPAuthBadStatusCode(SRPAuthenticationError): """ Exception raised when we received an unknown bad status code @@ -97,7 +90,7 @@ class SRPAuthJSONDecodeError(SRPAuthenticationError): pass -class SRPAuthBadPassword(SRPAuthenticationError): +class SRPAuthBadUserOrPassword(SRPAuthenticationError): """ Exception raised when the user provided a bad password to auth. """ @@ -219,7 +212,6 @@ class SRPAuth(QtCore.QObject): Might raise all SRPAuthenticationError based: SRPAuthenticationError SRPAuthConnectionError - SRPAuthUnknownUser SRPAuthBadStatusCode SRPAuthNoSalt SRPAuthNoB @@ -266,7 +258,7 @@ class SRPAuth(QtCore.QObject): "Status code = %r. Content: %r" % (init_session.status_code, content)) if init_session.status_code == 422: - raise SRPAuthUnknownUser(self._WRONG_USER_PASS) + raise SRPAuthBadUserOrPassword(self._WRONG_USER_PASS) raise SRPAuthBadStatusCode(self.tr("There was a problem with" " authentication")) @@ -296,7 +288,7 @@ class SRPAuth(QtCore.QObject): SRPAuthBadDataFromServer SRPAuthConnectionError SRPAuthJSONDecodeError - SRPAuthBadPassword + SRPAuthBadUserOrPassword :param salt_B: salt and B parameters for the username :type salt_B: tuple @@ -355,7 +347,7 @@ class SRPAuth(QtCore.QObject): "received: %s", (content,)) logger.error("[%s] Wrong password (HAMK): [%s]" % (auth_result.status_code, error)) - raise SRPAuthBadPassword(self._WRONG_USER_PASS) + raise SRPAuthBadUserOrPassword(self._WRONG_USER_PASS) if auth_result.status_code not in (200,): logger.error("No valid response (HAMK): " @@ -452,7 +444,7 @@ class SRPAuth(QtCore.QObject): It requires to be authenticated. Might raise: - SRPAuthBadPassword + SRPAuthBadUserOrPassword requests.exceptions.HTTPError :param current_password: the current password for the logged user. @@ -463,7 +455,7 @@ class SRPAuth(QtCore.QObject): leap_assert(self.get_uid() is not None) if current_password != self._password: - raise SRPAuthBadPassword + raise SRPAuthBadUserOrPassword url = "%s/%s/users/%s.json" % ( self._provider_config.get_api_uri(), diff --git a/src/leap/bitmask/crypto/tests/test_srpauth.py b/src/leap/bitmask/crypto/tests/test_srpauth.py index 6fb2b739..e63c1385 100644 --- a/src/leap/bitmask/crypto/tests/test_srpauth.py +++ b/src/leap/bitmask/crypto/tests/test_srpauth.py @@ -246,7 +246,7 @@ class SRPAuthTestCase(unittest.TestCase): d = self._prepare_auth_test(422) def wrapper(_): - with self.assertRaises(srpauth.SRPAuthUnknownUser): + with self.assertRaises(srpauth.SRPAuthBadUserOrPassword): with mock.patch( 'leap.bitmask.util.request_helpers.get_content', new=mock.create_autospec(get_content)) as content: @@ -425,7 +425,7 @@ class SRPAuthTestCase(unittest.TestCase): new=mock.create_autospec(get_content)) as \ content: content.return_value = ("", 0) - with self.assertRaises(srpauth.SRPAuthBadPassword): + with self.assertRaises(srpauth.SRPAuthBadUserOrPassword): self.auth_backend._process_challenge( salt_B, username=self.TEST_USER) @@ -449,7 +449,7 @@ class SRPAuthTestCase(unittest.TestCase): new=mock.create_autospec(get_content)) as \ content: content.return_value = ("[]", 0) - with self.assertRaises(srpauth.SRPAuthBadPassword): + with self.assertRaises(srpauth.SRPAuthBadUserOrPassword): self.auth_backend._process_challenge( salt_B, username=self.TEST_USER) @@ -680,10 +680,7 @@ class SRPAuthTestCase(unittest.TestCase): self.auth_backend._session.delete, side_effect=Exception()) - def wrapper(*args): - self.auth_backend.logout() - - d = threads.deferToThread(wrapper) + d = threads.deferToThread(self.auth.logout) return d @deferred() diff --git a/src/leap/bitmask/gui/preferenceswindow.py b/src/leap/bitmask/gui/preferenceswindow.py index 7e281b44..58cb05ba 100644 --- a/src/leap/bitmask/gui/preferenceswindow.py +++ b/src/leap/bitmask/gui/preferenceswindow.py @@ -27,11 +27,10 @@ from PySide import QtCore, QtGui from leap.bitmask.config.leapsettings import LeapSettings from leap.bitmask.gui.ui_preferences import Ui_Preferences from leap.soledad.client import NoStorageSecret -from leap.bitmask.crypto.srpauth import SRPAuthBadPassword +from leap.bitmask.crypto.srpauth import SRPAuthBadUserOrPassword from leap.bitmask.util.password import basic_password_checks from leap.bitmask.services import get_supported from leap.bitmask.config.providerconfig import ProviderConfig -from leap.bitmask.services.eip.eipconfig import EIPConfig, VPNGatewaySelector from leap.bitmask.services import get_service_display_name logger = logging.getLogger(__name__) @@ -179,7 +178,7 @@ class PreferencesWindow(QtGui.QDialog): logger.error("Error changing password: %s", (failure, )) problem = self.tr("There was a problem changing the password.") - if failure.check(SRPAuthBadPassword): + if failure.check(SRPAuthBadUserOrPassword): problem = self.tr("You did not enter a correct current password.") self._set_password_change_status(problem, error=True) diff --git a/src/leap/bitmask/provider/providerbootstrapper.py b/src/leap/bitmask/provider/providerbootstrapper.py index 751da828..a10973f0 100644 --- a/src/leap/bitmask/provider/providerbootstrapper.py +++ b/src/leap/bitmask/provider/providerbootstrapper.py @@ -156,7 +156,9 @@ class ProviderBootstrapper(AbstractBootstrapper): # Watch out! We're handling the verify paramenter differently here. headers = {} - provider_json = os.path.join(get_path_prefix(), "leap", "providers", + provider_json = os.path.join(get_path_prefix(), + "leap", + "providers", self._domain, "provider.json") mtime = get_mtime(provider_json) diff --git a/src/leap/bitmask/provider/tests/test_providerbootstrapper.py b/src/leap/bitmask/provider/tests/test_providerbootstrapper.py index 9b47d60e..fe5b52bd 100644 --- a/src/leap/bitmask/provider/tests/test_providerbootstrapper.py +++ b/src/leap/bitmask/provider/tests/test_providerbootstrapper.py @@ -42,6 +42,8 @@ from leap.bitmask.provider.providerbootstrapper import ProviderBootstrapper from leap.bitmask.provider.providerbootstrapper import UnsupportedProviderAPI from leap.bitmask.provider.providerbootstrapper import WrongFingerprint from leap.bitmask.provider.supportedapis import SupportedAPIs +from leap.bitmask import util +from leap.bitmask.util import get_path_prefix from leap.common.files import mkdir_p from leap.common.testing.https_server import where from leap.common.testing.basetest import BaseLeapTest @@ -315,16 +317,19 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): # tearDown so we are sure everything is as expected for each # test. If we do it inside each specific test, a failure in # the test will leave the implementation with the mock. - self.old_gpp = ProviderConfig.get_path_prefix + self.old_gpp = util.get_path_prefix + self.old_load = ProviderConfig.load self.old_save = ProviderConfig.save self.old_api_version = ProviderConfig.get_api_version + self.old_api_uri = ProviderConfig.get_api_uri def tearDown(self): - ProviderConfig.get_path_prefix = self.old_gpp + util.get_path_prefix = self.old_gpp ProviderConfig.load = self.old_load ProviderConfig.save = self.old_save ProviderConfig.get_api_version = self.old_api_version + ProviderConfig.get_api_uri = self.old_api_uri def test_check_https_succeeds(self): # XXX: Need a proper CA signed cert to test this @@ -372,10 +377,11 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): paths :type path_prefix: str """ - ProviderConfig.get_path_prefix = mock.MagicMock( - return_value=path_prefix) + util.get_path_prefix = mock.MagicMock(return_value=path_prefix) ProviderConfig.get_api_version = mock.MagicMock( return_value=api) + ProviderConfig.get_api_uri = mock.MagicMock( + return_value="https://localhost:%s" % (self.https_port,)) ProviderConfig.load = mock.MagicMock() ProviderConfig.save = mock.MagicMock() @@ -400,10 +406,8 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): :returns: the provider.json path used :rtype: str """ - provider_dir = os.path.join(ProviderConfig() - .get_path_prefix(), - "leap", - "providers", + provider_dir = os.path.join(get_path_prefix(), + "leap", "providers", self.pb._domain) mkdir_p(provider_dir) provider_path = os.path.join(provider_dir, @@ -413,6 +417,9 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): p.write("A") return provider_path + @mock.patch( + 'leap.bitmask.config.providerconfig.ProviderConfig.get_domain', + lambda x: where('testdomain.com')) def test_download_provider_info_new_provider(self): self._setup_provider_config_with("1", tempfile.mkdtemp()) self._setup_providerbootstrapper(True) @@ -431,10 +438,7 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): # set mtime to something really new os.utime(provider_path, (-1, time.time())) - with mock.patch.object( - ProviderConfig, 'get_api_uri', - return_value="https://localhost:%s" % (self.https_port,)): - self.pb._download_provider_info() + self.pb._download_provider_info() # we check that it doesn't save the provider # config, because it's new enough self.assertFalse(ProviderConfig.save.called) @@ -450,10 +454,7 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): # set mtime to something really new os.utime(provider_path, (-1, time.time())) - with mock.patch.object( - ProviderConfig, 'get_api_uri', - return_value="https://localhost:%s" % (self.https_port,)): - self.pb._download_provider_info() + self.pb._download_provider_info() # we check that it doesn't save the provider # config, because it's new enough self.assertFalse(ProviderConfig.save.called) @@ -469,10 +470,7 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): # set mtime to something really old os.utime(provider_path, (-1, 100)) - with mock.patch.object( - ProviderConfig, 'get_api_uri', - return_value="https://localhost:%s" % (self.https_port,)): - self.pb._download_provider_info() + self.pb._download_provider_info() self.assertTrue(ProviderConfig.load.called) self.assertTrue(ProviderConfig.save.called) @@ -484,11 +482,8 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): self._setup_providerbootstrapper(False) self._produce_dummy_provider_json() - with mock.patch.object( - ProviderConfig, 'get_api_uri', - return_value="https://localhost:%s" % (self.https_port,)): - with self.assertRaises(UnsupportedProviderAPI): - self.pb._download_provider_info() + with self.assertRaises(UnsupportedProviderAPI): + self.pb._download_provider_info() @mock.patch( 'leap.bitmask.config.providerconfig.ProviderConfig.get_ca_cert_path', @@ -499,10 +494,7 @@ class ProviderBootstrapperActiveTest(unittest.TestCase): self._setup_providerbootstrapper(False) self._produce_dummy_provider_json() - with mock.patch.object( - ProviderConfig, 'get_api_uri', - return_value="https://localhost:%s" % (self.https_port,)): - self.pb._download_provider_info() + self.pb._download_provider_info() @mock.patch( 'leap.bitmask.config.providerconfig.ProviderConfig.get_api_uri', diff --git a/src/leap/bitmask/services/__init__.py b/src/leap/bitmask/services/__init__.py index afce72f6..0d74e0e2 100644 --- a/src/leap/bitmask/services/__init__.py +++ b/src/leap/bitmask/services/__init__.py @@ -105,10 +105,12 @@ def download_service_config(provider_config, service_config, service_name = service_config.name service_json = "{0}-service.json".format(service_name) headers = {} - mtime = get_mtime(os.path.join(get_path_prefix(), + mtime = get_mtime(os.path.join(service_config + .get_path_prefix(), "leap", "providers", provider_config.get_domain(), service_json)) + if download_if_needed and mtime: headers['if-modified-since'] = mtime diff --git a/src/leap/bitmask/services/eip/tests/test_eipbootstrapper.py b/src/leap/bitmask/services/eip/tests/test_eipbootstrapper.py index d0d78eed..fed4a783 100644 --- a/src/leap/bitmask/services/eip/tests/test_eipbootstrapper.py +++ b/src/leap/bitmask/services/eip/tests/test_eipbootstrapper.py @@ -150,10 +150,10 @@ class EIPBootstrapperActiveTest(BaseLeapTest): def check(*args): self.eb._eip_config.save.assert_called_once_with( - ["leap", + ("leap", "providers", self.eb._provider_config.get_domain(), - "eip-service.json"]) + "eip-service.json")) d.addCallback(check) return d |