Browse files

Merge pull request #4718 from bmerry/fix-4716

Strip Authorization header whenever root URL changes
  • Loading branch information...
nateprewitt committed Sep 14, 2018
2 parents dd754d1 + 857e9b7 commit c45d7c49ea75133e52ab22a8e9e13173938e36ff
Showing with 44 additions and 11 deletions.
  1. +18 −6 requests/
  2. +26 −5 tests/
@@ -115,6 +115,22 @@ def get_redirect_target(self, resp):
return to_native_string(location, 'utf8')
return None

def should_strip_auth(self, old_url, new_url):
"""Decide whether Authorization header should be removed when redirecting"""
old_parsed = urlparse(old_url)
new_parsed = urlparse(new_url)
if old_parsed.hostname != new_parsed.hostname:
return True
# Special case: allow http -> https redirect when using the standard
# ports. This isn't specified by RFC 7235, but is kept to avoid
# breaking backwards compatibility with older versions of requests
# that allowed any redirects on the same host.
if (old_parsed.scheme == 'http' and old_parsed.port in (80, None)
and new_parsed.scheme == 'https' and new_parsed.port in (443, None)):
return False
# Standard case: root URI must match
return old_parsed.port != new_parsed.port or old_parsed.scheme != new_parsed.scheme

def resolve_redirects(self, resp, req, stream=False, timeout=None,
verify=True, cert=None, proxies=None, yield_requests=False, **adapter_kwargs):
"""Receives a Response. Returns a generator of Responses or Requests."""
@@ -236,14 +252,10 @@ def rebuild_auth(self, prepared_request, response):
headers = prepared_request.headers
url = prepared_request.url

if 'Authorization' in headers:
if 'Authorization' in headers and self.should_strip_auth(response.request.url, url):
# 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']
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
@@ -1573,15 +1573,15 @@ def test_nonhttp_schemes_dont_check_URLs(self):
preq = req.prepare()
assert test_url == preq.url

def test_auth_is_stripped_on_redirect_off_host(self, httpbin):
def test_auth_is_stripped_on_http_downgrade(self, httpbin, httpbin_secure, httpbin_ca_bundle):
r = requests.get(
params={'url': ''},
params={'url': httpbin('get')},
auth=('user', 'pass'),
assert r.history[0].request.headers['Authorization']
assert not r.request.headers.get('Authorization', '')
assert 'Authorization' not in r.request.headers

def test_auth_is_retained_for_redirect_on_host(self, httpbin):
r = requests.get(httpbin('redirect/1'), auth=('user', 'pass'))
@@ -1590,6 +1590,27 @@ def test_auth_is_retained_for_redirect_on_host(self, httpbin):

assert h1 == h2

def test_should_strip_auth_host_change(self):
s = requests.Session()
assert s.should_strip_auth('', '')

def test_should_strip_auth_http_downgrade(self):
s = requests.Session()
assert s.should_strip_auth('', '')

def test_should_strip_auth_https_upgrade(self):
s = requests.Session()
assert not s.should_strip_auth('', '')
assert not s.should_strip_auth('', '')
assert not s.should_strip_auth('', '')
# Non-standard ports should trigger stripping
assert s.should_strip_auth('', '')
assert s.should_strip_auth('', '')

def test_should_strip_auth_port_change(self):
s = requests.Session()
assert s.should_strip_auth('', '')

def test_manual_redirect_with_partial_body_read(self, httpbin):
s = requests.Session()
r1 = s.get(httpbin('redirect/2'), allow_redirects=False, stream=True)

0 comments on commit c45d7c4

Please sign in to comment.