From 66ba7ed4efca534e33597dbdec25962de3c03f9b Mon Sep 17 00:00:00 2001 From: jeanluc Date: Fri, 16 Oct 2026 04:58:34 +0200 Subject: [PATCH] Better account for GitHub App installation tokens --- src/saltext/github/utils/gh/client.py | 11 ++++ src/saltext/github/utils/gh/repos.py | 4 +- .../modules/github_repo/test_app_token.py | 51 ++++++++++++++++ tests/support/github/__init__.py | 2 + tests/support/github/base.py | 17 +++++- tests/unit/utils/test_client.py | 14 +++++ tests/unit/utils/test_repos.py | 60 +++++++++++++++++++ 7 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 tests/functional/modules/github_repo/test_app_token.py create mode 100644 tests/unit/utils/test_repos.py diff --git a/src/saltext/github/utils/gh/client.py b/src/saltext/github/utils/gh/client.py index da67c097..2246feb1 100644 --- a/src/saltext/github/utils/gh/client.py +++ b/src/saltext/github/utils/gh/client.py @@ -207,6 +207,17 @@ def auth_login(self): self.cache["auth_login"] = login return self.cache["auth_login"] + def is_auth_user(self, login): + """ + Whether a login belongs to the token's user. Returns ``False`` + for tokens that are not tied to a user account (e.g. GitHub App + installation tokens), whose user lookup fails. + """ + try: + return login.lower() == self.auth_login.lower() + except github.GithubException: + return False + def owner_type(self, owner=None): """ Return the account type of an owner login, either ``Organization`` diff --git a/src/saltext/github/utils/gh/repos.py b/src/saltext/github/utils/gh/repos.py index 59270143..645b18ec 100644 --- a/src/saltext/github/utils/gh/repos.py +++ b/src/saltext/github/utils/gh/repos.py @@ -83,7 +83,7 @@ def list_all(conn, *, owner=None, ignore_cache=False): if key not in conn.cache or ignore_cache: if conn.owner_type(owner) == "Organization": found = conn.get_org(owner).get_repos() - elif owner.lower() == conn.auth_login.lower(): + elif conn.is_auth_user(owner): # Listing the authenticated user's repos directly includes # private ones, which /users/{owner}/repos does not carry. found = conn.auth_user.get_repos(affiliation="owner") @@ -339,7 +339,7 @@ def create( owner = owner or conn.owner if conn.owner_type(owner) == "Organization": creator = conn.get_org(owner) - elif owner.lower() == conn.auth_user.login.lower(): + elif conn.is_auth_user(owner): creator = conn.auth_user else: raise CommandExecutionError( diff --git a/tests/functional/modules/github_repo/test_app_token.py b/tests/functional/modules/github_repo/test_app_token.py new file mode 100644 index 00000000..e0e3c9a5 --- /dev/null +++ b/tests/functional/modules/github_repo/test_app_token.py @@ -0,0 +1,51 @@ +""" +Verify behavior with GitHub App installation tokens, which are not tied +to a user account (``GET /user`` and other user-scoped endpoints fail +with a 403). +""" + +import pytest +from salt.exceptions import CommandExecutionError + +from tests.support.github import APP_TOKEN + + +@pytest.fixture(scope="module") +def minion_config_defaults(github_profile): + app_profile = { + "token": APP_TOKEN, + "api_url": github_profile["api_url"], + "owner": "testorg", + "repo_name": "repo-a", + "cache": False, + } + noowner_profile = {key: val for key, val in app_profile.items() if key != "owner"} + return { + "github": app_profile, + "github_noowner": noowner_profile, + } + + +def test_org_operations(github_repo, fake_org): + 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 + + +def test_missing_owner_hints_at_profile_parameter(github_repo): + with pytest.raises(CommandExecutionError, match="require the 'owner' profile parameter"): + github_repo.list(profile="github_noowner") + + +def test_list_user_owner_public_only(github_repo): + # Without a token user to compare against, the private-repos-included + # /user/repos listing is skipped in favor of the public one + ret = github_repo.list(owner="user1") + assert set(ret) == {"repo-a"} + assert ret["repo-a"]["description"] == "User-owned repo-a" + + +def test_create_user_owner_fails_with_intended_error(github_repo): + with pytest.raises(CommandExecutionError, match="only supports creating"): + github_repo.create("nope", owner="user1") diff --git a/tests/support/github/__init__.py b/tests/support/github/__init__.py index b8a1f5a5..b91c26ec 100644 --- a/tests/support/github/__init__.py +++ b/tests/support/github/__init__.py @@ -21,6 +21,7 @@ Per-area route mixins composed into :class:`FakeGitHubAPI` below. """ +from tests.support.github.base import APP_TOKEN 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 @@ -67,6 +68,7 @@ from tests.support.github.state import GlobalState __all__ = [ + "APP_TOKEN", "DEPLOY_KEY_A", "DEPLOY_KEY_B", "ORG", diff --git a/tests/support/github/base.py b/tests/support/github/base.py index b8889573..5523dcb8 100644 --- a/tests/support/github/base.py +++ b/tests/support/github/base.py @@ -24,6 +24,9 @@ from tests.support.github.state import GlobalState 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" ORG = "testorg" # Sample SSH public keys for deploy key tests @@ -47,11 +50,18 @@ class APIBase: """ def __init__( - self, token: str = TOKEN, state: GlobalState | None = None, auth_login: str = "user1" + self, + token: str = TOKEN, + state: GlobalState | None = None, + auth_login: str = "user1", + app_token: str | None = APP_TOKEN, ): 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.base_url = None self.lock = threading.Lock() self.requests = [] @@ -113,7 +123,10 @@ def dispatch(self, method, path, query, body, auth): """ with self.lock: self.requests.append(Request(method, path, query, body)) - if auth not in (f"token {self.token}", f"Bearer {self.token}"): # pragma: no cover + if self.app_token and auth in (f"token {self.app_token}", f"Bearer {self.app_token}"): + 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 return 401, {}, {"message": "Bad credentials"} for patterns, func in self._routes.get(method, ()): for route in patterns: diff --git a/tests/unit/utils/test_client.py b/tests/unit/utils/test_client.py index 5a5215b4..cbda0be7 100644 --- a/tests/unit/utils/test_client.py +++ b/tests/unit/utils/test_client.py @@ -116,6 +116,20 @@ def test_owner_lookup_failure_hints_at_profile_parameter(conn): conn.owner # pylint: disable=pointless-statement +def test_is_auth_user_matches_case_insensitively(conn): + conn.client.get_user.return_value.login = "TokenUser" + assert conn.is_auth_user("tokenuser") is True + assert conn.is_auth_user("otheruser") is False + + +def test_is_auth_user_false_for_non_user_tokens(conn): + # GitHub App installation tokens are not tied to a user account + conn.client.get_user.side_effect = pygithub.GithubException( + 403, {"message": "Resource not accessible by integration"}, None + ) + assert conn.is_auth_user("someuser") is False + + def test_explicit_owner_skips_token_lookup(): with patch("saltext.github.utils.gh.client.github.Github", autospec=True): explicit = gh_client.GitHubConnection("token", "testorg") diff --git a/tests/unit/utils/test_repos.py b/tests/unit/utils/test_repos.py new file mode 100644 index 00000000..ff26cfe3 --- /dev/null +++ b/tests/unit/utils/test_repos.py @@ -0,0 +1,60 @@ +""" +Repository operations with tokens that are not tied to a user account +(e.g. GitHub App installation tokens). The functional fake API always +models a user token, so these paths are unit-only. +""" + +from unittest.mock import MagicMock +from unittest.mock import patch + +import github as pygithub +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import client as gh_client +from saltext.github.utils.gh import repos as gh_repos + + +@pytest.fixture +def user_account(): + account = MagicMock() + account.type = "User" + account.login = "someuser" + return account + + +@pytest.fixture +def conn(user_account): + """ + A connection whose token cannot resolve ``GET /user``, knowing a + single user-type account ``someuser``. + """ + + def get_user(login=None): + if login is None: + raise pygithub.GithubException( + 403, {"message": "Resource not accessible by integration"}, None + ) + assert login == user_account.login + return user_account + + with patch("saltext.github.utils.gh.client.github.Github", autospec=True): + connection = gh_client.GitHubConnection("token") + connection.client.get_user.side_effect = get_user + return connection + + +def test_list_all_user_owner_falls_back_to_public_listing(conn, user_account): + repo = MagicMock() + repo.name = "repo-a" + user_account.get_repos.return_value = [repo] + + assert gh_repos.list_all(conn, owner="someuser") == [repo] + # The private-repos-included /user/repos listing is unavailable, + # /users/{owner}/repos takes no affiliation parameter + user_account.get_repos.assert_called_once_with() + + +def test_create_user_owner_raises_intended_error(conn): + with pytest.raises(CommandExecutionError, match="Cannot create a repository for 'someuser'"): + gh_repos.create(conn, "new-repo", owner="someuser")