Merge lp:~mandel/ubuntu-sso-client/keyring-integration into lp:ubuntu-sso-client

[Manuel de la Peña]

Allows the creds dialog to store the credentials in the keyring.

Allows the creds dialog to store the credentials in the keyring.

[lp:~mandel/ubuntu-sso-client/keyring-integration]

866. By Manuel de la Peña on 2012-02-10

Merged creds-dialog into keyring-integration.

867. By Manuel de la Peña on 2012-02-10

Merged creds-dialog into keyring-integration.

868. By Manuel de la Peña on 2012-02-10

Merged creds-dialog into keyring-integration.

869. By Manuel de la Peña on 2012-02-10

Typos...

[Alejandro J. Cura (alecu)]

Looks just fine.

review: Approve

[lp:~mandel/ubuntu-sso-client/keyring-integration]

870. By Manuel de la Peña on 2012-02-10

Use unicode.

[Roberto Alsina (ralsina)]

There is a problem in that using str(widget.text()) will cause a unicode exception if the user types a unicode character.

OTOH, the keyring uses urllib.urlencode on the credentials, which means we can't pass unicode to it:

>>> urllib.urlencode({u'á':1})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/urllib.py", line 1310, in urlencode
    k = quote_plus(str(k))
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe1' in position 0: ordinal not in range(128)

Also, it seems we are doing the same thing in other places. So, maybe this branch should land and a new bug be opened for that issue.

If that's the chosen path, feel free to land it with one review, since everything else looks ok.

[lp:~mandel/ubuntu-sso-client/keyring-integration]

871. By Manuel de la Peña on 2012-02-13

Ensure that we do pass bytes to the keyring.

872. By Manuel de la Peña on 2012-02-13

Merged with parent and fixed issues.

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'ubuntu_sso/qt/proxy_dialog.py'
--- ubuntu_sso/qt/proxy_dialog.py	2012-02-13 16:19:18 +0000
+++ ubuntu_sso/qt/proxy_dialog.py	2012-02-13 16:19:18 +0000
@@ -17,7 +17,10 @@
 
 
 from PyQt4.QtGui import QDialog, QIcon
+from twisted.internet import defer
 
+from ubuntu_sso.logger import setup_logging
+from ubuntu_sso.keyring import Keyring
 from ubuntu_sso.qt.ui.proxy_credentials_dialog_ui import Ui_ProxyCredsDialog
 from ubuntu_sso.utils.ui import (
     PROXY_CREDS_DIALOG_TITLE,
@@ -32,6 +35,12 @@
     PROXY_CREDS_SAVE_BUTTON,
 )
 
+CREDS_ACQUIRED = 0
+USER_CANCELATION = -1
+EXCEPTION_RAISED = -2
+
+logger = setup_logging("ubuntu_sso.qt.proxy_dialog")
+
 
 class ProxyCredsDialog(QDialog):
     """"Dialog used to require the proxy credentials."""
@@ -39,6 +48,12 @@
     def __init__(self, is_error=False, domain=None):
         """Create a new instance."""
         QDialog.__init__(self)
+
+        if domain is None:
+            logger.debug('Domain passed as None.')
+            domain = ''
+        self.domain = domain
+        self.keyring = Keyring()
         self.ui = Ui_ProxyCredsDialog()
         self.ui.setupUi(self)
         # lets set the different basic contents for the ui
@@ -49,10 +64,6 @@
             self.ui.error_label.setVisible(True)
         else:
             self.ui.error_label.setVisible(False)
-        if domain is None:
-            self.ui.domain_label.setText('')
-        else:
-            self.ui.domain_label.setText(domain)
 
     def _set_labels(self):
         """Set the labels translations."""
@@ -65,12 +76,36 @@
                                      % PROXY_CREDS_ERROR)
         self.ui.username_label.setText(PROXY_CREDS_USER_LABEL)
         self.ui.password_label.setText(PROXY_CREDS_PSWD_LABEL)
+        self.ui.domain_label.setText(self.domain)
+
+    @defer.inlineCallbacks
+    def _on_save_clicked(self, *args):
+        """Save the new credentials."""
+        username = unicode(self.ui.username_entry.text()).encode('utf8')
+        password = unicode(self.ui.password_entry.text()).encode('utf8')
+        creds = dict(username=username, password=password)
+        # pylint: disable=W0703, W0612
+        try:
+            logger.debug('Save credentials as for domain %s.', self.domain)
+            yield self.keyring.set_credentials(self.domain, creds)
+        except Exception, e:
+            logger.error('Could not retrieve credentials.')
+            self.done(EXCEPTION_RAISED)
+        # pylint: disable=W0703, W0612
+        self.done(CREDS_ACQUIRED)
+
+    def _on_cancel_clicked(self, *args):
+        """End the dialog."""
+        logger.debug('User canceled credentials dialog.')
+        self.done(USER_CANCELATION)
 
     def _set_buttons(self):
         """Set the labels of the buttons."""
         self.ui.help_button.setText(PROXY_CREDS_HELP_BUTTON)
         self.ui.cancel_button.setText(PROXY_CREDS_CANCEL_BUTTON)
+        self.ui.cancel_button.clicked.connect(self._on_cancel_clicked)
         self.ui.save_button.setText(PROXY_CREDS_SAVE_BUTTON)
+        self.ui.save_button.clicked.connect(self._on_save_clicked)
 
     def _set_icon(self):
         """Set the icon used in the dialog."""
 
=== modified file 'ubuntu_sso/qt/tests/test_proxy_dialog.py'
--- ubuntu_sso/qt/tests/test_proxy_dialog.py	2012-02-13 16:19:18 +0000
+++ ubuntu_sso/qt/tests/test_proxy_dialog.py	2012-02-13 16:19:18 +0000
@@ -20,25 +20,48 @@
 
 from ubuntu_sso.qt import proxy_dialog
 
-# pylint: disable=E1101,
-
-
-class FakeTextWidget(object):
+# pylint: disable=E1101,W0212,
+
+
+class FakeSignal(object):
+    """A fake qt signal."""
+
+    def __init__(self):
+        """Create a new instance."""
+        self.called = []
+
+    def connect(self, cb):
+        """Connect signals."""
+        self.called.append(('connect', cb))
+
+
+class FakeWidget(object):
     """A fake widget that contains text."""
 
-    def __init__(self, object_name, called):
+    def __init__(self, object_name, called, text=''):
         """Create a new instance."""
         self.object_name = object_name
         self.called = called
+        self.internal_text = text
+        self.clicked = FakeSignal()
 
     # pylint: disable=C0103
     def setText(self, text):
         """Set the text of the widget."""
+        self.internal_text = text
         if not self.object_name in self.called:
             self.called[self.object_name] = [('setText', text)]
         else:
             self.called[self.object_name].append(('setText', text))
 
+    def text(self):
+        """Return the text."""
+        if not self.object_name in self.called:
+            self.called[self.object_name] = [('text',)]
+        else:
+            self.called[self.object_name].append(('text',))
+        return self.internal_text
+
     def setPixmap(self, pixmap):
         """Set a pixmap."""
         if not self.object_name in self.called:
@@ -52,6 +75,7 @@
             self.called[self.object_name] = [('setVisible', visible)]
         else:
             self.called[self.object_name].append(('setVisible', visible))
+
     # pylint: enable=C0103
 
 
@@ -62,12 +86,13 @@
                    'error_label', 'username_label', 'password_label',
                    'domain_label', 'logo_label')
     ui_buttons = ('help_button', 'cancel_button', 'save_button')
+    ui_entries = ('username_entry', 'password_entry')
 
     def __init__(self):
         """Create a new instance."""
         self.called = {}
-        for name in self.ui_labels + self.ui_buttons:
-            setattr(self, name, FakeTextWidget(name, self.called))
+        for name in self.ui_labels + self.ui_buttons + self.ui_entries:
+            setattr(self, name, FakeWidget(name, self.called))
 
     # pylint: disable=C0103
     def setupUi(self, dialog):
@@ -121,6 +146,11 @@
         dialog = proxy_dialog.ProxyCredsDialog()
         for label in dialog.ui.ui_buttons:
             self.assertIn(label, dialog.ui.called)
+        # assert that the clicked was set
+        self.assertIn(('connect', dialog._on_save_clicked),
+                      dialog.ui.save_button.clicked.called)
+        self.assertIn(('connect', dialog._on_cancel_clicked),
+                      dialog.ui.cancel_button.clicked.called)
 
     def test_domain_empty(self):
         """Test that if the domain is not passed we set ''"""
@@ -156,3 +186,50 @@
                       self.icon.called)
         self.assertIn(('setPixmap', self.icon),
                       dialog.ui.called['logo_label'])
+
+    def test_on_cancel_clicked(self):
+        """Test the on cancel clicked event."""
+        dialog = proxy_dialog.ProxyCredsDialog()
+        called = []
+
+        def fake_done(result):
+            """Fake the done method."""
+            called.append(('done', result))
+
+        self.patch(dialog, 'done', fake_done)
+
+        dialog._on_cancel_clicked()
+        self.assertIn(('done', proxy_dialog.USER_CANCELATION), called)
+
+    def assert_save_button(self, set_creds_callback, result_number):
+        """Test the save button execution."""
+        dialog = proxy_dialog.ProxyCredsDialog()
+        called = []
+
+        def fake_done(result):
+            """Fake the done method."""
+            called.append(('done', result))
+
+        self.patch(dialog, 'done', fake_done)
+
+        def fake_set_credentials(app_name, creds):
+            """Fake an error."""
+            called.append(('set_credentials', app_name, creds))
+            return set_creds_callback()
+
+        self.patch(dialog.keyring, 'set_credentials', fake_set_credentials)
+
+        dialog._on_save_clicked()
+        self.assertIn(('done', result_number), called)
+
+    def test_on_save_clicked_exception(self):
+        """Test that the exception is handled correctly."""
+        set_creds_cb = lambda: defer.fail(Exception())
+        result_number = proxy_dialog.EXCEPTION_RAISED
+        self.assert_save_button(set_creds_cb, result_number)
+
+    def test_on_save_clicked_correct(self):
+        """Test that we do save the creds."""
+        set_creds_cb = lambda: defer.succeed(True)
+        result_number = proxy_dialog.CREDS_ACQUIRED
+        self.assert_save_button(set_creds_cb, result_number)