8000 Merge pull request #800 from electrofelix/token-auth-disables-netrc · pythonthings/github3.py@aa50e04 · GitHub
[go: up one dir, main page]
Skip to content

Commit aa50e04

Browse files
sigmavirus24sigmavirus24
authored
Merge pull request sigmavirus24#800 from electrofelix/token-auth-disables-netrc
Provide TokenAuth class to requests to ensure explicit usage
2 parents de04628 + 65eb7a2 commit aa50e04

File tree

2 files changed

+65
-16
lines changed

2 files changed

+65
-16
lines changed

‎github3/session.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ def requires_2fa(response):
1717
return False
1818

1919

20+
class TokenAuth(requests.auth.AuthBase):
21+
def __init__(self, token):
22+
self.token = token
23+
24+
def __ne__(self, other):
25+
return not self == other
26+
27+
def __eq__(self, other):
28+
return self.token == getattr(other, 'token', None)
29+
30+
def __call__(self, request):
31+
request.headers['Authorization'] = 'token {}'.format(self.token)
32+
return request
33+
34+
2035
class GitHubSession(requests.Session):
2136
auth = None
2237
__attrs__ = requests.Session.__attrs__ + ['base_url', 'two_factor_auth_cb']
@@ -48,9 +63,6 @@ def basic_auth(self, username, password):
4863

4964
self.auth = (username, password)
5065

51-
# Disable token authentication
52-
self.headers.pop('Authorization', None)
53-
5466
def build_url(self, *args, **kwargs):
5567
"""Builds a new API url from scratch."""
5668
parts = [kwargs.get('base_url') or self.base_url]
@@ -122,11 +134,7 @@ def token_auth(self, token):
122134
if not token:
123135
return
124136

125-
self.headers.update({
126-
'Authorization': 'token {0}'.format(token)
127-
})
128-
# Unset username/password so we stop sending them
129-
self.auth = None
137+
self.auth = TokenAuth(token)
130138

131139
@contextmanager
132140
def temporary_basic_auth(self, *auth):

‎tests/unit/test_github_session.py

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,12 @@ def test_basic_login_disables_token_auth(self):
8585
"""
8686
s = self.build_session()
8787
s.token_auth('token goes here')
88-
assert 'Authorization' in s.headers
88+
req = requests.Request('GET', 'https://api.github.com/')
89+
pr = s.prepare_request(req)
90+
assert 'token token goes here' == pr.headers['Authorization']
8991
s.basic_auth('username', 'password')
90-
assert 'Authorization' not in s.headers
92+
pr = s.prepare_request(req)
93+
assert 'token token goes here' != pr.headers['Authorization']
9194

9295
@mock.patch.object(requests.Session, 'request')
9396
def test_handle_two_factor_auth(self, request_mock):
@@ -136,7 +139,9 @@ def test_token_auth(self):
136139
"""Test that token auth will work with a valid token"""
137140
s = self.build_session()
138141
s.token_auth('token goes here')
139-
assert s.headers['Authorization'] == 'token token goes here'
142+
req = requests.Request('GET', 'https://api.github.com/')
143+
pr = s.prepare_request(req)
144+
assert pr.headers['Authorization'] == 'token token goes here'
140145

141146
def test_token_auth_disables_basic_auth(self):
142147
"""Test that using token auth removes the value of the auth attribute.
@@ -146,15 +151,49 @@ def test_token_auth_disables_basic_auth(self):
146151
s = self.build_session()
147152
s.auth = ('foo', 'bar')
148153
s.token_auth('token goes here')
149-
assert s.auth is None
154+
assert s.auth != ('foo', 'bar')
155+
assert isinstance(s.auth, session.TokenAuth)
150156

151157
def test_token_auth_does_not_use_falsey_values(self):
152158
"""Test that token auth will not authenticate with falsey values"""
153159
bad_tokens = [None, '']
160+
req = requests.Request('GET', 'https://api.github.com/')
154161
for token in bad_tokens:
155162
s = self.build_session()
156163
s.token_auth(token)
157-
assert 'Authorization' not in s.headers
164+
pr = s.prepare_request(req)
165+
assert 'Authorization' not in pr.headers
166+
167+
def test_token_auth_with_netrc_works(self, tmpdir):
168+
"""
169+
Test that token auth will be used instead of netrc.
170+
171+
With no auth specified, requests will use any matching auths
172+
in .netrc/_netrc files
173+
"""
174+
token = "my-valid-token"
175+
s = self.build_session()
176+
s.token_auth(token)
177+
178+
netrc_contents = (
179+
"machine api.github.com\n"
180+
"login sigmavirus24\n"
181+
"password invalid_token_for_test_verification\n"
182+
)
183+
# cover testing netrc behaviour on different OSs
184+
dotnetrc = tmpdir.join(".netrc")
185+
dotnetrc.write(netrc_contents)
186+
dashnetrc = tmpdir.join("_netrc")
187+
dashnetrc.write(netrc_contents)
188+
189+
with mock.patch.dict('os.environ', {'HOME': str(tmpdir)}):
190+
# prepare_request triggers reading of .netrc files
191+
pr = s.prepare_request(
192+
requests.Request(
193+
'GET', 'https://api.github.com/users/sigmavirus24')
194+
)
195+
auth_header = pr.headers['Authorization']
196+
assert auth_header == 'token {0}'.format(token)
158197

159198
def test_two_factor_auth_callback_handles_None(self):
160199
s = self.build_session()
@@ -214,13 +253,15 @@ def test_no_auth(self):
214253
"""Verify that no_auth removes existing authentication."""
215254
s = self.build_session()
216255
s.basic_auth('user', 'password')
217-
s.headers['Authorization'] = 'token foobarbogus'
256+
req = requests.Request('GET', 'https://api.github.com/')
218257

219258
with s.no_auth():
220-
assert 'Authentication' not in s.headers
259+
pr = s.prepare_request(req)
260+
assert 'Authorization' not in pr.headers
221261
assert s.auth is None
222262

223-
assert s.headers['Authorization'] == 'token foobarbogus'
263+
pr = s.prepare_request(req)
264+
assert 'Authorization' in pr.headers
224265
assert s.auth == ('user', 'password')
225266

226267
def test_retrieve_client_credentials_when_set(self):

0 commit comments

Comments
 (0)
0