Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions src/saltext/github/utils/gh/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``
Expand Down
4 changes: 2 additions & 2 deletions src/saltext/github/utils/gh/repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down
51 changes: 51 additions & 0 deletions tests/functional/modules/github_repo/test_app_token.py
Original file line number Diff line number Diff line change
@@ -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")
2 changes: 2 additions & 0 deletions tests/support/github/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -67,6 +68,7 @@
from tests.support.github.state import GlobalState

__all__ = [
"APP_TOKEN",
"DEPLOY_KEY_A",
"DEPLOY_KEY_B",
"ORG",
Expand Down
17 changes: 15 additions & 2 deletions tests/support/github/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = []
Expand Down Expand Up @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/utils/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
60 changes: 60 additions & 0 deletions tests/unit/utils/test_repos.py
Original file line number Diff line number Diff line change
@@ -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")
Loading