Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for JWT authentication #948

Merged
merged 3 commits into from Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions CONTRIBUTING.md
Expand Up @@ -45,7 +45,7 @@ from deprecated import deprecated
@deprecated
def rate(self):
pass

@deprecated(reason="Deprecated in favor of the new branch protection")
def get_protected_branch(self):
pass
Expand All @@ -65,10 +65,13 @@ You will need a `GithubCredentials.py` file at the root of the project with the
login = "my_login"
password = "my_password"
oauth_token = "my_token" # Can be left empty if not used
jwt = "my_json_web_token" # Can be left empty if not used
```

If you use 2 factor authentication on your Github account, tests that require a login/password authentication will fail.
You can use `python -m github.tests Issue139.testCompletion --record --auth_with_token` to use the `oauth_token` field specified in `GithubCredentials.py` when recording a unit test interaction. Note that the `password = ""` (empty string is ok) must still be present in `GithubCredentials.py` to run the tests even when the `--auth_with_token` arg is used. (Also note that if you record your test data with `--auth_with_token` then you also need to be in token authentication mode when running the test. A simple alternative it to replace `token private_token_removed` with `Basic login_and_password_removed` in all your newly generated ReplayData files.)
You can use `python -m github.tests Issue139.testCompletion --record --auth_with_token` to use the `oauth_token` field specified in `GithubCredentials.py` when recording a unit test interaction. Note that the `password = ""` (empty string is ok) must still be present in `GithubCredentials.py` to run the tests even when the `--auth_with_token` arg is used. (Also note that if you record your test data with `--auth_with_token` then you also need to be in token authentication mode when running the test. A simple alternative is to replace `token private_token_removed` with `Basic login_and_password_removed` in all your newly generated ReplayData files.)

Similarly, you can use `python -m github.tests Issue139.testCompletion --record --auth_with_jwt` to use the `jwt` field specified in `GithubCredentials.py` to access endpoints that require JWT.

To run manual tests with external scripts that use the PyGithub package, you can install your development version with:

Expand Down
5 changes: 3 additions & 2 deletions github/MainClass.py
Expand Up @@ -94,7 +94,7 @@ class Github(object):
This is the main class you instantiate to access the Github API v3. Optional parameters allow different authentication methods.
"""

def __init__(self, login_or_token=None, password=None, base_url=DEFAULT_BASE_URL, timeout=DEFAULT_TIMEOUT, client_id=None, client_secret=None, user_agent='PyGithub/Python', per_page=DEFAULT_PER_PAGE, api_preview=False, verify=True):
def __init__(self, login_or_token=None, password=None, jwt=None, base_url=DEFAULT_BASE_URL, timeout=DEFAULT_TIMEOUT, client_id=None, client_secret=None, user_agent='PyGithub/Python', per_page=DEFAULT_PER_PAGE, api_preview=False, verify=True):
"""
:param login_or_token: string
:param password: string
Expand All @@ -109,13 +109,14 @@ def __init__(self, login_or_token=None, password=None, base_url=DEFAULT_BASE_URL

assert login_or_token is None or isinstance(login_or_token, (str, unicode)), login_or_token
assert password is None or isinstance(password, (str, unicode)), password
assert jwt is None or isinstance(jwt, (str, unicode)), jwt
assert isinstance(base_url, (str, unicode)), base_url
assert isinstance(timeout, (int, long)), timeout
assert client_id is None or isinstance(client_id, (str, unicode)), client_id
assert client_secret is None or isinstance(client_secret, (str, unicode)), client_secret
assert user_agent is None or isinstance(user_agent, (str, unicode)), user_agent
assert isinstance(api_preview, (bool))
self.__requester = Requester(login_or_token, password, base_url, timeout, client_id, client_secret, user_agent, per_page, api_preview, verify)
self.__requester = Requester(login_or_token, password, jwt, base_url, timeout, client_id, client_secret, user_agent, per_page, api_preview, verify)

def __get_FIX_REPO_GET_GIT_REF(self):
"""
Expand Down
6 changes: 5 additions & 1 deletion github/Requester.py
Expand Up @@ -214,7 +214,7 @@ def _initializeDebugFeature(self):

#############################################################

def __init__(self, login_or_token, password, base_url, timeout, client_id, client_secret, user_agent, per_page, api_preview, verify):
def __init__(self, login_or_token, password, jwt, base_url, timeout, client_id, client_secret, user_agent, per_page, api_preview, verify):
self._initializeDebugFeature()

if password is not None:
Expand All @@ -226,6 +226,8 @@ def __init__(self, login_or_token, password, base_url, timeout, client_id, clien
elif login_or_token is not None:
token = login_or_token
self.__authorizationHeader = "token " + token
elif jwt is not None:
self.__authorizationHeader = "Bearer " + jwt
else:
self.__authorizationHeader = None

Expand Down Expand Up @@ -469,6 +471,8 @@ def __log(self, verb, url, requestHeaders, input, status, responseHeaders, outpu
requestHeaders["Authorization"] = "Basic (login and password removed)"
elif requestHeaders["Authorization"].startswith("token"):
requestHeaders["Authorization"] = "token (oauth token removed)"
elif requestHeaders["Authorization"].startswith("Bearer"):
requestHeaders["Authorization"] = "Bearer (jwt removed)"
else: # pragma no cover (Cannot happen, but could if we add an authentication method => be prepared)
requestHeaders["Authorization"] = "(unknown auth removed)" # pragma no cover (Cannot happen, but could if we add an authentication method => be prepared)
logger.debug("%s %s://%s%s %s %s ==> %i %s %s", str(verb), self.__scheme, self.__hostname, str(url), str(requestHeaders), str(input), status, str(responseHeaders), str(output))
6 changes: 5 additions & 1 deletion github/tests/Authentication.py
Expand Up @@ -45,7 +45,11 @@ def testOAuthAuthentication(self):
g = github.Github(self.oauth_token)
self.assertEqual(g.get_user("jacquev6").name, "Vincent Jacques")

# Warning: I don't have a scret key, so the requests for this test are forged
def testJWTAuthentication(self):
g = github.Github(jwt=self.jwt)
self.assertEqual(g.get_user("jacquev6").name, "Vincent Jacques")

# Warning: I don't have a secret key, so the requests for this test are forged
def testSecretKeyAuthentication(self):
g = github.Github(client_id=self.client_id, client_secret=self.client_secret)
self.assertListKeyEqual(g.get_organization("BeaverSoftware").get_repos("public"), lambda r: r.name, ["FatherBeaver", "PyGithub"])
Expand Down
11 changes: 11 additions & 0 deletions github/tests/Framework.py
Expand Up @@ -80,6 +80,8 @@ def fixAuthorizationHeader(headers):
headers["Authorization"] = "token private_token_removed"
elif headers["Authorization"].startswith("Basic "):
headers["Authorization"] = "Basic login_and_password_removed"
elif headers["Authorization"].startswith("Bearer "):
headers["Authorization"] = "Bearer jwt_removed"


class RecordingConnection: # pragma no cover (Class useful only when recording new tests, not used during automated tests)
Expand Down Expand Up @@ -213,6 +215,7 @@ def ReplayingHttpsConnection(testCase, file, *args, **kwds):
class BasicTestCase(unittest.TestCase):
recordMode = False
tokenAuthMode = False
jwtAuthMode = False
replayDataFolder = os.path.join(os.path.dirname(__file__), "ReplayData")

def setUp(self):
Expand All @@ -228,6 +231,7 @@ def setUp(self):
self.login = GithubCredentials.login
self.password = GithubCredentials.password
self.oauth_token = GithubCredentials.oauth_token
self.jwt = GithubCredentials.jwt
# @todo Remove client_id and client_secret from ReplayData (as we already remove login, password and oauth_token)
# self.client_id = GithubCredentials.client_id
# self.client_secret = GithubCredentials.client_secret
Expand All @@ -241,6 +245,7 @@ def setUp(self):
self.oauth_token = "oauth_token"
self.client_id = "client_id"
self.client_secret = "client_secret"
self.jwt = "jwt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you add a activateJwtAuthMode in this file as well? Since this PR adds support for authenticating with JWT, we should support recording tests through it right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfdye thanks for the suggestion, I added the mode and verified that it works.


def tearDown(self):
unittest.TestCase.tearDown(self)
Expand Down Expand Up @@ -294,6 +299,8 @@ def setUp(self):

if self.tokenAuthMode:
self.g = github.Github(self.oauth_token)
elif self.jwtAuthMode:
self.g = github.Github(jwt=self.jwt)
else:
self.g = github.Github(self.login, self.password)

Expand All @@ -304,3 +311,7 @@ def activateRecordMode(): # pragma no cover (Function useful only when recordin

def activateTokenAuthMode(): # pragma no cover (Function useful only when recording new tests, not used during automated tests)
BasicTestCase.tokenAuthMode = True


def activateJWTAuthMode(): # pragma no cover (Function useful only when recording new tests, not used during automated tests)
BasicTestCase.jwtAuthMode = True
11 changes: 11 additions & 0 deletions github/tests/ReplayData/Authentication.testJWTAuthentication.txt
@@ -0,0 +1,11 @@
https
GET
api.github.com
None
/users/jacquev6
{'Authorization': 'Bearer jwt_removed', 'User-Agent': 'PyGithub/Python'}
None
200
[('status', '200 OK'), ('x-ratelimit-remaining', '4994'), ('content-length', '623'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"0e3990a84c08ccd728a27dbe549d4f86"'), ('date', 'Sat, 26 May 2012 09:34:29 GMT'), ('x-oauth-scopes', ''), ('content-type', 'application/json; charset=utf-8'), ('x-accepted-oauth-scopes', 'user')]
{"type":"User","company":"Criteo","location":"Paris, France","hireable":false,"gravatar_id":"b68de5ae38616c296fa345d2b9df2225","bio":"","following":24,"blog":"http://vincent-jacques.net","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png","followers":13,"html_url":"https://github.com/jacquev6","url":"https://api.github.com/users/jacquev6","name":"Vincent Jacques","login":"jacquev6","public_repos":11,"public_gists":1,"email":"vincent@vincent-jacques.net","id":327146,"created_at":"2010-07-09T06:10:06Z"}

4 changes: 4 additions & 0 deletions github/tests/__main__.py
Expand Up @@ -44,6 +44,10 @@ def main(argv):
github.tests.Framework.activateTokenAuthMode()
argv = [arg for arg in argv if arg != "--auth_with_token"]

if "--auth_with_jwt" in argv:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One final touch, this could be documented here in the contribution guide:
https://github.com/PyGithub/PyGithub/blob/master/CONTRIBUTING.md#automated-tests

github.tests.Framework.activateJWTAuthMode()
argv = [arg for arg in argv if arg != "--auth_with_jwt"]

unittest.main(module=github.tests.AllTests, argv=argv)


Expand Down