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 Ability to Scope Install Access Tokens to a List of Repositories #2892

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 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: 4 additions & 3 deletions doc/examples/Authentication.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ expiration timeout. The access token is refreshed automatically.

.. code-block:: python

>>> auth = Auth.AppAuth(123456, private_key).get_installation_auth(installation_id, token_permissions)
>>> g = Github(auth=auth)
>>> app_auth = Auth.AppAuth(123456, private_key)
>>> inst_auth = app_auth.get_installation_auth(installation_id, token_permissions, token_repositories)
>>> g = Github(auth=inst_auth)
>>> g.get_repo("user/repo").name
'repo'

Expand All @@ -103,7 +104,7 @@ Alternatively, the `github.Github` instance can be retrieved via `github.GithubI

>>> auth = Auth.AppAuth(123456, private_key)
>>> gi = GithubIntegration(auth=auth)
>>> g = gi.get_github_for_installation(installation_id, token_permissions)
>>> g = gi.get_github_for_installation(installation_id, token_permissions, token_repositories)
>>> g.get_repo("user/repo").name
'repo'

Expand Down
4 changes: 3 additions & 1 deletion github/AppAuthentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
################################################################################


from typing import Dict, Optional, Union
from typing import Dict, List, Optional, Union

import deprecated

Expand All @@ -49,9 +49,11 @@ def __init__(
private_key: str,
installation_id: int,
token_permissions: Optional[Dict[str, str]] = None,
token_repositories: Optional[List[Union[str, int]]] = None,
):
super().__init__(
app_auth=AppAuth(app_id, private_key),
installation_id=installation_id,
token_permissions=token_permissions,
token_repositories=token_repositories,
)
18 changes: 14 additions & 4 deletions github/Auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import time
from abc import ABC
from datetime import datetime, timedelta, timezone
from typing import TYPE_CHECKING, Dict, Optional, Union
from typing import TYPE_CHECKING, Dict, List, Optional, Union

import jwt
from requests import utils
Expand Down Expand Up @@ -201,18 +201,21 @@ def get_installation_auth(
self,
installation_id: int,
token_permissions: Optional[Dict[str, str]] = None,
token_repositories: Optional[List[Union[str, int]]] = None,
*,
requester: Optional[Requester] = None,
) -> "AppInstallationAuth":
"""
Creates a github.Auth.AppInstallationAuth instance for an installation.

:param installation_id: installation id
:param token_permissions: optional permissions
:param token_repositories: optional repositories or repository ids
:param requester: optional requester with app authentication
:return:

"""
return AppInstallationAuth(self, installation_id, token_permissions, requester)
return AppInstallationAuth(self, installation_id, token_permissions, token_repositories, requester=requester)

def create_jwt(self, expiration: Optional[int] = None) -> str:
"""
Expand Down Expand Up @@ -273,18 +276,22 @@ def __init__(
app_auth: AppAuth,
installation_id: int,
token_permissions: Optional[Dict[str, str]] = None,
token_repositories: Optional[List[Union[str, int]]] = None,
*,
requester: Optional[Requester] = None,
):
super().__init__()

assert isinstance(app_auth, AppAuth), app_auth
assert isinstance(installation_id, int), installation_id
assert token_permissions is None or isinstance(token_permissions, dict), token_permissions
assert token_repositories is None or isinstance(token_repositories, list), token_repositories
assert requester is None or isinstance(requester, Requester), requester

self._app_auth = app_auth
self._installation_id = installation_id
self._token_permissions = token_permissions
self._token_repositories = token_repositories

if requester is not None:
self.withRequester(requester)
Expand Down Expand Up @@ -316,6 +323,10 @@ def installation_id(self) -> int:
def token_permissions(self) -> Optional[Dict[str, str]]:
return self._token_permissions

@property
def token_repositories(self) -> Optional[List[Union[str, int]]]:
return self._token_repositories

@property
def token_type(self) -> str:
return "token"
Expand All @@ -335,8 +346,7 @@ def _is_expired(self) -> bool:
def _get_installation_authorization(self) -> InstallationAuthorization:
assert self.__integration is not None, "Method withRequester(Requester) must be called first"
return self.__integration.get_access_token(
self._installation_id,
permissions=self._token_permissions,
self._installation_id, permissions=self._token_permissions, repositories=self._token_repositories
)


Expand Down
34 changes: 28 additions & 6 deletions github/GithubIntegration.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,14 @@ def __init__(
)

def close(self) -> None:
"""Close connections to the server. Alternatively, use the
GithubIntegration object as a context manager:
"""
Close connections to the server. Alternatively, use the GithubIntegration object as a context manager:

.. code-block:: python

with github.GithubIntegration(...) as gi:
# do something

"""
self.__requester.close()

Expand All @@ -174,10 +175,15 @@ def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
self.close()

def get_github_for_installation(
self, installation_id: int, token_permissions: dict[str, str] | None = None
self,
installation_id: int,
token_permissions: dict[str, str] | None = None,
token_repositories: list[str | int] | None = None,
) -> github.Github:
# The installation has to authenticate as an installation, not an app
auth = self.auth.get_installation_auth(installation_id, token_permissions, self.__requester)
auth = self.auth.get_installation_auth(
installation_id, token_permissions, token_repositories, requester=self.__requester
)
return github.Github(**self.__requester.withAuth(auth).kwargs)

def _get_headers(self) -> dict[str, str]:
Expand Down Expand Up @@ -212,18 +218,34 @@ def create_jwt(self, expiration: int | None = None) -> str:
return self.auth.create_jwt(expiration)

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

if repositories is not None and not (isinstance(repositories, list) and all(isinstance(r, (str, int)) for r in repositories)):
Copy link
Author

@bakosa bakosa Mar 9, 2024

Choose a reason for hiding this comment

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

The unit test fails here because in Python, booleans are subclass of integers 🙃

We could ether be explicit about the type check (exclude subclasses..)

Suggested change
if repositories is not None and not (isinstance(repositories, list) and all(isinstance(r, (str, int)) for r in repositories)):
if repositories is not None and not (isinstance(repositories, list) and all(type(r) in (str, int) for r in repositories)):

Or change the unit test. Not sure why someone would pass in a subclassed object or str or int into the list of repositories but I will leave it up to you!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's stick to isinstance as we use it everywhere while it is not accurate, as you pointed out. Can you fix the unittest by simply using object()?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, updated the test.

raise GithubException(status=400, data={"message": "Invalid repositories"}, headers=None)

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

body = {"permissions": permissions}
body: dict[str, Any] = {"permissions": permissions}
body["permissions"] = permissions
EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
if repositories:
repo_names = [r for r in repositories if isinstance(r, str)]
repo_ids = [r for r in repositories if isinstance(r, int)]

if repo_names:
body["repositories"] = repo_names
if repo_ids:
body["repository_ids"] = repo_ids

headers, response = self.__requester.requestJsonAndCheck(
"POST",
f"/app/installations/{installation_id}/access_tokens",
Expand Down
28 changes: 26 additions & 2 deletions tests/GithubIntegration.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,34 @@ def testGetAccessTokenWithInvalidPermissions(self):
def testGetAccessTokenWithInvalidData(self):
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
github_integration = github.GithubIntegration(auth=auth)
with self.assertRaises(github.GithubException) as raisedexp:

with self.assertRaises(github.GithubException) as raisedPermissionExp:
github_integration.get_access_token(self.repo_installation_id, permissions="invalid_data")
self.assertEqual(raisedPermissionExp.exception.status, 400)

with self.assertRaises(github.GithubException) as raisedRepositoriesExp:
github_integration.get_access_token(self.repo_installation_id, repositories="invalid_data")
self.assertEqual(raisedRepositoriesExp.exception.status, 400)

EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
with self.assertRaises(github.GithubException) as raisedRepositoriesExp:
github_integration.get_access_token(self.repo_installation_id, repositories=[False])
self.assertEqual(raisedRepositoriesExp.exception.status, 400)

def testGetAccessTokenWithInvalidRepositories(self):
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
github_integration = github.GithubIntegration(auth=auth)
with self.assertRaises(github.GithubException) as raisedexp:
github_integration.get_access_token(self.org_installation_id, repositories=["invalid-repo"])

self.assertEqual(raisedexp.exception.status, 400)
self.assertEqual(raisedexp.exception.status, 422)

def testGetAccessTokenWithInvalidRepositoryIds(self):
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
github_integration = github.GithubIntegration(auth=auth)
with self.assertRaises(github.GithubException) as raisedexp:
github_integration.get_access_token(self.org_installation_id, repositories=[123456, 7891011])

self.assertEqual(raisedexp.exception.status, 422)

def testGetApp(self):
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,25 @@ None
400
[('status', '400 BAD REQUEST'), ('server', 'Github.com'), ('date', 'Mon, 24 Oct 2022 23:11:45 GMT'), ('content-type', 'application/json; charset=utf-8'), ('connection', 'keep-alive'), ('content-length', '1962'), ('etag', 'W/"b11a1c9caabe35f1de0a13e597a3022d27d2bff0694c2ccb5a65edc3b4d18837"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('vary', 'Accept'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'), ('access-control-allow-origin', '*'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('x-frame-options', 'deny'), ('x-content-type-options', 'nosniff'), ('x-xss-protection', '0'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('x-github-request-id', "E475:53DD:8B7A89E:11E38A79:63571BB0"), ('vary', 'Accept-Encoding, Accept, X-Requested-With'), ('content-security-policy', "default-src 'none'")]
{"message":"Problems parsing JSON","documentation_url":"https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}

https
POST
api.github.com
None
/app/installations/30614431/access_tokens
{'Authorization': 'Bearer jwt_removed', 'User-Agent': 'PyGithub/Python', 'Content-Type': 'application/json'}
{"permissions": {}, "repositories": "invalid_data"}
400
[('status', '400 BAD REQUEST'), ('server', 'Github.com'), ('date', 'Mon, 24 Oct 2022 23:11:45 GMT'), ('content-type', 'application/json; charset=utf-8'), ('connection', 'keep-alive'), ('content-length', '1962'), ('etag', 'W/"b11a1c9caabe35f1de0a13e597a3022d27d2bff0694c2ccb5a65edc3b4d18837"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('vary', 'Accept'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'), ('access-control-allow-origin', '*'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('x-frame-options', 'deny'), ('x-content-type-options', 'nosniff'), ('x-xss-protection', '0'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('x-github-request-id', "E475:53DD:8B7A89E:11E38A79:63571BB0"), ('vary', 'Accept-Encoding, Accept, X-Requested-With'), ('content-security-policy', "default-src 'none'")]
{"message":"Problems parsing JSON","documentation_url":"https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}

https
POST
api.github.com
None
/app/installations/30614431/access_tokens
{'Authorization': 'Bearer jwt_removed', 'User-Agent': 'PyGithub/Python', 'Content-Type': 'application/json'}
{"permissions": {}, "repositories": [], "repository_ids": "invalid_data"}
400
[('status', '400 BAD REQUEST'), ('server', 'Github.com'), ('date', 'Mon, 24 Oct 2022 23:11:45 GMT'), ('content-type', 'application/json; charset=utf-8'), ('connection', 'keep-alive'), ('content-length', '1962'), ('etag', 'W/"b11a1c9caabe35f1de0a13e597a3022d27d2bff0694c2ccb5a65edc3b4d18837"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('vary', 'Accept'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'), ('access-control-allow-origin', '*'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('x-frame-options', 'deny'), ('x-content-type-options', 'nosniff'), ('x-xss-protection', '0'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('x-github-request-id', "E475:53DD:8B7A89E:11E38A79:63571BB0"), ('vary', 'Accept-Encoding, Accept, X-Requested-With'), ('content-security-policy', "default-src 'none'")]
{"message":"Problems parsing JSON","documentation_url":"https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
https
POST
api.github.com
None
/app/installations/30614487/access_tokens
{'Authorization': 'Bearer jwt_removed', 'Accept': 'application/vnd.github.machine-man-preview+json', 'User-Agent': 'PyGithub/Python', 'Content-Type': 'application/json'}
{"permissions": {}, "repositories": ["invalid-repo"]}
422
[('status', '422 UNPROCESSABLE ENTITY'), ('server', 'Github.com'), ('date', 'Mon, 24 Oct 2022 23:11:45 GMT'), ('content-type', 'application/json; charset=utf-8'), ('connection', 'keep-alive'), ('content-length', '1962'), ('etag', 'W/"b11a1c9caabe35f1de0a13e597a3022d27d2bff0694c2ccb5a65edc3b4d18837"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('vary', 'Accept'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'), ('access-control-allow-origin', '*'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('x-frame-options', 'deny'), ('x-content-type-options', 'nosniff'), ('x-xss-protection', '0'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('x-github-request-id', "E475:53DD:8B7A89E:11E38A79:63571BB0"), ('vary', 'Accept-Encoding, Accept, X-Requested-With'), ('content-security-policy', "default-src 'none'")]
{"message": "There is at least one repository that does not exist or is not accessible to the parent installation.", "documentation_url": "https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
https
POST
api.github.com
None
/app/installations/30614487/access_tokens
{'Authorization': 'Bearer jwt_removed', 'Accept': 'application/vnd.github.machine-man-preview+json', 'User-Agent': 'PyGithub/Python', 'Content-Type': 'application/json'}
{"permissions": {}, "repository_ids": [123456, 7891011]}
422
[('status', '422 UNPROCESSABLE ENTITY'), ('server', 'Github.com'), ('date', 'Mon, 24 Oct 2022 23:11:45 GMT'), ('content-type', 'application/json; charset=utf-8'), ('connection', 'keep-alive'), ('content-length', '1962'), ('etag', 'W/"b11a1c9caabe35f1de0a13e597a3022d27d2bff0694c2ccb5a65edc3b4d18837"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('vary', 'Accept'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'), ('access-control-allow-origin', '*'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('x-frame-options', 'deny'), ('x-content-type-options', 'nosniff'), ('x-xss-protection', '0'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('x-github-request-id', "E475:53DD:8B7A89E:11E38A79:63571BB0"), ('vary', 'Accept-Encoding, Accept, X-Requested-With'), ('content-security-policy', "default-src 'none'")]
{"message": "There is at least one repository that does not exist or is not accessible to the parent installation.", "documentation_url": "https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}