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

Support full GitHub app authentication #1986

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
885a522
Support full GitHub app authentication
dblanchette Jun 30, 2021
a719746
Fix main class type
dblanchette Jul 7, 2021
c39a183
Merge master and fix conflict
dblanchette Oct 15, 2021
d8b20e1
Remove unused imports
dblanchette Oct 15, 2021
30a11a0
Merge branch 'master' into feature/github-app-authentication
dblanchette Nov 3, 2021
c15de72
Code review changes
dblanchette Jan 11, 2022
f8a071b
Fix type annotation
dblanchette Jan 11, 2022
e957467
Remove default values in type hints
dblanchette Jan 14, 2022
add6683
Remove default values in .pyi (again)
dblanchette Jan 17, 2022
3ba6e76
Merge remote-tracking branch 'origin/master' into feature/github-app-…
dblanchette Jan 17, 2022
8df28e9
Merge branch 'master' into feature/github-app-authentication
dblanchette Oct 13, 2022
a461e0e
black
dblanchette Oct 13, 2022
94ca7be
Refactor GithubIntegration class and add test case for app authentica…
ammarmallik Oct 31, 2022
8e47431
Modify existing testcases for GithubIntegration as per the framework …
ammarmallik Nov 1, 2022
a9c2f81
Provide installation ID for creating the access token instead of gett…
ammarmallik Nov 2, 2022
b88f36f
Add test case for getting app installation
ammarmallik Nov 3, 2022
8534fe9
Add optional permissions support for installation access token
ammarmallik Nov 4, 2022
d502f27
Merge pull request #28 from ammarmallik/feature/github-app-authentica…
dblanchette Nov 29, 2022
350911e
Merge branch 'master' into feature/github-app-authentication
dblanchette Nov 29, 2022
15f1c0c
flake8 not on gitlab anymore?
dblanchette Nov 29, 2022
99e6b3a
Merge branch 'master' into feature/github-app-authentication
dblanchette Dec 19, 2022
ceb8d7d
Add lock around app authentication
dblanchette Jan 16, 2023
df38766
Remove unrelated fixes
dblanchette Jan 23, 2023
8899a9e
A tab
dblanchette Jan 23, 2023
38fb3cd
Keep compatibility for importing GithubIntegration from MainClass
dblanchette Jan 30, 2023
2c106c6
Group app authentication parameters in a class
dblanchette Jan 31, 2023
ce199bf
Merge branch 'master' into feature/github-app-authentication
dblanchette Jan 31, 2023
a50aecd
Remove useless exception handling
dblanchette Jan 31, 2023
0e245d0
Add isinstance checks in github/AppAuthentication.py
dblanchette Feb 1, 2023
7d61945
Code review changes
dblanchette Feb 1, 2023
a7ca0cb
Merge remote-tracking branch 'coveord/feature/github-app-authenticati…
dblanchette Feb 1, 2023
7dd9fa9
Fix an assert
dblanchette Feb 1, 2023
7e6d5b7
black
dblanchette Feb 1, 2023
148c14f
Remove useless check
dblanchette Feb 1, 2023
3e16166
Fix an assert and a docstring
dblanchette Feb 2, 2023
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
41 changes: 41 additions & 0 deletions github/AppAuthentication.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
############################ Copyrights and license ############################
# #
# Copyright 2023 Denis Blanchette <denisblanchette@gmail.com> #
# #
# This file is part of PyGithub. #
# http://pygithub.readthedocs.io/ #
# #
# PyGithub is free software: you can redistribute it and/or modify it under #
# the terms of the GNU Lesser General Public License as published by the Free #
# Software Foundation, either version 3 of the License, or (at your option) #
# any later version. #
# #
# PyGithub is distributed in the hope that it will be useful, but WITHOUT ANY #
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS #
# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more #
# details. #
# #
# You should have received a copy of the GNU Lesser General Public License #
# along with PyGithub. If not, see <http://www.gnu.org/licenses/>. #
# #
################################################################################


class AppAuthentication:
def __init__(
self,
app_id,
private_key,
installation_id,
token_permissions=None,
):
assert isinstance(app_id, (int, str)), app_id
assert isinstance(private_key, str)
assert isinstance(installation_id, int), installation_id
assert token_permissions is None or isinstance(
token_permissions, dict
), token_permissions
self.app_id = app_id
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
self.private_key = private_key
self.installation_id = installation_id
self.token_permissions = token_permissions
10 changes: 10 additions & 0 deletions github/AppAuthentication.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from typing import Optional, Dict, Union

class AppAuthentication:
def __init__(
self,
app_id: Union[int, str],
private_key: str,
installation_id: int,
token_permissions: Optional[Dict[str, str]] = ...,
): ...
17 changes: 17 additions & 0 deletions github/Consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,20 @@

# https://developer.github.com/changes/2019-12-03-internal-visibility-changes/
repoVisibilityPreview = "application/vnd.github.nebula-preview+json"

DEFAULT_BASE_URL = "https://api.github.com"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved those here since they are used in 2 files now (MainClass and GithubIntegration)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Comparing this is really difficult. Would have been better to do the refactoring (without changes) in a separate PR and base this PR on that. Then we would see only changes, not moving unchanged code around.

DEFAULT_STATUS_URL = "https://status.github.com"
# As of 2018-05-17, Github imposes a 10s limit for completion of API requests.
# Thus, the timeout should be slightly > 10s to account for network/front-end
# latency.
DEFAULT_TIMEOUT = 15
DEFAULT_PER_PAGE = 30

# JWT expiry in seconds. Could be set for max 600 seconds (10 minutes).
# https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#authenticating-as-a-github-app
DEFAULT_JWT_EXPIRY = 300
MIN_JWT_EXPIRY = 15
MAX_JWT_EXPIRY = 600
# https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#generating-a-json-web-token-jwt
# "The time the JWT was created. To protect against clock drift, we recommend you set this 60 seconds in the past."
DEFAULT_JWT_ISSUED_AT = -60
199 changes: 199 additions & 0 deletions github/GithubIntegration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
import time
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from MainClass, it's already big as it is and that does not really belong there.


import deprecated
import jwt

from github import Consts
from github.GithubException import GithubException
from github.Installation import Installation
from github.InstallationAuthorization import InstallationAuthorization
from github.PaginatedList import PaginatedList
from github.Requester import Requester


class GithubIntegration:
"""
Main class to obtain tokens for a GitHub integration.
"""

def __init__(
self,
integration_id,
private_key,
base_url=Consts.DEFAULT_BASE_URL,
jwt_expiry=Consts.DEFAULT_JWT_EXPIRY,
jwt_issued_at=Consts.DEFAULT_JWT_ISSUED_AT,
):
"""
:param integration_id: int
:param private_key: string
:param base_url: string
:param jwt_expiry: int. Expiry of the JWT used to get the information about this integration.
The default expiration is in 5 minutes and is capped at 10 minutes according to GitHub documentation
https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#generating-a-json-web-token-jwt
:param jwt_issued_at: int. Number of seconds, relative to now, to set for the "iat" (issued at) parameter.
The default value is -60 to protect against clock drift
"""
assert isinstance(integration_id, (int, str)), integration_id
assert isinstance(private_key, str), "supplied private key should be a string"
assert isinstance(base_url, str), base_url
assert isinstance(jwt_expiry, int), jwt_expiry
assert Consts.MIN_JWT_EXPIRY <= jwt_expiry <= Consts.MAX_JWT_EXPIRY, jwt_expiry
assert isinstance(jwt_issued_at, int)

self.base_url = base_url
self.integration_id = integration_id
self.private_key = private_key
self.jwt_expiry = jwt_expiry
self.jwt_issued_at = jwt_issued_at
self.__requester = Requester(
login_or_token=None,
password=None,
jwt=self.create_jwt(),
app_auth=None,
base_url=self.base_url,
timeout=Consts.DEFAULT_TIMEOUT,
user_agent="PyGithub/Python",
per_page=Consts.DEFAULT_PER_PAGE,
verify=True,
retry=None,
pool_size=None,
)

def _get_headers(self):
"""
Get headers for the requests.

:return: dict
"""
return {
"Authorization": f"Bearer {self.create_jwt()}",
"Accept": Consts.mediaTypeIntegrationPreview,
"User-Agent": "PyGithub/Python",
}

def _get_installed_app(self, url):
"""
Get installation for the given URL.

:param url: str
:rtype: :class:`github.Installation.Installation`
"""
headers, response = self.__requester.requestJsonAndCheck(
"GET", url, headers=self._get_headers()
)

return Installation(
requester=self.__requester,
headers=headers,
attributes=response,
completed=True,
)

def create_jwt(self):
"""
Create a signed JWT
https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#authenticating-as-a-github-app

:return string:
"""
now = int(time.time())
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
payload = {
"iat": now + self.jwt_issued_at,
"exp": now + self.jwt_expiry,
"iss": self.integration_id,
}
encrypted = jwt.encode(payload, key=self.private_key, algorithm="RS256")

if isinstance(encrypted, bytes):
encrypted = encrypted.decode("utf-8")

return encrypted

def get_access_token(self, installation_id, permissions=None):
"""
:calls: `POST /app/installations/{installation_id}/access_tokens <https://docs.github.com/en/rest/apps/apps#create-an-installation-access-token-for-an-app>`
:param installation_id: int
:param permissions: dict
:return: :class:`github.InstallationAuthorization.InstallationAuthorization`
"""
if permissions is None:
permissions = {}

if not isinstance(permissions, dict):
raise GithubException(
status=400, data={"message": "Invalid permissions"}, headers=None
)

body = {"permissions": permissions}
headers, response = self.__requester.requestJsonAndCheck(
"POST",
f"/app/installations/{installation_id}/access_tokens",
input=body,
)

return InstallationAuthorization(
requester=self.__requester,
headers=headers,
attributes=response,
completed=True,
)

@deprecated.deprecated("Use get_repo_installation")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if that does not suit you. I feel that this call is badly named

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the existing call? It should probably also state it's deprecated in the docstring. Also, it should mention the class, not just the bare function.

def get_installation(self, owner, repo):
"""
Deprecated by get_repo_installation

:calls: `GET /repos/{owner}/{repo}/installation <https://docs.github.com/en/rest/reference/apps#get-a-repository-installation-for-the-authenticated-app>`
:param owner: str
:param repo: str
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/repos/{owner}/{repo}/installation")

def get_installations(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part I added in that file. Users having a private app with more than one installations can use this call as the main class always uses the first found installation.

"""
:calls: GET /app/installations <https://docs.github.com/en/rest/reference/apps#list-installations-for-the-authenticated-app>
:rtype: :class:`github.PaginatedList.PaginatedList[github.Installation.Installation]`
"""
return PaginatedList(
contentClass=Installation,
requester=self.__requester,
firstUrl="/app/installations",
firstParams=None,
headers=self._get_headers(),
list_item="installations",
)

def get_org_installation(self, org):
"""
:calls: `GET /orgs/{org}/installation <https://docs.github.com/en/rest/apps/apps#get-an-organization-installation-for-the-authenticated-app>`
:param org: str
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/orgs/{org}/installation")

def get_repo_installation(self, owner, repo):
"""
:calls: `GET /repos/{owner}/{repo}/installation <https://docs.github.com/en/rest/reference/apps#get-a-repository-installation-for-the-authenticated-app>`
:param owner: str
:param repo: str
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/repos/{owner}/{repo}/installation")

def get_user_installation(self, username):
"""
:calls: `GET /users/{username}/installation <https://docs.github.com/en/rest/apps/apps#get-a-user-installation-for-the-authenticated-app>`
:param username: str
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/users/{username}/installation")

def get_app_installation(self, installation_id):
"""
:calls: `GET /app/installations/{installation_id} <https://docs.github.com/en/rest/apps/apps#get-an-installation-for-the-authenticated-app>`
:param installation_id: int
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/app/installations/{installation_id}")
34 changes: 34 additions & 0 deletions github/GithubIntegration.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from typing import Union, Optional, Dict

from github.Installation import Installation
from github.InstallationAuthorization import InstallationAuthorization
from github.PaginatedList import PaginatedList
from github.Requester import Requester

class GithubIntegration:
integration_id: Union[int, str] = ...
private_key: str = ...
base_url: str = ...
jwt_expiry: int = ...
jwt_issued_at: int = ...
__requester: Requester = ...
def __init__(
self,
integration_id: Union[int, str],
private_key: str,
base_url: str = ...,
jwt_expiry: int = ...,
jwt_issued_at: int = ...,
) -> None: ...
def _get_installed_app(self, url: str) -> Installation: ...
def _get_headers(self) -> Dict[str, str]: ...
def create_jwt(self, expiration: int = ...) -> str: ...
def get_access_token(
self, installation_id: int, permissions: Optional[Dict[str, str]] = ...
) -> InstallationAuthorization: ...
def get_app_installation(self, installation_id: int) -> Installation: ...
def get_installation(self, owner: str, repo: str) -> Installation: ...
def get_installations(self) -> PaginatedList[Installation]: ...
def get_org_installation(self, org: str) -> Installation: ...
def get_repo_installation(self, owner: str, repo: str) -> Installation: ...
def get_user_installation(self, username: str) -> Installation: ...
22 changes: 22 additions & 0 deletions github/InstallationAuthorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,26 @@ def on_behalf_of(self):
"""
return self._on_behalf_of.value

@property
def permissions(self):
"""
:type: dict
"""
return self._permissions.value

@property
def repository_selection(self):
"""
:type: string
"""
return self._repository_selection.value

def _initAttributes(self):
self._token = github.GithubObject.NotSet
self._expires_at = github.GithubObject.NotSet
self._on_behalf_of = github.GithubObject.NotSet
self._permissions = github.GithubObject.NotSet
self._repository_selection = github.GithubObject.NotSet

def _useAttributes(self, attributes):
if "token" in attributes: # pragma no branch
Expand All @@ -71,3 +87,9 @@ def _useAttributes(self, attributes):
self._on_behalf_of = self._makeClassAttribute(
github.NamedUser.NamedUser, attributes["on_behalf_of"]
)
if "permissions" in attributes: # pragma no branch
self._permissions = self._makeDictAttribute(attributes["permissions"])
if "repository_selection" in attributes: # pragma no branch
self._repository_selection = self._makeStringAttribute(
attributes["repository_selection"]
)
4 changes: 4 additions & 0 deletions github/InstallationAuthorization.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ class InstallationAuthorization(NonCompletableGithubObject):
def on_behalf_of(self) -> NamedUser: ...
@property
def token(self) -> str: ...
@property
def permissions(self) -> dict: ...
@property
def repository_selection(self) -> str: ...