-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Conversation
@@ -241,6 +243,7 @@ def setUp(self): | |||
self.oauth_token = "oauth_token" | |||
self.client_id = "client_id" | |||
self.client_secret = "client_secret" | |||
self.jwt = "jwt" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@ardakuyumcu Awesome addition you've got here. Did you test this change with an actual Github app? I will review the other two PRs once we close this one 👍 |
github/Requester.py
Outdated
self._initializeDebugFeature() | ||
|
||
if password is not None: | ||
if jwt is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind moving this case after the login_or_token
?
@@ -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: |
There was a problem hiding this comment.
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
@sfdye added the requested changes |
@ardakuyumcu Awesome, thank you very much! |
This option breaks positional argument compatibility 😞 |
APIs such as https://developer.github.com/v3/apps/#find-organization-installation use a JWT for authentication. Adding support for JWT auth.
APIs such as https://developer.github.com/v3/apps/#find-organization-installation use a JWT for authentication. Adding support for JWT auth.