Merge lp:~mandel/ubuntu-sso-client/fix-ssl into lp:ubuntu-sso-client

[Manuel de la Peña]

- Added extra logging when ssl errors occur (LP: #987405).

- Added extra logging when ssl errors occur (LP: #987405).

[lp:~mandel/ubuntu-sso-client/fix-ssl]

951. By Manuel de la Peña on 2012-04-23

Remove old comment.

[Roberto Alsina (ralsina)]

+1 but the diff lines 16-18 scare me slightly.

review: Approve

[Alejandro J. Cura (alecu)]

Nice branch.

I'm requesting a few fixes, as follow:

---

This comment is no longer valid:

29 # ask the user if the cer should be trusted

---

The name of this test is no longer valid:

test_ssl_fail_dialog_user_rejects

----

The changes in ubuntu_sso/utils/webclient/txweb.py are not used, and should be removed from this branch.

----

A typical error is currently logged as:

"""
2012-04-23 20:17:42,844:844.474077225 - ubuntu_sso.utils.webclient.qtnetwork - ERROR - SSL errors econuntered when performing a request to PyQt4.QtCore.QUrl(u'https://login.ubuntu.com/api/1.0/authentications')
:Error:
<TAB>Cert Details:
Organization: Mandel Corp
Common Name: mandel.biz
Locality Name: Barrio La Latina
Unit: Biohazards Division
Country: ES
State or Province: Madrid
<TAB>Error:
22
<TAB>Error msg:
The host name did not match any of the valid hosts for this certificate
"""

The error could be repeated multiple times in the log, and the wrongly placed tabs and the weird line breaks add a lot of noise. Please change it so looks like this:

msg.write('SSL errors found; url: %s\n' % reply.request().url())
for error in errors:
    msg.write('========Error=============\n%s (%s)\n' % (error.errorString(), error.error()))
    msg.write('--------Cert Details------\n%s\n' % self._get_certificate_details(error.certificate()))
msg.write('==========================\n')

review: Needs Fixing

[Manuel de la Peña (mandel)]

Indeed, is confusing (funny brain wired here), I'll push a version with the proposed message.

[lp:~mandel/ubuntu-sso-client/fix-ssl]

952. By Manuel de la Peña on 2012-04-24

Improve logging message.

[Alejandro J. Cura (alecu)]

It seems that these items were not fixed in the latest revision:

---

This comment is no longer valid:

29 # ask the user if the cer should be trusted

---

The name of this test is no longer valid:

test_ssl_fail_dialog_user_rejects

----

The changes in ubuntu_sso/utils/webclient/txweb.py are not used, and should be removed from this branch.

----

review: Needs Fixing

[lp:~mandel/ubuntu-sso-client/fix-ssl]

953. By Manuel de la Peña on 2012-04-24

Remov old comment and renamed test.

[Alejandro J. Cura (alecu)]

+1

review: Approve

=== modified file 'ubuntu_sso/utils/webclient/qtnetwork.py'
--- ubuntu_sso/utils/webclient/qtnetwork.py	2012-04-12 12:07:46 +0000
+++ ubuntu_sso/utils/webclient/qtnetwork.py	2012-04-24 14:09:21 +0000
@@ -30,6 +30,7 @@
 
 import sys
 
+from StringIO import StringIO
 # pylint: disable=E0611
 from PyQt4.QtCore import (
     QBuffer,
@@ -96,8 +97,7 @@
         self.nam.finished.connect(self._handle_finished)
         self.nam.authenticationRequired.connect(self._handle_authentication)
         self.nam.proxyAuthenticationRequired.connect(self.handle_proxy_auth)
-        # Disabled until we make this a per-instance option
-        #self.nam.sslErrors.connect(self._handle_ssl_errors)
+        self.nam.sslErrors.connect(self._handle_ssl_errors)
         self.replies = {}
         self.proxy_retry = False
         self.setup_proxy()
@@ -229,16 +229,18 @@
         """Return the host of the cert."""
         return str(cert.issuerInfo(QSslCertificate.CommonName))
 
-    @defer.inlineCallbacks
     def _handle_ssl_errors(self, reply, errors):
         """Handle the case in which we got an ssl error."""
-        # ask the user if the cer should be trusted
-        cert = errors[0].certificate()
-        trust_cert = yield self.request_ssl_cert_approval(
-                                    self._get_certificate_host(cert),
-                                    self._get_certificate_details(cert))
-        if trust_cert:
-            reply.ignoreSslErrors()
+        msg = StringIO()
+        msg.write('SSL errors found; url: %s\n' %
+                  reply.request().url().toString())
+        for error in errors:
+            msg.write('========Error=============\n%s (%s)\n' %
+                     (error.errorString(), error.error()))
+            msg.write('--------Cert Details------\n%s\n' %
+                      self._get_certificate_details(error.certificate()))
+            msg.write('==========================\n')
+        logger.error(msg.getvalue())
 
     def force_use_proxy(self, https_settings):
         """Setup this webclient to use the given proxy settings."""
 
=== modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py'
--- ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-04-18 15:24:12 +0000
+++ ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-04-24 14:09:21 +0000
@@ -28,6 +28,7 @@
 # files in the program, then also delete it here.
 """Integration tests for the proxy-enabled webclient."""
 
+import logging
 import os
 import shutil
 import sys
@@ -40,6 +41,7 @@
 from twisted.web import guard, http, resource
 from urllib2 import urlparse
 
+from ubuntuone.devtools.handlers import MementoHandler
 from ubuntuone.devtools.testcases import TestCase
 from ubuntuone.devtools.testcases.squid import SquidTestCase
 from ubuntuone.devtools.testing.txwebserver import (
@@ -1110,6 +1112,12 @@
         """Set the diff tests."""
         yield super(SSLTestCase, self).setUp()
 
+        self.memento = MementoHandler()
+        self.memento.setLevel(logging.DEBUG)
+        logger = webclient.webclient_module().logger
+        logger.addHandler(self.memento)
+        self.addCleanup(logger.removeHandler, self.memento)
+
         self.wc = webclient.webclient_factory()
         self.addCleanup(self.wc.shutdown)
 
@@ -1142,10 +1150,12 @@
         """Test showing the dialog and accepting when using a proxy."""
         self._assert_ssl_fail_user_accepts(self.get_nonauth_proxy_settings())
 
-    def test_ssl_fail_dialog_user_rejects(self):
+    def test_ssl_fail(self):
         """Test showing the dialog and rejecting."""
-        self.failUnlessFailure(self.wc.request(self.base_iri + SIMPLERESOURCE),
+        self.failUnlessFailure(self.wc.request(
+                                     self.base_iri + SIMPLERESOURCE),
                                      webclient.WebClientError)
+        self.assertNotEqual(None, self.memento.check_error('SSL errors'))
 
     def test_format_ssl_details(self):
         """Assert that details are correctly formatted"""
@@ -1158,4 +1168,4 @@
         reason = 'SSL support has not yet been implemented.'
         test_ssl_fail_dialog_user_accepts.skip = reason
         test_ssl_fail_dialog_user_accepts_via_proxy.skip = reason
-        test_ssl_fail_dialog_user_rejects.skip = reason
+        test_ssl_fail.skip = reason
 
=== modified file 'ubuntu_sso/utils/webclient/txweb.py'
--- ubuntu_sso/utils/webclient/txweb.py	2012-04-18 15:13:50 +0000
+++ ubuntu_sso/utils/webclient/txweb.py	2012-04-24 14:09:21 +0000
@@ -33,6 +33,7 @@
 
 from twisted.internet import defer
 
+from ubuntu_sso.logger import setup_logging
 from ubuntu_sso.utils.webclient.common import (
     BaseWebClient,
     HeaderDict,
@@ -41,6 +42,8 @@
     WebClientError,
 )
 
+logger = setup_logging("ubuntu_sso.utils.webclient.txweb")
+
 
 class RawResponse(object):
     """A raw response from the webcall."""