Merge pull request #1951 from Lukasa/proxyauth

Re-evaluate proxy authorization.

3 files changed, 102 insertions, 24 deletions

diff --git a/requests/sessions.py b/requests/sessions.py
index 425db22c..79ea7773 100644
--- a/requests/sessions.py
+++ b/requests/sessions.py
@@ -12,6 +12,7 @@ import os
 from collections import Mapping
 from datetime import datetime
 
+from .auth import _basic_auth_str
 from .compat import cookielib, OrderedDict, urljoin, urlparse, builtin_str
 from .cookies import (
     cookiejar_from_dict, extract_cookies_to_jar, RequestsCookieJar, merge_cookies)
@@ -23,7 +24,10 @@ from .structures import CaseInsensitiveDict
 
 from .adapters import HTTPAdapter
 
-from .utils import requote_uri, get_environ_proxies, get_netrc_auth
+from .utils import (
+    requote_uri, get_environ_proxies, get_netrc_auth, should_bypass_proxies,
+    get_auth_from_url
+)
 
 from .status_codes import codes
 
@@ -154,19 +158,9 @@ class SessionRedirectMixin(object):
             prepared_request._cookies.update(self.cookies)
             prepared_request.prepare_cookies(prepared_request._cookies)
 
-            if 'Authorization' in headers:
-                # If we get redirected to a new host, we should strip out any
-                # authentication headers.
-                original_parsed = urlparse(resp.request.url)
-                redirect_parsed = urlparse(url)
-
-                if (original_parsed.hostname != redirect_parsed.hostname):
-                    del headers['Authorization']
-
-            # .netrc might have more auth for us.
-            new_auth = get_netrc_auth(url) if self.trust_env else None
-            if new_auth is not None:
-                prepared_request.prepare_auth(new_auth)
+            # Rebuild auth and proxy information.
+            proxies = self.rebuild_proxies(prepared_request, proxies)
+            self.rebuild_auth(prepared_request, resp)
 
             resp = self.send(
                 prepared_request,
@@ -183,6 +177,68 @@ class SessionRedirectMixin(object):
             i += 1
             yield resp
 
+    def rebuild_auth(self, prepared_request, response):
+        """
+        When being redirected we may want to strip authentication from the
+        request to avoid leaking credentials. This method intelligently removes
+        and reapplies authentication where possible to avoid credential loss.
+        """
+        headers = prepared_request.headers
+        url = prepared_request.url
+
+        if 'Authorization' in headers:
+            # If we get redirected to a new host, we should strip out any
+            # authentication headers.
+            original_parsed = urlparse(response.request.url)
+            redirect_parsed = urlparse(url)
+
+            if (original_parsed.hostname != redirect_parsed.hostname):
+                del headers['Authorization']
+
+        # .netrc might have more auth for us on our new host.
+        new_auth = get_netrc_auth(url) if self.trust_env else None
+        if new_auth is not None:
+            prepared_request.prepare_auth(new_auth)
+
+        return
+
+    def rebuild_proxies(self, prepared_request, proxies):
+        """
+        This method re-evaluates the proxy configuration by considering the
+        environment variables. If we are redirected to a URL covered by
+        NO_PROXY, we strip the proxy configuration. Otherwise, we set missing
+        proxy keys for this URL (in case they were stripped by a previous
+        redirect).
+
+        This method also replaces the Proxy-Authorization header where
+        necessary.
+        """
+        headers = prepared_request.headers
+        url = prepared_request.url
+        new_proxies = {}
+
+        if not should_bypass_proxies(url):
+            environ_proxies = get_environ_proxies(url)
+            scheme = urlparse(url).scheme
+
+            proxy = environ_proxies.get(scheme)
+
+            if proxy:
+                new_proxies.setdefault(scheme, environ_proxies[scheme])
+
+        if 'Proxy-Authorization' in headers:
+            del headers['Proxy-Authorization']
+
+        try:
+            username, password = get_auth_from_url(new_proxies[scheme])
+        except KeyError:
+            username, password = None, None
+
+        if username and password:
+            headers['Proxy-Authorization'] = _basic_auth_str(username, password)
+
+        return new_proxies
+
 
 class Session(SessionRedirectMixin):
     """A Requests session.
diff --git a/requests/utils.py b/requests/utils.py
index 4d648bc5..6f4eb500 100644
--- a/requests/utils.py
+++ b/requests/utils.py
@@ -466,9 +466,10 @@ def is_valid_cidr(string_network):
     return True
 
 
-def get_environ_proxies(url):
-    """Return a dict of environment proxies."""
-
+def should_bypass_proxies(url):
+    """
+    Returns whether we should bypass proxies or not.
+    """
     get_proxy = lambda k: os.environ.get(k) or os.environ.get(k.upper())
 
     # First check whether no_proxy is defined. If it is, check that the URL
@@ -486,13 +487,13 @@ def get_environ_proxies(url):
             for proxy_ip in no_proxy:
                 if is_valid_cidr(proxy_ip):
                     if address_in_network(ip, proxy_ip):
-                        return {}
+                        return True
         else:
             for host in no_proxy:
                 if netloc.endswith(host) or netloc.split(':')[0].endswith(host):
                     # The URL does match something in no_proxy, so we don't want
                     # to apply the proxies on this URL.
-                    return {}
+                    return True
 
     # If the system proxy settings indicate that this URL should be bypassed,
     # don't proxy.
@@ -506,12 +507,16 @@ def get_environ_proxies(url):
         bypass = False
 
     if bypass:
-        return {}
+        return True
 
-    # If we get here, we either didn't have no_proxy set or we're not going
-    # anywhere that no_proxy applies to, and the system settings don't require
-    # bypassing the proxy for the current URL.
-    return getproxies()
+    return False
+
+def get_environ_proxies(url):
+    """Return a dict of environment proxies."""
+    if should_bypass_proxies(url):
+        return {}
+    else:
+        return getproxies()
 
 
 def default_user_agent(name="python-requests"):
diff --git a/test_requests.py b/test_requests.py
index 17de8491..1bebb1ad 100755
--- a/test_requests.py
+++ b/test_requests.py
@@ -17,6 +17,7 @@ from requests.compat import (
     Morsel, cookielib, getproxies, str, urljoin, urlparse)
 from requests.cookies import cookiejar_from_dict, morsel_to_cookie
 from requests.exceptions import InvalidURL, MissingSchema
+from requests.models import PreparedRequest, Response
 from requests.structures import CaseInsensitiveDict
 
 try:
@@ -865,6 +866,22 @@ class RequestsTestCase(unittest.TestCase):
             preq = req.prepare()
             assert test_url == preq.url
 
+    def test_auth_is_stripped_on_redirect_off_host(self):
+        r = requests.get(
+            httpbin('redirect-to'),
+            params={'url': 'http://www.google.co.uk'},
+            auth=('user', 'pass'),
+        )
+        assert r.history[0].request.headers['Authorization']
+        assert not r.request.headers.get('Authorization', '')
+
+    def test_auth_is_retained_for_redirect_on_host(self):
+        r = requests.get(httpbin('redirect/1'), auth=('user', 'pass'))
+        h1 = r.history[0].request.headers['Authorization']
+        h2 = r.request.headers['Authorization']
+
+        assert h1 == h2
+
 
 class TestContentEncodingDetection(unittest.TestCase):