You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2020/03/03 21:10:42 UTC
[allura] 01/01: [#8353] update several github api usages & auth to
latest way of doing it
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a commit to branch db/8353
in repository https://gitbox.apache.org/repos/asf/allura.git
commit e34506fab1d9db3a8551854bef91d91a92cdac16
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Tue Mar 3 15:56:48 2020 -0500
[#8353] update several github api usages & auth to latest way of doing it
---
Allura/allura/lib/helpers.py | 8 +++-
Allura/development.ini | 2 +-
ForgeBlog/forgeblog/command/rssfeeds.py | 2 +-
ForgeImporters/forgeimporters/base.py | 8 ++--
ForgeImporters/forgeimporters/github/__init__.py | 52 ++++++++++++++--------
.../forgeimporters/github/tests/test_oauth.py | 27 +++++++----
.../tests/github/functional/test_github.py | 11 ++---
.../forgeimporters/tests/github/test_extractor.py | 9 ++--
8 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index 13c14a8..5c0e7f8 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -1017,11 +1017,17 @@ def urlopen(url, retries=3, codes=(408, 500, 502, 503, 504), timeout=None):
url_string = url.get_full_url() # if url is Request obj
except Exception:
url_string = url
+ if hasattr(e, 'filename') and url_string != e.filename:
+ url_string += ' => {}'.format(e.filename)
if timeout is None:
timeout = socket.getdefaulttimeout()
+ if hasattr(e, 'fp'):
+ body = e.fp.read()
+ else:
+ body = ''
log.exception(
'Failed after %s retries on url with a timeout of %s: %s: %s',
- attempts, timeout, url_string, e)
+ attempts, timeout, url_string, body[:250])
raise e
diff --git a/Allura/development.ini b/Allura/development.ini
index ad56a7a..cdaf2bf 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -449,7 +449,7 @@ disable_entry_points.allura.theme.override = responsive
;hidden_importers = trac-tickets
; GitHub importer keys. For github ticket import, it is best to set
-; up an app at https://github.com/settings/applications Use the root URL
+; up an app at https://github.com/settings/developers Use the root URL
; of your Allura instance for both URLs, and enter client values here:
;github_importer.client_id =
;github_importer.client_secret =
diff --git a/ForgeBlog/forgeblog/command/rssfeeds.py b/ForgeBlog/forgeblog/command/rssfeeds.py
index d46319a..5ed3310 100644
--- a/ForgeBlog/forgeblog/command/rssfeeds.py
+++ b/ForgeBlog/forgeblog/command/rssfeeds.py
@@ -106,7 +106,7 @@ class RssFeedsCommand(base.BlogCommand):
c.app = app
allura_base.log.info("Get feed: %s" % feed_url)
- f = feedparser.parse(feed_url)
+ f = feedparser.parse(feed_url) # TODO: fetch first with requests w/ UA set, then pass r.text in
if f.bozo:
allura_base.log.exception("%s: %s" % (feed_url, f.bozo_exception))
return
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index f4e78c9..afbe7a9 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -171,10 +171,12 @@ class ProjectExtractor(object):
self.get_page(page_name, **kw)
@staticmethod
- def urlopen(url, retries=3, codes=(408, 500, 502, 503, 504), timeout=120, **kw):
+ def urlopen(url, retries=3, codes=(408, 500, 502, 503, 504), timeout=120, unredirected_hdrs=None, **kw):
req = six.moves.urllib.request.Request(url, **kw)
- req.add_header(
- 'User-Agent', 'Allura Data Importer (https://allura.apache.org/)')
+ if unredirected_hdrs:
+ for k, v in unredirected_hdrs.items():
+ req.add_unredirected_header(k, v)
+ req.add_header('User-Agent', 'Allura Data Importer (https://allura.apache.org/)')
return h.urlopen(req, retries=retries, codes=codes, timeout=timeout)
def get_page(self, page_name_or_url, parser=None, **kw):
diff --git a/ForgeImporters/forgeimporters/github/__init__.py b/ForgeImporters/forgeimporters/github/__init__.py
index 1b67d0a..a5e3ddb 100644
--- a/ForgeImporters/forgeimporters/github/__init__.py
+++ b/ForgeImporters/forgeimporters/github/__init__.py
@@ -75,10 +75,10 @@ class GitHubProjectExtractor(base.ProjectExtractor):
super(GitHubProjectExtractor, self).__init__(*args, **kw)
def add_token(self, url):
+ headers = {}
if self.token:
- glue = '&' if '?' in url else '?'
- url += glue + 'access_token=' + self.token
- return url
+ headers['Authorization'] = 'token {}'.format(self.token)
+ return url, headers
def wait_for_limit_reset(self, headers):
reset = headers.get('X-RateLimit-Reset')
@@ -89,10 +89,15 @@ class GitHubProjectExtractor(base.ProjectExtractor):
'Sleeping until %s UTC' % (limit, reset))
time.sleep((reset - now).total_seconds())
- def urlopen(self, url, **kw):
+ def urlopen(self, url, headers=None, **kw):
+ if headers is None:
+ headers = {}
try:
- resp = super(GitHubProjectExtractor, self).urlopen(
- self.add_token(url), **kw)
+ url, auth_headers = self.add_token(url)
+ # need to use unredirected_hdrs for Authorization for APIs that redirect to an AWS file asset which has
+ # separate authentication added automatically
+ resp = super(GitHubProjectExtractor, self).urlopen(url,
+ headers=headers, unredirected_hdrs=auth_headers, **kw)
except six.moves.urllib.error.HTTPError as e:
# GitHub will return 403 if rate limit exceeded.
# We're checking for limit on every request below, but we still
@@ -109,7 +114,9 @@ class GitHubProjectExtractor(base.ProjectExtractor):
return resp
def check_readable(self):
- resp = requests.head(self.add_token(self.get_page_url('project_info')), timeout=10)
+ url, headers = self.add_token(self.get_page_url('project_info'))
+ headers['User-Agent'] = 'Allura Data Importer (https://allura.apache.org/)'
+ resp = requests.head(url, headers=headers, timeout=10)
return resp.status_code == 200
def get_next_page_url(self, link):
@@ -179,18 +186,29 @@ class GitHubProjectExtractor(base.ProjectExtractor):
return self.get_page('project_info').get('has_issues')
-def valid_access_token(access_token):
- # https://developer.github.com/v3/oauth_authorizations/#check-an-authorization
+def oauth_app_basic_auth(config):
client_id = config['github_importer.client_id']
secret = config['github_importer.client_secret']
- token_valid_resp = requests.get('https://api.github.com/applications/{}/tokens/{}'.format(client_id, access_token),
- auth=requests.auth.HTTPBasicAuth(client_id, secret),
- timeout=10)
- return token_valid_resp.status_code == 200
+ return requests.auth.HTTPBasicAuth(client_id, secret)
+
+
+def valid_access_token(access_token):
+ return access_token_details(access_token).status_code == 200
+
+
+def access_token_details(access_token):
+ # https://developer.github.com/v3/apps/oauth_applications/#check-a-token
+ client_id = config['github_importer.client_id']
+ url = 'https://api.github.com/applications/{}/token'.format(client_id)
+ return requests.post(url, auth=oauth_app_basic_auth(config), timeout=10, json=dict(
+ access_token=access_token,
+ ))
class GitHubOAuthMixin(object):
- '''Support for github oauth web application flow.'''
+ '''
+ Support for github oauth web application flow. This is an "OAuth App" not a "GitHub App"
+ '''
def oauth_begin(self, scope=None):
client_id = config.get('github_importer.client_id')
@@ -241,8 +259,6 @@ class GitHubOAuthMixin(object):
token = c.user.get_tool_data('GitHubProjectImport', 'token')
if not token:
return False
- url = 'https://api.github.com/?access_token={}'.format(token)
- r = requests.head(url, timeout=10)
- scopes = r.headers.get('X-OAuth-Scopes', '')
- scopes = [s.strip() for s in scopes.split(',')]
+ r = access_token_details(token)
+ scopes = r.json()['scopes']
return scope in scopes
diff --git a/ForgeImporters/forgeimporters/github/tests/test_oauth.py b/ForgeImporters/forgeimporters/github/tests/test_oauth.py
index 7e60b7e..563869c 100644
--- a/ForgeImporters/forgeimporters/github/tests/test_oauth.py
+++ b/ForgeImporters/forgeimporters/github/tests/test_oauth.py
@@ -43,20 +43,31 @@ class TestGitHubOAuthMixin(TestController, TestCase):
c.user.get_tool_data.return_value = None
self.assertFalse(self.mix.oauth_has_access('write:repo_hook'))
+ @patch.dict(config, {'github_importer.client_id': '123456',
+ 'github_importer.client_secret': 'deadbeef'})
@patch('forgeimporters.github.requests')
- def test_oauth_has_access_no_headers(self, req):
- c.user.get_tool_data.return_value = 'token'
+ def test_oauth_has_access_no(self, req):
+ c.user.get_tool_data.return_value = 'some-token'
self.assertFalse(self.mix.oauth_has_access('write:repo_hook'))
- req.head.assert_called_once_with('https://api.github.com/?access_token=token', timeout=10)
+ call_args = req.post.call_args[0]
+ self.assertEqual(call_args, ('https://api.github.com/applications/123456/token',))
+ call_kwargs = req.post.call_args[1]
+ assert call_kwargs['auth']
+ self.assertEqual(call_kwargs['json'], {'access_token': 'some-token'})
+ @patch.dict(config, {'github_importer.client_id': '123456',
+ 'github_importer.client_secret': 'deadbeef'})
@patch('forgeimporters.github.requests')
- def test_oauth_has_access_with_headers(self, req):
- c.user.get_tool_data.return_value = 'token'
- req.head.return_value.headers = {'X-OAuth-Scopes': ''}
+ def test_oauth_has_access_yes(self, req):
+ c.user.get_tool_data.return_value = 'some-token'
+
+ req.post.return_value.json.return_value = {'scopes': []}
self.assertFalse(self.mix.oauth_has_access('write:repo_hook'))
- req.head.return_value.headers = {'X-OAuth-Scopes': 'some, other:scopes'}
+
+ req.post.return_value.json.return_value = {'scopes': ['some', 'other:scopes']}
self.assertFalse(self.mix.oauth_has_access('write:repo_hook'))
- req.head.return_value.headers = {'X-OAuth-Scopes': 'write:repo_hook, user'}
+
+ req.post.return_value.json.return_value = {'scopes': ['write:repo_hook', 'user']}
self.assertTrue(self.mix.oauth_has_access('write:repo_hook'))
@patch.dict(config, {'github_importer.client_id': '123456',
diff --git a/ForgeImporters/forgeimporters/tests/github/functional/test_github.py b/ForgeImporters/forgeimporters/tests/github/functional/test_github.py
index 1c0eb4d..f6b7ad5 100644
--- a/ForgeImporters/forgeimporters/tests/github/functional/test_github.py
+++ b/ForgeImporters/forgeimporters/tests/github/functional/test_github.py
@@ -83,13 +83,14 @@ class TestGitHubOAuth(TestController):
user = M.User.by_username('test-admin')
assert_equal(user.get_tool_data('GitHubProjectImport', 'token'), 'abc')
- with patch('forgeimporters.github.requests.get') as valid_access_token_get:
- valid_access_token_get.return_value = Mock(status_code=200)
+ with patch('forgeimporters.github.requests.post') as valid_access_token_post:
+ valid_access_token_post.return_value = Mock(status_code=200)
r = self.app.get('/p/import_project/github/')
# token in user data, so oauth isn't triggered
assert_equal(r.status_int, 200)
- valid_access_token_get.assert_called_once_with('https://api.github.com/applications/client_id/tokens/abc',
- auth=requests.auth.HTTPBasicAuth('client_id', 'secret'),
- timeout=10)
+ valid_access_token_post.assert_called_once_with('https://api.github.com/applications/client_id/token',
+ auth=requests.auth.HTTPBasicAuth('client_id', 'secret'),
+ json={'access_token': 'abc'},
+ timeout=10)
diff --git a/ForgeImporters/forgeimporters/tests/github/test_extractor.py b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
index 617e56b..2b44a40 100644
--- a/ForgeImporters/forgeimporters/tests/github/test_extractor.py
+++ b/ForgeImporters/forgeimporters/tests/github/test_extractor.py
@@ -150,12 +150,9 @@ class TestGitHubProjectExtractor(TestCase):
e = github.GitHubProjectExtractor('test_project', user=user)
e.urlopen(url)
request = urlopen.call_args[0][0]
- self.assertEqual(request.get_full_url(), url + '?access_token=abc')
-
- url = 'https://github.com/u/p/?p=1'
- e.urlopen(url)
- request = urlopen.call_args[0][0]
- self.assertEqual(request.get_full_url(), url + '&access_token=abc')
+ self.assertEqual(request.get_full_url(), url)
+ assert request.headers['User-agent']
+ self.assertEqual(request.unredirected_hdrs['Authorization'], 'token abc')
@patch('forgeimporters.base.h.urlopen')
@patch('forgeimporters.github.time.sleep')