Auth on redirect (#2328)

* Fix #2243: Refactor processing of URL credentials

* Fix URLs with user but without password

* Cleanup ClientRequest.update_host

* Work on

* Fix #1699: Clear auth information on redirecting to other domain

aiohttp/client.py

@@ -24,7 +24,8 @@
 from .connector import TCPConnector
 from .cookiejar import CookieJar
 from .helpers import (PY_35, CeilTimeout, TimeoutHandle, _BaseCoroMixin,
-                      deprecated_noop, proxies_from_env, sentinel)
+                      deprecated_noop, proxies_from_env, sentinel,
+                      strip_auth_from_url)
 from .http import WS_KEY, WebSocketReader, WebSocketWriter
 from .http_websocket import WSHandshakeError, ws_ext_gen, ws_ext_parse
 from .streams import FlowControlDataQueue
@@ -193,15 +194,10 @@ def _request(self, method, url, *,
         headers = self._prepare_headers(headers)
         proxy_headers = self._prepare_headers(proxy_headers)
 
-        if auth is None:
-            auth = self._default_auth
-        # It would be confusing if we support explicit Authorization header
-        # with `auth` argument
-        if (headers is not None and
-                auth is not None and
-                hdrs.AUTHORIZATION in headers):
-            raise ValueError("Can't combine `Authorization` header with "
-                             "`auth` argument")
+        try:
+            url = URL(url)
+        except ValueError:
+            raise InvalidURL(url)
 
         skip_headers = set(self._skip_auto_headers)
         if skip_auto_headers is not None:
@@ -214,11 +210,6 @@ def _request(self, method, url, *,
             except ValueError:
                 raise InvalidURL(url)
 
-        try:
-            url = URL(url)
-        except ValueError:
-            raise InvalidURL(url)
-
         # timeout is cumulative for all request operations
         # (request, redirects, responses, data consuming)
         tm = TimeoutHandle(
@@ -231,6 +222,24 @@ def _request(self, method, url, *,
         try:
             with timer:
                 while True:
+                    url, auth_from_url = strip_auth_from_url(url)
+                    if auth and auth_from_url:
+                        raise ValueError("Cannot combine AUTH argument with "
+                                         "credentials encoded in URL")
+
+                    if auth is None:
+                        auth = auth_from_url
+                    if auth is None:
+                        auth = self._default_auth
+                    # It would be confusing if we support explicit
+                    # Authorization header with auth argument
+                    if (headers is not None and
+                            auth is not None and
+                            hdrs.AUTHORIZATION in headers):
+                        raise ValueError("Cannot combine AUTHORIZATION header "
+                                         "with AUTH argument or credentials "
+                                         "encoded in URL")
+
                     url = url.with_fragment(None)
                     cookies = self._cookie_jar.filter_cookies(url)
 
@@ -322,6 +331,10 @@ def _request(self, method, url, *,
                         elif not scheme:
                             r_url = url.join(r_url)
 
+                        if url.origin() != r_url.origin():
+                            auth = None
+                            headers.pop(hdrs.AUTHORIZATION, None)
+
                         url = r_url
                         params = None
                         resp.release()

aiohttp/helpers.py

@@ -213,15 +213,15 @@ def from_url(cls, url, *, encoding='latin1'):
             raise TypeError("url should be yarl.URL instance")
         if url.user is None:
             return None
-        return cls(url.user, url.password, encoding=encoding)
+        return cls(url.user, url.password or '', encoding=encoding)
 
     def encode(self):
         """Encode credentials."""
         creds = ('%s:%s' % (self.login, self.password)).encode(self.encoding)
         return 'Basic %s' % base64.b64encode(creds).decode(self.encoding)
 
 
-def _strip_auth_from_url(url):
+def strip_auth_from_url(url):
     auth = BasicAuth.from_url(url)
     if auth is None:
         return url, None
@@ -235,7 +235,7 @@ def _strip_auth_from_url(url):
 def proxies_from_env():
     proxy_urls = {k: URL(v) for k, v in getproxies().items()
                   if k in ('http', 'https')}
-    stripped = {k: _strip_auth_from_url(v) for k, v in proxy_urls.items()}
+    stripped = {k: strip_auth_from_url(v) for k, v in proxy_urls.items()}
     ret = {}
     for proto, val in stripped.items():
         proxy, auth = val

changes/1699.bugfix

@@ -0,0 +1 @@
+Clear auth information on redirecting to other domain

changes/2243.misc

@@ -0,0 +1 @@
+Refactor processing of URL credentials.

tests/test_client_functional.py

@@ -5,6 +5,7 @@
 import io
 import json
 import pathlib
+import socket
 import ssl
 from unittest import mock
 
@@ -13,6 +14,7 @@
 
 import aiohttp
 from aiohttp import ServerFingerprintMismatch, hdrs, web
+from aiohttp.abc import AbstractResolver
 from aiohttp.helpers import create_future
 from aiohttp.multipart import MultipartWriter
 
@@ -2226,3 +2228,63 @@ def test_invalid_idna(loop):
             yield from session.get('http://\u2061owhefopw.com')
     finally:
         yield from session.close()
+
+
+@asyncio.coroutine
+def test_creds_in_auth_and_url(loop):
+    session = aiohttp.ClientSession(loop=loop)
+    try:
+        with pytest.raises(ValueError):
+            yield from session.get('http://user:[email protected]',
+                                   auth=aiohttp.BasicAuth('user2', 'pass2'))
+    finally:
+        yield from session.close()
+
+
+@asyncio.coroutine
+def test_drop_auth_on_redirect_to_other_host(test_server, loop):
+    @asyncio.coroutine
+    def srv1(request):
+        assert request.host == 'host1.com'
+        assert request.headers['Authorization'] == 'Basic dXNlcjpwYXNz'
+        raise web.HTTPFound('http://host2.com/path2')
+
+    @asyncio.coroutine
+    def srv2(request):
+        assert request.host == 'host2.com'
+        assert 'Authorization' not in request.headers
+        return web.Response()
+
+    app = web.Application()
+    app.router.add_route('GET', '/path1', srv1)
+    app.router.add_route('GET', '/path2', srv2)
+
+    server = yield from test_server(app)
+
+    class FakeResolver(AbstractResolver):
+
+        @asyncio.coroutine
+        def resolve(self, host, port=0, family=socket.AF_INET):
+            return [{'hostname': host,
+                     'host': server.host,
+                     'port': server.port,
+                     'family': socket.AF_INET,
+                     'proto': 0,
+                     'flags': socket.AI_NUMERICHOST}]
+
+        @asyncio.coroutine
+        def close(self):
+            pass
+
+    connector = aiohttp.TCPConnector(loop=loop, resolver=FakeResolver())
+    client = aiohttp.ClientSession(connector=connector)
+    try:
+        resp = yield from client.get('http://host1.com/path1',
+                                     auth=aiohttp.BasicAuth('user', 'pass'))
+        assert resp.status == 200
+        resp = yield from client.get('http://host1.com/path1',
+                                     headers={'Authorization':
+                                              'Basic dXNlcjpwYXNz'})
+        assert resp.status == 200
+    finally:
+        yield from client.close()