diff --git a/descriptions/edges/GH_MemberOf.md b/descriptions/edges/GH_MemberOf.md index f6e3ca5..0793dbe 100644 --- a/descriptions/edges/GH_MemberOf.md +++ b/descriptions/edges/GH_MemberOf.md @@ -1,3 +1,3 @@ ## General Information -The traversable GH_MemberOf edge represents team membership, linking a team role to its parent team or a child team to a parent team in nested team hierarchies. This edge is traversable because team membership extends access transitively -- a user who holds a role in a child team inherits the repository permissions of all ancestor teams in the nesting hierarchy, making it a key component of attack path analysis. +The traversable GH_MemberOf edge represents team membership and projection relationships, linking a team role to its parent team, a child team to a parent team in nested team hierarchies, or a GH_EnterpriseTeam to its projected GH_Team in an organization. This edge is traversable because these relationships carry effective team membership context through the graph: a user who holds a role in a child team inherits the repository permissions of ancestor teams, and enterprise-managed team membership flows into the projected organization team. diff --git a/extension/schema.json b/extension/schema.json index 26f6503..c74a4d5 100644 --- a/extension/schema.json +++ b/extension/schema.json @@ -467,7 +467,7 @@ }, { "name": "GH_MemberOf", - "description": "Team role is a member of a team, or team is a nested member of a parent team", + "description": "Team role is a member of a team, team is nested under a parent team, or enterprise team maps to its projected organization team", "is_traversable": true }, { diff --git a/src/openhound_github/github_rest_client.py b/src/openhound_github/github_rest_client.py new file mode 100644 index 0000000..7adca6f --- /dev/null +++ b/src/openhound_github/github_rest_client.py @@ -0,0 +1,3 @@ +from openhound_github.github_retry import should_retry_github_response + +__all__ = ["should_retry_github_response"] diff --git a/src/openhound_github/github_retry.py b/src/openhound_github/github_retry.py new file mode 100644 index 0000000..2cc5e68 --- /dev/null +++ b/src/openhound_github/github_retry.py @@ -0,0 +1,48 @@ +from typing import Optional + +from requests import Response + + +def _response_message(response: Response) -> str: + try: + response_data = response.json() + except ValueError: + return getattr(response, "text", "") + if isinstance(response_data, dict): + return str(response_data.get("message", "")) + return "" + + +def is_primary_rate_limit_response(response: Response) -> bool: + if response.status_code not in (403, 429): + return False + + message = _response_message(response).lower() + return ( + response.headers.get("x-ratelimit-remaining") == "0" + or "api rate limit exceeded" in message + ) + + +def is_secondary_rate_limit_response(response: Response) -> bool: + if response.status_code not in (403, 429): + return False + + message = _response_message(response).lower() + return "secondary rate limit" in message or "abuse detection" in message + + +def should_retry_github_response( + response: Optional[Response], exception: Optional[BaseException] +) -> bool: + if response is None: + return False + + return ( + is_primary_rate_limit_response(response) + or is_secondary_rate_limit_response(response) + or ( + response.status_code in (403, 429) + and bool(response.headers.get("Retry-After")) + ) + ) diff --git a/src/openhound_github/helpers.py b/src/openhound_github/helpers.py index aed2b99..fe26b7a 100644 --- a/src/openhound_github/helpers.py +++ b/src/openhound_github/helpers.py @@ -12,6 +12,10 @@ from requests import Request from openhound_github.auth import GitHubAppInstallationAuth +from openhound_github.github_retry import ( + is_primary_rate_limit_response, + is_secondary_rate_limit_response, +) logger = logging.getLogger(__name__) @@ -239,18 +243,14 @@ def retry_policy( if response.status_code not in (403, 429): return False - message = _response_message(response).lower() - if ( - headers.get("x-ratelimit-remaining") == "0" - or "api rate limit exceeded" in message - ): + if is_primary_rate_limit_response(response): reset_at = headers.get("x-ratelimit-reset") delay = int(reset_at) - now if reset_at else 0 headers["Retry-After"] = str(delay) logger.warning("Primary rate limit reached, retrying in %s seconds", delay) return True - if "secondary rate limit" in message or "abuse detection" in message: + if is_secondary_rate_limit_response(response): logger.warning("Secondary rate limit reached, retrying in 60 seconds") headers["Retry-After"] = "60" return True diff --git a/src/openhound_github/lookup.py b/src/openhound_github/lookup.py index 798a04e..6783e05 100644 --- a/src/openhound_github/lookup.py +++ b/src/openhound_github/lookup.py @@ -205,6 +205,42 @@ def projected_enterprise_team_id(self, org_login: str, slug: str) -> str | None: [org_login, slug], ) + @lru_cache + def external_group_for_team( + self, org_login: str, team_database_id: int + ) -> tuple[int, str] | None: + row = self._find_single_row( + f""" + SELECT external_group_id, external_group_name + FROM {self.schema}.team_external_groups + WHERE org_login = ? + AND team_database_id = ? + """, + [org_login, team_database_id], + ) + if row is None: + return None + return int(row[0]), str(row[1]) + + @lru_cache + def scim_group_id_for_team_external_group( + self, org_login: str, external_group_name: str + ) -> str | None: + rows = self._find_all_objects( + f""" + SELECT DISTINCT esg.id + FROM {self.schema}.enterprise_scim_groups esg + JOIN {self.schema}.enterprise_organizations eo + ON eo.enterprise_node_id = esg.enterprise_node_id + WHERE eo.login = ? + AND esg.display_name = ? + """, + [org_login, external_group_name], + ) + if not rows or len(rows) != 1: + return None + return str(rows[0][0]) + @lru_cache def external_identity_id_for_guid( self, guid: str, environment_slug: str diff --git a/src/openhound_github/main.py b/src/openhound_github/main.py index 778df58..b36636e 100644 --- a/src/openhound_github/main.py +++ b/src/openhound_github/main.py @@ -61,12 +61,14 @@ def preproc(ctx: PreProcContext): "repo_roles": "repo_roles", "users": "users", "teams": "teams", + "team_external_groups": "team_external_groups", "team_members": "team_members", "saml_provider": "saml_provider", "external_identities": "external_identities", "applications": "applications", "enterprise": "enterprise", "enterprise_organizations": "enterprise_organizations", + "enterprise_scim_groups": "enterprise_scim_groups", "enterprise_runner_groups": "enterprise_runner_groups", "enterprise_runner_group_organizations": "enterprise_runner_group_organizations", "enterprise_runner_group_memberships": "enterprise_runner_group_memberships", diff --git a/src/openhound_github/models/__init__.py b/src/openhound_github/models/__init__.py index cac60f6..8332d96 100644 --- a/src/openhound_github/models/__init__.py +++ b/src/openhound_github/models/__init__.py @@ -62,7 +62,7 @@ ScimUser, ) from .secret_scanning_alert import SecretScanningAlert -from .team import Team +from .team import Team, TeamExternalGroup from .team_member import TeamMember from .team_role import TeamRole from .user import User @@ -78,6 +78,7 @@ "ActionPermission", "User", "Team", + "TeamExternalGroup", "TeamRole", "TeamMember", "Repository", diff --git a/src/openhound_github/models/team.py b/src/openhound_github/models/team.py index d738ed2..5307a9b 100644 --- a/src/openhound_github/models/team.py +++ b/src/openhound_github/models/team.py @@ -15,7 +15,7 @@ @dataclass class GHTeamProperties(GHNodeProperties): """Team-specific properties and accordion panel queries. - + Attributes: collected: Collected/generated by OpenHound github_team_id: The raw GitHub team node ID. @@ -23,6 +23,8 @@ class GHTeamProperties(GHNodeProperties): description: The team's description. privacy: The team's privacy level (e.g., `visible`, `secret`). type: The team type, such as `enterprise` for projected enterprise teams. + external_group_id: GitHub's org-scoped external IdP group identifier. + external_group_name: The external IdP group name reported by GitHub. environment_name: The name of the environment (GitHub organization). query_first_degree_members: Query for first degree members. query_unrolled_members: Query for unrolled members. @@ -40,6 +42,8 @@ class GHTeamProperties(GHNodeProperties): description: str | None = None privacy: str | None = None type: str | None = None + external_group_id: int | None = None + external_group_name: str | None = None environment_name: str | None = None query_first_degree_members: str | None = None @@ -74,6 +78,16 @@ class ParentTeam(BaseModel): id: str +class TeamExternalGroup(BaseModel): + """Raw mapping between an org team and its linked external IdP group.""" + + org_login: str + team_database_id: int + external_group_id: int + external_group_name: str + external_group_updated_at: str | None = None + + @app.asset( node=NodeDef( kind=nk.TEAM, @@ -89,6 +103,13 @@ class ParentTeam(BaseModel): description="Team is a child of parent team", traversable=False, ), + EdgeDef( + start=nk.SCIM_GROUP, + end=nk.TEAM, + kind=ek.SCIM_PROVISIONED, + description="SCIM group is provisioned as team", + traversable=True, + ), ], ) class Team(BaseAsset): @@ -120,9 +141,16 @@ def node_id(self) -> str: """The ID from a GraphQL API response is the same as a regular node_id""" return self.id + @property + def external_group(self) -> tuple[int, str] | None: + if self.database_id is None: + return None + return self._lookup.external_group_for_team(self.org_login, self.database_id) + @property def as_node(self) -> GHNode: tid = self.node_id + external_group = self.external_group return GHNode( kinds=[nk.TEAM], properties=GHTeamProperties( @@ -132,6 +160,8 @@ def as_node(self) -> GHNode: slug=self.slug, description=self.description, privacy=self.privacy, + external_group_id=external_group[0] if external_group else None, + external_group_name=external_group[1] if external_group else None, environment_name=self.org_login, environmentid=self.org_node_id, query_first_degree_members=f"MATCH p=(:GH_User)-[:GH_HasRole]->(t:GH_TeamRole)-[:GH_MemberOf]->(:GH_Team {{node_id:'{tid}'}}) RETURN p", @@ -176,6 +206,19 @@ def edges(self): properties=EdgeProperties(traversable=True), ) + external_group = self.external_group + if external_group: + scim_group_id = self._lookup.scim_group_id_for_team_external_group( + self.org_login, external_group[1] + ) + if scim_group_id: + yield Edge( + kind=ek.SCIM_PROVISIONED, + start=EdgePath(value=scim_group_id, match_by="id"), + end=EdgePath(value=self.node_id, match_by="id"), + properties=EdgeProperties(traversable=True), + ) + # yield from self._branch_edges yield from self._bypass_pull_request_allowances_edges yield from self._bypass_push_allowances_edges diff --git a/src/openhound_github/resources/organization.py b/src/openhound_github/resources/organization.py index 931eb86..38d37d2 100644 --- a/src/openhound_github/resources/organization.py +++ b/src/openhound_github/resources/organization.py @@ -67,6 +67,7 @@ SelectedOrgSecret, SelectedOrgVariable, Team, + TeamExternalGroup, TeamMember, TeamRole, User, @@ -102,6 +103,7 @@ class SourceContext: github_web_origin: str = DEFAULT_GITHUB_WEB_ORIGIN cache_lock: Lock = field(default_factory=Lock) app_cache: dict[str, dict[str, Any]] = field(default_factory=dict) + team_rest_cache: dict[str, list[dict[str, Any]]] = field(default_factory=dict) actions_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) runner_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) workflow_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) @@ -199,6 +201,183 @@ def _workflow_permissions( ) +def _rest_teams_for_org( + ctx: SourceContext, client: RESTClient, org_name: str +) -> list[dict[str, Any]]: + if org_name not in ctx.team_rest_cache: + with ctx.cache_lock: + if org_name not in ctx.team_rest_cache: + ctx.team_rest_cache[org_name] = [ + team + for page in client.paginate( + f"/orgs/{org_name}/teams", params={"per_page": 100} + ) + for team in page + ] + return ctx.team_rest_cache[org_name] + + +def _external_group_skip_reason(exception: BaseException) -> str | None: + if not isinstance(exception, requests.HTTPError) or exception.response is None: + return None + + status_code = exception.response.status_code + if status_code in (401, 403): + return "the configured credentials do not have Members organization permission at write level" + if status_code == 404: + return "the GitHub scope does not expose external groups" + return None + + +def _external_group_failure_is_terminal(exception: BaseException) -> bool: + if not isinstance(exception, requests.HTTPError) or exception.response is None: + return False + return exception.response.status_code in (401, 403) + + +def _team_cannot_be_externally_managed(exception: BaseException) -> bool: + if not isinstance(exception, requests.HTTPError) or exception.response is None: + return False + if exception.response.status_code != 400: + return False + + try: + message = str(exception.response.json().get("message", "")).casefold() + except ValueError: + return False + return "cannot be externally managed" in message and "explicit members" in message + + +def _log_org_external_group_failure( + resource: str, + org_name: str, + exception: BaseException, + *, + group_id: int | str | None = None, +) -> None: + skip_reason = _external_group_skip_reason(exception) + if skip_reason: + logger.warning( + "Skipping %s for organization '%s': %s", + resource, + org_name, + skip_reason, + extra={"resource": resource, "phase": "resource_iteration"}, + ) + return + + group_context = f" external group '{group_id}'" if group_id is not None else "" + logger.error( + "Error in resource '%s' processing organization '%s'%s: %s", + resource, + org_name, + group_context, + exception, + extra={"resource": resource, "phase": "resource_iteration"}, + ) + + +def _team_database_id(team_id: Any) -> int | None: + try: + return int(team_id) + except (TypeError, ValueError): + return None + + +def _team_external_group_row( + org_name: str, + team_id: Any, + group_id: Any, + group_name: Any, + updated_at: Any, +) -> dict[str, Any] | None: + team_database_id = _team_database_id(team_id) + if group_id is None or not group_name: + return None + if team_database_id is None: + return None + try: + return { + "org_login": org_name, + "team_database_id": team_database_id, + "external_group_id": int(group_id), + "external_group_name": str(group_name), + "external_group_updated_at": updated_at, + } + except (TypeError, ValueError): + return None + + +def _team_external_groups_by_group( + client: RESTClient, + org_name: str, + groups: list[dict[str, Any]], + allowed_team_database_ids: set[int], +) -> Iterator[dict[str, Any]]: + for group in groups: + group_id = group.get("group_id") + try: + response = client.get(f"/orgs/{org_name}/external-group/{group_id}") + response.raise_for_status() + group_details = response.json() + except Exception as e: + _log_org_external_group_failure( + "team_external_groups", + org_name, + e, + group_id=group_id, + ) + if _external_group_failure_is_terminal(e): + return + continue + + group_name = group_details.get("group_name") or group.get("group_name") + updated_at = group_details.get("updated_at") or group.get("updated_at") + for team in group_details.get("teams") or []: + team_database_id = _team_database_id(team.get("team_id")) + if team_database_id not in allowed_team_database_ids: + continue + row = _team_external_group_row( + org_name, + team_database_id, + group_id, + group_name, + updated_at, + ) + if row: + yield row + + +def _team_external_groups_by_team( + client: RESTClient, org_name: str, teams: list[dict[str, Any]] +) -> Iterator[dict[str, Any]]: + for team in teams: + try: + response = client.get( + f"/orgs/{org_name}/teams/{_encode_path_segment(str(team['slug']))}/external-groups" + ) + response.raise_for_status() + group_details = response.json() + except Exception as e: + if _team_cannot_be_externally_managed(e): + continue + _log_org_external_group_failure("team_external_groups", org_name, e) + if _external_group_failure_is_terminal(e): + return + continue + + for group in group_details.get("groups") or []: + row = _team_external_group_row( + org_name, + team.get("id"), + group.get("group_id"), + group.get("group_name"), + group.get("updated_at"), + ) + if row: + yield row + + def _repo_permission_role(repo: dict[str, Any]) -> str: role_name = repo.get("role_name") if role_name: @@ -545,12 +724,9 @@ def projected_enterprise_teams(ctx: SourceContext): org_name = org.org_name client = org.client try: - for page in client.paginate( - f"/orgs/{org_name}/teams", params={"per_page": 100} - ): - for team in page: - if str(team.get("slug", "")).startswith("ent:") and team.get("node_id"): - yield {**team, "org_login": org_name} + for team in _rest_teams_for_org(ctx, client, org_name): + if str(team.get("slug", "")).startswith("ent:") and team.get("node_id"): + yield {**team, "org_login": org_name} except Exception as e: logger.error( f"Error in resource 'projected_enterprise_teams' processing organization '{org_name}': {e}", @@ -559,6 +735,58 @@ def projected_enterprise_teams(ctx: SourceContext): continue +@app.resource( + name="team_external_groups", + columns=TeamExternalGroup, + parallelized=True, +) +def team_external_groups(ctx: SourceContext): + """Collect external IdP group mappings for normal organization teams.""" + + for org in ctx.organizations: + org_name = org.org_name + client = org.client + try: + groups = [ + group + for page in client.paginate( + f"/orgs/{org_name}/external-groups", + params={"per_page": 100}, + data_selector="groups", + ) + for group in page + if group.get("group_id") is not None + ] + if not groups: + continue + + teams = [ + team + for team in _rest_teams_for_org(ctx, client, org_name) + if team.get("id") is not None + and team.get("slug") + and not str(team["slug"]).startswith("ent:") + ] + team_database_ids = { + team_database_id + for team in teams + if (team_database_id := _team_database_id(team["id"])) is not None + } + + if len(groups) <= len(teams): + yield from _team_external_groups_by_group( + client, + org_name, + groups, + team_database_ids, + ) + else: + yield from _team_external_groups_by_team(client, org_name, teams) + except Exception as e: + _log_org_external_group_failure("team_external_groups", org_name, e) + continue + + @app.transformer(name="team_roles", columns=TeamRole, parallelized=True) def team_roles(team: Team): """Yield the two built-in team roles (members and maintainers) for a team. @@ -2032,6 +2260,7 @@ def organization_resources(ctx: SourceContext): personal_access_tokens_resource = personal_access_tokens(ctx) teams_resource = teams(ctx) + team_external_groups_resource = team_external_groups(ctx) repositories_graphql_resource = repositories_graphql(ctx) app_installs_resource = app_installations(ctx) runner_groups_resource = runner_groups(ctx) @@ -2061,6 +2290,7 @@ def organization_resources(ctx: SourceContext): repos_resource | repository_secrets(ctx), repos_resource | repository_variables(ctx), teams_resource, + team_external_groups_resource, projected_enterprise_teams_resource, org_scim_organizations_resource, teams_resource | team_members(ctx), diff --git a/src/openhound_github/source.py b/src/openhound_github/source.py index 0999be1..1240967 100644 --- a/src/openhound_github/source.py +++ b/src/openhound_github/source.py @@ -54,6 +54,7 @@ class SourceContext: github_web_origin: str = DEFAULT_GITHUB_WEB_ORIGIN cache_lock: Lock = field(default_factory=Lock) app_cache: dict[str, dict[str, Any]] = field(default_factory=dict) + team_rest_cache: dict[str, list[dict[str, Any]]] = field(default_factory=dict) actions_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) runner_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) workflow_permissions_cache: dict[str, dict[str, Any]] = field(default_factory=dict) diff --git a/src/openhound_github/transforms.py b/src/openhound_github/transforms.py index 1b7aea2..4aa3bee 100644 --- a/src/openhound_github/transforms.py +++ b/src/openhound_github/transforms.py @@ -62,6 +62,13 @@ def ensure_optional_input_tables( team_id VARCHAR, id VARCHAR ); + CREATE TABLE IF NOT EXISTS {schema}.team_external_groups ( + org_login VARCHAR, + team_database_id BIGINT, + external_group_id BIGINT, + external_group_name VARCHAR, + external_group_updated_at VARCHAR + ); CREATE TABLE IF NOT EXISTS {schema}.external_identities ( id VARCHAR, guid VARCHAR, @@ -106,6 +113,12 @@ def ensure_optional_input_tables( ); CREATE TABLE IF NOT EXISTS {schema}.enterprise_organizations ( id VARCHAR, + login VARCHAR, + enterprise_node_id VARCHAR + ); + CREATE TABLE IF NOT EXISTS {schema}.enterprise_scim_groups ( + id VARCHAR, + display_name VARCHAR, enterprise_node_id VARCHAR ); CREATE TABLE IF NOT EXISTS {schema}.enterprise_runner_groups ( @@ -194,6 +207,17 @@ def ensure_optional_input_tables( ALTER TABLE {schema}.team_members ADD COLUMN IF NOT EXISTS id VARCHAR; + ALTER TABLE {schema}.team_external_groups + ADD COLUMN IF NOT EXISTS org_login VARCHAR; + ALTER TABLE {schema}.team_external_groups + ADD COLUMN IF NOT EXISTS team_database_id BIGINT; + ALTER TABLE {schema}.team_external_groups + ADD COLUMN IF NOT EXISTS external_group_id BIGINT; + ALTER TABLE {schema}.team_external_groups + ADD COLUMN IF NOT EXISTS external_group_name VARCHAR; + ALTER TABLE {schema}.team_external_groups + ADD COLUMN IF NOT EXISTS external_group_updated_at VARCHAR; + ALTER TABLE {schema}.org_roles ADD COLUMN IF NOT EXISTS id BIGINT; ALTER TABLE {schema}.org_roles @@ -243,6 +267,15 @@ def ensure_optional_input_tables( ALTER TABLE {schema}.enterprise_organizations ADD COLUMN IF NOT EXISTS id VARCHAR; ALTER TABLE {schema}.enterprise_organizations + ADD COLUMN IF NOT EXISTS login VARCHAR; + ALTER TABLE {schema}.enterprise_organizations + ADD COLUMN IF NOT EXISTS enterprise_node_id VARCHAR; + + ALTER TABLE {schema}.enterprise_scim_groups + ADD COLUMN IF NOT EXISTS id VARCHAR; + ALTER TABLE {schema}.enterprise_scim_groups + ADD COLUMN IF NOT EXISTS display_name VARCHAR; + ALTER TABLE {schema}.enterprise_scim_groups ADD COLUMN IF NOT EXISTS enterprise_node_id VARCHAR; ALTER TABLE {schema}.enterprise_runner_groups diff --git a/tests/test_error_resilience.py b/tests/test_error_resilience.py index 5f59289..a2c0597 100644 --- a/tests/test_error_resilience.py +++ b/tests/test_error_resilience.py @@ -83,7 +83,7 @@ def test_rest_resource_continues_after_org_failure(caplog) -> None: ) with caplog.at_level(logging.ERROR, logger="openhound_github.resources.organization"): - results = list(organizations.bind(ctx)) + results = list(organizations(ctx)) yielded_logins = {r.login for r in results} diff --git a/tests/test_lookup.py b/tests/test_lookup.py index a88e962..d5c9de5 100644 --- a/tests/test_lookup.py +++ b/tests/test_lookup.py @@ -2,6 +2,7 @@ import pytest from openhound_github.lookup import GithubLookup +from openhound_github.transforms import ensure_optional_input_tables def test_github_lookup_accepts_plain_schema_identifiers() -> None: @@ -26,3 +27,91 @@ def test_github_lookup_rejects_untrusted_schema_identifiers() -> None: with pytest.raises(ValueError, match="Invalid DuckDB schema identifier"): GithubLookup(connection, schema="github; DROP SCHEMA github") + + +def test_external_group_for_team_is_scoped_to_org_login() -> None: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github_test") + connection.execute( + "CREATE TABLE github_test.team_external_groups " + "(org_login VARCHAR, team_database_id BIGINT, " + "external_group_id BIGINT, external_group_name VARCHAR)" + ) + connection.execute( + "INSERT INTO github_test.team_external_groups VALUES " + "('acme', 7, 100, 'Acme Engineering'), " + "('other', 7, 200, 'Other Engineering')" + ) + + lookup = GithubLookup(connection, schema="github_test") + + assert lookup.external_group_for_team("acme", 7) == (100, "Acme Engineering") + assert lookup.external_group_for_team("other", 7) == (200, "Other Engineering") + + +def test_scim_group_id_for_team_external_group_is_scoped_to_enterprise_org() -> None: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github_test") + connection.execute( + "CREATE TABLE github_test.enterprise_organizations " + "(login VARCHAR, enterprise_node_id VARCHAR)" + ) + connection.execute( + "CREATE TABLE github_test.enterprise_scim_groups " + "(id VARCHAR, display_name VARCHAR, enterprise_node_id VARCHAR)" + ) + connection.execute( + "INSERT INTO github_test.enterprise_organizations VALUES " + "('acme', 'ENT_1'), ('other', 'ENT_2')" + ) + connection.execute( + "INSERT INTO github_test.enterprise_scim_groups VALUES " + "('SCIM_1', 'Engineering', 'ENT_1'), " + "('SCIM_2', 'Engineering', 'ENT_2')" + ) + + lookup = GithubLookup(connection, schema="github_test") + + assert ( + lookup.scim_group_id_for_team_external_group("acme", "Engineering") + == "SCIM_1" + ) + assert ( + lookup.scim_group_id_for_team_external_group("other", "Engineering") + == "SCIM_2" + ) + + +def test_scim_group_id_for_team_external_group_skips_ambiguous_names() -> None: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github_test") + connection.execute( + "CREATE TABLE github_test.enterprise_organizations " + "(login VARCHAR, enterprise_node_id VARCHAR)" + ) + connection.execute( + "CREATE TABLE github_test.enterprise_scim_groups " + "(id VARCHAR, display_name VARCHAR, enterprise_node_id VARCHAR)" + ) + connection.execute( + "INSERT INTO github_test.enterprise_organizations VALUES ('acme', 'ENT_1')" + ) + connection.execute( + "INSERT INTO github_test.enterprise_scim_groups VALUES " + "('SCIM_1', 'Engineering', 'ENT_1'), " + "('SCIM_2', 'Engineering', 'ENT_1')" + ) + + lookup = GithubLookup(connection, schema="github_test") + + assert lookup.scim_group_id_for_team_external_group("acme", "Engineering") is None + + +def test_scim_group_id_for_team_external_group_skips_org_only_context() -> None: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github_test") + ensure_optional_input_tables(connection, schema="github_test") + + lookup = GithubLookup(connection, schema="github_test") + + assert lookup.scim_group_id_for_team_external_group("acme", "Engineering") is None diff --git a/tests/test_team_external_groups.py b/tests/test_team_external_groups.py new file mode 100644 index 0000000..17f0316 --- /dev/null +++ b/tests/test_team_external_groups.py @@ -0,0 +1,384 @@ +import json +import logging +from unittest.mock import MagicMock + +import requests + +from openhound_github.kinds import edges as ek +from openhound_github.models import Team +from openhound_github.resources.organization import ( + OrgContext, + SourceContext, + team_external_groups, +) + + +class _FakeResponse: + def __init__(self, payload: dict, status_code: int = 200): + self._payload = payload + self.status_code = status_code + self.url = "https://api.github.com/example" + + def json(self) -> dict: + return self._payload + + def raise_for_status(self) -> None: + if self.status_code >= 400: + response = requests.Response() + response.status_code = self.status_code + response.url = self.url + response._content = json.dumps(self._payload).encode() + raise requests.HTTPError(response=response) + + +class _FakeClient: + def __init__( + self, + pages: dict[str, list[list[dict]]], + responses: dict[str, _FakeResponse] | None = None, + ): + self.pages = pages + self.responses = responses or {} + self.paginate_calls: list[tuple[str, dict]] = [] + self.get_calls: list[tuple[str, dict]] = [] + + def paginate(self, path: str, **kwargs): + self.paginate_calls.append((path, kwargs)) + return iter(self.pages.get(path, [])) + + def get(self, path: str, **kwargs): + self.get_calls.append((path, kwargs)) + return self.responses[path] + + +class _HTTPErrorPaginateClient(_FakeClient): + def __init__(self, status_code: int): + super().__init__({}) + self.status_code = status_code + + def paginate(self, path: str, **kwargs): + self.paginate_calls.append((path, kwargs)) + response = requests.Response() + response.status_code = self.status_code + response.url = f"https://api.github.com{path}" + raise requests.HTTPError(response=response) + + +def _ctx(client) -> SourceContext: + return SourceContext( + client=client, + organizations=[OrgContext(client=client, org_name="acme")], + ) + + +def test_team_external_groups_collects_group_first_team_mappings() -> None: + client = _FakeClient( + { + "/orgs/acme/external-groups": [ + [ + { + "group_id": 100, + "group_name": "Engineering", + "updated_at": "2026-08-24T16:34:05Z", + }, + { + "group_id": 200, + "group_name": "Security", + "updated_at": "2026-08-24T16:35:05Z", + }, + ] + ], + "/orgs/acme/teams": [ + [ + {"id": 7, "slug": "engineering"}, + {"id": 8, "slug": "platform"}, + {"id": 9, "slug": "operations"}, + {"id": 10, "slug": "ent:enterprise-team"}, + ] + ], + }, + { + "/orgs/acme/external-group/100": _FakeResponse( + { + "group_id": 100, + "group_name": "Engineering", + "updated_at": "2026-08-24T16:34:05Z", + "teams": [ + {"team_id": 7, "team_name": "engineering"}, + {"team_id": 8, "team_name": "platform"}, + {"team_id": 10, "team_name": "ent:enterprise-team"}, + ], + } + ), + "/orgs/acme/external-group/200": _FakeResponse( + { + "group_id": 200, + "group_name": "Security", + "updated_at": "2026-08-24T16:35:05Z", + "teams": [], + } + ), + }, + ) + + rows = list(team_external_groups.__wrapped__(_ctx(client))) + + assert rows == [ + { + "org_login": "acme", + "team_database_id": 7, + "external_group_id": 100, + "external_group_name": "Engineering", + "external_group_updated_at": "2026-08-24T16:34:05Z", + }, + { + "org_login": "acme", + "team_database_id": 8, + "external_group_id": 100, + "external_group_name": "Engineering", + "external_group_updated_at": "2026-08-24T16:34:05Z", + }, + ] + assert client.paginate_calls == [ + ( + "/orgs/acme/external-groups", + {"params": {"per_page": 100}, "data_selector": "groups"}, + ), + ("/orgs/acme/teams", {"params": {"per_page": 100}}), + ] + assert client.get_calls == [ + ("/orgs/acme/external-group/100", {}), + ("/orgs/acme/external-group/200", {}), + ] + + +def test_team_external_groups_uses_team_connections_when_teams_are_fewer() -> None: + client = _FakeClient( + { + "/orgs/acme/external-groups": [ + [ + {"group_id": 100, "group_name": "Engineering"}, + {"group_id": 200, "group_name": "Security"}, + ] + ], + "/orgs/acme/teams": [ + [ + {"id": 7, "slug": "engineering"}, + {"id": 8, "slug": "ent:enterprise-team"}, + ] + ], + }, + { + "/orgs/acme/teams/engineering/external-groups": _FakeResponse( + { + "groups": [ + { + "group_id": 100, + "group_name": "Engineering", + "updated_at": "2026-08-24T16:34:05Z", + } + ] + } + ) + }, + ) + + rows = list(team_external_groups.__wrapped__(_ctx(client))) + + assert rows == [ + { + "org_login": "acme", + "team_database_id": 7, + "external_group_id": 100, + "external_group_name": "Engineering", + "external_group_updated_at": "2026-08-24T16:34:05Z", + } + ] + assert client.get_calls == [ + ("/orgs/acme/teams/engineering/external-groups", {}), + ] + + +def test_team_external_groups_skips_missing_permission(caplog) -> None: + client = _HTTPErrorPaginateClient(status_code=403) + + with caplog.at_level( + logging.WARNING, logger="openhound_github.resources.organization" + ): + rows = list(team_external_groups.__wrapped__(_ctx(client))) + + assert rows == [] + assert any( + "Skipping team_external_groups for organization 'acme': " + "the configured credentials do not have Members organization permission at write level" + in message + for message in caplog.messages + ) + + +def test_team_external_groups_stops_after_detail_permission_failure(caplog) -> None: + client = _FakeClient( + { + "/orgs/acme/external-groups": [ + [ + {"group_id": 100, "group_name": "Engineering"}, + {"group_id": 200, "group_name": "Security"}, + ] + ], + "/orgs/acme/teams": [ + [ + {"id": 7, "slug": "engineering"}, + {"id": 8, "slug": "platform"}, + {"id": 9, "slug": "operations"}, + ] + ], + }, + { + "/orgs/acme/external-group/100": _FakeResponse({}, status_code=403), + "/orgs/acme/external-group/200": _FakeResponse( + { + "group_id": 200, + "group_name": "Security", + "teams": [{"team_id": 8, "team_name": "platform"}], + } + ), + }, + ) + + with caplog.at_level( + logging.WARNING, logger="openhound_github.resources.organization" + ): + rows = list(team_external_groups.__wrapped__(_ctx(client))) + + assert rows == [] + assert client.get_calls == [ + ("/orgs/acme/external-group/100", {}), + ] + assert any( + "Skipping team_external_groups for organization 'acme': " + "the configured credentials do not have Members organization permission at write level" + in message + for message in caplog.messages + ) + + +def test_team_external_groups_skips_teams_with_explicit_members() -> None: + client = _FakeClient( + { + "/orgs/acme/external-groups": [ + [ + {"group_id": 100, "group_name": "Engineering"}, + {"group_id": 200, "group_name": "Security"}, + {"group_id": 300, "group_name": "Operations"}, + ] + ], + "/orgs/acme/teams": [ + [ + {"id": 7, "slug": "engineering"}, + {"id": 8, "slug": "platform"}, + ] + ], + }, + { + "/orgs/acme/teams/engineering/external-groups": _FakeResponse( + { + "message": "This team cannot be externally managed since it has explicit members." + }, + status_code=400, + ), + "/orgs/acme/teams/platform/external-groups": _FakeResponse( + { + "groups": [ + { + "group_id": 200, + "group_name": "Security", + "updated_at": "2026-08-24T16:35:05Z", + } + ] + } + ), + }, + ) + + rows = list(team_external_groups.__wrapped__(_ctx(client))) + + assert rows == [ + { + "org_login": "acme", + "team_database_id": 8, + "external_group_id": 200, + "external_group_name": "Security", + "external_group_updated_at": "2026-08-24T16:35:05Z", + } + ] + assert client.get_calls == [ + ("/orgs/acme/teams/engineering/external-groups", {}), + ("/orgs/acme/teams/platform/external-groups", {}), + ] + + +def test_team_node_includes_external_group_evidence() -> None: + team = Team( + id="T_1", + databaseId=7, + name="engineering", + slug="engineering", + members={"edges": [], "pageInfo": {"endCursor": None, "hasNextPage": False}}, + org_login="acme", + ) + lookup = MagicMock() + lookup.org_id_for_login.return_value = "O_1" + lookup.external_group_for_team.return_value = (100, "Engineering") + team._lookup = lookup + + node = team.as_node + + assert node.properties.external_group_id == 100 + assert node.properties.external_group_name == "Engineering" + lookup.external_group_for_team.assert_called_once_with("acme", 7) + + +def test_team_emits_scim_provisioned_edge_for_external_group_match() -> None: + team = Team( + id="T_1", + databaseId=7, + name="engineering", + slug="engineering", + members={"edges": [], "pageInfo": {"endCursor": None, "hasNextPage": False}}, + org_login="acme", + ) + lookup = MagicMock() + lookup.external_group_for_team.return_value = (100, "Engineering") + lookup.scim_group_id_for_team_external_group.return_value = "SCIM_1" + lookup.bypass_pull_request_allowances.return_value = [] + lookup.bypass_push_restrictions.return_value = [] + team._lookup = lookup + + edges = list(team.edges) + + assert [edge.kind for edge in edges] == [ek.SCIM_PROVISIONED] + assert edges[0].start.value == "SCIM_1" + assert edges[0].end.value == "T_1" + assert edges[0].properties.traversable is True + lookup.scim_group_id_for_team_external_group.assert_called_once_with( + "acme", "Engineering" + ) + + +def test_team_skips_scim_provisioned_edge_without_unique_scim_group_match() -> None: + team = Team( + id="T_1", + databaseId=7, + name="engineering", + slug="engineering", + members={"edges": [], "pageInfo": {"endCursor": None, "hasNextPage": False}}, + org_login="acme", + ) + lookup = MagicMock() + lookup.external_group_for_team.return_value = (100, "Engineering") + lookup.scim_group_id_for_team_external_group.return_value = None + lookup.bypass_pull_request_allowances.return_value = [] + lookup.bypass_push_restrictions.return_value = [] + team._lookup = lookup + + assert list(team.edges) == []