Merge pull request sigmavirus24#800 from electrofelix/token-auth-disables-netrc

sigmavirus24 · web-flow · commit aa50e04ebf02 · 2018-03-21T19:10:37.000-05:00
Provide TokenAuth class to requests to ensure explicit usage
diff --git a/github3/session.py b/github3/session.py
@@ -17,6 +17,21 @@ def requires_2fa(response):
     return False
 
 
+class TokenAuth(requests.auth.AuthBase):
+    def __init__(self, token):
+        self.token = token
+
+    def __ne__(self, other):
+        return not self == other
+
+    def __eq__(self, other):
+        return self.token == getattr(other, 'token', None)
+
+    def __call__(self, request):
+        request.headers['Authorization'] = 'token {}'.format(self.token)
+        return request
+
+
 class GitHubSession(requests.Session):
     auth = None
     __attrs__ = requests.Session.__attrs__ + ['base_url', 'two_factor_auth_cb']
@@ -48,9 +63,6 @@ def basic_auth(self, username, password):
 
         self.auth = (username, password)
 
-        # Disable token authentication
-        self.headers.pop('Authorization', None)
-
     def build_url(self, *args, **kwargs):
         """Builds a new API url from scratch."""
         parts = [kwargs.get('base_url') or self.base_url]
@@ -122,11 +134,7 @@ def token_auth(self, token):
         if not token:
             return
 
-        self.headers.update({
-            'Authorization': 'token {0}'.format(token)
-            })
-        # Unset username/password so we stop sending them
-        self.auth = None
+        self.auth = TokenAuth(token)
 
     @contextmanager
     def temporary_basic_auth(self, *auth):
diff --git a/tests/unit/test_github_session.py b/tests/unit/test_github_session.py
@@ -85,9 +85,12 @@ def test_basic_login_disables_token_auth(self):
         """
         s = self.build_session()
         s.token_auth('token goes here')
-        assert 'Authorization' in s.headers
+        req = requests.Request('GET', 'https://api.github.com/')
+        pr = s.prepare_request(req)
+        assert 'token token goes here' == pr.headers['Authorization']
         s.basic_auth('username', 'password')
-        assert 'Authorization' not in s.headers
+        pr = s.prepare_request(req)
+        assert 'token token goes here' != pr.headers['Authorization']
 
     @mock.patch.object(requests.Session, 'request')
     def test_handle_two_factor_auth(self, request_mock):
@@ -136,7 +139,9 @@ def test_token_auth(self):
         """Test that token auth will work with a valid token"""
         s = self.build_session()
         s.token_auth('token goes here')
-        assert s.headers['Authorization'] == 'token token goes here'
+        req = requests.Request('GET', 'https://api.github.com/')
+        pr = s.prepare_request(req)
+        assert pr.headers['Authorization'] == 'token token goes here'
 
     def test_token_auth_disables_basic_auth(self):
         """Test that using token auth removes the value of the auth attribute.
@@ -146,15 +151,49 @@ def test_token_auth_disables_basic_auth(self):
         s = self.build_session()
         s.auth = ('foo', 'bar')
         s.token_auth('token goes here')
-        assert s.auth is None
+        assert s.auth != ('foo', 'bar')
+        assert isinstance(s.auth, session.TokenAuth)
 
     def test_token_auth_does_not_use_falsey_values(self):
         """Test that token auth will not authenticate with falsey values"""
         bad_tokens = [None, '']
+        req = requests.Request('GET', 'https://api.github.com/')
         for token in bad_tokens:
             s = self.build_session()
             s.token_auth(token)
-            assert 'Authorization' not in s.headers
+            pr = s.prepare_request(req)
+            assert 'Authorization' not in pr.headers
+
+    def test_token_auth_with_netrc_works(self, tmpdir):
+        """
+        Test that token auth will be used instead of netrc.
+
+        With no auth specified, requests will use any matching auths
+        in .netrc/_netrc files
+        """
+        token = "my-valid-token"
+        s = self.build_session()
+        s.token_auth(token)
+
+        netrc_contents = (
+            "machine api.github.com\n"
+            "login sigmavirus24\n"
+            "password invalid_token_for_test_verification\n"
+        )
+        # cover testing netrc behaviour on different OSs
+        dotnetrc = tmpdir.join(".netrc")
+        dotnetrc.write(netrc_contents)
+        dashnetrc = tmpdir.join("_netrc")
+        dashnetrc.write(netrc_contents)
+
+        with mock.patch.dict('os.environ', {'HOME': str(tmpdir)}):
+            # prepare_request triggers reading of .netrc files
+            pr = s.prepare_request(
+                requests.Request(
+                    'GET', 'https://api.github.com/users/sigmavirus24')
+            )
+            auth_header = pr.headers['Authorization']
+            assert auth_header == 'token {0}'.format(token)
 
     def test_two_factor_auth_callback_handles_None(self):
         s = self.build_session()
@@ -214,13 +253,15 @@ def test_no_auth(self):
         """Verify that no_auth removes existing authentication."""
         s = self.build_session()
         s.basic_auth('user', 'password')
-        s.headers['Authorization'] = 'token foobarbogus'
+        req = requests.Request('GET', 'https://api.github.com/')
 
         with s.no_auth():
-            assert 'Authentication' not in s.headers
+            pr = s.prepare_request(req)
+            assert 'Authorization' not in pr.headers
             assert s.auth is None
 
-        assert s.headers['Authorization'] == 'token foobarbogus'
+        pr = s.prepare_request(req)
+        assert 'Authorization' in pr.headers
         assert s.auth == ('user', 'password')
 
     def test_retrieve_client_credentials_when_set(self):
