From d88745d4179a55b3cf702fe8b2c082849b7382bc Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 15:08:50 -0700 Subject: [PATCH 1/8] BED-9446: collect enterprise SCIM by default --- README.md | 2 +- src/openhound_github/helpers.py | 13 ++ src/openhound_github/resources/enterprise.py | 121 ++++++++++++------ .../resources/organization.py | 120 ++++++++++++----- src/openhound_github/source.py | 4 - tests/test_app_auth.py | 1 - tests/test_enterprise_resources.py | 94 ++++++++++++++ tests/test_org_scim_resources.py | 93 ++++++++++++++ 8 files changed, 373 insertions(+), 75 deletions(-) create mode 100644 tests/test_org_scim_resources.py diff --git a/README.md b/README.md index 5bcc9a0..afb265c 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ identifier must be supplied together with `key_path` and `enterprise_name`. ### Enterprise SCIM and hybrid correlations -When `SOURCES__GITHUB__COLLECT_ENTERPRISE_SCIM=true`, a token with enterprise SCIM access is used to collect both `/scim/v2/enterprises/{enterprise}/Users` and `/scim/v2/enterprises/{enterprise}/Groups`. The collector emits normalized `SCIM_Organization`, `SCIM_User`, and `SCIM_Group` nodes plus `SCIM_Contains`, `SCIM_MemberOf`, and `SCIM_Provisioned` relationships. Install the BloodHound SCIM extension alongside this extension to register the shared SCIM kinds. +A token with enterprise SCIM access is used to collect both `/scim/v2/enterprises/{enterprise}/Users` and `/scim/v2/enterprises/{enterprise}/Groups`. The collector emits normalized `SCIM_Organization`, `SCIM_User`, and `SCIM_Group` nodes plus `SCIM_Contains`, `SCIM_MemberOf`, and `SCIM_Provisioned` relationships. Install the BloodHound SCIM extension alongside this extension to register the shared SCIM kinds. `SOURCES__GITHUB__EMIT_LEGACY_SCIM_CORRELATIONS=true` temporarily reproduces GitHound-style Okta-to-SCIM correlation relationships. It defaults to false because a dedicated hybrid correlator should own IdP-to-SCIM matching; GitHub remains authoritative for GitHub's SCIM resources and target-system provisioning relationships. diff --git a/src/openhound_github/helpers.py b/src/openhound_github/helpers.py index 04e7761..aed2b99 100644 --- a/src/openhound_github/helpers.py +++ b/src/openhound_github/helpers.py @@ -20,6 +20,19 @@ class GraphQLPaginationError(RuntimeError): pass +def scim_skip_reason(exception: BaseException) -> str | None: + """Return a user-facing reason for expected SCIM API unavailability.""" + 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 SCIM access" + if status_code == 404: + return "the GitHub scope does not expose SCIM endpoints" + return None + + class GraphQLCursorPaginator(JSONResponseCursorPaginator): def __init__( self, diff --git a/src/openhound_github/resources/enterprise.py b/src/openhound_github/resources/enterprise.py index 08ab1f5..f61c6ec 100644 --- a/src/openhound_github/resources/enterprise.py +++ b/src/openhound_github/resources/enterprise.py @@ -11,7 +11,7 @@ ENTERPRISE_QUERY, ENTERPRISE_SAML_QUERY, ) -from openhound_github.helpers import GraphQLCursorPaginator +from openhound_github.helpers import GraphQLCursorPaginator, scim_skip_reason from openhound_github.main import app from openhound_github.models import ( BaseUser, @@ -57,7 +57,6 @@ class SourceContext: scim_client: RESTClient | None = None org_name: str | None = None enterprise_name: str | None = None - collect_enterprise_scim: bool = False emit_legacy_scim_correlations: bool = False github_deployment_id: str = DEFAULT_GITHUB_DEPLOYMENT_ID github_web_origin: str = DEFAULT_GITHUB_WEB_ORIGIN @@ -67,25 +66,45 @@ def iter_enterprise_scim_resources( client: RESTClient, enterprise_slug: str, resource_kind: str, + *, + count: int = 100, ): if resource_kind not in {"Users", "Groups"}: raise ValueError(f"Unsupported enterprise SCIM resource: {resource_kind}") paginator = OffsetPaginator( offset_param="startIndex", limit_param="count", - limit=100, + limit=count, offset=1, total_path="totalResults", ) for page in client.paginate( f"/scim/v2/enterprises/{enterprise_slug}/{resource_kind}", - params={"startIndex": 1, "count": 100}, + params={"startIndex": 1, "count": count}, paginator=paginator, data_selector="Resources", ): yield from page +def _log_enterprise_scim_failure(resource: str, enterprise_name: str, exception: BaseException): + skip_reason = scim_skip_reason(exception) + if skip_reason: + logger.warning( + "Skipping %s for enterprise '%s': %s", + resource, + enterprise_name, + skip_reason, + extra={"resource": resource, "phase": "resource_iteration"}, + ) + return + + logger.error( + f"Error in resource '{resource}' processing enterprise '{enterprise_name}': {exception}", + extra={"resource": resource, "phase": "resource_iteration"}, + ) + + @app.resource(name="enterprise", columns=Enterprise, parallelized=True) def enterprise(ctx: SourceContext): data = { @@ -152,6 +171,28 @@ def enterprise_organizations(enterprise_data: Enterprise, ctx: SourceContext): parallelized=True, ) def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContext): + scim_client = ctx.scim_client or ctx.client + if not scim_client or not ctx.enterprise_name: + raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") + + try: + next( + iter_enterprise_scim_resources( + scim_client, + ctx.enterprise_name, + "Users", + count=1, + ), + None, + ) + except Exception as e: + _log_enterprise_scim_failure( + "enterprise_scim_organizations", + ctx.enterprise_name, + e, + ) + return + yield { "enterprise_node_id": enterprise_data.id, "enterprise_slug": ctx.enterprise_name, @@ -163,21 +204,25 @@ def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContex columns=ScimUser, parallelized=True, ) -def enterprise_scim_users(enterprise_data: Enterprise, ctx: SourceContext): +def enterprise_scim_users(scim_organization: ScimOrganization, ctx: SourceContext): scim_client = ctx.scim_client or ctx.client if not scim_client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") - for user in iter_enterprise_scim_resources( - scim_client, - ctx.enterprise_name, - "Users", - ): - yield { - **user, - "enterprise_node_id": enterprise_data.id, - "enterprise_slug": ctx.enterprise_name, - "emit_legacy_correlation": ctx.emit_legacy_scim_correlations, - } + try: + for user in iter_enterprise_scim_resources( + scim_client, + ctx.enterprise_name, + "Users", + ): + yield { + **user, + "enterprise_node_id": scim_organization.enterprise_node_id, + "enterprise_slug": ctx.enterprise_name, + "emit_legacy_correlation": ctx.emit_legacy_scim_correlations, + } + except Exception as e: + _log_enterprise_scim_failure("enterprise_scim_users", ctx.enterprise_name, e) + return @app.transformer( @@ -185,21 +230,25 @@ def enterprise_scim_users(enterprise_data: Enterprise, ctx: SourceContext): columns=ScimGroup, parallelized=True, ) -def enterprise_scim_groups(enterprise_data: Enterprise, ctx: SourceContext): +def enterprise_scim_groups(scim_organization: ScimOrganization, ctx: SourceContext): scim_client = ctx.scim_client or ctx.client if not scim_client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") - for group in iter_enterprise_scim_resources( - scim_client, - ctx.enterprise_name, - "Groups", - ): - yield { - **group, - "enterprise_node_id": enterprise_data.id, - "enterprise_slug": ctx.enterprise_name, - "emit_legacy_correlation": ctx.emit_legacy_scim_correlations, - } + try: + for group in iter_enterprise_scim_resources( + scim_client, + ctx.enterprise_name, + "Groups", + ): + yield { + **group, + "enterprise_node_id": scim_organization.enterprise_node_id, + "enterprise_slug": ctx.enterprise_name, + "emit_legacy_correlation": ctx.emit_legacy_scim_correlations, + } + except Exception as e: + _log_enterprise_scim_failure("enterprise_scim_groups", ctx.enterprise_name, e) + return @app.transformer(name="enterprise_members", columns=BaseUser, parallelized=True) @@ -794,6 +843,7 @@ def enterprise_resources(ctx: SourceContext): teams_resource = enterprise_teams(ctx) roles_resource = enterprise_roles(ctx) runner_groups_resource = enterprise_runner_groups(ctx) + scim_organizations_resource = enterprise_resource | enterprise_scim_organizations(ctx) resources = [ enterprise_resource, enterprise_resource | organizations_resource, @@ -830,13 +880,12 @@ def enterprise_resources(ctx: SourceContext): ] ) - if ctx.collect_enterprise_scim: - resources.extend( - [ - enterprise_resource | enterprise_scim_organizations(ctx), - enterprise_resource | enterprise_scim_users(ctx), - enterprise_resource | enterprise_scim_groups(ctx), - ] - ) + resources.extend( + [ + scim_organizations_resource, + scim_organizations_resource | enterprise_scim_users(ctx), + scim_organizations_resource | enterprise_scim_groups(ctx), + ] + ) return tuple(resources) diff --git a/src/openhound_github/resources/organization.py b/src/openhound_github/resources/organization.py index 68468bc..65d9df2 100644 --- a/src/openhound_github/resources/organization.py +++ b/src/openhound_github/resources/organization.py @@ -22,7 +22,7 @@ TEAM_MEMBERS_OVERFLOW_QUERY, TEAMS_QUERY, ) -from openhound_github.helpers import GraphQLCursorPaginator +from openhound_github.helpers import GraphQLCursorPaginator, scim_skip_reason from openhound_github.main import app from openhound_github.models import ( ActionPermission, @@ -1917,43 +1917,77 @@ def saml_issuer(saml_provider: SamlProvider, ctx: SourceContext): "github_web_origin": saml_provider.get("github_web_origin"), } -@app.resource(name="scim_users", columns=ScimResource, parallelized=True) -def scim_users(ctx: SourceContext): +def iter_organization_scim_users( + client: RESTClient, + org_name: str, + *, + items_per_page: int = 100, +): + scim_paginator = OffsetPaginator( + offset_param="startIndex", + limit_param="itemsPerPage", + limit=items_per_page, + total_path="totalResults", + ) + for page in client.paginate( + f"/scim/v2/organizations/{org_name}/Users", + params={"startIndex": 1, "itemsPerPage": items_per_page}, + paginator=scim_paginator, + data_selector="Resources", + ): + yield from page + + +def _org_context_for_login(ctx: SourceContext, org_login: str) -> OrgContext | None: + for org in ctx.organizations: + if org.org_name == org_login: + return org + return None + + +def _log_org_scim_failure(resource: str, org_name: str, exception: BaseException): + skip_reason = scim_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 + + logger.error( + f"Error in resource '{resource}' processing organization '{org_name}': {exception}", + extra={"resource": resource, "phase": "resource_iteration"}, + ) + + +@app.transformer(name="scim_users", columns=ScimResource, parallelized=True) +def scim_users(scim_organization: ScimOrganization, ctx: SourceContext): """Fetch SCIM users for the organization. Args: + scim_organization (ScimOrganization): The SCIM scope whose users should be fetched. ctx (SourceContext): The shared context containing the REST client and organization name. Yields: ScimResource (ScimResource): SCIM user record. """ - for org in ctx.organizations: - org_name = org.org_name - client = org.client - try: - scim_paginator = OffsetPaginator( - offset_param="startIndex", - limit_param="itemsPerPage", - limit=100, - total_path="totalResults", - ) - for page in client.paginate( - f"/scim/v2/organizations/{org_name}/Users", - params={"startIndex": 1, "itemsPerPage": 100}, - paginator=scim_paginator, - data_selector="Resources", - ): - for user in page: - yield { - **user, - "org_login": org_name, - } - except Exception as e: - logger.error( - f"Error in resource 'scim_users' processing organization '{org_name}': {e}", - extra={"resource": "scim_users", "phase": "resource_iteration"}, - ) - continue + org_name = scim_organization.org_login + org = _org_context_for_login(ctx, org_name) + if not org: + raise ValueError(f"SCIM collection requires a configured client for organization '{org_name}'") + + try: + for user in iter_organization_scim_users(org.client, org_name): + yield { + **user, + "org_login": org_name, + } + except Exception as e: + _log_org_scim_failure("scim_users", org_name, e) + return @app.transformer( @@ -1961,7 +1995,26 @@ def scim_users(ctx: SourceContext): columns=ScimOrganization, parallelized=True, ) -def org_scim_organizations(org: Organization): +def org_scim_organizations(org: Organization, ctx: SourceContext): + org_context = _org_context_for_login(ctx, org.login) + if not org_context: + raise ValueError( + f"SCIM collection requires a configured client for organization '{org.login}'" + ) + + try: + next( + iter_organization_scim_users( + org_context.client, + org.login, + items_per_page=1, + ), + None, + ) + except Exception as e: + _log_org_scim_failure("org_scim_organizations", org.login, e) + return + yield { "org_login": org.login, "org_node_id": org.node_id, @@ -1989,6 +2042,7 @@ def organization_resources(ctx: SourceContext): organization_vars_resource = organization_variables(ctx) projected_enterprise_teams_resource = projected_enterprise_teams(ctx) saml_resource = saml_provider(ctx) + org_scim_organizations_resource = org_resource | org_scim_organizations(ctx) return ( org_resource, @@ -2007,11 +2061,11 @@ def organization_resources(ctx: SourceContext): repos_resource | repository_variables(ctx), teams_resource, projected_enterprise_teams_resource, - org_resource | org_scim_organizations(), + org_scim_organizations_resource, teams_resource | team_members(ctx), teams_resource | team_roles(), teams_resource | team_repo_role_assignments(ctx, repo_roles_base), - scim_users(ctx), + org_scim_organizations_resource | scim_users(ctx), repositories_graphql_resource, repositories_graphql_resource | branches(ctx), branch_prot_rules_resource, diff --git a/src/openhound_github/source.py b/src/openhound_github/source.py index 4ec8466..3d3384d 100644 --- a/src/openhound_github/source.py +++ b/src/openhound_github/source.py @@ -50,7 +50,6 @@ class SourceContext: sso_client: RESTClient | None = None scim_client: RESTClient | None = None enterprise_name: str | None = None - collect_enterprise_scim: bool = False emit_legacy_scim_correlations: bool = False github_deployment_id: str = DEFAULT_GITHUB_DEPLOYMENT_ID github_web_origin: str = DEFAULT_GITHUB_WEB_ORIGIN @@ -122,7 +121,6 @@ def source( GithubEnterpriseAppCredentials, GithubOrgAppCredentials, GithubTokenCredentials ] = dlt.secrets.value, host: str = "https://api.github.com", - collect_enterprise_scim: bool | None = dlt.config.value, emit_legacy_scim_correlations: bool | None = dlt.config.value, ): """DLT source, defines GitHub collection resources and transformers. @@ -158,7 +156,6 @@ def token_client(token: str) -> RESTClient: ) ctx = SourceContext( enterprise_name=credentials.enterprise_name, - collect_enterprise_scim=bool(collect_enterprise_scim), emit_legacy_scim_correlations=bool(emit_legacy_scim_correlations), github_deployment_id=github_deployment_id, github_web_origin=github_web_origin, @@ -250,7 +247,6 @@ def token_client(token: str) -> RESTClient: if credentials.scim_token else token_api_client, enterprise_name=credentials.enterprise_name, - collect_enterprise_scim=bool(collect_enterprise_scim), emit_legacy_scim_correlations=bool(emit_legacy_scim_correlations), github_deployment_id=github_deployment_id, github_web_origin=github_web_origin, diff --git a/tests/test_app_auth.py b/tests/test_app_auth.py index bfdde26..12fa425 100644 --- a/tests/test_app_auth.py +++ b/tests/test_app_auth.py @@ -194,7 +194,6 @@ def __init__(self, **kwargs) -> None: enterprise_name="example-enterprise", ), host="https://api.github.com", - collect_enterprise_scim=False, emit_legacy_scim_correlations=False, ) diff --git a/tests/test_enterprise_resources.py b/tests/test_enterprise_resources.py index 61a5272..49cb77e 100644 --- a/tests/test_enterprise_resources.py +++ b/tests/test_enterprise_resources.py @@ -1,6 +1,8 @@ import logging from types import SimpleNamespace +import requests + from openhound_github.resources.enterprise import ( SourceContext, enterprise, @@ -10,6 +12,9 @@ enterprise_runner_group_organizations, enterprise_runner_groups, enterprise_runners, + enterprise_resources, + enterprise_scim_organizations, + enterprise_scim_users, enterprise_saml_provider, ) @@ -50,6 +55,19 @@ def post(self, path: str, json: dict): raise ConnectionError("GraphQL endpoint unreachable") +class _HTTPErrorPaginateClient(_FakeClient): + def __init__(self, status_code: int): + super().__init__(payload={}) + 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 test_enterprise_resource_yields_single_record() -> None: client = _FakeClient( { @@ -222,6 +240,82 @@ def test_enterprise_external_identity_logs_and_returns_on_pagination_failure( ) +def test_enterprise_scim_organization_skips_missing_permission(caplog) -> None: + client = _HTTPErrorPaginateClient(status_code=403) + ctx = SourceContext(client=client, enterprise_name="acme") + enterprise_data = SimpleNamespace(id="E_1") + + with caplog.at_level(logging.WARNING, logger="openhound_github.resources.enterprise"): + rows = list(enterprise_scim_organizations.__wrapped__(enterprise_data, ctx)) + + assert rows == [] + assert any( + "Skipping enterprise_scim_organizations for enterprise 'acme': " + "the configured credentials do not have SCIM access" + in message + for message in caplog.messages + ) + + +def test_enterprise_scim_organization_skips_unavailable_scope(caplog) -> None: + client = _HTTPErrorPaginateClient(status_code=404) + ctx = SourceContext(client=client, enterprise_name="acme") + enterprise_data = SimpleNamespace(id="E_1") + + with caplog.at_level(logging.WARNING, logger="openhound_github.resources.enterprise"): + rows = list(enterprise_scim_organizations.__wrapped__(enterprise_data, ctx)) + + assert rows == [] + assert any( + "Skipping enterprise_scim_organizations for enterprise 'acme': " + "the GitHub scope does not expose SCIM endpoints" + in message + for message in caplog.messages + ) + + +def test_enterprise_scim_users_logs_unexpected_failure_as_error(caplog) -> None: + client = _FailingPaginateClient(payload={}) + ctx = SourceContext(client=client, enterprise_name="acme") + scim_organization = SimpleNamespace(enterprise_node_id="E_1") + + with caplog.at_level(logging.ERROR, logger="openhound_github.resources.enterprise"): + rows = list(enterprise_scim_users.__wrapped__(scim_organization, ctx)) + + assert rows == [] + assert any( + "Error in resource 'enterprise_scim_users' processing enterprise 'acme'" + in message + for message in caplog.messages + ) + + +def test_enterprise_resources_register_scim_by_default() -> None: + ctx = SourceContext(client=_FakeClient(payload={}), enterprise_name="acme") + + resources = {resource.name: resource for resource in enterprise_resources(ctx)} + + assert "enterprise_scim_organizations" in resources + assert "enterprise_scim_users" in resources + assert "enterprise_scim_groups" in resources + + +def test_enterprise_scim_children_are_bound_to_successful_scim_scope() -> None: + ctx = SourceContext(client=_FakeClient(payload={}), enterprise_name="acme") + + resources = {resource.name: resource for resource in enterprise_resources(ctx)} + + assert resources["enterprise_scim_organizations"]._pipe.parent.name == "enterprise" + assert ( + resources["enterprise_scim_users"]._pipe.parent.name + == "enterprise_scim_organizations" + ) + assert ( + resources["enterprise_scim_groups"]._pipe.parent.name + == "enterprise_scim_organizations" + ) + + def test_enterprise_runner_groups_use_pat_backed_client() -> None: app_client = _FakeClient(payload={}) pat_client = _FakeClient( diff --git a/tests/test_org_scim_resources.py b/tests/test_org_scim_resources.py new file mode 100644 index 0000000..d8b042b --- /dev/null +++ b/tests/test_org_scim_resources.py @@ -0,0 +1,93 @@ +import logging +from types import SimpleNamespace + +import requests + +from openhound_github.resources.organization import ( + OrgContext, + SourceContext, + organization_resources, + org_scim_organizations, + scim_users, +) + + +class _HTTPErrorPaginateClient: + def __init__(self, status_code: int): + self.status_code = status_code + self.paginate_calls: list[tuple[str, dict]] = [] + + 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) + + +class _FailingPaginateClient: + def paginate(self, path: str, **kwargs): + raise ConnectionError("SCIM endpoint unreachable") + + +def _ctx(client) -> SourceContext: + return SourceContext( + client=client, + organizations=[OrgContext(client=client, org_name="acme")], + ) + + +def test_org_scim_organization_skips_unavailable_scope(caplog) -> None: + client = _HTTPErrorPaginateClient(status_code=404) + organization = SimpleNamespace(login="acme", node_id="O_1") + + with caplog.at_level(logging.WARNING, logger="openhound_github.resources.organization"): + rows = list(org_scim_organizations.__wrapped__(organization, _ctx(client))) + + assert rows == [] + assert any( + "Skipping org_scim_organizations for organization 'acme': " + "the GitHub scope does not expose SCIM endpoints" + in message + for message in caplog.messages + ) + + +def test_org_scim_users_skips_missing_permission(caplog) -> None: + client = _HTTPErrorPaginateClient(status_code=403) + scim_organization = SimpleNamespace(org_login="acme") + + with caplog.at_level(logging.WARNING, logger="openhound_github.resources.organization"): + rows = list(scim_users.__wrapped__(scim_organization, _ctx(client))) + + assert rows == [] + assert any( + "Skipping scim_users for organization 'acme': " + "the configured credentials do not have SCIM access" + in message + for message in caplog.messages + ) + + +def test_org_scim_users_logs_unexpected_failure_as_error(caplog) -> None: + client = _FailingPaginateClient() + scim_organization = SimpleNamespace(org_login="acme") + + with caplog.at_level(logging.ERROR, logger="openhound_github.resources.organization"): + rows = list(scim_users.__wrapped__(scim_organization, _ctx(client))) + + assert rows == [] + assert any( + "Error in resource 'scim_users' processing organization 'acme'" + in message + for message in caplog.messages + ) + + +def test_org_scim_users_are_bound_to_successful_scim_scope() -> None: + client = _HTTPErrorPaginateClient(status_code=404) + + resources = {resource.name: resource for resource in organization_resources(_ctx(client))} + + assert resources["org_scim_organizations"]._pipe.parent.name == "organizations" + assert resources["scim_users"]._pipe.parent.name == "org_scim_organizations" From c46a9f676f8525216d629a97145038ef12851311 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 16:41:15 -0700 Subject: [PATCH 2/8] BED-9446: use installation client for enterprise SCIM --- src/openhound_github/resources/enterprise.py | 16 ++++++---------- src/openhound_github/source.py | 10 ---------- tests/test_enterprise_resources.py | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/openhound_github/resources/enterprise.py b/src/openhound_github/resources/enterprise.py index f61c6ec..9a53f9f 100644 --- a/src/openhound_github/resources/enterprise.py +++ b/src/openhound_github/resources/enterprise.py @@ -54,7 +54,6 @@ class SourceContext: client: RESTClient sso_client: RESTClient | None = None - scim_client: RESTClient | None = None org_name: str | None = None enterprise_name: str | None = None emit_legacy_scim_correlations: bool = False @@ -171,14 +170,13 @@ def enterprise_organizations(enterprise_data: Enterprise, ctx: SourceContext): parallelized=True, ) def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContext): - scim_client = ctx.scim_client or ctx.client - if not scim_client or not ctx.enterprise_name: + if not ctx.client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") try: next( iter_enterprise_scim_resources( - scim_client, + ctx.client, ctx.enterprise_name, "Users", count=1, @@ -205,12 +203,11 @@ def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContex parallelized=True, ) def enterprise_scim_users(scim_organization: ScimOrganization, ctx: SourceContext): - scim_client = ctx.scim_client or ctx.client - if not scim_client or not ctx.enterprise_name: + if not ctx.client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") try: for user in iter_enterprise_scim_resources( - scim_client, + ctx.client, ctx.enterprise_name, "Users", ): @@ -231,12 +228,11 @@ def enterprise_scim_users(scim_organization: ScimOrganization, ctx: SourceContex parallelized=True, ) def enterprise_scim_groups(scim_organization: ScimOrganization, ctx: SourceContext): - scim_client = ctx.scim_client or ctx.client - if not scim_client or not ctx.enterprise_name: + if not ctx.client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") try: for group in iter_enterprise_scim_resources( - scim_client, + ctx.client, ctx.enterprise_name, "Groups", ): diff --git a/src/openhound_github/source.py b/src/openhound_github/source.py index 3d3384d..0999be1 100644 --- a/src/openhound_github/source.py +++ b/src/openhound_github/source.py @@ -48,7 +48,6 @@ class SourceContext: organizations: list[OrgContext] | None = field(default_factory=list) client: RESTClient | None = None sso_client: RESTClient | None = None - scim_client: RESTClient | None = None enterprise_name: str | None = None emit_legacy_scim_correlations: bool = False github_deployment_id: str = DEFAULT_GITHUB_DEPLOYMENT_ID @@ -80,7 +79,6 @@ class GithubEnterpriseAppCredentials(CredentialsConfiguration): key_path: str = None enterprise_name: str = None pat_token: str | None = None - scim_token: str | None = None api_uri: str = "https://api.github.com" @property @@ -104,7 +102,6 @@ def auth(self) -> str: @configspec class GithubTokenCredentials(GithubCredentials): token: str = None - scim_token: str | None = None @property def auth(self) -> str: @@ -162,10 +159,6 @@ def token_client(token: str) -> RESTClient: ) if credentials.pat_token: ctx.sso_client = token_client(credentials.pat_token) - if credentials.scim_token: - ctx.scim_client = token_client(credentials.scim_token) - elif credentials.pat_token: - ctx.scim_client = ctx.sso_client github_app_session = GithubApp( jwt_issuer=jwt_issuer, private_key_path=credentials.key_path, @@ -243,9 +236,6 @@ def token_client(token: str) -> RESTClient: ctx = SourceContext( client=token_api_client, sso_client=token_api_client, - scim_client=token_client(credentials.scim_token) - if credentials.scim_token - else token_api_client, enterprise_name=credentials.enterprise_name, emit_legacy_scim_correlations=bool(emit_legacy_scim_correlations), github_deployment_id=github_deployment_id, diff --git a/tests/test_enterprise_resources.py b/tests/test_enterprise_resources.py index 49cb77e..f8ca413 100644 --- a/tests/test_enterprise_resources.py +++ b/tests/test_enterprise_resources.py @@ -274,6 +274,23 @@ def test_enterprise_scim_organization_skips_unavailable_scope(caplog) -> None: ) +def test_enterprise_scim_uses_enterprise_client_when_sso_client_is_present() -> None: + enterprise_client = _FakeClient(payload={}, pages=[[]]) + sso_client = _FailingPaginateClient(payload={}) + ctx = SourceContext( + client=enterprise_client, + sso_client=sso_client, + enterprise_name="acme", + ) + enterprise_data = SimpleNamespace(id="E_1") + + rows = list(enterprise_scim_organizations.__wrapped__(enterprise_data, ctx)) + + assert rows == [{"enterprise_node_id": "E_1", "enterprise_slug": "acme"}] + assert enterprise_client.paginate_calls[0][0] == "/scim/v2/enterprises/acme/Users" + assert sso_client.paginate_calls == [] + + def test_enterprise_scim_users_logs_unexpected_failure_as_error(caplog) -> None: client = _FailingPaginateClient(payload={}) ctx = SourceContext(client=client, enterprise_name="acme") From 34a3664fa0c71d9490c47d95ecb950489a9783b5 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 19:10:31 -0700 Subject: [PATCH 3/8] BED-9446: preserve enterprise SCIM conversion tables --- src/openhound_github/models/__init__.py | 11 ++++++++++- src/openhound_github/models/scim_user.py | 14 ++++++++++++++ src/openhound_github/resources/enterprise.py | 16 ++++++++++------ tests/test_enterprise_resources.py | 6 ++++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/openhound_github/models/__init__.py b/src/openhound_github/models/__init__.py index 0c52692..cac60f6 100644 --- a/src/openhound_github/models/__init__.py +++ b/src/openhound_github/models/__init__.py @@ -53,7 +53,14 @@ from .saml_provider import SamlProvider from .saml_service_provider import SamlServiceProvider from .saml_issuer import SamlIssuer -from .scim_user import ScimGroup, ScimOrganization, ScimResource, ScimUser +from .scim_user import ( + EnterpriseScimOrganization, + EnterpriseScimUser, + ScimGroup, + ScimOrganization, + ScimResource, + ScimUser, +) from .secret_scanning_alert import SecretScanningAlert from .team import Team from .team_member import TeamMember @@ -117,6 +124,8 @@ "ScimUser", "ScimGroup", "ScimOrganization", + "EnterpriseScimUser", + "EnterpriseScimOrganization", "RepoRoleAssignment", "Environment", "EnvironmentSecret", diff --git a/src/openhound_github/models/scim_user.py b/src/openhound_github/models/scim_user.py index 3151c7d..e576d04 100644 --- a/src/openhound_github/models/scim_user.py +++ b/src/openhound_github/models/scim_user.py @@ -137,6 +137,15 @@ def edges(self): return [] +@app.asset() +class EnterpriseScimOrganization(ScimOrganization): + """Enterprise-scoped SCIM organization input model. + + This remains a distinct asset class so the converter can map enterprise and + organization SCIM tables independently while emitting the same graph kind. + """ + + @app.asset( node=NodeDef( kind=nk.SCIM_USER, @@ -231,6 +240,11 @@ def edges(self): ) +@app.asset() +class EnterpriseScimUser(ScimUser): + """Enterprise-scoped SCIM user input model.""" + + @app.asset( node=NodeDef( kind=nk.SCIM_GROUP, diff --git a/src/openhound_github/resources/enterprise.py b/src/openhound_github/resources/enterprise.py index 9a53f9f..389ca47 100644 --- a/src/openhound_github/resources/enterprise.py +++ b/src/openhound_github/resources/enterprise.py @@ -31,14 +31,14 @@ EnterpriseTeamOrganization, EnterpriseTeamRole, EnterpriseUser, + EnterpriseScimOrganization, + EnterpriseScimUser, SamlProvider, SamlServiceProvider, SamlAssertionConsumerService, SamlIssuer, ExternalIdentity, ScimGroup, - ScimOrganization, - ScimUser, ) from openhound_github.models.saml_helpers import ( DEFAULT_GITHUB_DEPLOYMENT_ID, @@ -166,7 +166,7 @@ def enterprise_organizations(enterprise_data: Enterprise, ctx: SourceContext): @app.transformer( name="enterprise_scim_organizations", - columns=ScimOrganization, + columns=EnterpriseScimOrganization, parallelized=True, ) def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContext): @@ -199,10 +199,12 @@ def enterprise_scim_organizations(enterprise_data: Enterprise, ctx: SourceContex @app.transformer( name="enterprise_scim_users", - columns=ScimUser, + columns=EnterpriseScimUser, parallelized=True, ) -def enterprise_scim_users(scim_organization: ScimOrganization, ctx: SourceContext): +def enterprise_scim_users( + scim_organization: EnterpriseScimOrganization, ctx: SourceContext +): if not ctx.client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") try: @@ -227,7 +229,9 @@ def enterprise_scim_users(scim_organization: ScimOrganization, ctx: SourceContex columns=ScimGroup, parallelized=True, ) -def enterprise_scim_groups(scim_organization: ScimOrganization, ctx: SourceContext): +def enterprise_scim_groups( + scim_organization: EnterpriseScimOrganization, ctx: SourceContext +): if not ctx.client or not ctx.enterprise_name: raise ValueError("Enterprise SCIM collection requires a client and enterprise slug") try: diff --git a/tests/test_enterprise_resources.py b/tests/test_enterprise_resources.py index f8ca413..895d809 100644 --- a/tests/test_enterprise_resources.py +++ b/tests/test_enterprise_resources.py @@ -3,6 +3,7 @@ import requests +from openhound_github.models import EnterpriseScimOrganization, EnterpriseScimUser from openhound_github.resources.enterprise import ( SourceContext, enterprise, @@ -315,6 +316,11 @@ def test_enterprise_resources_register_scim_by_default() -> None: assert "enterprise_scim_organizations" in resources assert "enterprise_scim_users" in resources assert "enterprise_scim_groups" in resources + assert ( + resources["enterprise_scim_organizations"].validator.model + is EnterpriseScimOrganization + ) + assert resources["enterprise_scim_users"].validator.model is EnterpriseScimUser def test_enterprise_scim_children_are_bound_to_successful_scim_scope() -> None: From 2fd1a814edcbafe90950b932e60588146bb2f0eb Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 19:37:24 -0700 Subject: [PATCH 4/8] BED-9446: use SCIM username for user node names --- src/openhound_github/models/scim_user.py | 10 +++++----- tests/test_scim_models.py | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/openhound_github/models/scim_user.py b/src/openhound_github/models/scim_user.py index e576d04..d5e2198 100644 --- a/src/openhound_github/models/scim_user.py +++ b/src/openhound_github/models/scim_user.py @@ -32,7 +32,7 @@ def scim_organization_id(scope_node_id: str) -> str: class ScimNodeProperties(NodeProperties): collected: bool = True external_id: str | None = None - user_name: str | None = None + username: str | None = None enabled: bool | None = None given_name: str | None = None family_name: str | None = None @@ -181,7 +181,7 @@ class EnterpriseScimOrganization(ScimOrganization): class ScimUser(ScimScopeAsset): id: str external_id: str | None = Field(default=None, alias="externalId") - user_name: str | None = Field(default=None, alias="userName") + username: str | None = Field(default=None, alias="userName") display_name: str | None = Field(default=None, alias="displayName") name: Name | None = None emails: list[dict[str, Any]] = Field(default_factory=list) @@ -193,16 +193,16 @@ class ScimUser(ScimScopeAsset): @property def as_node(self) -> ScimNode: - display_name = self.display_name or self.user_name or self.id + display_name = self.display_name or self.username or self.id return ScimNode( id=self.id, kinds=[nk.SCIM_USER], properties=ScimNodeProperties( - name=self.id, + name=self.username or self.id, displayname=display_name, environmentid=self.scope_node_id, external_id=self.external_id, - user_name=self.user_name, + username=self.username, enabled=self.active, given_name=self.name.given_name if self.name else None, family_name=self.name.family_name if self.name else None, diff --git a/tests/test_scim_models.py b/tests/test_scim_models.py index 72663db..4cbb89e 100644 --- a/tests/test_scim_models.py +++ b/tests/test_scim_models.py @@ -52,6 +52,8 @@ def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default( assert node.kinds == [nk.SCIM_USER] assert node.properties.environmentid == "ENT_NODE_1" assert node.properties.external_id == "00u-okta-1" + assert node.properties.name == "alice@example.test" + assert node.properties.username == "alice@example.test" assert [edge.kind for edge in edges] == [ ek.SCIM_CONTAINS, ek.SCIM_PROVISIONED, From 036b45d430c343a5e2f8d637037246c8b98d4c31 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 19:46:40 -0700 Subject: [PATCH 5/8] BED-9446: preserve SCIM user_name property --- src/openhound_github/models/scim_user.py | 10 +++++----- tests/test_scim_models.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/openhound_github/models/scim_user.py b/src/openhound_github/models/scim_user.py index d5e2198..0ac5f38 100644 --- a/src/openhound_github/models/scim_user.py +++ b/src/openhound_github/models/scim_user.py @@ -32,7 +32,7 @@ def scim_organization_id(scope_node_id: str) -> str: class ScimNodeProperties(NodeProperties): collected: bool = True external_id: str | None = None - username: str | None = None + user_name: str | None = None enabled: bool | None = None given_name: str | None = None family_name: str | None = None @@ -181,7 +181,7 @@ class EnterpriseScimOrganization(ScimOrganization): class ScimUser(ScimScopeAsset): id: str external_id: str | None = Field(default=None, alias="externalId") - username: str | None = Field(default=None, alias="userName") + user_name: str | None = Field(default=None, alias="userName") display_name: str | None = Field(default=None, alias="displayName") name: Name | None = None emails: list[dict[str, Any]] = Field(default_factory=list) @@ -193,16 +193,16 @@ class ScimUser(ScimScopeAsset): @property def as_node(self) -> ScimNode: - display_name = self.display_name or self.username or self.id + display_name = self.display_name or self.user_name or self.id return ScimNode( id=self.id, kinds=[nk.SCIM_USER], properties=ScimNodeProperties( - name=self.username or self.id, + name=self.user_name or self.id, displayname=display_name, environmentid=self.scope_node_id, external_id=self.external_id, - username=self.username, + user_name=self.user_name, enabled=self.active, given_name=self.name.given_name if self.name else None, family_name=self.name.family_name if self.name else None, diff --git a/tests/test_scim_models.py b/tests/test_scim_models.py index 4cbb89e..b8fb808 100644 --- a/tests/test_scim_models.py +++ b/tests/test_scim_models.py @@ -53,7 +53,7 @@ def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default( assert node.properties.environmentid == "ENT_NODE_1" assert node.properties.external_id == "00u-okta-1" assert node.properties.name == "alice@example.test" - assert node.properties.username == "alice@example.test" + assert node.properties.user_name == "alice@example.test" assert [edge.kind for edge in edges] == [ ek.SCIM_CONTAINS, ek.SCIM_PROVISIONED, From 56fc04d3eaa66dedd42455417d3a1a879ad4e212 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 20:31:03 -0700 Subject: [PATCH 6/8] BED-9446: resolve projected enterprise teams by id --- src/openhound_github/lookup.py | 4 +-- .../models/enterprise_helpers.py | 4 +++ .../models/enterprise_team_organization.py | 27 ++++++++++--------- .../models/projected_enterprise_team.py | 3 ++- tests/test_enterprise_organization.py | 15 ++++------- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/openhound_github/lookup.py b/src/openhound_github/lookup.py index 4c37782..439b188 100644 --- a/src/openhound_github/lookup.py +++ b/src/openhound_github/lookup.py @@ -188,9 +188,9 @@ def org_login_for_id(self, org_node_id: str) -> str | None: ) @lru_cache - def projected_enterprise_team_exists(self, org_login: str, slug: str): + def projected_enterprise_team_id(self, org_login: str, slug: str) -> str | None: return self._find_single_object( - f"""SELECT slug FROM {self.schema}.projected_enterprise_teams WHERE org_login = ? AND slug = ?""", + f"""SELECT node_id FROM {self.schema}.projected_enterprise_teams WHERE org_login = ? AND slug = ?""", [org_login, slug], ) diff --git a/src/openhound_github/models/enterprise_helpers.py b/src/openhound_github/models/enterprise_helpers.py index 2b382ac..e96b569 100644 --- a/src/openhound_github/models/enterprise_helpers.py +++ b/src/openhound_github/models/enterprise_helpers.py @@ -2,5 +2,9 @@ def enterprise_team_node_id(enterprise_id: str, team_id: str | int) -> str: return f"GH_EnterpriseTeam_{enterprise_id}_{team_id}" +def projected_enterprise_team_node_id(org_id: str | None, team_node_id: str) -> str: + return f"GH_Team_{org_id}_{team_node_id}" + + def enterprise_role_node_id(enterprise_id: str, role_id: str | int) -> str: return f"GH_EnterpriseRole_{enterprise_id}_{role_id}" diff --git a/src/openhound_github/models/enterprise_team_organization.py b/src/openhound_github/models/enterprise_team_organization.py index 51613e9..5aed94a 100644 --- a/src/openhound_github/models/enterprise_team_organization.py +++ b/src/openhound_github/models/enterprise_team_organization.py @@ -1,16 +1,17 @@ from openhound.core.asset import BaseAsset, EdgeDef from openhound.core.models.entries_dataclass import ( - ConditionalEdgePath, Edge, EdgePath, EdgeProperties, - PropertyMatch, ) from openhound_github.kinds import edges as ek from openhound_github.kinds import nodes as nk from openhound_github.main import app -from openhound_github.models.enterprise_helpers import enterprise_team_node_id +from openhound_github.models.enterprise_helpers import ( + enterprise_team_node_id, + projected_enterprise_team_node_id, +) @app.asset( @@ -59,18 +60,20 @@ def _assigned_to_edge(self): @property def member_of_team_edges(self): org_login = self.login or self._lookup.org_login_for_id(self.node_id) - if org_login and self._lookup.projected_enterprise_team_exists( - org_login, self.projected_slug - ): + projected_team_id = ( + self._lookup.projected_enterprise_team_id(org_login, self.projected_slug) + if org_login + else None + ) + if projected_team_id: yield Edge( kind=ek.MEMBER_OF, start=EdgePath(value=self.enterprise_team_node_id, match_by="id"), - end=ConditionalEdgePath( - kind=nk.TEAM, - property_matchers=[ - PropertyMatch(key="environmentid", value=self.node_id), - PropertyMatch(key="slug", value=self.projected_slug), - ], + end=EdgePath( + value=projected_enterprise_team_node_id( + self.node_id, projected_team_id + ), + match_by="id", ), properties=EdgeProperties(traversable=True), ) diff --git a/src/openhound_github/models/projected_enterprise_team.py b/src/openhound_github/models/projected_enterprise_team.py index 8edb4ff..fe98e4a 100644 --- a/src/openhound_github/models/projected_enterprise_team.py +++ b/src/openhound_github/models/projected_enterprise_team.py @@ -7,6 +7,7 @@ from openhound_github.graph import GHNode from openhound_github.kinds import nodes as nk from openhound_github.main import app +from openhound_github.models.enterprise_helpers import projected_enterprise_team_node_id from openhound_github.models.team import GHTeamProperties @@ -43,7 +44,7 @@ def as_node(self) -> GHNode: properties=GHTeamProperties( name=self.name, displayname=self.name, - node_id=f"GH_Team_{self.org_node_id}_{self.node_id}", + node_id=projected_enterprise_team_node_id(self.org_node_id, self.node_id), github_team_id=self.node_id, collected=False, slug=self.slug, diff --git a/tests/test_enterprise_organization.py b/tests/test_enterprise_organization.py index 5006130..b15c4e9 100644 --- a/tests/test_enterprise_organization.py +++ b/tests/test_enterprise_organization.py @@ -69,7 +69,7 @@ def fake_lookup(node_id: str): assert stub_org.as_node.properties.collected is False -def test_enterprise_team_organization_matcher_preserves_org_node_id_case() -> None: +def test_enterprise_team_organization_uses_projected_team_node_id() -> None: org_node_id = "MDEyOk9yZ2FuaXphdGlvbjE=" assignment = EnterpriseTeamOrganization( node_id=org_node_id, @@ -80,15 +80,10 @@ def test_enterprise_team_organization_matcher_preserves_org_node_id_case() -> No enterprise_slug="example-enterprise", ) lookup = MagicMock() - lookup.projected_enterprise_team_exists.return_value = True + lookup.projected_enterprise_team_id.return_value = "TEAM_NODE_1" assignment._lookup = lookup edge = next(assignment.member_of_team_edges) - matcher_values = { - matcher.key: matcher.value for matcher in edge.end.property_matchers - } - - assert matcher_values == { - "environmentid": org_node_id, - "slug": "ent:engineering", - } + + assert edge.end.value == f"GH_Team_{org_node_id}_TEAM_NODE_1" + assert edge.end.match_by == "id" From 6ae184191cc6c7186208ef3b873a0135f9dc5a37 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 20:54:48 -0700 Subject: [PATCH 7/8] BED-9446: resolve SCIM external identities by id --- src/openhound_github/lookup.py | 9 +++++++++ src/openhound_github/main.py | 1 + src/openhound_github/models/scim_user.py | 17 +++++++++-------- src/openhound_github/transforms.py | 5 +++++ tests/test_scim_models.py | 15 ++++++++++----- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/openhound_github/lookup.py b/src/openhound_github/lookup.py index 439b188..b4458c4 100644 --- a/src/openhound_github/lookup.py +++ b/src/openhound_github/lookup.py @@ -194,6 +194,15 @@ def projected_enterprise_team_id(self, org_login: str, slug: str) -> str | None: [org_login, slug], ) + @lru_cache + def external_identity_id_for_guid( + self, guid: str, environment_slug: str + ) -> str | None: + return self._find_single_object( + f"""SELECT id FROM {self.schema}.external_identities WHERE guid = ? AND environment_slug = ?""", + [guid, environment_slug], + ) + @lru_cache def repository_node_ids(self): return self._find_all_objects( diff --git a/src/openhound_github/main.py b/src/openhound_github/main.py index 708bcdf..778df58 100644 --- a/src/openhound_github/main.py +++ b/src/openhound_github/main.py @@ -63,6 +63,7 @@ def preproc(ctx: PreProcContext): "teams": "teams", "team_members": "team_members", "saml_provider": "saml_provider", + "external_identities": "external_identities", "applications": "applications", "enterprise": "enterprise", "enterprise_organizations": "enterprise_organizations", diff --git a/src/openhound_github/models/scim_user.py b/src/openhound_github/models/scim_user.py index 0ac5f38..48d3fe4 100644 --- a/src/openhound_github/models/scim_user.py +++ b/src/openhound_github/models/scim_user.py @@ -222,15 +222,16 @@ def edges(self): end=EdgePath(value=self.id, match_by="id"), properties=EdgeProperties(traversable=True), ) - yield Edge( - kind=ek.SCIM_PROVISIONED, - start=EdgePath(value=self.id, match_by="id"), - end=ConditionalEdgePath( - kind=nk.EXTERNAL_IDENTITY, - property_matchers=[PropertyMatch(key="guid", value=self.id)], - ), - properties=EdgeProperties(traversable=True), + external_identity_id = self._lookup.external_identity_id_for_guid( + self.id, self.scope_name ) + if external_identity_id: + yield Edge( + kind=ek.SCIM_PROVISIONED, + start=EdgePath(value=self.id, match_by="id"), + end=EdgePath(value=external_identity_id, match_by="id"), + properties=EdgeProperties(traversable=True), + ) if self.emit_legacy_correlation and self.external_id: yield Edge( kind=ek.SCIM_PROVISIONED, diff --git a/src/openhound_github/transforms.py b/src/openhound_github/transforms.py index 006fb2e..1b7aea2 100644 --- a/src/openhound_github/transforms.py +++ b/src/openhound_github/transforms.py @@ -62,6 +62,11 @@ def ensure_optional_input_tables( team_id VARCHAR, id VARCHAR ); + CREATE TABLE IF NOT EXISTS {schema}.external_identities ( + id VARCHAR, + guid VARCHAR, + environment_slug VARCHAR + ); CREATE TABLE IF NOT EXISTS {schema}.org_roles ( id BIGINT, name VARCHAR, diff --git a/tests/test_scim_models.py b/tests/test_scim_models.py index b8fb808..87d0fa5 100644 --- a/tests/test_scim_models.py +++ b/tests/test_scim_models.py @@ -11,7 +11,7 @@ def _scim_user(*, legacy: bool = False) -> ScimUser: - return ScimUser( + user = ScimUser( id="scim-user-1", externalId="00u-okta-1", userName="alice@example.test", @@ -23,6 +23,10 @@ def _scim_user(*, legacy: bool = False) -> ScimUser: enterprise_slug="example-enterprise", emit_legacy_correlation=legacy, ) + lookup = MagicMock() + lookup.external_identity_id_for_guid.return_value = "external-identity-1" + user._lookup = lookup + return user def test_enterprise_scim_uses_enterprise_endpoint_and_scim_pagination() -> None: @@ -58,10 +62,11 @@ def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default( ek.SCIM_CONTAINS, ek.SCIM_PROVISIONED, ] - assert isinstance(edges[1].end, ConditionalEdgePath) - assert edges[1].end.kind == nk.EXTERNAL_IDENTITY - assert edges[1].end.property_matchers[0].key == "guid" - assert edges[1].end.property_matchers[0].value == "scim-user-1" + assert edges[1].end.value == "external-identity-1" + assert edges[1].end.match_by == "id" + user._lookup.external_identity_id_for_guid.assert_called_once_with( + "scim-user-1", "example-enterprise" + ) def test_scim_user_legacy_idp_correlation_is_explicitly_gated() -> None: From a2a34ec8e033277702d706f321505d377a1fcf42 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Thu, 20 Aug 2026 21:37:21 -0700 Subject: [PATCH 8/8] BED-9446: address SCIM review feedback --- src/openhound_github/lookup.py | 15 ++++- .../resources/organization.py | 11 ++-- tests/test_enterprise_organization.py | 3 + tests/test_lookup.py | 28 ++++++++ tests/test_org_scim_resources.py | 18 ++++++ tests/test_scim_models.py | 64 +++++++++++++++---- 6 files changed, 119 insertions(+), 20 deletions(-) create mode 100644 tests/test_lookup.py diff --git a/src/openhound_github/lookup.py b/src/openhound_github/lookup.py index b4458c4..798a04e 100644 --- a/src/openhound_github/lookup.py +++ b/src/openhound_github/lookup.py @@ -1,4 +1,5 @@ import json +import re from functools import lru_cache import duckdb @@ -8,10 +9,20 @@ from openhound_github.runner_ids import runner_group_node_id, runner_node_id +_SCHEMA_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + + +def _validate_schema_identifier(schema: str) -> str: + if not _SCHEMA_IDENTIFIER_RE.fullmatch(schema): + raise ValueError(f"Invalid DuckDB schema identifier: {schema!r}") + return schema + + class GithubLookup(LookupManager): def __init__(self, client: DuckDBPyConnection, schema: str = "github"): - super().__init__(client, schema) - self.schema = schema + validated_schema = _validate_schema_identifier(schema) + super().__init__(client, validated_schema) + self.schema = validated_schema self.client = client def _find_single_row(self, *args): diff --git a/src/openhound_github/resources/organization.py b/src/openhound_github/resources/organization.py index 65d9df2..931eb86 100644 --- a/src/openhound_github/resources/organization.py +++ b/src/openhound_github/resources/organization.py @@ -1921,17 +1921,18 @@ def iter_organization_scim_users( client: RESTClient, org_name: str, *, - items_per_page: int = 100, + count: int = 100, ): scim_paginator = OffsetPaginator( offset_param="startIndex", - limit_param="itemsPerPage", - limit=items_per_page, + limit_param="count", + limit=count, + offset=1, total_path="totalResults", ) for page in client.paginate( f"/scim/v2/organizations/{org_name}/Users", - params={"startIndex": 1, "itemsPerPage": items_per_page}, + params={"startIndex": 1, "count": count}, paginator=scim_paginator, data_selector="Resources", ): @@ -2007,7 +2008,7 @@ def org_scim_organizations(org: Organization, ctx: SourceContext): iter_organization_scim_users( org_context.client, org.login, - items_per_page=1, + count=1, ), None, ) diff --git a/tests/test_enterprise_organization.py b/tests/test_enterprise_organization.py index b15c4e9..2fc9069 100644 --- a/tests/test_enterprise_organization.py +++ b/tests/test_enterprise_organization.py @@ -85,5 +85,8 @@ def test_enterprise_team_organization_uses_projected_team_node_id() -> None: edge = next(assignment.member_of_team_edges) + lookup.projected_enterprise_team_id.assert_called_once_with( + "github", "ent:engineering" + ) assert edge.end.value == f"GH_Team_{org_node_id}_TEAM_NODE_1" assert edge.end.match_by == "id" diff --git a/tests/test_lookup.py b/tests/test_lookup.py new file mode 100644 index 0000000..a88e962 --- /dev/null +++ b/tests/test_lookup.py @@ -0,0 +1,28 @@ +import duckdb +import pytest + +from openhound_github.lookup import GithubLookup + + +def test_github_lookup_accepts_plain_schema_identifiers() -> None: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github_test") + connection.execute( + "CREATE TABLE github_test.projected_enterprise_teams " + "(node_id VARCHAR, org_login VARCHAR, slug VARCHAR)" + ) + connection.execute( + "INSERT INTO github_test.projected_enterprise_teams VALUES (?, ?, ?)", + ["TEAM_1", "acme", "ent:security"], + ) + + lookup = GithubLookup(connection, schema="github_test") + + assert lookup.projected_enterprise_team_id("acme", "ent:security") == "TEAM_1" + + +def test_github_lookup_rejects_untrusted_schema_identifiers() -> None: + connection = duckdb.connect(":memory:") + + with pytest.raises(ValueError, match="Invalid DuckDB schema identifier"): + GithubLookup(connection, schema="github; DROP SCHEMA github") diff --git a/tests/test_org_scim_resources.py b/tests/test_org_scim_resources.py index d8b042b..6c429cd 100644 --- a/tests/test_org_scim_resources.py +++ b/tests/test_org_scim_resources.py @@ -1,11 +1,13 @@ import logging from types import SimpleNamespace +from unittest.mock import MagicMock import requests from openhound_github.resources.organization import ( OrgContext, SourceContext, + iter_organization_scim_users, organization_resources, org_scim_organizations, scim_users, @@ -37,6 +39,22 @@ def _ctx(client) -> SourceContext: ) +def test_org_scim_uses_scim_count_pagination() -> None: + client = MagicMock() + client.paginate.return_value = [[{"id": "u1"}], [{"id": "u2"}]] + + rows = list(iter_organization_scim_users(client, "acme")) + + assert rows == [{"id": "u1"}, {"id": "u2"}] + args, kwargs = client.paginate.call_args + assert args[0] == "/scim/v2/organizations/acme/Users" + assert kwargs["params"] == {"startIndex": 1, "count": 100} + assert kwargs["data_selector"] == "Resources" + assert kwargs["paginator"].param_name == "startIndex" + assert kwargs["paginator"].initial_value == 1 + assert kwargs["paginator"].limit_param == "count" + + def test_org_scim_organization_skips_unavailable_scope(caplog) -> None: client = _HTTPErrorPaginateClient(status_code=404) organization = SimpleNamespace(login="acme", node_id="O_1") diff --git a/tests/test_scim_models.py b/tests/test_scim_models.py index 87d0fa5..0db8f8d 100644 --- a/tests/test_scim_models.py +++ b/tests/test_scim_models.py @@ -1,18 +1,23 @@ +import gzip +import json +from pathlib import Path from unittest.mock import MagicMock import duckdb from openhound.core.models.entries_dataclass import ConditionalEdgePath +from openhound.core.progress import Progress from openhound_github.kinds import edges as ek from openhound_github.kinds import nodes as nk from openhound_github.lookup import GithubLookup +from openhound_github.main import preproc from openhound_github.models import EnterpriseTeam, ScimGroup, ScimOrganization, ScimUser from openhound_github.resources.enterprise import iter_enterprise_scim_resources -def _scim_user(*, legacy: bool = False) -> ScimUser: - user = ScimUser( - id="scim-user-1", +def _scim_user(*, legacy: bool = False, user_id: str = "scim-user-1") -> ScimUser: + return ScimUser( + id=user_id, externalId="00u-okta-1", userName="alice@example.test", displayName="Alice Example", @@ -23,10 +28,35 @@ def _scim_user(*, legacy: bool = False) -> ScimUser: enterprise_slug="example-enterprise", emit_legacy_correlation=legacy, ) - lookup = MagicMock() - lookup.external_identity_id_for_guid.return_value = "external-identity-1" - user._lookup = lookup - return user + + +def _external_identity_lookup(tmp_path: Path) -> GithubLookup: + resource_dir = tmp_path / "external_identities" + resource_dir.mkdir() + with gzip.open(resource_dir / "rows.jsonl.gz", "wt", encoding="utf-8") as f: + for row in [ + { + "id": "external-identity-wrong-guid", + "guid": "scim-user-2", + "environment_slug": "example-enterprise", + }, + { + "id": "external-identity-wrong-scope", + "guid": "scim-user-1", + "environment_slug": "other-enterprise", + }, + { + "id": "external-identity-1", + "guid": "scim-user-1", + "environment_slug": "example-enterprise", + }, + ]: + f.write(json.dumps(row)) + f.write("\n") + + lookup_file = tmp_path / "lookup.duckdb" + preproc(tmp_path, lookup_file, progress=Progress.log) + return GithubLookup(duckdb.connect(str(lookup_file))) def test_enterprise_scim_uses_enterprise_endpoint_and_scim_pagination() -> None: @@ -47,11 +77,18 @@ def test_enterprise_scim_uses_enterprise_endpoint_and_scim_pagination() -> None: assert kwargs["paginator"].limit_param == "count" -def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default() -> None: +def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default( + tmp_path: Path, +) -> None: + lookup = _external_identity_lookup(tmp_path) user = _scim_user() + user._lookup = lookup + unmatched_user = _scim_user(user_id="scim-user-missing") + unmatched_user._lookup = lookup node = user.as_node edges = list(user.edges) + unmatched_edges = list(unmatched_user.edges) assert node.kinds == [nk.SCIM_USER] assert node.properties.environmentid == "ENT_NODE_1" @@ -64,13 +101,14 @@ def test_scim_user_emits_normalized_edges_without_legacy_correlation_by_default( ] assert edges[1].end.value == "external-identity-1" assert edges[1].end.match_by == "id" - user._lookup.external_identity_id_for_guid.assert_called_once_with( - "scim-user-1", "example-enterprise" - ) + assert [edge.kind for edge in unmatched_edges] == [ek.SCIM_CONTAINS] -def test_scim_user_legacy_idp_correlation_is_explicitly_gated() -> None: - edges = list(_scim_user(legacy=True).edges) +def test_scim_user_legacy_idp_correlation_is_explicitly_gated(tmp_path: Path) -> None: + user = _scim_user(legacy=True) + user._lookup = _external_identity_lookup(tmp_path) + + edges = list(user.edges) legacy_edge = edges[-1] assert legacy_edge.kind == ek.SCIM_PROVISIONED