[PR #9443/06b2398 backport][3.11] Fix handling of redirects with authentication (#9571)

Co-authored-by: Pierre-Louis Peeters <[email protected]>
Co-authored-by: Pierre-Louis Peeters <[email protected]>

Author: 3 people

CHANGES/9436.bugfix.rst

@@ -0,0 +1 @@
+Authentication provided by a redirect now takes precedence over provided ``auth`` when making requests with the client -- by :user:`PLPeeters`.

aiohttp/client.py

@@ -523,7 +523,7 @@ async def _request(
             warnings.warn("Chunk size is deprecated #1615", DeprecationWarning)
 
         redirects = 0
-        history = []
+        history: List[ClientResponse] = []
         version = self._version
         params = params or {}
 
@@ -614,13 +614,18 @@ async def _request(
                             else InvalidUrlClientError
                         )
                         raise err_exc_cls(url)
-                    if auth and auth_from_url:
+                    # If `auth` was passed for an already authenticated URL,
+                    # disallow only if this is the initial URL; this is to avoid issues
+                    # with sketchy redirects that are not the caller's responsibility
+                    if not history and (auth and auth_from_url):
                         raise ValueError(
                             "Cannot combine AUTH argument with "
                             "credentials encoded in URL"
                         )
 
-                    if auth is None:
+                    # Override the auth with the one from the URL only if we
+                    # have no auth, or if we got an auth from a redirect URL
+                    if auth is None or (history and auth_from_url is not None):
                         auth = auth_from_url
 
                     if (

docs/client_advanced.rst

@@ -56,6 +56,44 @@ For *text/plain* ::
 
     await session.post(url, data='Привет, Мир!')
 
+Authentication
+--------------
+
+Instead of setting the ``Authorization`` header directly,
+:class:`ClientSession` and individual request methods provide an ``auth``
+argument. An instance of :class:`BasicAuth` can be passed in like this::
+
+    auth = BasicAuth(login="...", password="...")
+    async with ClientSession(auth=auth) as session:
+        ...
+
+Note that if the request is redirected and the redirect URL contains
+credentials, those credentials will supersede any previously set credentials.
+In other words, if ``http://[email protected]`` redirects to
+``http://[email protected]``, the second request will be authenticated
+as ``other_user``. Providing both the ``auth`` parameter and authentication in
+the *initial* URL will result in a :exc:`ValueError`.
+
+For other authentication flows, the ``Authorization`` header can be set
+directly::
+
+    headers = {"Authorization": "Bearer eyJh...0M30"}
+    async with ClientSession(headers=headers) as session:
+        ...
+
+The authentication header for a session may be updated as and when required.
+For example::
+
+    session.headers["Authorization"] = "Bearer eyJh...1OH0"
+
+Note that a *copy* of the headers dictionary is set as an attribute when
+creating a :class:`ClientSession` instance (as a :class:`multidict.CIMultiDict`
+object). Updating the original dictionary does not have any effect.
+
+In cases where the authentication header value expires periodically, an
+:mod:`asyncio` task may be used to update the session's default headers in the
+background.
+
 .. note::
    ``Authorization`` header will be removed if you get redirected
    to a different host or protocol.

tests/test_client_functional.py

@@ -12,7 +12,7 @@
 import tarfile
 import time
 import zipfile
-from typing import Any, AsyncIterator, Optional, Type
+from typing import Any, AsyncIterator, Awaitable, Callable, List, Optional, Type
 from unittest import mock
 
 import pytest
@@ -21,7 +21,7 @@
 
 import aiohttp
 from aiohttp import Fingerprint, ServerFingerprintMismatch, hdrs, web
-from aiohttp.abc import AbstractResolver
+from aiohttp.abc import AbstractResolver, ResolveResult
 from aiohttp.client_exceptions import (
     ClientResponseError,
     InvalidURL,
@@ -35,8 +35,9 @@
 from aiohttp.client_reqrep import ClientRequest
 from aiohttp.connector import Connection
 from aiohttp.http_writer import StreamWriter
-from aiohttp.pytest_plugin import AiohttpClient, AiohttpServer, TestClient
-from aiohttp.test_utils import unused_port
+from aiohttp.pytest_plugin import AiohttpClient, AiohttpServer
+from aiohttp.test_utils import TestClient, TestServer, unused_port
+from aiohttp.typedefs import Handler
 
 
 @pytest.fixture
@@ -2888,6 +2889,68 @@ async def test_creds_in_auth_and_url() -> None:
         await session.close()
 
 
+async def test_creds_in_auth_and_redirect_url(
+    create_server_for_url_and_handler: Callable[[URL, Handler], Awaitable[TestServer]],
+) -> None:
+    """Verify that credentials in redirect URLs can and do override any previous credentials."""
+    url_from = URL("http://example.com")
+    url_to = URL("http://[email protected]")
+    redirected = False
+
+    async def srv(request: web.Request) -> web.Response:
+        nonlocal redirected
+
+        assert request.host == url_from.host
+
+        if not redirected:
+            redirected = True
+            raise web.HTTPMovedPermanently(url_to)
+
+        return web.Response()
+
+    server = await create_server_for_url_and_handler(url_from, srv)
+
+    etc_hosts = {
+        (url_from.host, 80): server,
+    }
+
+    class FakeResolver(AbstractResolver):
+        async def resolve(
+            self,
+            host: str,
+            port: int = 0,
+            family: socket.AddressFamily = socket.AF_INET,
+        ) -> List[ResolveResult]:
+            server = etc_hosts[(host, port)]
+            assert server.port is not None
+
+            return [
+                {
+                    "hostname": host,
+                    "host": server.host,
+                    "port": server.port,
+                    "family": socket.AF_INET,
+                    "proto": 0,
+                    "flags": socket.AI_NUMERICHOST,
+                }
+            ]
+
+        async def close(self) -> None:
+            """Dummy"""
+
+    connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False)
+
+    async with aiohttp.ClientSession(connector=connector) as client, client.get(
+        url_from, auth=aiohttp.BasicAuth("user", "pass")
+    ) as resp:
+        assert len(resp.history) == 1
+        assert str(resp.url) == "http://example.com"
+        assert resp.status == 200
+        assert (
+            resp.request_info.headers.get("authorization") == "Basic dXNlcjo="
+        ), "Expected redirect credentials to take precedence over provided auth"
+
+
 @pytest.fixture
 def create_server_for_url_and_handler(aiohttp_server, tls_certificate_authority):
     def create(url, srv):