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:11:07 UTC
[allura] branch db/8353 updated (e34506f -> a4445f6)
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a change to branch db/8353
in repository https://gitbox.apache.org/repos/asf/allura.git.
discard e34506f [#8353] update several github api usages & auth to latest way of doing it
new a4445f6 [#8353] update several github api usages & auth to latest way of doing it
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (e34506f)
\
N -- N -- N refs/heads/db/8353 (a4445f6)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
ForgeBlog/forgeblog/command/rssfeeds.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[allura] 01/01: [#8353] update several github api usages & auth to
latest way of doing it
Posted by br...@apache.org.
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 a4445f676c5c52ca065061915077f50bb7420516
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Tue Mar 3 16:10:57 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 +-
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 ++--
7 files changed, 75 insertions(+), 42 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/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')