Refactor keyring usage and fix saving problem

3 files changed, 45 insertions, 29 deletions

diff --git a/changes/bug_fix_keyring_save b/changes/bug_fix_keyring_save
new file mode 100644
index 00000000..3be0e05b
--- /dev/null
+++ b/changes/bug_fix_keyring_save
@@ -0,0 +1,2 @@
+  - Refactor keyring handling and make it properly save user and
+    password. Fixes #4190.
\ No newline at end of file
diff --git a/src/leap/bitmask/gui/login.py b/src/leap/bitmask/gui/login.py
index 255a06d3..b21057f0 100644
--- a/src/leap/bitmask/gui/login.py
+++ b/src/leap/bitmask/gui/login.py
@@ -49,6 +49,9 @@ class LoginWidget(QtGui.QWidget):
 
     BARE_USERNAME_REGEX = r"^[A-Za-z\d_]+$"
 
+    # Keyring
+    KEYRING_KEY = "bitmask"
+
     def __init__(self, settings, parent=None):
         """
         Constructs the LoginWidget.
@@ -177,7 +180,7 @@ class LoginWidget(QtGui.QWidget):
         Sets the password for the widget
 
         :param password: password to set
-        :type password: str
+        :type password: unicode
         """
         self.ui.lnPassword.setText(password)
 
@@ -366,3 +369,41 @@ class LoginWidget(QtGui.QWidget):
         self.ui.btnLogout.setText(self.tr("Logout"))
         self.ui.btnLogout.setEnabled(True)
         self.ui.clblErrorMsg.hide()
+
+    def load_user_from_keyring(self, saved_user):
+        """
+        Tries to load a user from the keyring, returns True if it was
+        loaded successfully, False otherwise.
+
+        :param saved_user: String containing the saved username as
+                           user@domain
+        :type saved_user: unicode
+
+        :rtype: bool
+        """
+        leap_assert_type(saved_user, unicode)
+
+        try:
+            username, domain = saved_user.split('@')
+        except ValueError as e:
+            # if the saved_user does not contain an '@'
+            logger.error('Username@provider malformed. %r' % (e, ))
+            return False
+
+        self.set_user(username)
+
+        self.set_remember(True)
+
+        saved_password = None
+        try:
+            saved_password = keyring.get_password(self.KEYRING_KEY,
+                                                  saved_user
+                                                  .encode("utf8"))
+        except ValueError as e:
+            logger.debug("Incorrect Password. %r." % (e,))
+
+        if saved_password is not None:
+            self.set_password(saved_password)
+            return True
+
+        return False
diff --git a/src/leap/bitmask/gui/mainwindow.py b/src/leap/bitmask/gui/mainwindow.py
index 6af71e24..ce3f113b 100644
--- a/src/leap/bitmask/gui/mainwindow.py
+++ b/src/leap/bitmask/gui/mainwindow.py
@@ -20,8 +20,6 @@ Main window for Bitmask.
 import logging
 import os
 
-import keyring
-
 from PySide import QtCore, QtGui
 from twisted.internet import threads
 from zope.proxy import ProxyBase, setProxiedObject, sameProxiedObjects
@@ -88,9 +86,6 @@ class MainWindow(QtGui.QMainWindow):
     LOGIN_INDEX = 0
     EIP_STATUS_INDEX = 1
 
-    # Keyring
-    KEYRING_KEY = "bitmask"
-
     OPENVPN_SERVICE = "openvpn"
     MX_SERVICE = "mx"
 
@@ -579,30 +574,8 @@ class MainWindow(QtGui.QMainWindow):
 
             saved_user = self._settings.get_user()
 
-            try:
-                username, domain = saved_user.split('@')
-            except (ValueError, AttributeError) as e:
-                # if the saved_user does not contain an '@' or its None
-                logger.error('Username@provider malformed. %r' % (e, ))
-                saved_user = None
-
             if saved_user is not None and has_keyring():
-                # fill the username
-                self._login_widget.set_user(username)
-
-                self._login_widget.set_remember(True)
-
-                saved_password = None
-                try:
-                    saved_password = keyring.get_password(self.KEYRING_KEY,
-                                                          saved_user
-                                                          .encode("utf8"))
-                except ValueError, e:
-                    logger.debug("Incorrect Password. %r." % (e,))
-
-                if saved_password is not None:
-                    self._login_widget.set_password(
-                        saved_password.decode("utf8"))
+                if self._login_widget.load_user_from_keyring(saved_user):
                     self._login()
 
     def _hide_unsupported_services(self):