Repository: planetlabs/planet-client-python (python)

Issue:
download scene has stopped working
Appears the client is not passing authorization headers when following a redirect.

Pull request:
fix passing of auth during redirects
- requests strips Authorization header from redirects
  when the host differs
- new S3 redirect needs api_key as parameter but does not accept
  via the Authorization headers

closes #43 

Patch:
diff --git a/planet/api/dispatch.py b/planet/api/dispatch.py
index 54a46e5..27e2b09 100644
--- a/planet/api/dispatch.py
+++ b/planet/api/dispatch.py
@@ -12,16 +12,47 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import re
 from requests_futures.sessions import FuturesSession
 from . utils import check_status
 from . models import Response
 from . exceptions import InvalidAPIKey
+from requests.compat import urlparse
+
+
+def _is_subdomain_of_tld(url1, url2):
+    orig_host = urlparse(url1).hostname
+    re_host = urlparse(url2).hostname
+    return orig_host.split('.')[-2:] == re_host.split('.')[-2:]
+
+
+class RedirectSession(FuturesSession):
+    '''This exists to override the existing behavior of requests that will
+    strip Authorization headers from any redirect requests that resolve to a
+    new domain. Instead, we'll keep headers if the redirect is a subdomain
+    and if not, extract the api-key from the header and add it to the url
+    as a parameter.
+    '''
+    def rebuild_auth(self, prepared_request, response):
+        existing_auth = prepared_request.headers.get('Authorization', None)
+        if existing_auth:
+            orig = response.request.url
+            redir = prepared_request.url
+            if not _is_subdomain_of_tld(orig, redir):
+                prepared_request.headers.pop('Authorization')
+            key = re.match('api-key (\S+)', existing_auth)
+            if key:
+                prepared_request.prepare_url(
+                    prepared_request.url, {
+                        'api_key': key.group(1)
+                    }
+                )
 
 
 class RequestsDispatcher(object):
 
     def __init__(self, workers=4):
-        self.session = FuturesSession(max_workers=workers)
+        self.session = RedirectSession(max_workers=workers)
 
     def response(self, request):
         return Response(request, self)
@@ -30,11 +61,16 @@ class RequestsDispatcher(object):
         auth = request.auth
         if not auth:
             raise InvalidAPIKey('No API key provided')
-        self.session.headers.update({
-            'Authorization': 'api-key %s' % auth.value
-        })
+
+        def _auth_callback(req):
+            req.headers.update({
+                'Authorization': 'api-key %s' % auth.value
+            })
+            return req
+
         return self.session.get(request.url, params=request.params,
-                                stream=True, background_callback=callback)
+                                stream=True, background_callback=callback,
+                                auth=_auth_callback)
 
     def _dispatch(self, request, callback=None):
         response = self._dispatch_async(request, callback).result()


Test patch:
diff --git a/tests/test_dispatch.py b/tests/test_dispatch.py
new file mode 100644
index 0000000..f6bc076
--- /dev/null
+++ b/tests/test_dispatch.py
@@ -0,0 +1,58 @@
+# Copyright 2015 Planet Labs, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from planet.api.dispatch import RedirectSession
+from planet.api.dispatch import _is_subdomain_of_tld
+import requests_mock
+
+
+def test_redirectsession_rebuilt_auth_called():
+    '''verify our hacking around with Session behavior works'''
+    session = RedirectSession()
+    with requests_mock.Mocker() as m:
+        m.get('http://redirect.com', status_code=302, headers={
+            'Location': 'http://newredirect.com'
+        })
+        m.get('http://newredirect.com', text='redirected!')
+
+        # base assertion, works as intended
+        resp = session.get('http://redirect.com').result()
+        assert resp.url == 'http://newredirect.com'
+        assert resp.text == 'redirected!'
+
+        # Authorization headers unpacked and URL is rewritten
+        resp = session.get('http://redirect.com', headers={
+            'Authorization': 'api-key foobar'
+        }).result()
+        assert resp.url == 'http://newredirect.com/?api_key=foobar'
+        assert resp.text == 'redirected!'
+
+        # Authorization headers unpacked and URL is rewritten, params saved
+        m.get('http://redirect.com', status_code=302, headers={
+            'Location': 'http://newredirect.com?param=yep'
+        })
+        m.get('http://newredirect.com?param=yep', text='param!')
+        resp = session.get('http://redirect.com?param=yep', headers={
+            'Authorization': 'api-key foobar'
+        }).result()
+        assert resp.url == 'http://newredirect.com/?param=yep&api_key=foobar'
+        assert resp.text == 'param!'
+
+
+def test_is_subdomain_of_tld():
+    assert _is_subdomain_of_tld('http://foo.bar', 'http://foo.bar')
+    assert _is_subdomain_of_tld('http://one.foo.bar', 'http://foo.bar')
+    assert _is_subdomain_of_tld('http://foo.bar', 'http://one.foo.bar')
+    assert not _is_subdomain_of_tld('http://foo.bar', 'http://bar.foo')
+    assert not _is_subdomain_of_tld('http://one.foo.bar', 'http://bar.foo')
+    assert not _is_subdomain_of_tld('http://foo.bar', 'http://one.bar.foo')