From 9de9bf8905b5256e8702b44ee893cfbc257395b7 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Fri, 16 Oct 2026 11:39:02 +0200 Subject: [PATCH] Support GitHub App auth --- changelog/+app_auth.added.md | 1 + changelog/+revoke_token.added.md | 1 + docs/topics/configuration.md | 113 +++++++- src/saltext/github/modules/github.py | 37 +++ src/saltext/github/utils/gh/client.py | 163 +++++++++++- src/saltext/github/utils/gh/profile.py | 113 +++++++- .../modules/github_repo/test_app_auth.py | 241 ++++++++++++++++++ .../modules/github_repo/test_app_token.py | 6 +- tests/support/github/__init__.py | 12 +- tests/support/github/base.py | 97 ++++++- tests/support/github/routes/__init__.py | 2 + tests/support/github/routes/app.py | 57 +++++ tests/unit/utils/conftest.py | 19 ++ tests/unit/utils/test_cache.py | 3 +- tests/unit/utils/test_client.py | 130 ++++++++++ tests/unit/utils/test_profile.py | 129 ++++++++++ 16 files changed, 1090 insertions(+), 34 deletions(-) create mode 100644 changelog/+app_auth.added.md create mode 100644 changelog/+revoke_token.added.md create mode 100644 tests/functional/modules/github_repo/test_app_auth.py create mode 100644 tests/support/github/routes/app.py create mode 100644 tests/unit/utils/conftest.py create mode 100644 tests/unit/utils/test_profile.py diff --git a/changelog/+app_auth.added.md b/changelog/+app_auth.added.md new file mode 100644 index 00000000..bfca0d4b --- /dev/null +++ b/changelog/+app_auth.added.md @@ -0,0 +1 @@ +Added native GitHub App authentication via the {ghconf}`app_id`, {ghconf}`private_key`/{ghconf}`private_key_path` and optional {ghconf}`installation_id` and {ghconf}`permissions` profile parameters. Installation access tokens are minted and refreshed automatically (optionally restricted to a subset of the app's granted permissions), the app's installation on {ghconf}`owner` is discovered when not pinned, and the connection identity derives from the app's identity instead of its short-lived tokens, keeping the persistent {ghconf}`cache` effective across token rotation (e.g. in CI) diff --git a/changelog/+revoke_token.added.md b/changelog/+revoke_token.added.md new file mode 100644 index 00000000..51a5b87a --- /dev/null +++ b/changelog/+revoke_token.added.md @@ -0,0 +1 @@ +Added `github.revoke_app_token` for immediately revoking a GitHub App profile's current installation access token (otherwise valid for one hour), e.g. as the last step of an ephemeral CI run diff --git a/docs/topics/configuration.md b/docs/topics/configuration.md index 782d7f37..541d2f37 100644 --- a/docs/topics/configuration.md +++ b/docs/topics/configuration.md @@ -8,7 +8,8 @@ All modules provided by this extension read their configuration from profile : A profile is a mapping under a top-level key in the minion configuration, minion pillar or master configuration - (grains work also, but should not contain secrets such as the required {ghconf}`token`). + (grains work also, but should not contain secrets such as + {ghconf}`token` or {ghconf}`private_key`). Functions and states use the profile named `github` by default and accept a `profile` argument to select a different one. @@ -21,7 +22,9 @@ The following keys can be specified in the profile: :::{ghconf} token ::: `token` -: **Required**. Access token to authenticate with. +: Access token to authenticate with. Either this or GitHub App + credentials ({ghconf}`app_id` + a private key) are required, + mutually exclusively. :::{ghconf} owner ::: @@ -33,6 +36,7 @@ The following keys can be specified in the profile: Needs to be set explicitly when the token does not have an associated resolvable user account such as GitHub App installation tokens. + Required for GitHub App ({ghconf}`app_id`) profiles. :::{versionadded} 1.1.0 Renamed from `org_name`. The new {py:mod}`github_repo ` @@ -52,8 +56,12 @@ The following keys can be specified in the profile: `cache` : Keep a persistent cross-process cache for specific API data, stored in the minion's {conf_minion}`cachedir`. - All entries are namespaced by a token/API URL-derived identity, + All entries are namespaced by a credential/API URL-derived identity, hence rotated credentials never serve stale data. + For GitHub App profiles, the identity derives from the app's identity + ({ghconf}`app_id` + {ghconf}`owner` + {ghconf}`permissions`) rather + than its short-lived installation tokens, so the cache survives token + rotation. The data that is cached is either static or refreshed as soon as it changes. Defaults to true. @@ -65,6 +73,7 @@ The following keys can be specified in the profile: * {ghconf}`token`-associated user handle (never changes) * account name => account type lookups (should never change) + * discovered GitHub App installation IDs (see {ghconf}`app_id`) When the {py:mod}`github resource type ` is configured with {ghconf}`grains_files `, file contents @@ -110,6 +119,7 @@ my_user: salt-call github_repo.list profile=my_user ``` +(secret-management)= ## Secret management Profile values containing [SDB URIs](https://docs.saltproject.io/en/latest/topics/sdb/index.html) @@ -129,6 +139,103 @@ github: owner: my_organization ``` +## GitHub App authentication + +:::{versionadded} 1.1.0 +::: + +Instead of a static {ghconf}`token`, a profile can authenticate as a +[GitHub App installation](https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/authenticating-as-a-github-app-installation). + +:::{hint} +Two short-lived credentials are involved: an app JWT (signed with the +private key, freshly minted per app-scoped request, only used to +discover installations and mint tokens) and the installation access +tokens performing the actual API calls, which are valid for one hour +(fixed by GitHub) and never persisted by this extension. For explicit +cleanup – e.g. as the last step of an ephemeral CI run – +{py:func}`github.revoke_app_token ` +revokes the current token immediately. +::: + +:::{ghconf} app_id +::: +`app_id` +: App ID (or client ID) of a GitHub App to authenticate as, together + with exactly one of {ghconf}`private_key` or {ghconf}`private_key_path` + and a required {ghconf}`owner`. Installation access tokens are minted + (and refreshed before expiry) automatically, removing the need to + pass short-lived tokens into the profile (e.g. via + `actions/create-github-app-token` in CI). + + The app's installation on {ghconf}`owner` is discovered via the API + (an organization, falling back to user accounts) unless pinned with + {ghconf}`installation_id`. The discovered ID is kept in the + persistent {ghconf}`cache`, when enabled. + + Minted tokens carry all of the app's granted permissions on all + repositories the installation covers, unless restricted via + {ghconf}`permissions` (per token) or by installing the app on + selected repositories only (per installation). + + :::{versionadded} 1.1.0 + ::: + +:::{ghconf} private_key +::: +`private_key` +: Private key of the GitHub App ({ghconf}`app_id`) in PEM format. + Like all profile values, it can be an [SDB URI](#secret-management), + avoiding plain text keys in the configuration. + + :::{versionadded} 1.1.0 + ::: + +:::{ghconf} private_key_path +::: +`private_key_path` +: Path to a local file containing the private key of the GitHub App + ({ghconf}`app_id`) in PEM format, as an alternative to + {ghconf}`private_key`. + + :::{versionadded} 1.1.0 + ::: + +:::{ghconf} installation_id +::: +`installation_id` +: ID of the GitHub App's ({ghconf}`app_id`) installation on + {ghconf}`owner`. Optional – when unset, it is discovered via the API. + + :::{versionadded} 1.1.0 + ::: + +:::{ghconf} permissions +::: +`permissions` +: Restrict the installation access tokens minted for this GitHub App + ({ghconf}`app_id`) profile to a subset of the app's granted + permissions – a mapping of permission name to access level, e.g. + `contents: read`. Defaults to all granted permissions. + + Allows profiles with different privileges to share a single app, + e.g. a read-only profile for CI validation runs (`test=True`) + next to an unrestricted one for applying: + + ```yaml + github_readonly: + app_id: 123456 + private_key: sdb://osenv/GITHUB_APP_PRIVATE_KEY + owner: my_organization + permissions: + metadata: read + contents: read + administration: read + ``` + + :::{versionadded} 1.1.0 + ::: + (github-resource-configuration)= ## Repository resource declaration diff --git a/src/saltext/github/modules/github.py b/src/saltext/github/modules/github.py index ac9723eb..c25ebcc0 100644 --- a/src/saltext/github/modules/github.py +++ b/src/saltext/github/modules/github.py @@ -164,6 +164,43 @@ def rate_limit(profile="github"): return gh_client.rate_limit(_get_conn(profile)) +def revoke_app_token(profile="github"): + """ + Revoke the GitHub App installation access token the profile + currently authenticates with and drop the associated cached + connections, ensuring a subsequent call mints a fresh token. + Only valid for GitHub App (:ghconf:`app_id`) profiles. + + Minted tokens are valid for one hour (fixed by GitHub) and are + never persisted by this extension, so revocation is optional + hygiene – useful e.g. as the last step of an ephemeral CI run. + + .. versionadded:: 1.1.0 + + CLI Example: + + .. code-block:: bash + + salt-call github.revoke_app_token + + profile + Profile configuration name to use. Defaults to ``github``. + """ + conn = _get_conn(profile) + gh_client.revoke_installation_token(conn) + # The revoked token stays attached to the connection's auth + # (PyGithub only re-mints on expiry, not on auth failures) - + # evict all context entries sharing it so later invocations + # build a fresh connection. + for key in list(__context__): + if key == f"github.conn.{conn.ident}" or ( + key.startswith("github.profile.") + and getattr(__context__[key], "ident", None) == conn.ident + ): + __context__.pop(key) + return True + + def account_info(name=None, profile="github"): """ Return the public profile of any user or organization account diff --git a/src/saltext/github/utils/gh/client.py b/src/saltext/github/utils/gh/client.py index 2246feb1..157c7d1b 100644 --- a/src/saltext/github/utils/gh/client.py +++ b/src/saltext/github/utils/gh/client.py @@ -69,14 +69,126 @@ def get_connection_ident(token, api_url): return hashlib.sha256(f"{token}:{api_url}".encode()).hexdigest()[:16] +def get_app_connection_ident(app_id, owner, api_url, permissions=None): + """ + Render the internal identity for a GitHub App's installation on an + owner, the app-authenticated counterpart of + :func:`get_connection_ident`. Derived from the app's identity + rather than its short-lived installation tokens, so it stays stable + across token minting/rotation (keeping e.g. persistent cache banks + alive between processes). + + app_id + App ID (or client ID) from profile. + + owner + Owner (organization or user) login the app installation targets. + + api_url + API URL from profile. + + permissions + Optional permission restrictions the minted tokens are subject + to. Profiles restricted differently must not share connections + (and thus identities), their tokens are not interchangeable. + """ + perms = "" + if permissions: + perms = ":" + json.dumps(permissions, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(f"app:{app_id}:{owner.lower()}:{api_url}{perms}".encode()).hexdigest()[ + :16 + ] + + +def _app_auth(app_id, private_key): + """ + Build PyGithub app (JWT) auth, surfacing invalid credentials early + with a helpful error. + """ + try: + auth = github.Auth.AppAuth(app_id, private_key) + # JWT creation (and thus private key parsing) usually happens + # lazily during the first request - fail fast instead. + auth.create_jwt() + except Exception as err: # pylint: disable=broad-except + raise CommandExecutionError( + f"Failed initializing GitHub App authentication: {err}" + ) from err + return auth + + +def build_app_auth(app_id, private_key, installation_id, permissions=None): + """ + Return PyGithub auth for a GitHub App installation. It mints + installation tokens on demand and refreshes them before expiry. + + app_id + App ID (or client ID) of the GitHub App. + + private_key + Private key of the GitHub App in PEM format. + + installation_id + ID of the app's installation on the target owner, see + :func:`resolve_app_installation_id`. + + permissions + Optional mapping of permission name to access level the minted + tokens are restricted to. Defaults to all the app's granted + permissions. + """ + return _app_auth(app_id, private_key).get_installation_auth( + int(installation_id), token_permissions=permissions or None + ) + + +def resolve_app_installation_id(app_id, private_key, owner, api_url): + """ + Resolve the ID of a GitHub App's installation on an owner + (an organization, falling back to user accounts). + + app_id + App ID (or client ID) of the GitHub App. + + private_key + Private key of the GitHub App in PEM format. + + owner + Owner (organization or user) login the app installation targets. + + api_url + Base URL of the GitHub API. + """ + integration = github.GithubIntegration(auth=_app_auth(app_id, private_key), base_url=api_url) + try: + try: + return integration.get_org_installation(owner).id + except UnknownObjectException: + return integration.get_user_installation(owner).id + except UnknownObjectException as err: + raise GitHubNotFoundError( + f"GitHub App {app_id} does not appear to be installed on '{owner}'", + status=err.status, + data=err.data, + ) from err + except github.GithubException as err: + raise GitHubApiError( + _error_message(err.status, err.data), status=err.status, data=err.data + ) from err + finally: + integration.close() + + @dataclass(slots=True) class GitHubConnection: """ An authenticated PyGithub client bound to a default repository owner (an organization or user), together with caches for API responses. - token - Personal access token to authenticate with. + auth + Personal access token to authenticate with, or a PyGithub + ``Auth`` instance (e.g. the result of :func:`build_app_auth`). + Passing the latter requires ``ident``. owner Login of the organization or user this connection is scoped to. @@ -88,29 +200,36 @@ class GitHubConnection: Base URL of the GitHub API. Set this when targeting a GitHub Enterprise Server instance. Defaults to the public GitHub API. + ident + Identity of the connection's credentials/endpoint, safe to + expose (e.g. as a persistent cache namespace). Defaults to + :func:`get_connection_ident` for token auth; required for + ``Auth`` instances (whose tokens can be short-lived, see + :func:`get_app_connection_ident`). + store Optional :class:`~saltext.github.utils.gh.cache.PersistentCache` for slow-changing API data, surviving across processes. """ - token: InitVar[str] + auth: InitVar[str | github.Auth.Auth] _owner: str | None = None _: KW_ONLY api_url: str = API_URL_DEFAULT cache: dict = field(default_factory=dict) client: github.Github = field(init=False, repr=False) - #: Identity of the connection's credentials/endpoint, safe to expose - #: (e.g. as a persistent cache namespace). - ident: str = field(init=False, default="") + ident: str = "" store: PersistentCache | None = field(default=None, repr=False) - def __post_init__(self, token: str): + def __post_init__(self, auth: str | github.Auth.Auth): self.api_url = (self.api_url or API_URL_DEFAULT).rstrip("/") - self.ident = get_connection_ident(token, self.api_url) - self.client = github.Github( - auth=github.Auth.Token(token), base_url=self.api_url, per_page=100 - ) + if isinstance(auth, str): + self.ident = self.ident or get_connection_ident(auth, self.api_url) + auth = github.Auth.Token(auth) + elif not self.ident: + raise ValueError("Passing an Auth instance requires 'ident'") + self.client = github.Github(auth=auth, base_url=self.api_url, per_page=100) def with_( self, @@ -130,7 +249,7 @@ def with_( """ if owner is ... and store is ...: return self - # `token` is an InitVar, so a replace() does not work. + # `auth` is an InitVar, so a replace() does not work. # We're intentionally sharing the same client and context cache. # Implies that sharing the same token+api_url in profiles # with different cache config and accessing them in the same run @@ -336,6 +455,26 @@ def query(self, path, *, params=None, method="GET", headers=None, data=None, per return complete_result +def revoke_installation_token(conn): + """ + Revoke the GitHub App installation access token a connection + currently authenticates with (would otherwise stay valid for one + hour after minting). + + The connection – and any others sharing its client – must not be + used afterwards: PyGithub only mints a new token once the revoked + one would have expired, not in response to auth failures. Callers + should evict the connection from their caches, ensuring later + invocations build a fresh one. + + conn + :class:`GitHubConnection` to use. Must be app-authenticated. + """ + if not isinstance(conn.client.requester.auth, github.Auth.AppInstallationAuth): + raise CommandExecutionError("Only GitHub App ('app_id') profiles support token revocation.") + conn.query("installation/token", method="DELETE") + + def rate_limit(conn): """ Return the API rate limit status of a connection's token, keyed by diff --git a/src/saltext/github/utils/gh/profile.py b/src/saltext/github/utils/gh/profile.py index ab8b46af..c8e8f693 100644 --- a/src/saltext/github/utils/gh/profile.py +++ b/src/saltext/github/utils/gh/profile.py @@ -14,13 +14,18 @@ """ import logging +from pathlib import Path from salt.exceptions import CommandExecutionError +from saltext.github.utils.gh.cache import MISS from saltext.github.utils.gh.cache import PersistentCache from saltext.github.utils.gh.client import API_URL_DEFAULT from saltext.github.utils.gh.client import GitHubConnection +from saltext.github.utils.gh.client import build_app_auth +from saltext.github.utils.gh.client import get_app_connection_ident from saltext.github.utils.gh.client import get_connection_ident +from saltext.github.utils.gh.client import resolve_app_installation_id log = logging.getLogger(__name__) @@ -183,14 +188,53 @@ def get_conn(profile, *, opts, context, utils=None): if profile_key in context: return context[profile_key] - token = get_config_value(profile, "token", opts=opts, context=context, utils=utils) + token = get_config_value( + profile, "token", opts=opts, context=context, utils=utils, default=None + ) + app_id = get_config_value( + profile, "app_id", opts=opts, context=context, utils=utils, default=None + ) api_url = get_api_url(profile, opts=opts, context=context, utils=utils) - # Hash the token to keep the secret out of context dumps - ident = get_connection_ident(token, api_url) + owner = get_owner(profile, opts=opts, context=context, utils=utils) + permissions = get_config_value( + profile, "permissions", opts=opts, context=context, utils=utils, default=None + ) + if token is not None and app_id is not None: + raise CommandExecutionError( + f"The '{profile}' profile specifies both 'token' and 'app_id', " + "which are mutually exclusive." + ) + if token is not None: + if permissions is not None: + raise CommandExecutionError( + f"The 'permissions' parameter is only valid for GitHub App ('app_id') " + f"profiles, but the '{profile}' profile uses token authentication." + ) + # Hash the token to keep the secret out of context dumps + ident = get_connection_ident(token, api_url) + elif app_id is not None: + if not owner: + raise CommandExecutionError( + f"GitHub App authentication requires the 'owner' parameter " + f"in the '{profile}' profile." + ) + if permissions is not None and not isinstance(permissions, dict): + raise CommandExecutionError( + f"The 'permissions' parameter of the '{profile}' profile must be a " + "mapping of permission name to access level." + ) + # Derived from the app's identity, not its short-lived + # installation tokens - persistent cache banks survive rotation. + # Differently restricted profiles must not share connections. + ident = get_app_connection_ident(app_id, owner, api_url, permissions) + else: + raise CommandExecutionError( + f"The '{profile}' profile is missing authentication credentials. " + "Specify either 'token' or 'app_id' + 'private_key'/'private_key_path'." + ) conn_key = f"github.conn.{ident}" # This config is HTTP connection-independent. We should reuse a client, if it exists. - owner = get_owner(profile, opts=opts, context=context, utils=utils) cache_enabled = get_config_value( profile, "cache", opts=opts, context=context, utils=utils, default=True ) @@ -202,15 +246,72 @@ def get_conn(profile, *, opts, context, utils=None): store = PersistentCache(opts, ident) if cache_enabled else None context[profile_key] = context[conn_key].with_(owner=owner, store=store) else: + store = PersistentCache(opts, ident) if cache_enabled else None + auth = token + if auth is None: + auth = _get_app_auth( + profile, + app_id, + owner=owner, + api_url=api_url, + permissions=permissions, + store=store, + opts=opts, + context=context, + utils=utils, + ) context[profile_key] = context[conn_key] = GitHubConnection( - token, + auth, owner, api_url=api_url, - store=cache_enabled and PersistentCache(opts, ident) or None, + ident=ident, + store=store, ) return context[profile_key] +def _get_app_auth( + profile, app_id, *, owner, api_url, permissions, store, opts, context, utils=None +): + """ + Build the PyGithub auth object for a GitHub App profile. When the + profile does not pin :ghconf:`installation_id`, the app's + installation on ``owner`` is discovered via the API (cached in + ``store``, when available). + """ + private_key = get_config_value( + profile, "private_key", opts=opts, context=context, utils=utils, default=None + ) + private_key_path = get_config_value( + profile, "private_key_path", opts=opts, context=context, utils=utils, default=None + ) + if (private_key is None) is (private_key_path is None): + raise CommandExecutionError( + f"GitHub App authentication requires exactly one of 'private_key' or " + f"'private_key_path', the '{profile}' profile specifies " + f"{'both' if private_key is not None else 'neither'}." + ) + if private_key_path is not None: + try: + private_key = Path(private_key_path).read_text(encoding="utf-8") + except OSError as err: + raise CommandExecutionError( + f"Failed reading 'private_key_path' of the '{profile}' profile: {err}" + ) from err + installation_id = get_config_value( + profile, "installation_id", opts=opts, context=context, utils=utils, default=None + ) + if installation_id is None and store is not None: + installation_id = store.get_meta("installation_id") + if installation_id is MISS: + installation_id = None + if installation_id is None: + installation_id = resolve_app_installation_id(app_id, private_key, owner, api_url) + if store is not None: + store.set_meta("installation_id", installation_id) + return build_app_auth(app_id, private_key, installation_id, permissions) + + def resource_decl(opts): """ Return the ``github`` resource type's declaration from the resource diff --git a/tests/functional/modules/github_repo/test_app_auth.py b/tests/functional/modules/github_repo/test_app_auth.py new file mode 100644 index 00000000..37e2545a --- /dev/null +++ b/tests/functional/modules/github_repo/test_app_auth.py @@ -0,0 +1,241 @@ +""" +Verify native GitHub App authentication: profiles carrying ``app_id`` + +``private_key`` mint installation tokens via a signed JWT, discover the +app's installation on the profile's ``owner`` (unless pinned via +``installation_id``) and derive a token-independent connection identity. +""" + +import copy +import os +import shutil +from datetime import timedelta + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh.cache import CACHE_ROOT +from saltext.github.utils.gh.client import get_app_connection_ident +from saltext.github.utils.gh.client import revoke_installation_token +from saltext.github.utils.gh.exceptions import GitHubApiError +from tests.support.github import APP_INSTALLATION_ID +from tests.support.github import TOKEN + + +@pytest.fixture(scope="module") +def minion_config_defaults(fake_github): + app_profile = { + "app_id": fake_github.app_id, + "private_key": fake_github.app_private_key, + "api_url": fake_github.url, + "owner": "testorg", + "repo_name": "repo-a", + "cache": False, + } + return { + "github": app_profile, + "github_inst": {**app_profile, "installation_id": APP_INSTALLATION_ID}, + "github_user": {**app_profile, "owner": "user1"}, + "github_noowner": {key: val for key, val in app_profile.items() if key != "owner"}, + # otherorg exists, but the app is not installed on it + "github_badowner": {**app_profile, "owner": "otherorg"}, + "github_scoped": {**app_profile, "permissions": {"contents": "read", "metadata": "read"}}, + "github_pat": { + "token": TOKEN, + "api_url": fake_github.url, + "owner": "testorg", + "cache": False, + }, + } + + +def _discovery_requests(fake_github): + return [req.path for req in fake_github.filter_requests(path="*/installation")] + + +def test_org_operations(github_repo, fake_org, fake_github): + assert github_repo.info()["full_name"] == "testorg/repo-a" + assert set(github_repo.list()) == {"repo-a", "repo-b"} + assert github_repo.create("app-created")["full_name"] == "testorg/app-created" + assert "app-created" in fake_org.repos + # The installation token was minted via the JWT-authenticated app endpoint + assert fake_github.filter_requests(path="/app/installations/*/access_tokens") + # The installation was discovered from the profile's owner, once per + # context (the connection is reused) + assert _discovery_requests(fake_github) == ["/orgs/testorg/installation"] + + +def test_explicit_installation_id_skips_discovery(github_repo, fake_github): + assert set(github_repo.list(profile="github_inst")) == {"repo-a", "repo-b"} + assert not _discovery_requests(fake_github) + + +def test_user_owner_falls_back_to_user_installation(github_repo, fake_github): + # Private repos are not included: installation tokens have no + # associated user to compare the owner against + assert set(github_repo.list(profile="github_user")) == {"repo-a"} + assert _discovery_requests(fake_github) == [ + "/orgs/user1/installation", + "/users/user1/installation", + ] + # The token was minted for the user installation + (token_meta,) = ( + meta for tok, meta in fake_github.app_tokens.items() if tok != fake_github.app_token + ) + assert token_meta["account"] == "user1" + + +def test_scoped_permissions_are_requested(github_repo, fake_github): + assert set(github_repo.list(profile="github_scoped")) == {"repo-a", "repo-b"} + mints = fake_github.filter_requests(path="/app/installations/*/access_tokens") + assert len(mints) == 1 + assert mints[0].body == {"permissions": {"contents": "read", "metadata": "read"}} + # The minted token itself carries the restrictions + (token_meta,) = ( + meta for tok, meta in fake_github.app_tokens.items() if tok != fake_github.app_token + ) + assert token_meta["permissions"] == {"contents": "read", "metadata": "read"} + + +def test_scoped_profiles_do_not_share_connections(github_repo, fake_github): + """ + The same app+owner with different permission restrictions derives + distinct connection identities - each profile mints its own, + appropriately restricted token. + """ + github_repo.list() + github_repo.list(profile="github_scoped") + mints = fake_github.filter_requests(path="/app/installations/*/access_tokens") + assert [req.body["permissions"] for req in mints] == [ + {}, + {"contents": "read", "metadata": "read"}, + ] + + +def test_missing_owner_fails_with_intended_error(github_repo): + with pytest.raises(CommandExecutionError, match="requires the 'owner' parameter"): + github_repo.list(profile="github_noowner") + + +def test_not_installed_fails_with_intended_error(github_repo): + with pytest.raises(CommandExecutionError, match="does not appear to be installed"): + github_repo.list(profile="github_badowner") + + +def _mints(fake_github): + return fake_github.filter_requests(path="/app/installations/*/access_tokens") + + +def _minted_tokens(fake_github): + """ + The currently valid app tokens minted during the test, excluding + the static one (used by the installation token tests). + """ + return {tok for tok in fake_github.app_tokens if tok != fake_github.app_token} + + +def test_revoke_app_token(modules, github_repo, fake_github): + github_repo.list() + (initial_token,) = _minted_tokens(fake_github) + assert modules.github.revoke_app_token() is True + assert len(fake_github.filter_requests(method="DELETE", path="/installation/token")) == 1 + # The token was really invalidated - requests with it would 401 + assert not _minted_tokens(fake_github) + # The cached connection was evicted, hence the next call succeeds + # by minting a fresh token + github_repo.list() + assert _minted_tokens(fake_github) + assert initial_token not in _minted_tokens(fake_github) + + +def test_revoked_connection_requires_eviction(minion_opts): + """ + PyGithub only mints a new token once the current one nears expiry, + not in response to auth failures: a connection surviving revocation + keeps presenting the revoked token. This pins the behavior + ``github.revoke_app_token``'s context eviction accounts for - if + this test starts failing, the eviction may have become obsolete. + """ + conn = gh_profile.get_conn("github", opts=minion_opts, context={}) + assert conn.query(["orgs", "testorg"])["login"] == "testorg" + revoke_installation_token(conn) + with pytest.raises(GitHubApiError, match="401"): + conn.query(["orgs", "testorg"]) + + +def test_revoke_app_token_evicts_shared_profiles(modules, github_repo, fake_github): + """ + ``github`` and ``github_inst`` share one connection (same app, + owner and API URL). Revoking via one profile must evict the other + as well - it holds the same, now invalid, token. + """ + github_repo.list() + github_repo.list(profile="github_inst") + assert len(_mints(fake_github)) == 1 + modules.github.revoke_app_token() + github_repo.list(profile="github_inst") + assert len(_mints(fake_github)) == 2 + + +def test_revoke_app_token_requires_app_profile(modules): + with pytest.raises(CommandExecutionError, match="Only GitHub App"): + modules.github.revoke_app_token(profile="github_pat") + + +@pytest.fixture +def short_lived_tokens(fake_github): + """ + Mint tokens that are already within PyGithub's refresh threshold + (20s), making it refresh the token before each request. + """ + default_validity = fake_github.app_token_validity + fake_github.app_token_validity = timedelta(seconds=15) + try: + yield + finally: + fake_github.app_token_validity = default_validity + + +@pytest.mark.usefixtures("short_lived_tokens") +def test_tokens_are_refreshed_before_expiry(modules, github_repo, fake_github): + """ + Tokens nearing expiry are replaced transparently (and the fake + rejects expired ones), so calls keep succeeding without any + token handling by the caller. + """ + assert set(github_repo.list()) == {"repo-a", "repo-b"} + mints = len(_mints(fake_github)) + assert mints >= 1 + # Served from the API (never conn-cached), requiring a live token + assert modules.github.rate_limit()["core"]["limit"] == 5000 + assert len(_mints(fake_github)) > mints + + +@pytest.fixture +def cached_app_opts(minion_opts, fake_github): # pylint: disable=unused-argument + """ + Opts with an app profile that keeps the default-enabled persistent + cache, flushed after the test (it outlives the fake state reset by + design). + """ + opts = copy.deepcopy(minion_opts) + opts["github_cached"] = {key: val for key, val in opts["github"].items() if key != "cache"} + try: + yield opts + finally: + shutil.rmtree(os.path.join(minion_opts["cachedir"], CACHE_ROOT), ignore_errors=True) + + +def test_installation_discovery_persists_across_processes(cached_app_opts, fake_github): + """ + Fresh contexts simulate separate processes (as with + ``salt-call --local``): the connection identity derives from the + app's identity instead of its short-lived installation tokens, so + the discovered installation is served from the persistent cache. + """ + conn = gh_profile.get_conn("github_cached", opts=cached_app_opts, context={}) + assert conn.ident == get_app_connection_ident(fake_github.app_id, "testorg", fake_github.url) + assert len(_discovery_requests(fake_github)) == 1 + conn2 = gh_profile.get_conn("github_cached", opts=cached_app_opts, context={}) + assert conn2.ident == conn.ident + assert len(_discovery_requests(fake_github)) == 1 diff --git a/tests/functional/modules/github_repo/test_app_token.py b/tests/functional/modules/github_repo/test_app_token.py index e0e3c9a5..29acf4a0 100644 --- a/tests/functional/modules/github_repo/test_app_token.py +++ b/tests/functional/modules/github_repo/test_app_token.py @@ -7,13 +7,11 @@ import pytest from salt.exceptions import CommandExecutionError -from tests.support.github import APP_TOKEN - @pytest.fixture(scope="module") -def minion_config_defaults(github_profile): +def minion_config_defaults(fake_github, github_profile): app_profile = { - "token": APP_TOKEN, + "token": fake_github.app_token, "api_url": github_profile["api_url"], "owner": "testorg", "repo_name": "repo-a", diff --git a/tests/support/github/__init__.py b/tests/support/github/__init__.py index b91c26ec..b52e2a7a 100644 --- a/tests/support/github/__init__.py +++ b/tests/support/github/__init__.py @@ -21,7 +21,10 @@ Per-area route mixins composed into :class:`FakeGitHubAPI` below. """ -from tests.support.github.base import APP_TOKEN +from tests.support.github.base import APP_ID +from tests.support.github.base import APP_INSTALLATION_ID +from tests.support.github.base import APP_TOKEN_PREFIX +from tests.support.github.base import APP_USER_INSTALLATION_ID from tests.support.github.base import DEPLOY_KEY_A from tests.support.github.base import DEPLOY_KEY_B from tests.support.github.base import ORG @@ -52,6 +55,7 @@ from tests.support.github.entities import TeamMember from tests.support.github.entities import User from tests.support.github.routes import ActionsRoutes +from tests.support.github.routes import AppRoutes from tests.support.github.routes import ContentsRoutes from tests.support.github.routes import DeployKeyRoutes from tests.support.github.routes import EnvironmentRoutes @@ -68,7 +72,10 @@ from tests.support.github.state import GlobalState __all__ = [ - "APP_TOKEN", + "APP_ID", + "APP_INSTALLATION_ID", + "APP_TOKEN_PREFIX", + "APP_USER_INSTALLATION_ID", "DEPLOY_KEY_A", "DEPLOY_KEY_B", "ORG", @@ -118,6 +125,7 @@ class FakeGitHubAPI( # pylint: disable=too-many-ancestors PagesRoutes, MetaRoutes, ContentsRoutes, + AppRoutes, ): """ Dict-backed implementation of the consumed GitHub API surface. diff --git a/tests/support/github/base.py b/tests/support/github/base.py index 5523dcb8..fee0d769 100644 --- a/tests/support/github/base.py +++ b/tests/support/github/base.py @@ -9,13 +9,20 @@ import traceback from collections.abc import Callable from contextlib import contextmanager +from datetime import datetime +from datetime import timedelta +from datetime import timezone from http.server import BaseHTTPRequestHandler from http.server import ThreadingHTTPServer from urllib.parse import parse_qs from urllib.parse import unquote from urllib.parse import urlsplit +import jwt +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa from nacl.public import PrivateKey +from saltfactories.utils import random_string from tests.support.github.entities import Organization from tests.support.github.entities import Repo @@ -26,9 +33,20 @@ TOKEN = "test-token" # Accepted alongside TOKEN, modeling a GitHub App installation token # (not tied to a user account, user-scoped endpoints are inaccessible) -APP_TOKEN = "test-app-token" +APP_TOKEN_PREFIX = "test-app-token" ORG = "testorg" +# GitHub App identity for app (JWT) authentication. The app-scoped +# endpoints mint APP_TOKEN as the installation token. +APP_ID = 121212 +# Default installations of the app (owner login -> installation ID), +# see APIBase.app_installations +APP_INSTALLATION_ID = 10001 # on ORG +APP_USER_INSTALLATION_ID = 10002 # on user1 + +# Endpoints that require app (JWT) authentication instead of a token +APP_JWT_ROUTES = re.compile(r"/app(?:/.*)?|/(?:orgs|users)/[^/]+/installation") + # Sample SSH public keys for deploy key tests DEPLOY_KEY_A = ( "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPDeployKeyAForTesting0000000000000000000A existing" @@ -42,7 +60,7 @@ USER_KEY_B = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUserKeyBForTesting000000000000000000000B" -class APIBase: +class APIBase: # pylint: disable=too-many-instance-attributes """ Request dispatch and shared helpers. Routes are contributed by the mixin classes in :mod:`tests.support.github.routes` and discovered @@ -54,14 +72,38 @@ def __init__( token: str = TOKEN, state: GlobalState | None = None, auth_login: str = "user1", - app_token: str | None = APP_TOKEN, + app_token_prefix: str = APP_TOKEN_PREFIX, ): self.token = token # The user the token belongs to (GET /user, /user/repos) self.auth_login = auth_login + # Second accepted token without an associated user account, # modeling a GitHub App installation token - self.app_token = app_token + self.app_token_prefix = app_token_prefix or APP_TOKEN_PREFIX + self.app_tokens = {} + # Validity of minted app tokens (1h on real GitHub). Short + # validities (< PyGithub's 20s refresh threshold) force a + # refresh before each request. + self.app_token_validity = timedelta(hours=1) + # GitHub App identity: JWTs signed with app_private_key are + # accepted on the app-scoped endpoints (APP_JWT_ROUTES), which + # mint app_token as the installation token. + self.app_id = APP_ID + self._app_rsa = rsa.generate_private_key(public_exponent=65537, key_size=2048) + self.app_private_key = self._app_rsa.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ).decode() + # Installations of the app: owner login -> installation ID + self.app_installations = { + ORG: APP_INSTALLATION_ID, + "user1": APP_USER_INSTALLATION_ID, + } + self.app_token = self._get_app_token(ORG) + self._static_app_token = self.app_tokens[self.app_token] + self.base_url = None self.lock = threading.Lock() self.requests = [] @@ -84,8 +126,21 @@ def reset(self): """ with self.lock: self.requests = [] + self.app_tokens = {self.app_token: self._static_app_token} self.state.restore() + def _get_app_token(self, account, permissions=None): + token_prefix = f"{self.app_token_prefix}-{account}-" + num = sum(1 for token in self.app_tokens if token.startswith(token_prefix)) + final_token = random_string(f"{token_prefix}{num}-") + expires_at = datetime.now(timezone.utc) + self.app_token_validity + self.app_tokens[final_token] = { + "account": account, + "expires_at": expires_at, + "permissions": permissions or {}, + } + return final_token + # ------------------------------------------------------------------ # Shared route helpers # ------------------------------------------------------------------ @@ -113,6 +168,23 @@ def _resolve_repo(self, match: re.Match | dict[str, str]) -> Repo: raise TypeError(f"no owner/repo specified: {groups}") # pragma: no cover return self.state.get_repo((owner, groups["repo"])) + def _valid_app_jwt(self, auth_type: str, token: str) -> bool: + """ + Whether the Authorization header carries a JWT signed by the + app's private key, issued by the app. + """ + if auth_type != "Bearer": + return False + try: + claims = jwt.decode( + token, + self._app_rsa.public_key(), + algorithms=["RS256"], + ) + except jwt.InvalidTokenError: + return False + return claims.get("iss") == str(self.app_id) + # ------------------------------------------------------------------ # Dispatch # ------------------------------------------------------------------ @@ -123,10 +195,23 @@ def dispatch(self, method, path, query, body, auth): """ with self.lock: self.requests.append(Request(method, path, query, body)) - if self.app_token and auth in (f"token {self.app_token}", f"Bearer {self.app_token}"): + auth_type, token = (auth or " ").split(" ", maxsplit=1) + if auth_type not in ("token", "Bearer") or not token: + return 401, {}, {"message": "Bad credentials"} + if APP_JWT_ROUTES.fullmatch(path): + # App-scoped endpoints require app (JWT) authentication + if not self._valid_app_jwt(auth_type, token): + return 401, {}, {"message": "A JSON web token could not be decoded"} + elif token.startswith(self.app_token_prefix): + meta = self.app_tokens.get(token) + if meta is None or meta["expires_at"] <= datetime.now(timezone.utc): + return 401, {}, {"message": "Bad credentials"} if path == "/user" or path.startswith("/user/"): return 403, {}, {"message": "Resource not accessible by integration"} - elif auth not in (f"token {self.token}", f"Bearer {self.token}"): # pragma: no cover + if method == "DELETE" and path == "/installation/token": + self.app_tokens.pop(token) + return 204, {}, None + elif token != self.token: # pragma: no cover return 401, {}, {"message": "Bad credentials"} for patterns, func in self._routes.get(method, ()): for route in patterns: diff --git a/tests/support/github/routes/__init__.py b/tests/support/github/routes/__init__.py index b5a9355f..c1b2983a 100644 --- a/tests/support/github/routes/__init__.py +++ b/tests/support/github/routes/__init__.py @@ -3,6 +3,7 @@ """ from tests.support.github.routes.actions import ActionsRoutes +from tests.support.github.routes.app import AppRoutes from tests.support.github.routes.contents import ContentsRoutes from tests.support.github.routes.deploy_keys import DeployKeyRoutes from tests.support.github.routes.environments import EnvironmentRoutes @@ -18,6 +19,7 @@ __all__ = [ "ActionsRoutes", + "AppRoutes", "ContentsRoutes", "DeployKeyRoutes", "EnvironmentRoutes", diff --git a/tests/support/github/routes/app.py b/tests/support/github/routes/app.py new file mode 100644 index 00000000..c9796de3 --- /dev/null +++ b/tests/support/github/routes/app.py @@ -0,0 +1,57 @@ +""" +Routes: GitHub App installations. These endpoints require app (JWT) +authentication, which dispatch enforces (see ``APP_JWT_ROUTES``). +""" + +from tests.support.github.base import APIBase +from tests.support.github.routing import DELETE +from tests.support.github.routing import GET +from tests.support.github.routing import POST + + +class AppRoutes(APIBase): + def _installation(self, login, target_type): + for owner, installation_id in self.app_installations.items(): + if owner.lower() == login.lower(): + return { + "id": installation_id, + "app_id": self.app_id, + "target_type": target_type, + "account": {"login": login, "type": target_type}, + } + raise KeyError(f"App {self.app_id} is not installed on {login}") + + @GET("orgs//installation") + def _org_installation(self, match, _query, _body): + org = self._resolve_org(match) + return 200, self._installation(org.data.login, "Organization") + + @GET("users//installation") + def _user_installation(self, match, _query, _body): + user = self.state.get_user(match["username"]) + return 200, self._installation(user.data.login, "User") + + @POST("app/installations//access_tokens") + def _create_installation_token(self, match, _query, body): + installation_id = int(match["installation_id"]) + for account, account_install_id in self.app_installations.items(): + if account_install_id == installation_id: + break + else: + raise KeyError(f"No app installation with ID {installation_id}") + token = self._get_app_token(account, (body or {}).get("permissions")) + token_meta = self.app_tokens[token] + return 201, { + "token": token, + "expires_at": token_meta["expires_at"].strftime("%Y-%m-%dT%H:%M:%SZ"), + # Unrestricted tokens carry the app's granted permissions, + # which this fake does not model in detail + "permissions": token_meta["permissions"].copy(), + "repository_selection": "all", + } + + @DELETE("installation/token") + def _revoke_installation_token(self, _match, _query, _body): + # Actual revocation is handled in the base class dispatch method, + # this path is only hit when the token does not exist + return 404 diff --git a/tests/unit/utils/conftest.py b/tests/unit/utils/conftest.py new file mode 100644 index 00000000..bf1f24f1 --- /dev/null +++ b/tests/unit/utils/conftest.py @@ -0,0 +1,19 @@ +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa + + +@pytest.fixture(scope="session") +def app_private_key(): + """ + A syntactically valid GitHub App private key (PEM). + """ + return ( + rsa.generate_private_key(public_exponent=65537, key_size=2048) + .private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ) + .decode() + ) diff --git a/tests/unit/utils/test_cache.py b/tests/unit/utils/test_cache.py index b19ce493..a46e8d9b 100644 --- a/tests/unit/utils/test_cache.py +++ b/tests/unit/utils/test_cache.py @@ -93,7 +93,8 @@ def test_flush_repo(store): assert store.get_file("org", "other", "a.yml", PUSHED) == "other" -def test_ident_namespacing(backend): # pylint: disable=unused-argument +@pytest.mark.usefixtures("backend") +def test_ident_namespacing(): one = gh_cache.PersistentCache({}, "ident1") two = gh_cache.PersistentCache({}, "ident2") one.set_meta("auth_login", "user1") diff --git a/tests/unit/utils/test_client.py b/tests/unit/utils/test_client.py index cbda0be7..aa64f0a6 100644 --- a/tests/unit/utils/test_client.py +++ b/tests/unit/utils/test_client.py @@ -135,3 +135,133 @@ def test_explicit_owner_skips_token_lookup(): explicit = gh_client.GitHubConnection("token", "testorg") assert explicit.owner == "testorg" explicit.client.get_user.assert_not_called() + + +def test_get_app_connection_ident_owner_case_insensitive(): + ident = gh_client.get_app_connection_ident(123, "TestOrg", "https://api.example.com") + assert ident == gh_client.get_app_connection_ident(123, "testorg", "https://api.example.com") + # Any other component changes the identity + assert ident != gh_client.get_app_connection_ident(124, "testorg", "https://api.example.com") + assert ident != gh_client.get_app_connection_ident(123, "otherorg", "https://api.example.com") + assert ident != gh_client.get_app_connection_ident(123, "testorg", "https://api.example.org") + assert ident != gh_client.get_connection_ident("token", "https://api.example.com") + + +def test_get_app_connection_ident_includes_permissions(): + def _ident(permissions=None): + return gh_client.get_app_connection_ident( + 123, "testorg", "https://api.example.com", permissions + ) + + # Differently restricted profiles must not share connections/banks + assert _ident({"contents": "read"}) != _ident() + assert _ident({"contents": "read"}) != _ident({"contents": "write"}) + # Unset and empty are equivalent (both mean unrestricted), key order is irrelevant + assert _ident({}) == _ident() + assert _ident({"contents": "read", "issues": "write"}) == _ident( + {"issues": "write", "contents": "read"} + ) + + +def test_connection_with_auth_instance_requires_ident(): + with pytest.raises(ValueError, match="requires 'ident'"): + gh_client.GitHubConnection(pygithub.Auth.Token("token")) + + +def test_connection_with_auth_instance(): + auth = pygithub.Auth.Token("token") + with patch("saltext.github.utils.gh.client.github.Github", autospec=True) as ghub: + conn = gh_client.GitHubConnection(auth, "testorg", ident="abc123") + assert conn.ident == "abc123" + assert ghub.call_args.kwargs["auth"] is auth + + +def test_connection_explicit_ident_overrides_token_derivation(): + with patch("saltext.github.utils.gh.client.github.Github", autospec=True): + conn = gh_client.GitHubConnection("token", ident="abc123") + assert conn.ident == "abc123" + + +def test_build_app_auth_invalid_private_key(): + with pytest.raises(CommandExecutionError, match="Failed initializing GitHub App"): + gh_client.build_app_auth(123, "not a private key", 42) + + +def test_build_app_auth(app_private_key): + auth = gh_client.build_app_auth(123, app_private_key, "42") + assert isinstance(auth, pygithub.Auth.AppInstallationAuth) + assert auth.installation_id == 42 + assert auth.app_id == "123" + assert auth.token_permissions is None + + +def test_build_app_auth_permissions(app_private_key): + auth = gh_client.build_app_auth(123, app_private_key, 42, {"contents": "read"}) + assert auth.token_permissions == {"contents": "read"} + + +def test_revoke_installation_token(conn, requester): + requester.auth = pygithub.Auth.AppAuth(123, "key").get_installation_auth(42) + requester.requestJson.return_value = (204, {}, "") + + gh_client.revoke_installation_token(conn) + + requester.requestJson.assert_called_once_with( + "DELETE", "/installation/token", parameters=None, headers=None, input=None + ) + + +def test_revoke_installation_token_requires_app_auth(conn, requester): + requester.auth = pygithub.Auth.Token("token") + with pytest.raises(CommandExecutionError, match="Only GitHub App"): + gh_client.revoke_installation_token(conn) + requester.requestJson.assert_not_called() + + +@pytest.fixture +def integration(): + with ( + patch("saltext.github.utils.gh.client._app_auth"), + patch( + "saltext.github.utils.gh.client.github.GithubIntegration", autospec=True + ) as integration_cls, + ): + yield integration_cls.return_value + + +def _resolve(): + return gh_client.resolve_app_installation_id(123, "pem", "someowner", "https://example.com") + + +def test_resolve_app_installation_id_org(integration): + integration.get_org_installation.return_value.id = 7 + assert _resolve() == 7 + integration.get_user_installation.assert_not_called() + integration.close.assert_called_once_with() + + +def test_resolve_app_installation_id_user_fallback(integration): + integration.get_org_installation.side_effect = pygithub.UnknownObjectException( + 404, {"message": "Not Found"} + ) + integration.get_user_installation.return_value.id = 8 + assert _resolve() == 8 + + +def test_resolve_app_installation_id_not_installed(integration): + integration.get_org_installation.side_effect = pygithub.UnknownObjectException( + 404, {"message": "Not Found"} + ) + integration.get_user_installation.side_effect = pygithub.UnknownObjectException( + 404, {"message": "Not Found"} + ) + with pytest.raises(GitHubNotFoundError, match="does not appear to be installed"): + _resolve() + + +def test_resolve_app_installation_id_api_error(integration): + integration.get_org_installation.side_effect = pygithub.GithubException( + 502, {"message": "bad gateway"} + ) + with pytest.raises(GitHubApiError, match="502 bad gateway"): + _resolve() diff --git a/tests/unit/utils/test_profile.py b/tests/unit/utils/test_profile.py new file mode 100644 index 00000000..85db3aec --- /dev/null +++ b/tests/unit/utils/test_profile.py @@ -0,0 +1,129 @@ +""" +Tests for profile resolution into connections, particularly the +credential validation (token vs GitHub App auth). +""" + +from unittest.mock import patch + +import github as pygithub +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh.client import get_app_connection_ident +from saltext.github.utils.gh.client import get_connection_ident + +API_URL = "https://api.example.com" + + +def _opts(**profile): + profile.setdefault("api_url", API_URL) + profile.setdefault("cache", False) + return {"github": profile} + + +def _get_conn(opts, context=None): + return gh_profile.get_conn("github", opts=opts, context=context if context is not None else {}) + + +def test_get_conn_rejects_token_and_app_id(): + with pytest.raises(CommandExecutionError, match="mutually exclusive"): + _get_conn(_opts(token="hunter2", app_id=123, owner="testorg")) + + +def test_get_conn_requires_credentials(): + with pytest.raises(CommandExecutionError, match="missing authentication credentials"): + _get_conn(_opts(owner="testorg")) + + +def test_get_conn_app_requires_owner(): + with pytest.raises(CommandExecutionError, match="requires the 'owner' parameter"): + _get_conn(_opts(app_id=123, private_key="pem")) + + +@pytest.mark.parametrize( + "extra", + ( + {"private_key": "pem", "private_key_path": "/some/path"}, + {}, + ), + ids=("both", "neither"), +) +def test_get_conn_app_requires_exactly_one_private_key(extra): + with pytest.raises(CommandExecutionError, match="exactly one of 'private_key'"): + _get_conn(_opts(app_id=123, owner="testorg", **extra)) + + +def test_get_conn_app_private_key_path_read_error(tmp_path): + with pytest.raises(CommandExecutionError, match="Failed reading 'private_key_path'"): + _get_conn( + _opts(app_id=123, owner="testorg", private_key_path=str(tmp_path / "missing.pem")) + ) + + +def test_get_conn_app_private_key_path(tmp_path, app_private_key): + """ + App profiles with a key file and a pinned installation build a + connection without any API request, deriving a token-independent + identity. + """ + key_file = tmp_path / "app.pem" + key_file.write_text(app_private_key) + conn = _get_conn( + _opts( + app_id=123, + owner="testorg", + private_key_path=str(key_file), + installation_id=42, + ) + ) + assert conn.ident == get_app_connection_ident(123, "testorg", API_URL) + assert conn.owner == "testorg" + + +def test_get_conn_app_resolves_installation(app_private_key): + with patch( + "saltext.github.utils.gh.profile.resolve_app_installation_id", return_value=42 + ) as resolve: + conn = _get_conn(_opts(app_id=123, owner="testorg", private_key=app_private_key)) + resolve.assert_called_once_with(123, app_private_key, "testorg", API_URL) + auth = conn.client.requester.auth + assert isinstance(auth, pygithub.Auth.AppInstallationAuth) + assert auth.installation_id == 42 + + +def test_get_conn_token_ident_unchanged(): + conn = _get_conn(_opts(token="hunter2", owner="testorg")) + assert conn.ident == get_connection_ident("hunter2", API_URL) + + +def test_get_conn_token_rejects_permissions(): + with pytest.raises(CommandExecutionError, match="only valid for GitHub App"): + _get_conn(_opts(token="hunter2", owner="testorg", permissions={"contents": "read"})) + + +def test_get_conn_app_permissions_must_be_a_mapping(): + with pytest.raises(CommandExecutionError, match="must be a mapping"): + _get_conn( + _opts(app_id=123, owner="testorg", private_key="pem", permissions="contents: read") + ) + + +def test_get_conn_app_permissions(app_private_key): + """ + Restricted profiles pass the permissions through to the token mint + and derive a distinct connection identity, so differently + restricted profiles never share a connection. + """ + permissions = {"contents": "read"} + profile = _opts( + app_id=123, + owner="testorg", + private_key=app_private_key, + installation_id=42, + permissions=permissions, + ) + conn = _get_conn(profile) + assert conn.client.requester.auth.token_permissions == permissions + assert conn.ident == get_app_connection_ident(123, "testorg", API_URL, permissions) + assert conn.ident != get_app_connection_ident(123, "testorg", API_URL)