Merge lp:~mandel/ubuntu-sso-client/qt-ssl-dialog into lp:ubuntu-sso-client
- qt-ssl-dialog
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Natalia Bidart |
Approved revision: | 940 |
Merged at revision: | 904 |
Proposed branch: | lp:~mandel/ubuntu-sso-client/qt-ssl-dialog |
Merge into: | lp:ubuntu-sso-client |
Prerequisite: | lp:~mandel/ubuntu-sso-client/webclient-use-dialog |
Diff against target: |
844 lines (+511/-103) 7 files modified
ubuntu_sso/utils/ui.py (+6/-0) ubuntu_sso/utils/webclient/common.py (+34/-1) ubuntu_sso/utils/webclient/gsettings.py (+21/-14) ubuntu_sso/utils/webclient/qtnetwork.py (+113/-21) ubuntu_sso/utils/webclient/tests/__init__.py (+74/-6) ubuntu_sso/utils/webclient/tests/test_gsettings.py (+44/-61) ubuntu_sso/utils/webclient/tests/test_webclient.py (+219/-0) |
To merge this branch: | bzr merge lp:~mandel/ubuntu-sso-client/qt-ssl-dialog |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Roberto Alsina (community) | Approve | ||
Alejandro J. Cura (community) | Approve | ||
Review via email: mp+96624@code.launchpad.net |
Commit message
- Added a translatable string to give more context of the ssl cert info to the user
(LP: #948119).
- Provided the logic required for the Qt webclient implementation to detect ssl errors
and spawn the ssl dialog to allow the user accept the ssl cert exceptions
(LP: #948134).
- Changed the qt webclient implementation to use a proxy factory so that the correct
proxy is chosen according to the request (LP: #950088).
Description of the change
This branch fixes a number of bugs:
Added a translatable string to give more context of the ssl cert info to the user.
The Qt implementation of the webclient detects ssl errors and spawns the ssl dialog to allow the user to accept the ssl cert exception.
lp:950088
Implemented the qt webclient using a proxy factory over a proxy so that the correct proxy is returned.
Roberto Alsina (ralsina) wrote : | # |
+1 with what alecu said.
Manuel de la Peña (mandel) wrote : | # |
> Very good branch.
> I can't comment on anything but typos!
>
> ---
>
> I looks like it will be a problem with SSL_DETAILS_
> translations, since the \t tabs will not likely align with the different text
> lengths of the headers. I think we should either drop tabs or use html tables,
> since QTextEdit supports those. Although I wouldn't mind this change happening
> in a later branch, in fact I recommend it :-)
>
> ----
>
> This comment is outdated: "# define here the return codes from the proxy creds
> dialogs"
>
> "formated stirng" -> "formatted string"
>
> "is used in according" -> "is used according"
>
> "accoding" -> "according"
>
> "in witch" -> "in which"
>
> "this funtion most return" ->
> "this function must return".
>
> "a proxu set for the query lets return" ->
> ^ ^
> "a proxy set for the query let's return
>
> "lets retry" -> "let's retry"
>
> "the port being listen" -> "the port where we are listening."
> "the ssl port being listen" -> "the ssl port where we are listening."
>
> "Assert the parsing anonymous settings" ->
> "Assert the parsing of anonymous settings"
>
> "for the http requets" ->
> "for the http request"
>
> "SSL support has not yet implemented" ->
> "SSL support has not yet been implemented" or "SSL support not yet
> implemented"
Fixing all those, thx!
Manuel de la Peña (mandel) wrote : | # |
> Very good branch.
> I can't comment on anything but typos!
>
> ---
>
> I looks like it will be a problem with SSL_DETAILS_
> translations, since the \t tabs will not likely align with the different text
> lengths of the headers. I think we should either drop tabs or use html tables,
> since QTextEdit supports those. Although I wouldn't mind this change happening
> in a later branch, in fact I recommend it :-)
>
> ----
>
> This comment is outdated: "# define here the return codes from the proxy creds
> dialogs"
>
> "formated stirng" -> "formatted string"
>
> "is used in according" -> "is used according"
>
> "accoding" -> "according"
>
> "in witch" -> "in which"
>
> "this funtion most return" ->
> "this function must return".
>
> "a proxu set for the query lets return" ->
> ^ ^
> "a proxy set for the query let's return
>
> "lets retry" -> "let's retry"
>
> "the port being listen" -> "the port where we are listening."
> "the ssl port being listen" -> "the ssl port where we are listening."
>
> "Assert the parsing anonymous settings" ->
> "Assert the parsing of anonymous settings"
>
> "for the http requets" ->
> "for the http request"
>
> "SSL support has not yet implemented" ->
> "SSL support has not yet been implemented" or "SSL support not yet
> implemented"
Stupid wired brain... I'll take care of them, thx!
- 939. By Manuel de la Peña
-
Merged webclient-
use-dialog into qt-ssl-dialog. - 940. By Manuel de la Peña
-
Fixed typos.
Preview Diff
1 | === modified file 'ubuntu_sso/utils/ui.py' |
2 | --- ubuntu_sso/utils/ui.py 2012-03-06 14:13:10 +0000 |
3 | +++ ubuntu_sso/utils/ui.py 2012-03-09 12:09:20 +0000 |
4 | @@ -133,6 +133,12 @@ |
5 | SSL_CERT_DETAILS = _('Certificate details') |
6 | SSL_CONNECT_BUTTON = _('Connect') |
7 | SSL_DETAILS_HELP = _('the details ssl certificate we are going to show.') |
8 | +SSL_DETAILS_TEMPLATE = ('Organization:\t%(organization)s\n' |
9 | + 'Common Name:\t%(common_name)s\n' |
10 | + 'Locality Name:\t%(locality_name)s\n' |
11 | + 'Unit:\t%(unit)s\n' |
12 | + 'Country:\t%(country_name)s\n' |
13 | + 'State or Province:\t%(state_name)s') |
14 | SSL_DESCRIPTION = _('Open the SSL certificate UI.') |
15 | SSL_DIALOG_TITLE = _('SSL Certificate Not Valid') |
16 | SSL_DOMAIN_HELP = _('the domain whose ssl certificate we are going to show.') |
17 | |
18 | === modified file 'ubuntu_sso/utils/webclient/common.py' |
19 | --- ubuntu_sso/utils/webclient/common.py 2012-03-09 12:09:20 +0000 |
20 | +++ ubuntu_sso/utils/webclient/common.py 2012-03-09 12:09:20 +0000 |
21 | @@ -27,8 +27,11 @@ |
22 | from ubuntu_sso import USER_SUCCESS, UI_PROXY_CREDS_DIALOG |
23 | from ubuntu_sso.logger import setup_logging |
24 | from ubuntu_sso.utils.runner import spawn_program |
25 | +from ubuntu_sso.utils.ui import SSL_DETAILS_TEMPLATE |
26 | from ubuntu_sso.utils.webclient.timestamp import TimestampChecker |
27 | |
28 | +SSL_DIALOG = 'ubuntu-sso-ssl-certificate-qt' |
29 | + |
30 | logger = setup_logging("ubuntu_sso.utils.webclient.common") |
31 | |
32 | |
33 | @@ -87,8 +90,10 @@ |
34 | |
35 | timestamp_checker = None |
36 | |
37 | - def __init__(self, username=None, password=None, oauth_sign_plain=False): |
38 | + def __init__(self, appname='', username=None, password=None, |
39 | + oauth_sign_plain=False): |
40 | """Initialize this instance.""" |
41 | + self.appname = appname |
42 | self.username = username |
43 | self.password = password |
44 | self.proxy_username = None |
45 | @@ -222,3 +227,31 @@ |
46 | logger.debug('Could not retrieve the credentials. Return code: %r', |
47 | return_code) |
48 | defer.returnValue(False) |
49 | + |
50 | + def format_ssl_details(self, details): |
51 | + """Return a formatted string with the details.""" |
52 | + return SSL_DETAILS_TEMPLATE % details |
53 | + |
54 | + def _launch_ssl_dialog(self, domain, details): |
55 | + """Launch a dialog used to approve the ssl cert.""" |
56 | + from ubuntu_sso.utils import get_bin_dir |
57 | + |
58 | + bin_dir = get_bin_dir() |
59 | + args = (os.path.join(bin_dir, SSL_DIALOG), '--domain', domain, |
60 | + '--details', details, |
61 | + '--appname', self.appname) |
62 | + return spawn_program(args) |
63 | + |
64 | + def _was_ssl_accepted(self, cert_details): |
65 | + """Return if the cert was already accepted.""" |
66 | + # TODO: Ensure that we look at pinned certs in a following branch |
67 | + return False |
68 | + |
69 | + @defer.inlineCallbacks |
70 | + def request_ssl_cert_approval(self, domain, details): |
71 | + """Request the user for ssl approval.""" |
72 | + if self._was_ssl_accepted(details): |
73 | + defer.returnValue(True) |
74 | + |
75 | + return_code = yield self._launch_ssl_dialog(domain, details) |
76 | + defer.returnValue(return_code == USER_SUCCESS) |
77 | |
78 | === modified file 'ubuntu_sso/utils/webclient/gsettings.py' |
79 | --- ubuntu_sso/utils/webclient/gsettings.py 2012-02-10 14:46:16 +0000 |
80 | +++ ubuntu_sso/utils/webclient/gsettings.py 2012-03-09 12:09:20 +0000 |
81 | @@ -31,6 +31,24 @@ |
82 | return hostname, username, password |
83 | |
84 | |
85 | +def parse_manual_proxy_settings(scheme, gsettings): |
86 | + """Parse the settings for a given scheme.""" |
87 | + host, username, pwd = parse_proxy_host(gsettings[scheme + ".host"]) |
88 | + settings = { |
89 | + "host": host, |
90 | + "port": gsettings[scheme + ".port"], |
91 | + } |
92 | + if scheme == "http" and gsettings["http.use-authentication"]: |
93 | + username = gsettings["http.authentication-user"] |
94 | + pwd = gsettings["http.authentication-password"] |
95 | + if username is not None and pwd is not None: |
96 | + settings.update({ |
97 | + "username": username, |
98 | + "password": pwd, |
99 | + }) |
100 | + return settings |
101 | + |
102 | + |
103 | def get_proxy_settings(): |
104 | """Parse the proxy settings as returned by the gsettings executable.""" |
105 | output = subprocess.check_output(GSETTINGS_CMDLINE.split()) |
106 | @@ -56,20 +74,9 @@ |
107 | if mode == "none": |
108 | settings = {} |
109 | elif mode == "manual": |
110 | - # attempt to parse the host |
111 | - host, username, pwd = parse_proxy_host(gsettings["http.host"]) |
112 | - settings = { |
113 | - "host": host, |
114 | - "port": gsettings["http.port"], |
115 | - } |
116 | - if gsettings["http.use-authentication"]: |
117 | - username = gsettings["http.authentication-user"] |
118 | - pwd = gsettings["http.authentication-password"] |
119 | - if username is not None and pwd is not None: |
120 | - settings.update({ |
121 | - "username": username, |
122 | - "password": pwd, |
123 | - }) |
124 | + settings = {} |
125 | + for scheme in ["http", "https"]: |
126 | + settings[scheme] = parse_manual_proxy_settings(scheme, gsettings) |
127 | else: |
128 | # If mode is automatic the PAC javascript should be interpreted |
129 | # on each request. That is out of scope so it's ignored for now |
130 | |
131 | === modified file 'ubuntu_sso/utils/webclient/qtnetwork.py' |
132 | --- ubuntu_sso/utils/webclient/qtnetwork.py 2012-03-09 12:09:20 +0000 |
133 | +++ ubuntu_sso/utils/webclient/qtnetwork.py 2012-03-09 12:09:20 +0000 |
134 | @@ -30,6 +30,7 @@ |
135 | QNetworkProxyFactory, |
136 | QNetworkReply, |
137 | QNetworkRequest, |
138 | + QSslCertificate, |
139 | ) |
140 | from twisted.internet import defer |
141 | |
142 | @@ -47,6 +48,77 @@ |
143 | logger = setup_logging("ubuntu_sso.utils.webclient.qtnetwork") |
144 | |
145 | |
146 | +class WebClientProxyFactory(QNetworkProxyFactory): |
147 | + """Proxy factory used by the web client. |
148 | + |
149 | + The proxy factory ensures that the correct proxy is used according to |
150 | + the request performed by the web client. For example, if the system has |
151 | + different proxies set for the http and the https the proxy factory will |
152 | + return the correct proxy according to the scheme of the request. |
153 | + """ |
154 | + |
155 | + def __init__(self): |
156 | + """Create a new instance.""" |
157 | + super(WebClientProxyFactory, self).__init__() |
158 | + self._proxies = dict(default=QNetworkProxy(QNetworkProxy.DefaultProxy)) |
159 | + self._setup_proxies() |
160 | + |
161 | + def _get_proxy_for_settings(self, settings): |
162 | + """Return a proxy that uses the given settings.""" |
163 | + if "username" in settings and "password" in settings: |
164 | + logger.debug('Setting auth %s:%s proxy', |
165 | + settings.get("host", ""), settings.get("port", 0)) |
166 | + proxy = QNetworkProxy(QNetworkProxy.HttpProxy, |
167 | + hostName=settings.get("host", ""), |
168 | + port=settings.get("port", 0), |
169 | + user=settings.get("username", ""), |
170 | + password=settings.get("password", "")) |
171 | + else: |
172 | + logger.debug('Using nonauth proxy %s:%s', settings.get("host", ""), |
173 | + settings.get("port", 0)) |
174 | + proxy = QNetworkProxy(QNetworkProxy.HttpProxy, |
175 | + hostName=settings.get("host", ""), |
176 | + port=settings.get("port", 0)) |
177 | + return proxy |
178 | + |
179 | + def _setup_proxies(self): |
180 | + """Set the different proxies used in the system.""" |
181 | + proxy_settings = gsettings.get_proxy_settings() |
182 | + for scheme, settings in proxy_settings.iteritems(): |
183 | + self._proxies[scheme] = self._get_proxy_for_settings(settings) |
184 | + |
185 | + # ignore the complain since we have to use the Qt naming convention |
186 | + # pylint: disable=C0103 |
187 | + def queryProxy(self, query): |
188 | + """Return the proxy servers to be used in order of preference.""" |
189 | + if 'force' in self._proxies: |
190 | + return [self._proxies['force']] |
191 | + if self._proxies: |
192 | + scheme = str(query.protocolTag()) |
193 | + if scheme in self._proxies: |
194 | + return [self._proxies[scheme]] |
195 | + # this function most return always at least one item, since we do not |
196 | + # have a proxy set for the query let's return the system default proxy |
197 | + return [self._proxies['default']] |
198 | + # pylint: enable=C0103 |
199 | + |
200 | + def update_proxy(self, scheme, settings): |
201 | + """Allows to update the proxy to be used for a scheme.""" |
202 | + self._proxies[scheme] = self._get_proxy_for_settings(settings) |
203 | + |
204 | + def force_use_proxy(self, settings): |
205 | + """Force to use the proxies in the given settings.""" |
206 | + self._proxies['force'] = self._get_proxy_for_settings(settings) |
207 | + |
208 | + def find_proxy_schemes(self, proxy): |
209 | + """Return the scheme in wich the proxy is used.""" |
210 | + schemes = [] |
211 | + for scheme, scheme_proxy in self._proxies.iteritems(): |
212 | + if scheme_proxy == proxy: |
213 | + schemes.append(scheme) |
214 | + return schemes |
215 | + |
216 | + |
217 | class WebClient(BaseWebClient): |
218 | """A webclient with a qtnetwork backend.""" |
219 | |
220 | @@ -58,16 +130,17 @@ |
221 | self.nam.authenticationRequired.connect(self._handle_authentication) |
222 | self.nam.proxyAuthenticationRequired.connect( |
223 | self._handle_bad_proxy_authentication) |
224 | + self.nam.sslErrors.connect(self._handle_ssl_errors) |
225 | self.replies = {} |
226 | + self.proxy_factory = None |
227 | self.setup_proxy() |
228 | |
229 | def setup_proxy(self): |
230 | """Setup the proxy settings if needed.""" |
231 | # QtNetwork knows how to use the system settings on both Win and Mac |
232 | if sys.platform.startswith("linux"): |
233 | - settings = gsettings.get_proxy_settings() |
234 | - if settings: |
235 | - self.force_use_proxy(settings) |
236 | + self.proxy_factory = WebClientProxyFactory() |
237 | + self.nam.setProxyFactory(self.proxy_factory) |
238 | else: |
239 | QNetworkProxyFactory.setUseSystemConfiguration(True) |
240 | |
241 | @@ -121,9 +194,8 @@ |
242 | @defer.inlineCallbacks |
243 | def _handle_bad_proxy_authentication(self, proxy, authenticator): |
244 | """Handle when we have a bad proxy.""" |
245 | - current_proxy = self.nam.proxy() |
246 | - host = unicode(current_proxy.hostName()).encode('utf8') |
247 | - port = current_proxy.port() |
248 | + host = unicode(proxy.hostName()).encode('utf8') |
249 | + port = proxy.port() |
250 | settings = dict(host=host, port=port) |
251 | # HACK: work around for reported bugs: QTBUG-16728 and #QTBUG-14850 |
252 | got_creds = yield self.request_proxy_auth_credentials(host, True) |
253 | @@ -135,7 +207,9 @@ |
254 | and 'password' in settings): |
255 | authenticator.setUser(self.proxy_username) |
256 | authenticator.setPassword(self.proxy_password) |
257 | - self.force_use_proxy(settings) |
258 | + schemes = self.proxy_factory.find_proxy_schemes(proxy) |
259 | + for scheme in schemes: |
260 | + self.proxy_factory.update_proxy(scheme, settings) |
261 | |
262 | @defer.inlineCallbacks |
263 | def _handle_proxy_authentication(self, failure, method, reply, |
264 | @@ -158,9 +232,10 @@ |
265 | 'password': self.proxy_password}) |
266 | if (settings and 'username' in settings |
267 | and 'password' in settings): |
268 | - self.force_use_proxy(settings) |
269 | - # lets retry using the request |
270 | request = reply.request() |
271 | + scheme = str(request.url().scheme()) |
272 | + self.proxy_factory.update_proxy(scheme, settings) |
273 | + # let's retry using the request |
274 | result = yield self._perform_request(request, method, post_buffer) |
275 | defer.returnValue(result) |
276 | else: |
277 | @@ -191,20 +266,37 @@ |
278 | exception = WebClientError(error_string, content) |
279 | d.errback(exception) |
280 | |
281 | + def _get_certificate_details(self, cert): |
282 | + """Return an string with the details of the certificate.""" |
283 | + detail_titles = {QSslCertificate.Organization: 'organization', |
284 | + QSslCertificate.CommonName: 'common_name', |
285 | + QSslCertificate.LocalityName: 'locality_name', |
286 | + QSslCertificate.OrganizationalUnitName: 'unit', |
287 | + QSslCertificate.CountryName: 'country_name', |
288 | + QSslCertificate.StateOrProvinceName: 'state_name'} |
289 | + details = {} |
290 | + for info, title in detail_titles.iteritems(): |
291 | + details[title] = str(cert.issuerInfo(info)) |
292 | + return self.format_ssl_details(details) |
293 | + |
294 | + def _get_certificate_host(self, cert): |
295 | + """Return the host of the cert.""" |
296 | + return str(cert.issuerInfo(QSslCertificate.CommonName)) |
297 | + |
298 | + @defer.inlineCallbacks |
299 | + def _handle_ssl_errors(self, reply, errors): |
300 | + """Handle the case in which we got an ssl error.""" |
301 | + # ask the user if the cer should be trusted |
302 | + cert = errors[0].certificate() |
303 | + trust_cert = yield self.request_ssl_cert_approval( |
304 | + self._get_certificate_host(cert), |
305 | + self._get_certificate_details(cert)) |
306 | + if trust_cert: |
307 | + reply.ignoreSslErrors() |
308 | + |
309 | def force_use_proxy(self, settings): |
310 | """Setup this webclient to use the given proxy settings.""" |
311 | - host = settings.get("host", "") |
312 | - port = settings.get("port", 0) |
313 | - if "username" in settings and "password" in settings: |
314 | - logger.debug('Using auth proxy for %s:%s', host, port) |
315 | - proxy = QNetworkProxy(QNetworkProxy.HttpProxy, hostName=host, |
316 | - port=port, user=settings.get("username", ""), |
317 | - password=settings.get("password", "")) |
318 | - else: |
319 | - logger.debug('Using nonauth proxy for %s:%s', host, port) |
320 | - proxy = QNetworkProxy(QNetworkProxy.HttpProxy, hostName=host, |
321 | - port=port) |
322 | - self.nam.setProxy(proxy) |
323 | + self.proxy_factory.force_use_proxy(settings) |
324 | |
325 | def shutdown(self): |
326 | """Shut down all pending requests (if possible).""" |
327 | |
328 | === modified file 'ubuntu_sso/utils/webclient/tests/__init__.py' |
329 | --- ubuntu_sso/utils/webclient/tests/__init__.py 2012-02-07 19:36:50 +0000 |
330 | +++ ubuntu_sso/utils/webclient/tests/__init__.py 2012-03-09 12:09:20 +0000 |
331 | @@ -17,9 +17,48 @@ |
332 | """Tests for the proxy-aware webclient.""" |
333 | |
334 | from twisted.application import internet, service |
335 | +from twisted.internet import ssl |
336 | from twisted.web import http, server |
337 | |
338 | |
339 | +# Some settings are not used as described in: |
340 | +# https://bugzilla.gnome.org/show_bug.cgi?id=648237 |
341 | + |
342 | +TEMPLATE_GSETTINGS_OUTPUT = """\ |
343 | +org.gnome.system.proxy autoconfig-url '{autoconfig_url}' |
344 | +org.gnome.system.proxy ignore-hosts {ignore_hosts:s} |
345 | +org.gnome.system.proxy mode '{mode}' |
346 | +org.gnome.system.proxy.ftp host '{ftp_host}' |
347 | +org.gnome.system.proxy.ftp port {ftp_port} |
348 | +org.gnome.system.proxy.http authentication-password '{auth_password}' |
349 | +org.gnome.system.proxy.http authentication-user '{auth_user}' |
350 | +org.gnome.system.proxy.http host '{http_host}' |
351 | +org.gnome.system.proxy.http port {http_port} |
352 | +org.gnome.system.proxy.http use-authentication {http_use_auth} |
353 | +org.gnome.system.proxy.https host '{https_host}' |
354 | +org.gnome.system.proxy.https port {https_port} |
355 | +org.gnome.system.proxy.socks host '{socks_host}' |
356 | +org.gnome.system.proxy.socks port {socks_port} |
357 | +""" |
358 | + |
359 | +BASE_GSETTINGS_VALUES = { |
360 | + "autoconfig_url": "", |
361 | + "ignore_hosts": ["localhost", "127.0.0.0/8"], |
362 | + "mode": "none", |
363 | + "ftp_host": "", |
364 | + "ftp_port": 0, |
365 | + "auth_password": "", |
366 | + "auth_user": "", |
367 | + "http_host": "", |
368 | + "http_port": 0, |
369 | + "http_use_auth": "false", |
370 | + "https_host": "", |
371 | + "https_port": 0, |
372 | + "socks_host": "", |
373 | + "socks_port": 0, |
374 | +} |
375 | + |
376 | + |
377 | class SaveHTTPChannel(http.HTTPChannel): |
378 | """A save protocol to be used in tests.""" |
379 | |
380 | @@ -48,15 +87,26 @@ |
381 | class BaseMockWebServer(object): |
382 | """A mock webserver for testing""" |
383 | |
384 | - def __init__(self): |
385 | + def __init__(self, ssl_settings=None): |
386 | """Start up this instance.""" |
387 | self.root = self.get_root_resource() |
388 | self.site = SaveSite(self.root) |
389 | application = service.Application('web') |
390 | self.service_collection = service.IServiceCollection(application) |
391 | #pylint: disable=E1101 |
392 | - self.tcpserver = internet.TCPServer(0, self.site) |
393 | - self.tcpserver.setServiceParent(self.service_collection) |
394 | + ssl_context = None |
395 | + if (ssl_settings is not None |
396 | + and 'key' in ssl_settings |
397 | + and 'cert' in ssl_settings): |
398 | + ssl_context = ssl.DefaultOpenSSLContextFactory(ssl_settings['key'], |
399 | + ssl_settings['cert']) |
400 | + self.ssl_server = internet.SSLServer(0, self.site, ssl_context) |
401 | + else: |
402 | + self.ssl_server = None |
403 | + self.server = internet.TCPServer(0, self.site) |
404 | + self.server.setServiceParent(self.service_collection) |
405 | + if self.ssl_server: |
406 | + self.ssl_server.setServiceParent(self.service_collection) |
407 | self.service_collection.startService() |
408 | |
409 | def get_root_resource(self): |
410 | @@ -65,9 +115,27 @@ |
411 | |
412 | def get_iri(self): |
413 | """Build the iri for this mock server.""" |
414 | - #pylint: disable=W0212 |
415 | - port_num = self.tcpserver._port.getHost().port |
416 | - return u"http://127.0.0.1:%d/" % port_num |
417 | + url = u"http://127.0.0.1:%d/" |
418 | + return url % self.get_port() |
419 | + |
420 | + def get_ssl_iri(self): |
421 | + """Build the ssl iri for this mock server.""" |
422 | + if self.ssl_server: |
423 | + url = u"https://127.0.0.1:%d/" |
424 | + return url % self.get_ssl_port() |
425 | + |
426 | + def get_port(self): |
427 | + """Return the port where we are listening.""" |
428 | + # pylint: disable=W0212 |
429 | + return self.server._port.getHost().port |
430 | + # pylint: enable=W0212 |
431 | + |
432 | + def get_ssl_port(self): |
433 | + """Return the ssl port where we are listening.""" |
434 | + # pylint: disable=W0212 |
435 | + if self.ssl_server: |
436 | + return self.ssl_server._port.getHost().port |
437 | + # pylint: enable=W0212 |
438 | |
439 | def stop(self): |
440 | """Shut it down.""" |
441 | |
442 | === modified file 'ubuntu_sso/utils/webclient/tests/test_gsettings.py' |
443 | --- ubuntu_sso/utils/webclient/tests/test_gsettings.py 2012-02-10 19:20:33 +0000 |
444 | +++ ubuntu_sso/utils/webclient/tests/test_gsettings.py 2012-03-09 12:09:20 +0000 |
445 | @@ -18,43 +18,10 @@ |
446 | from twisted.trial.unittest import TestCase |
447 | |
448 | from ubuntu_sso.utils.webclient import gsettings |
449 | - |
450 | -# Some settings are not used as described in: |
451 | -# https://bugzilla.gnome.org/show_bug.cgi?id=648237 |
452 | - |
453 | -TEMPLATE_GSETTINGS_OUTPUT = """\ |
454 | -org.gnome.system.proxy autoconfig-url '{autoconfig_url}' |
455 | -org.gnome.system.proxy ignore-hosts {ignore_hosts:s} |
456 | -org.gnome.system.proxy mode '{mode}' |
457 | -org.gnome.system.proxy.ftp host '{ftp_host}' |
458 | -org.gnome.system.proxy.ftp port {ftp_port} |
459 | -org.gnome.system.proxy.http authentication-password '{auth_password}' |
460 | -org.gnome.system.proxy.http authentication-user '{auth_user}' |
461 | -org.gnome.system.proxy.http host '{http_host}' |
462 | -org.gnome.system.proxy.http port {http_port} |
463 | -org.gnome.system.proxy.http use-authentication {http_use_auth} |
464 | -org.gnome.system.proxy.https host '{https_host}' |
465 | -org.gnome.system.proxy.https port {https_port} |
466 | -org.gnome.system.proxy.socks host '{socks_host}' |
467 | -org.gnome.system.proxy.socks port {socks_port} |
468 | -""" |
469 | - |
470 | -BASE_GSETTINGS_VALUES = { |
471 | - "autoconfig_url": "", |
472 | - "ignore_hosts": ["localhost", "127.0.0.0/8"], |
473 | - "mode": "none", |
474 | - "ftp_host": "", |
475 | - "ftp_port": 0, |
476 | - "auth_password": "", |
477 | - "auth_user": "", |
478 | - "http_host": "", |
479 | - "http_port": 0, |
480 | - "http_use_auth": "false", |
481 | - "https_host": "", |
482 | - "https_port": 0, |
483 | - "socks_host": "", |
484 | - "socks_port": 0, |
485 | -} |
486 | +from ubuntu_sso.utils.webclient.tests import ( |
487 | + BASE_GSETTINGS_VALUES, |
488 | + TEMPLATE_GSETTINGS_OUTPUT, |
489 | +) |
490 | |
491 | |
492 | class ProxySettingsTestCase(TestCase): |
493 | @@ -83,25 +50,33 @@ |
494 | ps = gsettings.get_proxy_settings() |
495 | self.assertEqual(ps, expected) |
496 | |
497 | + def _assert_parser_anonymous(self, scheme): |
498 | + """Assert the parsing of anonymous settings.""" |
499 | + template_values = dict(BASE_GSETTINGS_VALUES) |
500 | + expected_host = "expected_host" |
501 | + expected_port = 54321 |
502 | + expected = { |
503 | + "host": expected_host, |
504 | + "port": expected_port, |
505 | + } |
506 | + template_values.update({ |
507 | + "mode": "manual", |
508 | + scheme + "_host": expected_host, |
509 | + scheme + "_port": expected_port, |
510 | + }) |
511 | + fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values) |
512 | + self.patch(gsettings.subprocess, "check_output", |
513 | + lambda _: fake_output) |
514 | + ps = gsettings.get_proxy_settings() |
515 | + self.assertEqual(ps[scheme], expected) |
516 | + |
517 | def test_gsettings_parser_http_anonymous(self): |
518 | """Test a parser of gsettings.""" |
519 | - template_values = dict(BASE_GSETTINGS_VALUES) |
520 | - expected_host = "expected_host" |
521 | - expected_port = 54321 |
522 | - expected = { |
523 | - "host": expected_host, |
524 | - "port": expected_port, |
525 | - } |
526 | - template_values.update({ |
527 | - "mode": "manual", |
528 | - "http_host": expected_host, |
529 | - "http_port": expected_port, |
530 | - }) |
531 | - fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values) |
532 | - self.patch(gsettings.subprocess, "check_output", |
533 | - lambda _: fake_output) |
534 | - ps = gsettings.get_proxy_settings() |
535 | - self.assertEqual(ps, expected) |
536 | + self._assert_parser_anonymous('http') |
537 | + |
538 | + def test_gsettings_parser_https_anonymus(self): |
539 | + """Test a parser of gsettings.""" |
540 | + self._assert_parser_anonymous('https') |
541 | |
542 | def test_gsettings_parser_http_authenticated(self): |
543 | """Test a parser of gsettings.""" |
544 | @@ -128,9 +103,9 @@ |
545 | self.patch(gsettings.subprocess, "check_output", |
546 | lambda _: fake_output) |
547 | ps = gsettings.get_proxy_settings() |
548 | - self.assertEqual(ps, expected) |
549 | + self.assertEqual(ps["http"], expected) |
550 | |
551 | - def test_gsettings_parser_authenticated_url(self): |
552 | + def _assert_parser_authenticated_url(self, scheme): |
553 | """Test a parser of gsettings with creds in the url.""" |
554 | template_values = dict(BASE_GSETTINGS_VALUES) |
555 | expected_host = "expected_host" |
556 | @@ -147,15 +122,23 @@ |
557 | } |
558 | template_values.update({ |
559 | "mode": "manual", |
560 | - "http_host": composed_url, |
561 | - "http_port": expected_port, |
562 | + scheme + "_host": composed_url, |
563 | + scheme + "_port": expected_port, |
564 | "http_use_auth": "false", |
565 | }) |
566 | fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values) |
567 | self.patch(gsettings.subprocess, "check_output", |
568 | lambda _: fake_output) |
569 | ps = gsettings.get_proxy_settings() |
570 | - self.assertEqual(ps, expected) |
571 | + self.assertEqual(ps[scheme], expected) |
572 | + |
573 | + def test_gsettings_parser_http_authenticated_url(self): |
574 | + """Test a parser of gsettings with creds in the url.""" |
575 | + self._assert_parser_authenticated_url('http') |
576 | + |
577 | + def test_gsettings_parser_https_authenticated_url(self): |
578 | + """Test a parser of gsettings with creds in the url.""" |
579 | + self._assert_parser_authenticated_url('https') |
580 | |
581 | def test_gsettings_auth_over_url(self): |
582 | """Test that the settings are more important that the url.""" |
583 | @@ -166,7 +149,7 @@ |
584 | expected_password = "very secret password" |
585 | composed_url = '%s:%s@%s' % ('user', 'random', |
586 | expected_host) |
587 | - expected = { |
588 | + http_expected = { |
589 | "host": expected_host, |
590 | "port": expected_port, |
591 | "username": expected_user, |
592 | @@ -184,7 +167,7 @@ |
593 | self.patch(gsettings.subprocess, "check_output", |
594 | lambda _: fake_output) |
595 | ps = gsettings.get_proxy_settings() |
596 | - self.assertEqual(ps, expected) |
597 | + self.assertEqual(ps["http"], http_expected) |
598 | |
599 | |
600 | class ParseProxyHostTestCase(TestCase): |
601 | |
602 | === modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py' |
603 | --- ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-03-09 12:09:20 +0000 |
604 | +++ ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-03-09 12:09:20 +0000 |
605 | @@ -16,9 +16,12 @@ |
606 | """Integration tests for the proxy-enabled webclient.""" |
607 | |
608 | import os |
609 | +import shutil |
610 | import sys |
611 | import urllib |
612 | |
613 | +from OpenSSL import crypto |
614 | +from socket import gethostname |
615 | from twisted.cred import checkers, portal |
616 | from twisted.internet import defer |
617 | from twisted.web import guard, http, resource |
618 | @@ -34,6 +37,8 @@ |
619 | USER_CANCELLATION, |
620 | ) |
621 | from ubuntu_sso.utils import webclient |
622 | +from ubuntu_sso.utils.ui import SSL_DETAILS_TEMPLATE |
623 | +from ubuntu_sso.utils.webclient import gsettings |
624 | from ubuntu_sso.utils.webclient.common import BaseWebClient, HeaderDict, oauth |
625 | from ubuntu_sso.utils.webclient.tests import BaseMockWebServer |
626 | |
627 | @@ -767,3 +772,217 @@ |
628 | got_creds = yield self.wc.request_proxy_auth_credentials(self.domain, |
629 | self.retry) |
630 | self.assertFalse(got_creds, 'Return true when user cancels.') |
631 | + |
632 | + |
633 | +class BaseSSLTestCase(SquidTestCase): |
634 | + """Base test that allows to use ssl connections.""" |
635 | + |
636 | + @defer.inlineCallbacks |
637 | + def setUp(self): |
638 | + """Set the diff tests.""" |
639 | + yield super(BaseSSLTestCase, self).setUp() |
640 | + self.cert_dir = os.path.join(self.tmpdir, 'cert') |
641 | + self.cert_details = dict(organization='Canonical', |
642 | + common_name=gethostname(), |
643 | + locality_name='London', |
644 | + unit='Ubuntu One', |
645 | + country_name='UK', |
646 | + state_name='London',) |
647 | + self.ssl_settings = self._generate_self_signed_certificate( |
648 | + self.cert_dir, |
649 | + self.cert_details) |
650 | + self.addCleanup(self._clean_ssl_certificate_files) |
651 | + |
652 | + self.ws = MockWebServer(self.ssl_settings) |
653 | + self.addCleanup(self.ws.stop) |
654 | + self.base_iri = self.ws.get_iri() |
655 | + self.base_ssl_iri = self.ws.get_ssl_iri() |
656 | + |
657 | + def _clean_ssl_certificate_files(self): |
658 | + """Remove the certificate files.""" |
659 | + if os.path.exists(self.cert_dir): |
660 | + shutil.rmtree(self.cert_dir) |
661 | + |
662 | + def _generate_self_signed_certificate(self, cert_dir, cert_details): |
663 | + """Generate the required SSL certificates.""" |
664 | + if not os.path.exists(cert_dir): |
665 | + os.makedirs(cert_dir) |
666 | + cert_path = os.path.join(cert_dir, 'cert.crt') |
667 | + key_path = os.path.join(cert_dir, 'cert.key') |
668 | + |
669 | + if os.path.exists(cert_path): |
670 | + os.unlink(cert_path) |
671 | + if os.path.exists(key_path): |
672 | + os.unlink(key_path) |
673 | + |
674 | + # create a key pair |
675 | + key = crypto.PKey() |
676 | + key.generate_key(crypto.TYPE_RSA, 1024) |
677 | + |
678 | + # create a self-signed cert |
679 | + cert = crypto.X509() |
680 | + cert.get_subject().C = cert_details['country_name'] |
681 | + cert.get_subject().ST = cert_details['state_name'] |
682 | + cert.get_subject().L = cert_details['locality_name'] |
683 | + cert.get_subject().O = cert_details['organization'] |
684 | + cert.get_subject().OU = cert_details['unit'] |
685 | + cert.get_subject().CN = cert_details['common_name'] |
686 | + cert.set_serial_number(1000) |
687 | + cert.gmtime_adj_notBefore(0) |
688 | + cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60) |
689 | + cert.set_issuer(cert.get_subject()) |
690 | + cert.set_pubkey(key) |
691 | + cert.sign(key, 'sha1') |
692 | + |
693 | + with open(cert_path, 'wt') as fd: |
694 | + fd.write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert)) |
695 | + |
696 | + with open(key_path, 'wt') as fd: |
697 | + fd.write(crypto.dump_privatekey(crypto.FILETYPE_PEM, key)) |
698 | + |
699 | + return dict(key=key_path, cert=cert_path) |
700 | + |
701 | + |
702 | +class CorrectProxyTestCase(BaseSSLTestCase): |
703 | + """Test the interaction with a SSL enabled proxy.""" |
704 | + |
705 | + @defer.inlineCallbacks |
706 | + def setUp(self): |
707 | + """Set the tests.""" |
708 | + yield super(CorrectProxyTestCase, self).setUp() |
709 | + |
710 | + # fake the gsettings to have diff settings for https and http |
711 | + http_settings = self.get_auth_proxy_settings() |
712 | + |
713 | + #remember so that we can use them in the creds request |
714 | + proxy_username = http_settings['username'] |
715 | + proxy_password = http_settings['password'] |
716 | + |
717 | + # delete the username and password so that we get a 407 for testing |
718 | + del http_settings['username'] |
719 | + del http_settings['password'] |
720 | + |
721 | + https_settings = self.get_nonauth_proxy_settings() |
722 | + |
723 | + proxy_settings = dict(http=http_settings, https=https_settings) |
724 | + self.patch(gsettings, "get_proxy_settings", lambda: proxy_settings) |
725 | + |
726 | + self.wc = webclient.webclient_factory() |
727 | + self.addCleanup(self.wc.shutdown) |
728 | + |
729 | + self.called = [] |
730 | + |
731 | + def fake_creds_request(domain, retry): |
732 | + """Fake user interaction.""" |
733 | + self.called.append('request_proxy_auth_credentials') |
734 | + self.wc.proxy_username = proxy_username |
735 | + self.wc.proxy_password = proxy_password |
736 | + return defer.succeed(True) |
737 | + |
738 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
739 | + fake_creds_request) |
740 | + |
741 | + def assert_header_contains(self, headers, expected): |
742 | + """One of the headers matching key must contain a given value.""" |
743 | + self.assertTrue(any(expected in value for value in headers)) |
744 | + |
745 | + def test_https_request(self): |
746 | + """Test using the correct proxy for the ssl request. |
747 | + |
748 | + In order to assert that the correct proxy is used we expect not to call |
749 | + the auth dialog since we set the https proxy not to use the auth proxy |
750 | + and to fail because we are reaching a https page with bad self-signed |
751 | + certs. |
752 | + """ |
753 | + # we fail due to the fake ssl cert |
754 | + self.failUnlessFailure(self.wc.request( |
755 | + self.base_ssl_iri + SIMPLERESOURCE), |
756 | + webclient.WebClientError) |
757 | + # https requests do not use the auth proxy therefore called should be |
758 | + # empty. This asserts that we are using the correct settings for the |
759 | + # request. |
760 | + self.assertEqual([], self.called) |
761 | + |
762 | + @defer.inlineCallbacks |
763 | + def test_http_request(self): |
764 | + """Test using the correct proxy for the plain request. |
765 | + |
766 | + This tests does the opposite to the https tests. We did set the auth |
767 | + proxy for the http request therefore we expect the proxy dialog to be |
768 | + used and not to get an error since we are not visiting a https with bad |
769 | + self-signed certs. |
770 | + """ |
771 | + # we do not fail since we are not going to the https page |
772 | + result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
773 | + self.assert_header_contains(result.headers["Via"], "squid") |
774 | + # assert that we did go through the auth proxy |
775 | + self.assertIn('request_proxy_auth_credentials', self.called) |
776 | + |
777 | + if WEBCLIENT_MODULE_NAME.endswith(".txweb"): |
778 | + reason = 'Multiple proxy settings is not supported.' |
779 | + test_https_request.skip = reason |
780 | + test_http_request.skip = reason |
781 | + |
782 | + if WEBCLIENT_MODULE_NAME.endswith(".libsoup"): |
783 | + reason = 'Hard to test since we need to fully mock gsettings.' |
784 | + test_https_request.skip = reason |
785 | + test_http_request.skip = reason |
786 | + |
787 | + |
788 | +class SSLTestCase(BaseSSLTestCase): |
789 | + """Test error handling when dealing with ssl.""" |
790 | + |
791 | + @defer.inlineCallbacks |
792 | + def setUp(self): |
793 | + """Set the diff tests.""" |
794 | + yield super(SSLTestCase, self).setUp() |
795 | + |
796 | + self.wc = webclient.webclient_factory() |
797 | + self.addCleanup(self.wc.shutdown) |
798 | + |
799 | + self.return_code = USER_CANCELLATION |
800 | + self.called = [] |
801 | + |
802 | + def fake_launch_ssl_dialog(client, domain, details): |
803 | + """Fake the ssl dialog.""" |
804 | + self.called.append(('_launch_ssl_dialog', domain, details)) |
805 | + return defer.succeed(self.return_code) |
806 | + |
807 | + self.patch(BaseWebClient, '_launch_ssl_dialog', fake_launch_ssl_dialog) |
808 | + |
809 | + @defer.inlineCallbacks |
810 | + def _assert_ssl_fail_user_accepts(self, proxy_settings=None): |
811 | + """Assert the dialog is shown in an ssl fail.""" |
812 | + self.return_code = USER_SUCCESS |
813 | + if proxy_settings: |
814 | + self.wc.force_use_proxy(proxy_settings) |
815 | + yield self.wc.request(self.base_ssl_iri + SIMPLERESOURCE) |
816 | + details = SSL_DETAILS_TEMPLATE % self.cert_details |
817 | + self.assertIn(('_launch_ssl_dialog', gethostname(), details), |
818 | + self.called) |
819 | + |
820 | + def test_ssl_fail_dialog_user_accepts(self): |
821 | + """Test showing the dialog and accepting.""" |
822 | + self._assert_ssl_fail_user_accepts() |
823 | + |
824 | + def test_ssl_fail_dialog_user_accepts_via_proxy(self): |
825 | + """Test showing the dialog and accepting when using a proxy.""" |
826 | + self._assert_ssl_fail_user_accepts(self.get_nonauth_proxy_settings()) |
827 | + |
828 | + def test_ssl_fail_dialog_user_rejects(self): |
829 | + """Test showing the dialog and rejecting.""" |
830 | + self.failUnlessFailure(self.wc.request(self.base_iri + SIMPLERESOURCE), |
831 | + webclient.WebClientError) |
832 | + |
833 | + def test_format_ssl_details(self): |
834 | + """Assert that details are correctly formatted""" |
835 | + details = SSL_DETAILS_TEMPLATE % self.cert_details |
836 | + self.assertEqual(details, |
837 | + self.wc.format_ssl_details(self.cert_details)) |
838 | + |
839 | + if (WEBCLIENT_MODULE_NAME.endswith(".txweb") or |
840 | + WEBCLIENT_MODULE_NAME.endswith(".libsoup")): |
841 | + reason = 'SSL support has not yet been implemented.' |
842 | + test_ssl_fail_dialog_user_accepts.skip = reason |
843 | + test_ssl_fail_dialog_user_accepts_via_proxy.skip = reason |
844 | + test_ssl_fail_dialog_user_rejects.skip = reason |
Very good branch.
I can't comment on anything but typos!
---
I looks like it will be a problem with SSL_DETAILS_ TEMPLATE, tabs (\t) and translations, since the \t tabs will not likely align with the different text lengths of the headers. I think we should either drop tabs or use html tables, since QTextEdit supports those. Although I wouldn't mind this change happening in a later branch, in fact I recommend it :-)
----
This comment is outdated: "# define here the return codes from the proxy creds dialogs"
"formated stirng" -> "formatted string"
"is used in according" -> "is used according"
"accoding" -> "according"
"in witch" -> "in which"
"this funtion most return" ->
"this function must return".
"a proxu set for the query lets return" ->
^ ^
"a proxy set for the query let's return
"lets retry" -> "let's retry"
"the port being listen" -> "the port where we are listening."
"the ssl port being listen" -> "the ssl port where we are listening."
"Assert the parsing anonymous settings" ->
"Assert the parsing of anonymous settings"
"for the http requets" ->
"for the http request"
"SSL support has not yet implemented" ->
"SSL support has not yet been implemented" or "SSL support not yet implemented"