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 22 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
9 changes: 7 additions & 2 deletions github/ApplicationOAuth.pyi
Expand Up @@ -12,6 +12,11 @@ class ApplicationOAuth(NonCompletableGithubObject):
@property
def client_secret(self) -> str: ...
def get_login_url(
self, redirect_uri: Optional[str], state: Optional[str], login: Optional[str]
self,
redirect_uri: Optional[str] = ...,
state: Optional[str] = ...,
login: Optional[str] = ...,
) -> str: ...
def get_access_token(self, code: str, state: Optional[str]) -> AccessToken: ...
def get_access_token(
self, code: str, state: Optional[str] = ...
) -> AccessToken: ...
3 changes: 2 additions & 1 deletion github/AuthenticatedUser.pyi
@@ -1,5 +1,5 @@
from datetime import datetime
from typing import Any, Dict, List, Optional, Union, NamedTuple
from typing import Any, Dict, List, NamedTuple, Union

from github.Authorization import Authorization
from github.Event import Event
Expand Down Expand Up @@ -85,6 +85,7 @@ class AuthenticatedUser(CompletableGithubObject):
allow_squash_merge: Union[bool, _NotSetType] = ...,
allow_merge_commit: Union[bool, _NotSetType] = ...,
allow_rebase_merge: Union[bool, _NotSetType] = ...,
delete_branch_on_merge: Union[bool, _NotSetType] = ...,
) -> Repository: ...
@property
def created_at(self) -> datetime: ...
Expand Down
2 changes: 0 additions & 2 deletions github/Branch.pyi
Expand Up @@ -42,8 +42,6 @@ class Branch(NonCompletableGithubObject):
strict: Union[_NotSetType, bool] = ...,
contexts: Union[List[str], _NotSetType] = ...,
) -> None: ...
def edit_team_push_restrictions(self, *teams: str) -> None: ...
def edit_user_push_restrictions(self, *users: str) -> None: ...
def get_admin_enforcement(self) -> bool: ...
def get_protection(self) -> BranchProtection: ...
def get_required_pull_request_reviews(self) -> RequiredPullRequestReviews: ...
Expand Down
9 changes: 6 additions & 3 deletions github/CheckSuite.pyi
@@ -1,10 +1,10 @@
from datetime import datetime
from typing import Any, Dict, List
from typing import Any, Dict, List, Union

from github.CheckRun import CheckRun
from github.GitCommit import GitCommit
from github.GithubApp import GithubApp
from github.GithubObject import CompletableGithubObject
from github.GithubObject import CompletableGithubObject, _NotSetType
from github.PaginatedList import PaginatedList
from github.PullRequest import PullRequest
from github.Repository import Repository
Expand Down Expand Up @@ -47,5 +47,8 @@ class CheckSuite(CompletableGithubObject):
def url(self) -> str: ...
def rerequest(self) -> bool: ...
def get_check_runs(
self, check_name: str, status: str, filter: str
self,
check_name: Union[str, _NotSetType] = ...,
status: Union[str, _NotSetType] = ...,
filter: Union[str, _NotSetType] = ...,
) -> PaginatedList[CheckRun]: ...
4 changes: 2 additions & 2 deletions github/Commit.pyi
Expand Up @@ -45,8 +45,8 @@ class Commit(CompletableGithubObject):
def files(self) -> List[File]: ...
def get_check_suites(
self,
app_id: Union[_NotSetType, int],
check_name: Union[_NotSetType, str],
app_id: Union[_NotSetType, int] = ...,
check_name: Union[_NotSetType, str] = ...,
) -> PaginatedList[CheckSuite]: ...
def get_combined_status(self) -> CommitCombinedStatus: ...
def get_comments(self) -> PaginatedList[CommitComment]: ...
Expand Down
12 changes: 12 additions & 0 deletions github/Consts.py
Expand Up @@ -131,3 +131,15 @@

# 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
JWT_EXPIRY = 60
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
198 changes: 198 additions & 0 deletions github/GithubIntegration.py
@@ -0,0 +1,198 @@
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, UnknownObjectException
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.JWT_EXPIRY,
):
"""
:param integration_id: int
:param private_key: string
:param base_url: string
:param jwt_expiry: int
"""
assert isinstance(integration_id, (int, str)), integration_id
assert isinstance(private_key, str), private_key
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
assert isinstance(base_url, str), base_url
assert isinstance(jwt_expiry, int), jwt_expiry
assert 15 <= jwt_expiry <= 600, jwt_expiry
dblanchette marked this conversation as resolved.
Show resolved Hide resolved

self.base_url = base_url
self.integration_id = integration_id
self.private_key = private_key
self.jwt_expiry = jwt_expiry
self.__requester = Requester(
login_or_token=None,
password=None,
jwt=self.create_jwt(),
app_id=None,
app_private_key=None,
app_installation_id=None,
app_token_permissions=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`
"""
try:
headers, response = self.__requester.requestJsonAndCheck(
"GET", url, headers=self._get_headers()
)
except UnknownObjectException:
raise
except GithubException:
raise
return Installation(
requester=self.__requester,
headers=headers,
attributes=response,
completed=True,
)

def create_jwt(self):
"""
Creates a signed JWT, valid for 60 seconds by default. Could be set to a maximum of 600 seconds.
https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#authenticating-as-a-github-app

dblanchette marked this conversation as resolved.
Show resolved Hide resolved
:return string:
"""
now = int(time.time())
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
payload = {"iat": now, "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}
try:
headers, response = self.__requester.requestJsonAndCheck(
"POST",
f"/app/installations/{installation_id}/access_tokens",
input=body,
)
except UnknownObjectException:
s-t-e-v-e-n-k marked this conversation as resolved.
Show resolved Hide resolved
raise
except GithubException:
raise

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):
"""
: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}")
26 changes: 26 additions & 0 deletions github/GithubIntegration.pyi
@@ -0,0 +1,26 @@
from typing import Union, Optional

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

class GithubIntegration:
def __init__(
self,
integration_id: Union[int, str],
private_key: str,
base_url: str = ...,
jwt_expiry: int = ...,
) -> None: ...
def _get_installed_app(self, url: str) -> Installation: ...
def _get_headers(self) -> dict: ...
dblanchette marked this conversation as resolved.
Show resolved Hide resolved
def create_jwt(self, expiration: int = ...) -> str: ...
def get_access_token(
self, installation_id: int, permissions: Optional[dict] = ...
) -> 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: ...
9 changes: 3 additions & 6 deletions github/GithubObject.pyi
@@ -1,11 +1,6 @@
from typing import Any, Callable, Dict, List, Optional, Type, Union

from github.Commit import Commit
from github.GistFile import GistFile
from github.GitRelease import GitRelease
from github.NamedUser import NamedUser
from github.Organization import Organization
from github.PullRequestReview import PullRequestReview
from github.Requester import Requester

class GithubObject:
Expand Down Expand Up @@ -108,7 +103,7 @@ class CompletableGithubObject(GithubObject):
def _completeIfNotSet(
self, value: Union[_ValuedAttribute, _BadAttribute, _NotSetType]
) -> None: ...
def update(self) -> bool: ...
def update(self, additional_headers: Optional[Dict[str, str]] = ...) -> bool: ...

class _BadAttribute:
def __init__(
Expand All @@ -120,5 +115,7 @@ class _BadAttribute:
class _NotSetType:
def __repr__(self) -> str: ...

NotSet: _NotSetType

class _ValuedAttribute:
def __init__(self, value: Any) -> None: ...
22 changes: 22 additions & 0 deletions github/InstallationAuthorization.py
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
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: ...