Merge lp:~mandel/ubuntu-sso-client/webclient-use-dialog into lp:ubuntu-sso-client
- webclient-use-dialog
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Natalia Bidart | ||||
Approved revision: | 929 | ||||
Merged at revision: | 903 | ||||
Proposed branch: | lp:~mandel/ubuntu-sso-client/webclient-use-dialog | ||||
Merge into: | lp:ubuntu-sso-client | ||||
Prerequisite: | lp:~mandel/ubuntu-sso-client/fix-credentials-text | ||||
Diff against target: |
699 lines (+419/-29) 8 files modified
ubuntu_sso/__init__.py (+2/-0) ubuntu_sso/qt/proxy_dialog.py (+4/-7) ubuntu_sso/qt/tests/test_proxy_dialog.py (+2/-2) ubuntu_sso/utils/webclient/__init__.py (+1/-0) ubuntu_sso/utils/webclient/common.py (+69/-1) ubuntu_sso/utils/webclient/libsoup.py (+48/-4) ubuntu_sso/utils/webclient/qtnetwork.py (+88/-15) ubuntu_sso/utils/webclient/tests/test_webclient.py (+205/-0) |
||||
To merge this branch: | bzr merge lp:~mandel/ubuntu-sso-client/webclient-use-dialog | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Natalia Bidart (community) | Approve | ||
Alejandro J. Cura (community) | Approve | ||
Review via email: mp+94416@code.launchpad.net |
Commit message
- Added the required code to allow the webclient use authenticated proxies and request the creds when needed (LP: #933727).
Description of the change
Provides the required code to use auth proxies within sso.
Brian Curtin (brian.curtin) wrote : | # |
Brian Curtin (brian.curtin) wrote : | # |
> 140 + if (self.proxy_
> 141 + and self.proxy_password is not None):
>
> """if all((self.
> verbose way of doing the same thing. I think it's clearer, but it's no more
> correct than what you have so it's merely a comment and not a requirement to
> fix.
I misunderstood the original lines. If "" and "" are acceptable username and password values, then my suggestion is incorrect and would change the behavior.
- 911. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 912. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 913. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 914. By Manuel de la Peña
-
Merged with parent.
- 915. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 916. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 917. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 918. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 919. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 920. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 921. By Manuel de la Peña
-
Merged fix-credentials
-text into webclient- use-dialog. - 922. By Manuel de la Peña
-
Merged with trunk.
- 923. By Manuel de la Peña
-
Use add_feature instead of add_feature_
by_type. - 924. By Manuel de la Peña
-
Merged with trunk.
- 925. By Manuel de la Peña
-
Merged with trunk.
- 926. By Manuel de la Peña
-
Reuse vars from ubuntu-sso
Natalia Bidart (nataliabidart) wrote : | # |
Thanks for this work!
* Could you please change this line:
to be:
so the exception is logged and we know what happened? Please apply the same to every logger.error inside a try-except block.
* This new import from ubuntu_
* Can you please move the constants EXCEPTION_RAISED and PROXY_CREDS_DIALOG to ubuntu_
* Unless strictly necessary, please avoid having imports inside functions. Can this import be moved at the beginning of the python file?
def _load_proxy_
"""Load the proxy creds from the keyring."""
from ubuntu_sso.keyring import Keyring
Same for:
def _launch_
"""Launch the dialog used to get the creds."""
from ubuntu_sso.utils import get_bin_dir
* I would guess you need to pass the app_name here?
args = (os.path.
* Why are you defining new variables:
given that the BaseWebClient class already has:
?
* The import of ProxyUnauthoriz
* Shouldn't the self.session.
* Instead of calling settings.
Manuel de la Peña (mandel) wrote : | # |
> Thanks for this work!
>
> * Could you please change this line:
>
> logger.error('Could not set credentials.')
>
> to be:
>
> logger.
>
> so the exception is logged and we know what happened? Please apply the same to
> every logger.error inside a try-except block.
Sorted, I always forget about the diff, thx!
>
> * This new import from ubuntu_
> alphabetically ordered.
>
Sorted out.
> * Can you please move the constants EXCEPTION_RAISED and PROXY_CREDS_DIALOG to
> ubuntu_
>
Done.
> * Unless strictly necessary, please avoid having imports inside functions. Can
> this import be moved at the beginning of the python file?
>
> def _load_proxy_
> """Load the proxy creds from the keyring."""
> from ubuntu_sso.keyring import Keyring
>
>
> Same for:
>
> def _launch_
> """Launch the dialog used to get the creds."""
> from ubuntu_sso.utils import get_bin_dir
>
Unfortunately there is a circular import due to the fact that keyring is in utils too, there is no other way to fix this atm.
> * I would guess you need to pass the app_name here?
>
> args = (os.path.
>
By the looks at the strings proposed by design (we need to talk about this with them) there is no need at the moment. This looks like a bug in the ui design.
> * Why are you defining new variables:
>
> self.proxy_username = None
> self.proxy_password = None
>
> given that the BaseWebClient class already has:
>
> self.username = username
> self.password = password
> ?
>
There is a diff between the username and password in the server side and the proxy username and password. We have two different pairs, when there is an auth issue coming from the server (that is username/password) and when there is an auth issue coming from the proxy (proxy_
> * The import of ProxyUnauthoriz
>
Sorted!
> * Shouldn't the self.session.
>
Is in order once the exception var is moved out.
> * Instead of calling settings.
> that to a variable? same for port=settings.
Certainly.
- 927. By Manuel de la Peña
-
Fixed according to the reviews.
- 928. By Manuel de la Peña
-
Use try/finally to ensure that we always unpause the message.
Alejandro J. Cura (alecu) wrote : | # |
Lovely branch!
I really like that you managed to work around the qt issue regarding proxy authentication.
just two typos ;-)
* "going throw a proxy" -> *thru*
* _on_aunthenticate
Otherwise a great branch.
Manuel de la Peña (mandel) wrote : | # |
> Lovely branch!
> I really like that you managed to work around the qt issue regarding proxy
> authentication.
>
> just two typos ;-)
>
> * "going throw a proxy" -> *thru*
> * _on_aunthenticate
>
> Otherwise a great branch.
Fixed, thx for the review!!
- 929. By Manuel de la Peña
-
Fixed typos.
Natalia Bidart (nataliabidart) wrote : | # |
Looks good!
Preview Diff
1 | === modified file 'ubuntu_sso/__init__.py' |
2 | --- ubuntu_sso/__init__.py 2012-02-11 19:25:01 +0000 |
3 | +++ ubuntu_sso/__init__.py 2012-03-09 11:54:19 +0000 |
4 | @@ -29,7 +29,9 @@ |
5 | # return codes for UIs |
6 | USER_SUCCESS = 0 |
7 | USER_CANCELLATION = 10 |
8 | +EXCEPTION_RAISED = 11 |
9 | |
10 | # available UIs |
11 | UI_EXECUTABLE_GTK = 'ubuntu-sso-login-gtk' |
12 | UI_EXECUTABLE_QT = 'ubuntu-sso-login-qt' |
13 | +UI_PROXY_CREDS_DIALOG = 'ubuntu-sso-proxy-creds-qt' |
14 | |
15 | === modified file 'ubuntu_sso/qt/proxy_dialog.py' |
16 | --- ubuntu_sso/qt/proxy_dialog.py 2012-03-05 20:30:57 +0000 |
17 | +++ ubuntu_sso/qt/proxy_dialog.py 2012-03-09 11:54:19 +0000 |
18 | @@ -21,6 +21,7 @@ |
19 | from PyQt4.QtGui import QApplication, QDialog, QIcon |
20 | from twisted.internet import defer |
21 | |
22 | +from ubuntu_sso import EXCEPTION_RAISED, USER_SUCCESS, USER_CANCELLATION |
23 | from ubuntu_sso.logger import setup_gui_logging |
24 | from ubuntu_sso.keyring import Keyring |
25 | from ubuntu_sso.qt.ui.proxy_credentials_dialog_ui import Ui_ProxyCredsDialog |
26 | @@ -37,10 +38,6 @@ |
27 | PROXY_CREDS_SAVE_BUTTON, |
28 | ) |
29 | |
30 | -CREDS_ACQUIRED = 0 |
31 | -USER_CANCELATION = -1 |
32 | -EXCEPTION_RAISED = -2 |
33 | - |
34 | logger = setup_gui_logging("ubuntu_sso.qt.proxy_dialog") |
35 | |
36 | |
37 | @@ -107,15 +104,15 @@ |
38 | logger.debug('Save credentials as for domain %s.', self.domain) |
39 | yield self.keyring.set_credentials(self.domain, creds) |
40 | except Exception, e: |
41 | - logger.error('Could not retrieve credentials.') |
42 | + logger.exception('Could not set credentials:') |
43 | self.done(EXCEPTION_RAISED) |
44 | # pylint: disable=W0703, W0612 |
45 | - self.done(CREDS_ACQUIRED) |
46 | + self.done(USER_SUCCESS) |
47 | |
48 | def _on_cancel_clicked(self, *args): |
49 | """End the dialog.""" |
50 | logger.debug('User canceled credentials dialog.') |
51 | - self.done(USER_CANCELATION) |
52 | + self.done(USER_CANCELLATION) |
53 | |
54 | def _set_buttons(self): |
55 | """Set the labels of the buttons.""" |
56 | |
57 | === modified file 'ubuntu_sso/qt/tests/test_proxy_dialog.py' |
58 | --- ubuntu_sso/qt/tests/test_proxy_dialog.py 2012-02-13 16:16:03 +0000 |
59 | +++ ubuntu_sso/qt/tests/test_proxy_dialog.py 2012-03-09 11:54:19 +0000 |
60 | @@ -256,7 +256,7 @@ |
61 | self.patch(dialog, 'done', fake_done) |
62 | |
63 | dialog._on_cancel_clicked() |
64 | - self.assertIn(('done', proxy_dialog.USER_CANCELATION), called) |
65 | + self.assertIn(('done', proxy_dialog.USER_CANCELLATION), called) |
66 | |
67 | def assert_save_button(self, set_creds_callback, result_number): |
68 | """Test the save button execution.""" |
69 | @@ -288,7 +288,7 @@ |
70 | def test_on_save_clicked_correct(self): |
71 | """Test that we do save the creds.""" |
72 | set_creds_cb = lambda: defer.succeed(True) |
73 | - result_number = proxy_dialog.CREDS_ACQUIRED |
74 | + result_number = proxy_dialog.USER_SUCCESS |
75 | self.assert_save_button(set_creds_cb, result_number) |
76 | |
77 | |
78 | |
79 | === modified file 'ubuntu_sso/utils/webclient/__init__.py' |
80 | --- ubuntu_sso/utils/webclient/__init__.py 2012-02-09 18:11:02 +0000 |
81 | +++ ubuntu_sso/utils/webclient/__init__.py 2012-03-09 11:54:19 +0000 |
82 | @@ -19,6 +19,7 @@ |
83 | |
84 | # pylint: disable=W0611 |
85 | from ubuntu_sso.utils.webclient.common import ( |
86 | + ProxyUnauthorizedError, |
87 | UnauthorizedError, |
88 | WebClientError, |
89 | ) |
90 | |
91 | === modified file 'ubuntu_sso/utils/webclient/common.py' |
92 | --- ubuntu_sso/utils/webclient/common.py 2012-02-09 18:11:02 +0000 |
93 | +++ ubuntu_sso/utils/webclient/common.py 2012-03-09 11:54:19 +0000 |
94 | @@ -17,14 +17,20 @@ |
95 | |
96 | import cgi |
97 | import collections |
98 | +import os |
99 | |
100 | from httplib2 import iri2uri |
101 | from oauth import oauth |
102 | from twisted.internet import defer |
103 | from urlparse import urlparse |
104 | |
105 | +from ubuntu_sso import USER_SUCCESS, UI_PROXY_CREDS_DIALOG |
106 | +from ubuntu_sso.logger import setup_logging |
107 | +from ubuntu_sso.utils.runner import spawn_program |
108 | from ubuntu_sso.utils.webclient.timestamp import TimestampChecker |
109 | |
110 | +logger = setup_logging("ubuntu_sso.utils.webclient.common") |
111 | + |
112 | |
113 | class WebClientError(Exception): |
114 | """An http error happened while calling the webservice.""" |
115 | @@ -34,8 +40,12 @@ |
116 | """The request ended with bad_request, unauthorized or forbidden.""" |
117 | |
118 | |
119 | +class ProxyUnauthorizedError(WebClientError): |
120 | + """Failure raised when there is an issue with the proxy auth.""" |
121 | + |
122 | + |
123 | class Response(object): |
124 | - """A reponse object.""" |
125 | + """A response object.""" |
126 | |
127 | def __init__(self, content, headers=None): |
128 | """Initialize this instance.""" |
129 | @@ -81,6 +91,8 @@ |
130 | """Initialize this instance.""" |
131 | self.username = username |
132 | self.password = password |
133 | + self.proxy_username = None |
134 | + self.proxy_password = None |
135 | self.oauth_sign_plain = oauth_sign_plain |
136 | |
137 | def request(self, iri, method="GET", extra_headers=None, |
138 | @@ -154,3 +166,59 @@ |
139 | |
140 | def shutdown(self): |
141 | """Shut down all pending requests (if possible).""" |
142 | + |
143 | + @defer.inlineCallbacks |
144 | + def _load_proxy_creds_from_keyring(self, domain): |
145 | + """Load the proxy creds from the keyring.""" |
146 | + from ubuntu_sso.keyring import Keyring |
147 | + keyring = Keyring() |
148 | + try: |
149 | + creds = yield keyring.get_credentials(domain) |
150 | + logger.debug('Got credentials from keyring.') |
151 | + except Exception, e: |
152 | + logger.error('Error when retrieving the creds.') |
153 | + raise WebClientError('Error when retrieving the creds.', e) |
154 | + if creds is not None: |
155 | + self.proxy_username = creds['username'] |
156 | + self.proxy_password = creds['password'] |
157 | + defer.returnValue(True) |
158 | + defer.returnValue(False) |
159 | + |
160 | + def _launch_proxy_creds_dialog(self, domain, retry): |
161 | + """Launch the dialog used to get the creds.""" |
162 | + from ubuntu_sso.utils import get_bin_dir |
163 | + |
164 | + bin_dir = get_bin_dir() |
165 | + args = (os.path.join(bin_dir, UI_PROXY_CREDS_DIALOG), '--domain', |
166 | + domain) |
167 | + if retry: |
168 | + args += ('--retry',) |
169 | + return spawn_program(args) |
170 | + |
171 | + @defer.inlineCallbacks |
172 | + def request_proxy_auth_credentials(self, domain, retry): |
173 | + """Request the auth creds to the user.""" |
174 | + if not retry: |
175 | + if (self.proxy_username is not None |
176 | + and self.proxy_password is not None): |
177 | + logger.debug('Not retry and credentials are present.') |
178 | + defer.returnValue(True) |
179 | + else: |
180 | + creds_loaded = yield self._load_proxy_creds_from_keyring( |
181 | + domain) |
182 | + if creds_loaded: |
183 | + defer.returnValue(True) |
184 | + |
185 | + try: |
186 | + return_code = yield self._launch_proxy_creds_dialog(domain, retry) |
187 | + except Exception, e: |
188 | + logger.error('Error when running external ui process.') |
189 | + raise WebClientError('Error when running external ui process.', e) |
190 | + |
191 | + if return_code == USER_SUCCESS: |
192 | + creds_loaded = yield self._load_proxy_creds_from_keyring(domain) |
193 | + defer.returnValue(creds_loaded) |
194 | + else: |
195 | + logger.debug('Could not retrieve the credentials. Return code: %r', |
196 | + return_code) |
197 | + defer.returnValue(False) |
198 | |
199 | === modified file 'ubuntu_sso/utils/webclient/libsoup.py' |
200 | --- ubuntu_sso/utils/webclient/libsoup.py 2012-02-07 19:36:50 +0000 |
201 | +++ ubuntu_sso/utils/webclient/libsoup.py 2012-03-09 11:54:19 +0000 |
202 | @@ -19,10 +19,12 @@ |
203 | |
204 | from twisted.internet import defer |
205 | |
206 | +from ubuntu_sso.logger import setup_logging |
207 | from ubuntu_sso.utils.webclient.common import ( |
208 | BaseWebClient, |
209 | HeaderDict, |
210 | Response, |
211 | + ProxyUnauthorizedError, |
212 | UnauthorizedError, |
213 | WebClientError, |
214 | ) |
215 | @@ -30,6 +32,8 @@ |
216 | URI_ANONYMOUS_TEMPLATE = "http://{host}:{port}/" |
217 | URI_USERNAME_TEMPLATE = "http://{username}:{password}@{host}:{port}/" |
218 | |
219 | +logger = setup_logging("ubuntu_sso.utils.webclient.libsoup") |
220 | + |
221 | |
222 | class WebClient(BaseWebClient): |
223 | """A webclient with a libsoup backend.""" |
224 | @@ -41,11 +45,12 @@ |
225 | from gi.repository import Soup, SoupGNOME |
226 | self.soup = Soup |
227 | self.session = Soup.SessionAsync() |
228 | - self.session.add_feature_by_type(SoupGNOME.ProxyResolverGNOME) |
229 | + self.session.add_feature(SoupGNOME.ProxyResolverGNOME()) |
230 | self.session.connect("authenticate", self._on_authenticate) |
231 | |
232 | def _on_message(self, session, message, d): |
233 | """Handle the result of an http message.""" |
234 | + logger.debug('_on_message status code is %s', message.status_code) |
235 | if message.status_code == httplib.OK: |
236 | headers = HeaderDict() |
237 | response_headers = message.get_property("response-headers") |
238 | @@ -57,14 +62,51 @@ |
239 | elif message.status_code == httplib.UNAUTHORIZED: |
240 | e = UnauthorizedError(message.reason_phrase) |
241 | d.errback(e) |
242 | + elif message.status_code == httplib.PROXY_AUTHENTICATION_REQUIRED: |
243 | + e = ProxyUnauthorizedError(message.reason_phrase) |
244 | + d.errback(e) |
245 | else: |
246 | e = WebClientError(message.reason_phrase) |
247 | d.errback(e) |
248 | |
249 | - def _on_authenticate(self, sesion, message, auth, retrying, data=None): |
250 | + @defer.inlineCallbacks |
251 | + def _on_authenticate(self, session, message, auth, retrying, data=None): |
252 | """Handle the "authenticate" signal.""" |
253 | - if not retrying and self.username and self.password: |
254 | - auth.authenticate(self.username, self.password) |
255 | + self.session.pause_message(message) |
256 | + try: |
257 | + logger.debug('_on_authenticate: message status code is %s', |
258 | + message.status_code) |
259 | + if not retrying and self.username and self.password: |
260 | + auth.authenticate(self.username, self.password) |
261 | + if auth.is_for_proxy(): |
262 | + logger.debug('_on_authenticate auth is for proxy.') |
263 | + got_creds = yield self.request_proxy_auth_credentials( |
264 | + self.session.props.proxy_uri.host, |
265 | + retrying) |
266 | + if got_creds: |
267 | + logger.debug('Got proxy credentials from user.') |
268 | + auth.authenticate(self.proxy_username, self.proxy_password) |
269 | + finally: |
270 | + self.session.unpause_message(message) |
271 | + |
272 | + @defer.inlineCallbacks |
273 | + def _on_proxy_authenticate(self, failure, iri, method="GET", |
274 | + extra_headers=None, oauth_credentials=None, post_content=None): |
275 | + """Deal with wrong settings.""" |
276 | + failure.trap(ProxyUnauthorizedError) |
277 | + logger.debug('Proxy settings are wrong.') |
278 | + got_creds = yield self.request_proxy_auth_credentials( |
279 | + self.session.props.proxy_uri.host, |
280 | + True) |
281 | + if got_creds: |
282 | + settings = dict(host=self.session.props.proxy_uri.host, |
283 | + port=self.session.props.proxy_uri.port, |
284 | + username=self.proxy_username, |
285 | + password=self.proxy_password) |
286 | + self.force_use_proxy(settings) |
287 | + response = yield self.request(iri, method, extra_headers, |
288 | + oauth_credentials, post_content) |
289 | + defer.returnValue(response) |
290 | |
291 | @defer.inlineCallbacks |
292 | def request(self, iri, method="GET", extra_headers=None, |
293 | @@ -92,6 +134,8 @@ |
294 | message.request_body.append(post_content) |
295 | |
296 | self.session.queue_message(message, self._on_message, d) |
297 | + d.addErrback(self._on_proxy_authenticate, iri, method, extra_headers, |
298 | + oauth_credentials, post_content) |
299 | response = yield d |
300 | defer.returnValue(response) |
301 | |
302 | |
303 | === modified file 'ubuntu_sso/utils/webclient/qtnetwork.py' |
304 | --- ubuntu_sso/utils/webclient/qtnetwork.py 2012-02-07 19:36:50 +0000 |
305 | +++ ubuntu_sso/utils/webclient/qtnetwork.py 2012-03-09 11:54:19 +0000 |
306 | @@ -33,15 +33,19 @@ |
307 | ) |
308 | from twisted.internet import defer |
309 | |
310 | +from ubuntu_sso.logger import setup_logging |
311 | from ubuntu_sso.utils.webclient.common import ( |
312 | BaseWebClient, |
313 | HeaderDict, |
314 | + ProxyUnauthorizedError, |
315 | Response, |
316 | UnauthorizedError, |
317 | WebClientError, |
318 | ) |
319 | from ubuntu_sso.utils.webclient import gsettings |
320 | |
321 | +logger = setup_logging("ubuntu_sso.utils.webclient.qtnetwork") |
322 | + |
323 | |
324 | class WebClient(BaseWebClient): |
325 | """A webclient with a qtnetwork backend.""" |
326 | @@ -52,6 +56,8 @@ |
327 | self.nam = QNetworkAccessManager(QCoreApplication.instance()) |
328 | self.nam.finished.connect(self._handle_finished) |
329 | self.nam.authenticationRequired.connect(self._handle_authentication) |
330 | + self.nam.proxyAuthenticationRequired.connect( |
331 | + self._handle_bad_proxy_authentication) |
332 | self.replies = {} |
333 | self.setup_proxy() |
334 | |
335 | @@ -65,6 +71,22 @@ |
336 | else: |
337 | QNetworkProxyFactory.setUseSystemConfiguration(True) |
338 | |
339 | + def _perform_request(self, request, method, post_buffer): |
340 | + """Return a deferred that will be fired with a Response object.""" |
341 | + d = defer.Deferred() |
342 | + if method == "GET": |
343 | + reply = self.nam.get(request) |
344 | + elif method == "HEAD": |
345 | + reply = self.nam.head(request) |
346 | + else: |
347 | + reply = self.nam.sendCustomRequest(request, method, post_buffer) |
348 | + # deal with auth errors in case we get |
349 | + # ProxyAuthenticationRequiredError |
350 | + d.addErrback(self._handle_proxy_authentication, method, reply, |
351 | + post_buffer) |
352 | + self.replies[reply] = d |
353 | + return d |
354 | + |
355 | @defer.inlineCallbacks |
356 | def request(self, iri, method="GET", extra_headers=None, |
357 | oauth_credentials=None, post_content=None): |
358 | @@ -86,17 +108,9 @@ |
359 | for key, value in headers.iteritems(): |
360 | request.setRawHeader(key, value) |
361 | |
362 | - d = defer.Deferred() |
363 | - if method == "GET": |
364 | - reply = self.nam.get(request) |
365 | - elif method == "HEAD": |
366 | - reply = self.nam.head(request) |
367 | - else: |
368 | - post_buffer = QBuffer() |
369 | - post_buffer.setData(post_content) |
370 | - reply = self.nam.sendCustomRequest(request, method, post_buffer) |
371 | - self.replies[reply] = d |
372 | - result = yield d |
373 | + post_buffer = QBuffer() |
374 | + post_buffer.setData(post_content) |
375 | + result = yield self._perform_request(request, method, post_buffer) |
376 | defer.returnValue(result) |
377 | |
378 | def _handle_authentication(self, reply, authenticator): |
379 | @@ -104,6 +118,54 @@ |
380 | authenticator.setUser(self.username) |
381 | authenticator.setPassword(self.password) |
382 | |
383 | + @defer.inlineCallbacks |
384 | + def _handle_bad_proxy_authentication(self, proxy, authenticator): |
385 | + """Handle when we have a bad proxy.""" |
386 | + current_proxy = self.nam.proxy() |
387 | + host = unicode(current_proxy.hostName()).encode('utf8') |
388 | + port = current_proxy.port() |
389 | + settings = dict(host=host, port=port) |
390 | + # HACK: work around for reported bugs: QTBUG-16728 and #QTBUG-14850 |
391 | + got_creds = yield self.request_proxy_auth_credentials(host, True) |
392 | + if got_creds: |
393 | + logger.debug('Got proxy credentials from user.') |
394 | + settings.update({'username': self.proxy_username, |
395 | + 'password': self.proxy_password}) |
396 | + if (settings and 'username' in settings |
397 | + and 'password' in settings): |
398 | + authenticator.setUser(self.proxy_username) |
399 | + authenticator.setPassword(self.proxy_password) |
400 | + self.force_use_proxy(settings) |
401 | + |
402 | + @defer.inlineCallbacks |
403 | + def _handle_proxy_authentication(self, failure, method, reply, |
404 | + post_buffer=None, retry=False): |
405 | + """The reply needs authentication.""" |
406 | + failure.trap(ProxyUnauthorizedError) |
407 | + current_proxy = self.nam.proxy() |
408 | + host = unicode(current_proxy.hostName()).encode('utf8') |
409 | + port = current_proxy.port() |
410 | + settings = dict(host=host, port=port) |
411 | + # we have to auth the request and retry it |
412 | + if not retry and self.proxy_username and self.proxy_password: |
413 | + settings.update({'username': self.proxy_username, |
414 | + 'password': self.proxy_password}) |
415 | + if retry or self.proxy_username is None or self.proxy_password is None: |
416 | + got_creds = yield self.request_proxy_auth_credentials(host, retry) |
417 | + if got_creds: |
418 | + logger.debug('Got proxy credentials from user.') |
419 | + settings.update({'username': self.proxy_username, |
420 | + 'password': self.proxy_password}) |
421 | + if (settings and 'username' in settings |
422 | + and 'password' in settings): |
423 | + self.force_use_proxy(settings) |
424 | + # lets retry using the request |
425 | + request = reply.request() |
426 | + result = yield self._perform_request(request, method, post_buffer) |
427 | + defer.returnValue(result) |
428 | + else: |
429 | + defer.returnValue(ProxyUnauthorizedError()) |
430 | + |
431 | def _handle_finished(self, reply): |
432 | """The reply has finished processing.""" |
433 | assert reply in self.replies |
434 | @@ -118,19 +180,30 @@ |
435 | d.callback(response) |
436 | else: |
437 | error_string = reply.errorString() |
438 | + logger.debug('_handle_finished error (%s,%s).', error, |
439 | + error_string) |
440 | if error == QNetworkReply.AuthenticationRequiredError: |
441 | exception = UnauthorizedError(error_string, content) |
442 | + elif error == QNetworkReply.ProxyAuthenticationRequiredError: |
443 | + # we are going thru a proxy and we did not auth |
444 | + exception = ProxyUnauthorizedError(error_string, content) |
445 | else: |
446 | exception = WebClientError(error_string, content) |
447 | d.errback(exception) |
448 | |
449 | def force_use_proxy(self, settings): |
450 | """Setup this webclient to use the given proxy settings.""" |
451 | - proxy = QNetworkProxy(QNetworkProxy.HttpProxy, |
452 | - hostName=settings.get("host", ""), |
453 | - port=settings.get("port", 0), |
454 | - user=settings.get("username", ""), |
455 | + host = settings.get("host", "") |
456 | + port = settings.get("port", 0) |
457 | + if "username" in settings and "password" in settings: |
458 | + logger.debug('Using auth proxy for %s:%s', host, port) |
459 | + proxy = QNetworkProxy(QNetworkProxy.HttpProxy, hostName=host, |
460 | + port=port, user=settings.get("username", ""), |
461 | password=settings.get("password", "")) |
462 | + else: |
463 | + logger.debug('Using nonauth proxy for %s:%s', host, port) |
464 | + proxy = QNetworkProxy(QNetworkProxy.HttpProxy, hostName=host, |
465 | + port=port) |
466 | self.nam.setProxy(proxy) |
467 | |
468 | def shutdown(self): |
469 | |
470 | === modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py' |
471 | --- ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-02-13 13:14:18 +0000 |
472 | +++ ubuntu_sso/utils/webclient/tests/test_webclient.py 2012-03-09 11:54:19 +0000 |
473 | @@ -27,6 +27,12 @@ |
474 | from ubuntuone.devtools.testcases import TestCase |
475 | from ubuntuone.devtools.testcases.squid import SquidTestCase |
476 | |
477 | +from ubuntu_sso import ( |
478 | + keyring, |
479 | + EXCEPTION_RAISED, |
480 | + USER_SUCCESS, |
481 | + USER_CANCELLATION, |
482 | +) |
483 | from ubuntu_sso.utils import webclient |
484 | from ubuntu_sso.utils.webclient.common import BaseWebClient, HeaderDict, oauth |
485 | from ubuntu_sso.utils.webclient.tests import BaseMockWebServer |
486 | @@ -404,10 +410,119 @@ |
487 | result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
488 | self.assert_header_contains(result.headers["Via"], "squid") |
489 | |
490 | + @defer.inlineCallbacks |
491 | + def test_auth_proxy_is_used_creds_requested(self): |
492 | + """The authenticated proxy is used by the webclient.""" |
493 | + settings = self.get_auth_proxy_settings() |
494 | + partial_settings = dict(host=settings['host'], port=settings['port']) |
495 | + |
496 | + def fake_creds_request(domain, retry): |
497 | + """Fake user interaction.""" |
498 | + self.wc.proxy_username = settings['username'] |
499 | + self.wc.proxy_password = settings['password'] |
500 | + return defer.succeed(True) |
501 | + |
502 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
503 | + fake_creds_request) |
504 | + |
505 | + self.wc.force_use_proxy(partial_settings) |
506 | + result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
507 | + self.assert_header_contains(result.headers["Via"], "squid") |
508 | + |
509 | + @defer.inlineCallbacks |
510 | + def test_auth_proxy_is_requested_creds_bad_details(self): |
511 | + """Test using wrong credentials with the proxy.""" |
512 | + settings = self.get_auth_proxy_settings() |
513 | + wrong_settings = dict(host=settings['host'], port=settings['port'], |
514 | + username=settings['password'], |
515 | + password=settings['username']) |
516 | + |
517 | + def fake_creds_request(domain, retry): |
518 | + """Fake user interaction.""" |
519 | + self.wc.proxy_username = settings['username'] |
520 | + self.wc.proxy_password = settings['password'] |
521 | + return defer.succeed(True) |
522 | + |
523 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
524 | + fake_creds_request) |
525 | + |
526 | + self.wc.force_use_proxy(wrong_settings) |
527 | + result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
528 | + self.assert_header_contains(result.headers["Via"], "squid") |
529 | + |
530 | + @defer.inlineCallbacks |
531 | + def test_auth_proxy_is_requested_creds_bad_details_user(self): |
532 | + """Test using no creds and user providing the wrong ones.""" |
533 | + settings = self.get_auth_proxy_settings() |
534 | + partial_settings = dict(host=settings['host'], port=settings['port']) |
535 | + |
536 | + def fake_creds_request(domain, retry): |
537 | + """Fake user interaction.""" |
538 | + if retry: |
539 | + self.wc.proxy_username = settings['username'] |
540 | + self.wc.proxy_password = settings['password'] |
541 | + else: |
542 | + self.wc.proxy_username = settings['password'] |
543 | + self.wc.proxy_password = settings['username'] |
544 | + return defer.succeed(True) |
545 | + |
546 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
547 | + fake_creds_request) |
548 | + |
549 | + self.wc.force_use_proxy(partial_settings) |
550 | + result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
551 | + self.assert_header_contains(result.headers["Via"], "squid") |
552 | + |
553 | + @defer.inlineCallbacks |
554 | + def test_auth_proxy_is_requested_creds_bad_details_everywhere(self): |
555 | + """Test when we pass the wrong settings and get the wrong settings.""" |
556 | + settings = self.get_auth_proxy_settings() |
557 | + wrong_settings = dict(host=settings['host'], port=settings['port'], |
558 | + username=settings['password'], |
559 | + password=settings['username']) |
560 | + |
561 | + def fake_creds_request(domain, retry): |
562 | + """Fake user interaction.""" |
563 | + if retry: |
564 | + self.wc.proxy_username = settings['username'] |
565 | + self.wc.proxy_password = settings['password'] |
566 | + else: |
567 | + self.wc.proxy_username = settings['password'] |
568 | + self.wc.proxy_password = settings['username'] |
569 | + return defer.succeed(True) |
570 | + |
571 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
572 | + fake_creds_request) |
573 | + |
574 | + self.wc.force_use_proxy(wrong_settings) |
575 | + result = yield self.wc.request(self.base_iri + SIMPLERESOURCE) |
576 | + self.assert_header_contains(result.headers["Via"], "squid") |
577 | + |
578 | + def test_auth_proxy_is_requested_user_cancels(self): |
579 | + """Test when the user cancels the creds dialog.""" |
580 | + settings = self.get_auth_proxy_settings() |
581 | + partial_settings = dict(host=settings['host'], port=settings['port']) |
582 | + |
583 | + def fake_creds_request(domain, retry): |
584 | + """Fake user interaction.""" |
585 | + return defer.succeed(False) |
586 | + |
587 | + self.patch(self.wc, 'request_proxy_auth_credentials', |
588 | + fake_creds_request) |
589 | + |
590 | + self.wc.force_use_proxy(partial_settings) |
591 | + self.failUnlessFailure(self.wc.request(self.base_iri + SIMPLERESOURCE), |
592 | + webclient.WebClientError) |
593 | + |
594 | if WEBCLIENT_MODULE_NAME.endswith(".txweb"): |
595 | reason = "txweb does not support proxies." |
596 | test_anonymous_proxy_is_used.skip = reason |
597 | test_authenticated_proxy_is_used.skip = reason |
598 | + test_auth_proxy_is_used_creds_requested.skip = reason |
599 | + test_auth_proxy_is_requested_creds_bad_details.skip = reason |
600 | + test_auth_proxy_is_requested_creds_bad_details_user.skip = reason |
601 | + test_auth_proxy_is_requested_creds_bad_details_everywhere.skip = reason |
602 | + test_auth_proxy_is_requested_user_cancels.skip = reason |
603 | |
604 | |
605 | class HeaderDictTestCase(TestCase): |
606 | @@ -562,3 +677,93 @@ |
607 | """Test for the oauth signing code using HMAC-SHA1.""" |
608 | |
609 | oauth_sign = "HMAC-SHA1" |
610 | + |
611 | + |
612 | +class FakeKeyring(object): |
613 | + """A fake keyring.""" |
614 | + |
615 | + def __init__(self, creds): |
616 | + """A fake keyring.""" |
617 | + self.creds = creds |
618 | + |
619 | + def __call__(self): |
620 | + """Fake instance callable.""" |
621 | + return self |
622 | + |
623 | + def get_credentials(self, domain): |
624 | + """A fake get_credentials.""" |
625 | + if isinstance(self.creds, Exception): |
626 | + return defer.fail(self.creds) |
627 | + return defer.succeed(self.creds) |
628 | + |
629 | + |
630 | +class RequestProxyAuthTestCase(TestCase): |
631 | + """Test the spawn of the creds dialog.""" |
632 | + |
633 | + @defer.inlineCallbacks |
634 | + def setUp(self): |
635 | + """Set the different tests.""" |
636 | + yield super(RequestProxyAuthTestCase, self).setUp() |
637 | + self.wc = webclient.webclient_factory() |
638 | + self.addCleanup(self.wc.shutdown) |
639 | + self.domain = 'domain' |
640 | + self.retry = False |
641 | + self.creds = dict(username='username', password='password') |
642 | + |
643 | + self.keyring = FakeKeyring(self.creds) |
644 | + self.patch(keyring, 'Keyring', self.keyring) |
645 | + |
646 | + self.spawn_return_code = USER_SUCCESS |
647 | + |
648 | + def fake_spawn_process(args): |
649 | + """Fake spawning a process.""" |
650 | + if isinstance(self.spawn_return_code, Exception): |
651 | + return defer.fail(self.spawn_return_code) |
652 | + return defer.succeed(self.spawn_return_code) |
653 | + |
654 | + self.patch(webclient.common, 'spawn_program', fake_spawn_process) |
655 | + |
656 | + def test_spawn_error(self): |
657 | + """Test the case when we cannot spawn the process.""" |
658 | + self.spawn_return_code = Exception() |
659 | + self.failUnlessFailure(self.wc.request_proxy_auth_credentials( |
660 | + self.domain, True), |
661 | + webclient.WebClientError) |
662 | + |
663 | + @defer.inlineCallbacks |
664 | + def test_creds_acquired(self): |
665 | + """Test the case in which we do get the creds.""" |
666 | + got_creds = yield self.wc.request_proxy_auth_credentials(self.domain, |
667 | + self.retry) |
668 | + self.assertTrue(got_creds, 'Return true when creds are present.') |
669 | + self.assertEqual(self.wc.proxy_username, self.creds['username']) |
670 | + self.assertEqual(self.wc.proxy_password, self.creds['password']) |
671 | + |
672 | + def test_creds_acquired_keyring_error(self): |
673 | + """Test the case in which we cannot access the keyring.""" |
674 | + self.keyring.creds = Exception() |
675 | + self.failUnlessFailure(self.wc.request_proxy_auth_credentials( |
676 | + self.domain, self.retry), |
677 | + webclient.WebClientError) |
678 | + |
679 | + @defer.inlineCallbacks |
680 | + def test_creds_none(self): |
681 | + """Test the case in which we got None from the keyring.""" |
682 | + self.keyring.creds = None |
683 | + got_creds = yield self.wc.request_proxy_auth_credentials(self.domain, |
684 | + self.retry) |
685 | + self.assertFalse(got_creds, 'Return false when creds are not present.') |
686 | + |
687 | + def test_user_cancelation(self): |
688 | + """Test the case in which the user cancels.""" |
689 | + self.spawn_return_code = USER_CANCELLATION |
690 | + got_creds = yield self.wc.request_proxy_auth_credentials(self.domain, |
691 | + self.retry) |
692 | + self.assertFalse(got_creds, 'Return true when user cancels.') |
693 | + |
694 | + def test_exception_error(self): |
695 | + """Test the case in which something bad happened.""" |
696 | + self.spawn_return_code = EXCEPTION_RAISED |
697 | + got_creds = yield self.wc.request_proxy_auth_credentials(self.domain, |
698 | + self.retry) |
699 | + self.assertFalse(got_creds, 'Return true when user cancels.') |
140 + if (self.proxy_ username is not None
141 + and self.proxy_password is not None):
"""if all((self. proxy_username, self.self. proxy_password) ):""" would be a less verbose way of doing the same thing. I think it's clearer, but it's no more correct than what you have so it's merely a comment and not a requirement to fix.
424 + if "username" in settings and "password" in settings:
Just so you know I'm not crazy with the any/all builtins, I don't suggest """if all(key in settings for key in ("user", "pass"))""" :)
(comment for now - will approve/reject after I can run the tests)