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

GitHub App authentication #1830

Merged
merged 24 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
22d47cd
add config options for github_app_*
cbartz Jul 31, 2024
cde2475
add github app auth
cbartz Jul 31, 2024
ec37ec2
add/adapt integration tests for github app auth
cbartz Jul 31, 2024
04a6369
fix scope of fixture
cbartz Jul 31, 2024
17ef3e5
pass env vars in tox
cbartz Jul 31, 2024
db7d14c
add unit test to ensure coverage
cbartz Jul 31, 2024
4836602
change tests to run additionally for github app auth
cbartz Jul 31, 2024
5e1dc45
add reason to skipif
cbartz Jul 31, 2024
c00d4fc
Merge branch 'refs/heads/main' into feat/github-app-auth-ISD2110
cbartz Jul 31, 2024
bccad20
skip some tests for github app auth
cbartz Jul 31, 2024
c77a572
fix if condition
cbartz Jul 31, 2024
f5f619a
bump minor version
cbartz Jul 31, 2024
2148cdd
update docs
cbartz Aug 1, 2024
a61108c
Kick off CI build
cbartz Aug 1, 2024
5a9ce51
try out github app on cbartz-org/cbartz-repo-policy-compliance-tests
cbartz Aug 1, 2024
2792fee
update README on test repository requirements
cbartz Aug 1, 2024
20dc596
skip non-applicable tests
cbartz Aug 1, 2024
3b54797
Revert "try out github app on cbartz-org/cbartz-repo-policy-complianc…
cbartz Aug 1, 2024
5938fd3
cleanup
cbartz Aug 1, 2024
d1d6742
update docs
cbartz Aug 1, 2024
a058f19
use AuthMode enum and remove asserts
cbartz Aug 2, 2024
6db8f17
try out github app on cbartz-org/cbartz-repo-policy-compliance-tests
cbartz Aug 1, 2024
90b735a
Revert "try out github app on cbartz-org/cbartz-repo-policy-complianc…
cbartz Aug 2, 2024
eb4c64b
Merge branch 'refs/heads/main' into feat/github-app-auth-ISD2110
cbartz Aug 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ jobs:
- name: Run tests (unit + integration)
id: run-tests
env:
GITHUB_TOKEN: ${{ secrets.PERSONAL_GITHUB_TOKEN }}
AUTH_GITHUB_TOKEN: ${{ secrets.PERSONAL_GITHUB_TOKEN }}
AUTH_GITHUB_APP_ID : ${{ secrets.TEST_GITHUB_APP_ID }}
AUTH_GITHUB_APP_INSTALLATION_ID : ${{ secrets.TEST_GITHUB_APP_INSTALLATION_ID }}
AUTH_GITHUB_APP_PRIVATE_KEY : ${{ secrets.TEST_GITHUB_APP_PRIVATE_KEY }}
run: |
# Ensure that stdout appears as normal and redirect to file and exit depends on exit code of first command
STDOUT_LOG=$(mktemp --suffix=stdout.log)
Expand Down
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,14 @@ failing check to be used for testing purposes.
There are two types of test: the application test and the charm test.

### Application tests
To run the application tests, the `GITHUB_TOKEN` environment variable must be set. This
To run the application tests, the `AUTH_GITHUB_TOKEN` environment variable must be set. This
should be a token of a user with full repo permissions for the test repository.
You can also pass in `AUTH_APP_ID`, `AUTH_INSTALLATION_ID`, and `AUTH_PRIVATE_KEY`
to test the authentication using Github App Auth. In that case, the tests will randomly select
either the token or the app auth to run the tests. Note that the Github app should be installed
in the test repository organisation/user namespace, with access granted to the test repository
and in the user namespace that owns the Github token with access to the forked repository.
cbartz marked this conversation as resolved.
Show resolved Hide resolved

The command `tox -e test` can be used to run all tests, which are primarily integration tests.
You can also select the repository against which to run the tests by setting
the `--repository` flag. The tests will fork the repository and create PRs against it.
Expand All @@ -66,6 +72,8 @@ bot to test things like comments from a user with no write permissions or above.
GitHub actions should have access to the GitHub token via a secret
called `PERSONAL_GITHUB_TOKEN`. It is recommended to use either a fine-grained PAT or a
token that is short-lived, e.g. 7 days. When it expires, a new token must be set.
For the GitHub App Auth, the `TEST_GITHUB_APP_ID`, `TEST_GIHUB_APP_INSTALLATION_ID`, and `TEST_GITHUB_APPP_RIVATE_KEY`
should be set as secrets.

### Charm tests

Expand Down
26 changes: 26 additions & 0 deletions charm/charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,35 @@ config:
write and higher permissions for the repository to run jobs from forks.
type: boolean
default: false
github_app_id:
description: >-
The app or client ID of the GitHub App to use for communication with GitHub.
If provided, the other github_app_* options must also be provided.
The Github App needs to have read permission for Administration. If private repositories
are checked, the Github App does also need read permission for Contents and Pull request.
Either this or the github_token must be provided.
type: string
github_app_installation_id:
description: >-
The installation ID of the GitHub App to use for communication with GitHub.
If provided, the other github_app_* options must also be provided.
The Github App needs to have read permission for Administration. If private repositories
are checked, the Github App does also need read permission for Contents and Pull request.
Either this or the github_token must be provided.
type: string
github_app_private_key:
# this will become a juju user secret once paas-app-charmer supports it
description: >-
The private key of the GitHub App to use for communication with GitHub.
If provided, the other github_app_* options must also be provided.
The Github App needs to have read permission for Administration. If private repositories
are checked, the Github App does also need read permission for Contents and Pull request.
Either this or the github_token must be provided.
type: string
github_token:
description: >-
The token to use for communication with GitHub. This can be a PAT (with repo scope)
or a fine-grained token with read permission for Administration. If private repositories
are checked, the fine-grained token does also need read permission for Contents and
Pull request.
Either this or the GitHub App configuration must be provided.
146 changes: 136 additions & 10 deletions repo_policy_compliance/github_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from urllib import parse

from github import BadCredentialsException, Github, GithubException, RateLimitExceededException
from github.Auth import Token
from github.Auth import AppAuth, AppInstallationAuth, Auth, Token
from github.Branch import Branch
from github.Repository import Repository
from urllib3 import Retry
Expand All @@ -27,6 +27,24 @@

# Bandit thinks this constant is the real Github token
GITHUB_TOKEN_ENV_NAME = "GITHUB_TOKEN" # nosec
GITHUB_APP_ID_ENV_NAME = "GITHUB_APP_ID"
GITHUB_APP_INSTALLATION_ID_ENV_NAME = "GITHUB_APP_INSTALLATION_ID"
GITHUB_APP_PRIVATE_KEY_ENV_NAME = "GITHUB_APP_PRIVATE_KEY"

MISSING_GITHUB_CONFIG_ERR_MSG = (
f"Either the {GITHUB_TOKEN_ENV_NAME} or not all of {GITHUB_APP_ID_ENV_NAME},"
f" {GITHUB_APP_INSTALLATION_ID_ENV_NAME}, {GITHUB_APP_PRIVATE_KEY_ENV_NAME} "
f"environment variables were not provided or empty, "
"it is needed for interactions with GitHub, "
)
NOT_ALL_GITHUB_APP_CONFIG_ERR_MSG = (
f"Not all of {GITHUB_APP_ID_ENV_NAME}, {GITHUB_APP_INSTALLATION_ID_ENV_NAME},"
f" {GITHUB_APP_PRIVATE_KEY_ENV_NAME} environment variables were provided, "
)
# the following is no hardcoded password
PROVIDED_GITHUB_TOKEN_AND_APP_CONFIG_ERR_MSG = ( # nosec
"Provided github app config and github token, only one of them should be provided, "
)


def get() -> Github:
Expand All @@ -36,14 +54,10 @@ def get() -> Github:
A GitHub client that is configured with a token from the environment.

Raises:
ConfigurationError: If the GitHub token environment variable is not provided or empty.
"""
github_token = os.getenv(GITHUB_TOKEN_ENV_NAME) or os.getenv(f"FLASK_{GITHUB_TOKEN_ENV_NAME}")
if not github_token:
raise ConfigurationError(
f"The {GITHUB_TOKEN_ENV_NAME} environment variable was not provided or empty, "
f"it is needed for interactions with GitHub, got: {github_token!r}"
)
ConfigurationError: If the GitHub auth config is not valid.
""" # noqa: DCO051 error raised is useful to know for the user of the public interface
auth = _get_auth()

# Only retry on 5xx and only retry once after 20 secs
retry_config = Retry(
total=1,
Expand All @@ -53,7 +67,119 @@ def get() -> Github:
raise_on_status=False,
raise_on_redirect=False,
)
return Github(auth=Token(github_token), retry=retry_config)
return Github(auth=auth, retry=retry_config)


def _get_auth() -> Auth:
"""Get a GitHub auth object.

Returns:
A GitHub auth object that is configured with a token from the environment.
"""
github_token = os.getenv(GITHUB_TOKEN_ENV_NAME) or os.getenv(f"FLASK_{GITHUB_TOKEN_ENV_NAME}")
github_app_id = os.getenv(GITHUB_APP_ID_ENV_NAME) or os.getenv(
f"FLASK_{GITHUB_APP_ID_ENV_NAME}"
)
github_app_installation_id_str = os.getenv(GITHUB_APP_INSTALLATION_ID_ENV_NAME) or os.getenv(
f"FLASK_{GITHUB_APP_INSTALLATION_ID_ENV_NAME}"
)
github_app_private_key = os.getenv(GITHUB_APP_PRIVATE_KEY_ENV_NAME) or os.getenv(
f"FLASK_{GITHUB_APP_PRIVATE_KEY_ENV_NAME}"
)

_ensure_either_github_token_or_app_config(
github_token=github_token,
github_app_id=github_app_id,
github_app_installation_id_str=github_app_installation_id_str,
github_app_private_key=github_app_private_key,
)
cbartz marked this conversation as resolved.
Show resolved Hide resolved

auth: Auth
# we use asserts here to make mypy happy, we have already checked that the values are not None
# we know asserts are optimised away in production, so ignore bandit warnings
if github_app_id:
assert github_app_installation_id_str is not None # nosec
assert github_app_private_key is not None # nosec
auth = _get_github_app_installation_auth(
amandahla marked this conversation as resolved.
Show resolved Hide resolved
github_app_id=github_app_id,
github_app_installation_id_str=github_app_installation_id_str,
github_app_private_key=github_app_private_key,
)
else:
assert github_token is not None # nosec
auth = Token(github_token)

return auth


def _ensure_either_github_token_or_app_config(
github_token: str | None,
github_app_id: str | None,
github_app_installation_id_str: str | None,
github_app_private_key: str | None,
) -> None:
"""Ensure that only one of github_token or github app config is provided and is valid.

Args:
github_token: The GitHub token.
github_app_id: The GitHub App ID or Client ID.
github_app_installation_id_str: The GitHub App Installation ID as a string.
github_app_private_key: The GitHub App private key.

Raises:
ConfigurationError: If both github_token and github app config are provided.
"""
if not github_token and not (
github_app_id or github_app_installation_id_str or github_app_private_key
):
raise ConfigurationError(
f"{MISSING_GITHUB_CONFIG_ERR_MSG}"
f"got: {github_token!r}, {github_app_id!r},"
f" {github_app_installation_id_str!r}, {github_app_private_key!r}"
)
if github_token and (
github_app_id or github_app_installation_id_str or github_app_private_key
):
raise ConfigurationError(
f"{PROVIDED_GITHUB_TOKEN_AND_APP_CONFIG_ERR_MSG}"
f"got: {github_token!r}, {github_app_id!r}, {github_app_installation_id_str!r},"
f" {github_app_private_key!r}"
)

if github_app_id or github_app_installation_id_str or github_app_private_key:
if not (github_app_id and github_app_installation_id_str and github_app_private_key):
raise ConfigurationError(
f"{NOT_ALL_GITHUB_APP_CONFIG_ERR_MSG}"
f"got: {github_app_id!r}, {github_app_installation_id_str!r}, "
f"{github_app_private_key!r}"
)


def _get_github_app_installation_auth(
github_app_id: str, github_app_installation_id_str: str, github_app_private_key: str
) -> AppInstallationAuth:
"""Get a GitHub App Installation Auth object.

Args:
github_app_id: The GitHub App ID or Client ID.
github_app_installation_id_str: The GitHub App Installation ID as a string.
github_app_private_key: The GitHub App private key.

Returns:
A GitHub App Installation Auth object.

Raises:
ConfigurationError: If the GitHub App Installation Auth config is not valid.
"""
try:
github_app_installation_id = int(github_app_installation_id_str)
except ValueError as exc:
raise ConfigurationError(
f"Invalid github app installation id {github_app_installation_id_str!r}, "
f"it should be an integer."
) from exc
app_auth = AppAuth(app_id=github_app_id, private_key=github_app_private_key)
return AppInstallationAuth(app_auth=app_auth, installation_id=github_app_installation_id)


def inject(func: Callable[Concatenate[Github, P], R]) -> Callable[P, R]:
Expand Down
18 changes: 12 additions & 6 deletions src-docs/github_client.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ Module for GitHub client.
**Global Variables**
---------------
- **GITHUB_TOKEN_ENV_NAME**
- **GITHUB_APP_ID_ENV_NAME**
- **GITHUB_APP_INSTALLATION_ID_ENV_NAME**
- **GITHUB_APP_PRIVATE_KEY_ENV_NAME**
- **MISSING_GITHUB_CONFIG_ERR_MSG**
- **NOT_ALL_GITHUB_APP_CONFIG_ERR_MSG**
- **PROVIDED_GITHUB_TOKEN_AND_APP_CONFIG_ERR_MSG**

---

<a href="../repo_policy_compliance/github_client.py#L32"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../repo_policy_compliance/github_client.py#L50"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `get`

Expand All @@ -30,12 +36,12 @@ Get a GitHub client.

**Raises:**

- <b>`ConfigurationError`</b>: If the GitHub token environment variable is not provided or empty.
- <b>`ConfigurationError`</b>: If the GitHub auth config is not valid.


---

<a href="../repo_policy_compliance/github_client.py#L59"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../repo_policy_compliance/github_client.py#L185"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `inject`

Expand All @@ -59,7 +65,7 @@ Injects a GitHub client as the first argument to a function.

---

<a href="../repo_policy_compliance/github_client.py#L112"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../repo_policy_compliance/github_client.py#L238"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `get_collaborators`

Expand Down Expand Up @@ -89,7 +95,7 @@ Get collaborators with a given affiliation and permission.

---

<a href="../repo_policy_compliance/github_client.py#L147"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../repo_policy_compliance/github_client.py#L273"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `get_branch`

Expand Down Expand Up @@ -119,7 +125,7 @@ Get the branch for the check.

---

<a href="../repo_policy_compliance/github_client.py#L162"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../repo_policy_compliance/github_client.py#L288"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `get_collaborator_permission`

Expand Down
Loading
Loading