BED-8788: add PAT-backed enterprise SSO collection and fix repeated enterprise SSO pagination#22
Conversation
…nterprise SSO pagination - add optional pat_token support for enterprise app sources - create a dedicated SSO client/context when a PAT is provided - gate enterprise SSO collections on the presence of that client - run enterprise SAML provider and external identity collection with the PAT client - fix enterprise resource iteration so SSO collection does not repeat per org page - give PAT-backed requests the same retry/backoff behavior as the app client - add enterprise resource coverage tests
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds PAT-backed SSO support, environment-scoped SAML graph assets and identity mapping, enterprise and organization resource changes, workflow containment queries, empty-input table initialization, and enterprise resource tests. ChangesEnterprise SAML and graph integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Source as enterprise_app
participant Resources as enterprise_resources
participant SSO as sso_client
participant API as GraphQL API
participant Models as SAML graph assets
Source->>Resources: provide optional PAT-backed sso_client
Resources->>SSO: request EnterpriseSAMLProvider
SSO->>API: send GraphQL request
API-->>SSO: return provider data
SSO-->>Resources: yield environment-scoped provider
Resources->>Models: build service provider, issuer, ACS, and identity nodes
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tests/test_enterprise_resources.py (4)
55-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the post-call assertion.
Only the call count is checked; the actual GraphQL variables (e.g.,
enterprise_name) sent in the post body aren't verified. A regression that sends the wrong enterprise slug/name would pass this test silently.Suggested strengthening
assert len(rows) == 1 assert rows[0].id == "E_1" assert len(client.post_calls) == 1 + _, posted_json = client.post_calls[0] + assert posted_json["variables"]["slug"] == "acme"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_enterprise_resources.py` at line 55, Strengthen the enterprise resources test by verifying the actual GraphQL payload, not just the number of post calls. In the test around client.post_calls, assert the posted body/variables include the expected enterprise_name (and any other key GraphQL variables) so a wrong slug/name in the request will fail. Use the existing client.post_calls structure in tests/test_enterprise_resources.py to inspect the single post call and validate its variables.
89-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlso assert the pagination call arguments.
Only
len(client.paginate_calls) == 1is checked (line 93); the kwargs passed topaginate(query, variables including the enterprise node id/cursor) aren't verified. Given the PR's stated fix is specifically about correct enterprise resource iteration/pagination variables, asserting onclient.paginate_calls[0][1]would give stronger regression protection for the exact bug this PR fixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_enterprise_resources.py` around lines 89 - 93, The test for enterprise resource iteration only checks that paginate was called once, but it does not verify the query or variables used. Update the test around the paginate call in the enterprise resources flow to assert the captured arguments from client.paginate_calls[0][1], including the enterprise node id and cursor-related variables, so the regression coverage matches the pagination fix.
87-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReliance on
__wrapped__couples the test to decorator internals.Accessing
enterprise_organizations.__wrapped__assumes the resource/transformer decorator preserves the original callable viafunctools.wraps(or equivalent). If the decorator implementation changes (e.g., wraps differently or omits__wrapped__), this test breaks with an unrelatedAttributeErrorrather than a meaningful assertion failure. Consider invoking the resource through its public/documented test surface if the underlying decorator (likely dlt's@dlt.transformer) exposes one, or add a comment explaining why__wrapped__is required here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_enterprise_resources.py` at line 87, The test is reaching into `enterprise_organizations.__wrapped__`, which ties it to decorator internals and can fail with an `AttributeError` if the wrapper implementation changes. Update the test to use the public/documented way to invoke the `enterprise_organizations` resource/transformer in `tests/test_enterprise_resources.py`, or add a short justification comment if calling the underlying callable directly is truly required. Keep the assertion focused on the emitted rows rather than the decorator behavior.
58-93: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftMissing coverage for the SSO-gated resources.
The PR's core change is gating
enterprise_saml_provider/enterprise_external_identitiesonsso_clientpresence, but this test file only coversenterpriseandenterprise_organizations. No test verifies the skip/warning behavior whensso_clientis absent, nor the happy path when it is present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_enterprise_resources.py` around lines 58 - 93, Add test coverage for the SSO-gated enterprise resources so the new gating behavior is verified. In the enterprise resources test module, add one case for the absence of sso_client that confirms enterprise_saml_provider and enterprise_external_identities are skipped or warn as intended, and another case for the present sso_client path that confirms both resources execute normally through the enterprise_saml_provider and enterprise_external_identities entry points. Use the existing SourceContext, _FakeClient, and enterprise_data setup patterns to keep the tests aligned with the current enterprise_organizations coverage.src/openhound_github/graphql.py (1)
169-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate field list between the two SAML queries.
ENTERPRISE_SAML_PROVIDER_QUERY'ssamlIdentityProviderfield selection is byte-for-byte duplicated fromENTERPRISE_SAML_QUERY(Lines 125-131). Consider extracting a shared GraphQL fragment for the provider fields to keep both queries in sync as the schema evolves.♻️ Example using a shared fragment
fragment SamlIdentityProviderFields on SamlIdentityProvider { id issuer ssoUrl digestMethod signatureMethod idpCertificate }Then reference
...SamlIdentityProviderFieldsin both queries instead of repeating the field list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/graphql.py` around lines 169 - 188, The SAML provider field selection is duplicated between ENTERPRISE_SAML_QUERY and ENTERPRISE_SAML_PROVIDER_QUERY, so extract the repeated samlIdentityProvider fields into a shared GraphQL fragment and reuse it in both query strings. Update the query definitions in graphql.py to reference the new fragment from the existing ENTERPRISE_SAML_QUERY and ENTERPRISE_SAML_PROVIDER_QUERY symbols so future schema changes only need one edit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhound_github/resources/enterprise.py`:
- Around line 70-96: Add the same graceful error handling used in enterprise()
around the new enterprise_organizations GraphQL pagination flow. Wrap the
ctx.client.paginate("/graphql", ...) loop in try/except (including HTTPError or
a broader request exception if consistent with the module), log the failure with
enough context, and return/stop cleanly instead of letting the exception escape.
Use the enterprise_organizations paginator setup and the ctx.client.paginate
call as the key locations to update.
- Around line 345-372: Add error handling around the SSO GraphQL request in
enterprise_saml_provider: the unguarded client.post("/graphql",
json=data).json() call can raise on non-2xx or network failures and should
degrade gracefully like enterprise(). Wrap the POST/JSON fetch in a try/except,
log a warning or error with ctx.enterprise_name and the exception details, then
return early when the SSO-backed lookup fails; keep the existing saml_provider
parsing and yield logic unchanged when the request succeeds.
---
Nitpick comments:
In `@src/openhound_github/graphql.py`:
- Around line 169-188: The SAML provider field selection is duplicated between
ENTERPRISE_SAML_QUERY and ENTERPRISE_SAML_PROVIDER_QUERY, so extract the
repeated samlIdentityProvider fields into a shared GraphQL fragment and reuse it
in both query strings. Update the query definitions in graphql.py to reference
the new fragment from the existing ENTERPRISE_SAML_QUERY and
ENTERPRISE_SAML_PROVIDER_QUERY symbols so future schema changes only need one
edit.
In `@tests/test_enterprise_resources.py`:
- Line 55: Strengthen the enterprise resources test by verifying the actual
GraphQL payload, not just the number of post calls. In the test around
client.post_calls, assert the posted body/variables include the expected
enterprise_name (and any other key GraphQL variables) so a wrong slug/name in
the request will fail. Use the existing client.post_calls structure in
tests/test_enterprise_resources.py to inspect the single post call and validate
its variables.
- Around line 89-93: The test for enterprise resource iteration only checks that
paginate was called once, but it does not verify the query or variables used.
Update the test around the paginate call in the enterprise resources flow to
assert the captured arguments from client.paginate_calls[0][1], including the
enterprise node id and cursor-related variables, so the regression coverage
matches the pagination fix.
- Line 87: The test is reaching into `enterprise_organizations.__wrapped__`,
which ties it to decorator internals and can fail with an `AttributeError` if
the wrapper implementation changes. Update the test to use the public/documented
way to invoke the `enterprise_organizations` resource/transformer in
`tests/test_enterprise_resources.py`, or add a short justification comment if
calling the underlying callable directly is truly required. Keep the assertion
focused on the emitted rows rather than the decorator behavior.
- Around line 58-93: Add test coverage for the SSO-gated enterprise resources so
the new gating behavior is verified. In the enterprise resources test module,
add one case for the absence of sso_client that confirms
enterprise_saml_provider and enterprise_external_identities are skipped or warn
as intended, and another case for the present sso_client path that confirms both
resources execute normally through the enterprise_saml_provider and
enterprise_external_identities entry points. Use the existing SourceContext,
_FakeClient, and enterprise_data setup patterns to keep the tests aligned with
the current enterprise_organizations coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b1f1ac8-1163-4a8d-9e47-6bb356ee1893
📒 Files selected for processing (5)
src/openhound_github/graphql.pysrc/openhound_github/helpers.pysrc/openhound_github/resources/enterprise.pysrc/openhound_github/source.pytests/test_enterprise_resources.py
| paginator = GraphQLCursorPaginator( | ||
| page_info_path="data.enterprise.organizations.pageInfo", | ||
| cursor_variable="after", | ||
| cursor_field="endCursor", | ||
| has_next_field="hasNextPage", | ||
| ) | ||
| data = { | ||
| "query": ENTERPRISE_QUERY, | ||
| "variables": {"slug": ctx.enterprise_name, "after": None}, | ||
| } | ||
|
|
||
| for page_data in ctx.client.paginate( | ||
| "/graphql", | ||
| method="POST", | ||
| json=data, | ||
| paginator=paginator, | ||
| data_selector="data", | ||
| ): | ||
| for enterprise_object in page_data: | ||
| es_data = enterprise_object.get("enterprise", {}) | ||
| orgs = (es_data.get("organizations") or {}).get("nodes", []) | ||
| for org in orgs: | ||
| yield { | ||
| **org, | ||
| "enterprise_node_id": enterprise_data.id, | ||
| "enterprise_slug": ctx.enterprise_name, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add error handling around the new enterprise_organizations GraphQL pagination call.
Unlike enterprise() (Lines 53-62), which wraps its GraphQL POST in try/except with graceful logging, this newly-added independent pagination call has no error handling. Since client.paginate()/.post() raise HTTPError on non-2xx responses by default, a transient or permission error here will propagate as an unhandled exception and fail this resource entirely instead of degrading gracefully.
🛡️ Suggested fix
def enterprise_organizations(enterprise_data: Enterprise, ctx: SourceContext):
paginator = GraphQLCursorPaginator(
page_info_path="data.enterprise.organizations.pageInfo",
cursor_variable="after",
cursor_field="endCursor",
has_next_field="hasNextPage",
)
data = {
"query": ENTERPRISE_QUERY,
"variables": {"slug": ctx.enterprise_name, "after": None},
}
- for page_data in ctx.client.paginate(
- "/graphql",
- method="POST",
- json=data,
- paginator=paginator,
- data_selector="data",
- ):
- for enterprise_object in page_data:
- es_data = enterprise_object.get("enterprise", {})
- orgs = (es_data.get("organizations") or {}).get("nodes", [])
- for org in orgs:
- yield {
- **org,
- "enterprise_node_id": enterprise_data.id,
- "enterprise_slug": ctx.enterprise_name,
- }
+ try:
+ for page_data in ctx.client.paginate(
+ "/graphql",
+ method="POST",
+ json=data,
+ paginator=paginator,
+ data_selector="data",
+ ):
+ for enterprise_object in page_data:
+ es_data = enterprise_object.get("enterprise", {})
+ orgs = (es_data.get("organizations") or {}).get("nodes", [])
+ for org in orgs:
+ yield {
+ **org,
+ "enterprise_node_id": enterprise_data.id,
+ "enterprise_slug": ctx.enterprise_name,
+ }
+ except Exception as e:
+ logger.error(
+ f"Error in resource 'enterprise_organizations' processing enterprise '{ctx.enterprise_name}': {e}",
+ extra={"resource": "enterprise_organizations", "phase": "resource_iteration"},
+ )
+ return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| paginator = GraphQLCursorPaginator( | |
| page_info_path="data.enterprise.organizations.pageInfo", | |
| cursor_variable="after", | |
| cursor_field="endCursor", | |
| has_next_field="hasNextPage", | |
| ) | |
| data = { | |
| "query": ENTERPRISE_QUERY, | |
| "variables": {"slug": ctx.enterprise_name, "after": None}, | |
| } | |
| for page_data in ctx.client.paginate( | |
| "/graphql", | |
| method="POST", | |
| json=data, | |
| paginator=paginator, | |
| data_selector="data", | |
| ): | |
| for enterprise_object in page_data: | |
| es_data = enterprise_object.get("enterprise", {}) | |
| orgs = (es_data.get("organizations") or {}).get("nodes", []) | |
| for org in orgs: | |
| yield { | |
| **org, | |
| "enterprise_node_id": enterprise_data.id, | |
| "enterprise_slug": ctx.enterprise_name, | |
| } | |
| paginator = GraphQLCursorPaginator( | |
| page_info_path="data.enterprise.organizations.pageInfo", | |
| cursor_variable="after", | |
| cursor_field="endCursor", | |
| has_next_field="hasNextPage", | |
| ) | |
| data = { | |
| "query": ENTERPRISE_QUERY, | |
| "variables": {"slug": ctx.enterprise_name, "after": None}, | |
| } | |
| try: | |
| for page_data in ctx.client.paginate( | |
| "/graphql", | |
| method="POST", | |
| json=data, | |
| paginator=paginator, | |
| data_selector="data", | |
| ): | |
| for enterprise_object in page_data: | |
| es_data = enterprise_object.get("enterprise", {}) | |
| orgs = (es_data.get("organizations") or {}).get("nodes", []) | |
| for org in orgs: | |
| yield { | |
| **org, | |
| "enterprise_node_id": enterprise_data.id, | |
| "enterprise_slug": ctx.enterprise_name, | |
| } | |
| except Exception as e: | |
| logger.error( | |
| f"Error in resource 'enterprise_organizations' processing enterprise '{ctx.enterprise_name}': {e}", | |
| extra={"resource": "enterprise_organizations", "phase": "resource_iteration"}, | |
| ) | |
| return |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhound_github/resources/enterprise.py` around lines 70 - 96, Add the
same graceful error handling used in enterprise() around the new
enterprise_organizations GraphQL pagination flow. Wrap the
ctx.client.paginate("/graphql", ...) loop in try/except (including HTTPError or
a broader request exception if consistent with the module), log the failure with
enough context, and return/stop cleanly instead of letting the exception escape.
Use the enterprise_organizations paginator setup and the ctx.client.paginate
call as the key locations to update.
| client = ctx.sso_client | ||
| if not client: | ||
| logger.info( | ||
| "Skipping enterprise_saml_provider for enterprise '%s': no SSO client configured", | ||
| ctx.enterprise_name, | ||
| ) | ||
| return | ||
|
|
||
| data = { | ||
| "query": ENTERPRISE_SAML_QUERY, | ||
| "variables": {"slug": ctx.enterprise_name, "count": 1, "after": None}, | ||
| "query": ENTERPRISE_SAML_PROVIDER_QUERY, | ||
| "variables": {"slug": ctx.enterprise_name}, | ||
| } | ||
|
|
||
| for page_data in ctx.client.paginate( | ||
| "/graphql", | ||
| method="POST", | ||
| json=data, | ||
| paginator=paginator, | ||
| data_selector="data", | ||
| ): | ||
| for enterprise_object in page_data: | ||
| es_data = enterprise_object.get("enterprise", {}) | ||
| saml_provider = (es_data.get("ownerInfo") or {}).get("samlIdentityProvider") | ||
| if not saml_provider: | ||
| return | ||
| yield { | ||
| **{k: v for k, v in saml_provider.items() if k != "externalIdentities"}, | ||
| "enterprise_node_id": enterprise_data.id, | ||
| "enterprise_slug": ctx.enterprise_name, | ||
| } | ||
| response = client.post("/graphql", json=data).json() | ||
| enterprise_object = (response.get("data") or {}).get("enterprise", {}) | ||
| saml_provider = (enterprise_object.get("ownerInfo") or {}).get("samlIdentityProvider") | ||
| if not saml_provider: | ||
| logger.warning( | ||
| "No enterprise SAML provider returned for enterprise '%s'", | ||
| ctx.enterprise_name, | ||
| ) | ||
| return | ||
|
|
||
| yield { | ||
| **saml_provider, | ||
| "enterprise_node_id": enterprise_data.id, | ||
| "enterprise_slug": ctx.enterprise_name, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Missing error handling around the SSO-backed GraphQL POST.
client.post("/graphql", json=data).json() at Line 358 is unwrapped, unlike enterprise()'s equivalent call in the same file/diff which catches exceptions and logs gracefully (Lines 53-62). A non-2xx response (e.g., insufficient PAT scope, transient network failure) raises HTTPError by default and will crash this resource instead of degrading gracefully like its sibling.
🛡️ Suggested fix
- response = client.post("/graphql", json=data).json()
- enterprise_object = (response.get("data") or {}).get("enterprise", {})
- saml_provider = (enterprise_object.get("ownerInfo") or {}).get("samlIdentityProvider")
+ try:
+ response = client.post("/graphql", json=data).json()
+ except Exception as e:
+ logger.error(
+ f"Error in resource 'enterprise_saml_provider' processing enterprise '{ctx.enterprise_name}': {e}",
+ extra={"resource": "enterprise_saml_provider", "phase": "resource_iteration"},
+ )
+ return
+ enterprise_object = (response.get("data") or {}).get("enterprise", {})
+ saml_provider = (enterprise_object.get("ownerInfo") or {}).get("samlIdentityProvider")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client = ctx.sso_client | |
| if not client: | |
| logger.info( | |
| "Skipping enterprise_saml_provider for enterprise '%s': no SSO client configured", | |
| ctx.enterprise_name, | |
| ) | |
| return | |
| data = { | |
| "query": ENTERPRISE_SAML_QUERY, | |
| "variables": {"slug": ctx.enterprise_name, "count": 1, "after": None}, | |
| "query": ENTERPRISE_SAML_PROVIDER_QUERY, | |
| "variables": {"slug": ctx.enterprise_name}, | |
| } | |
| for page_data in ctx.client.paginate( | |
| "/graphql", | |
| method="POST", | |
| json=data, | |
| paginator=paginator, | |
| data_selector="data", | |
| ): | |
| for enterprise_object in page_data: | |
| es_data = enterprise_object.get("enterprise", {}) | |
| saml_provider = (es_data.get("ownerInfo") or {}).get("samlIdentityProvider") | |
| if not saml_provider: | |
| return | |
| yield { | |
| **{k: v for k, v in saml_provider.items() if k != "externalIdentities"}, | |
| "enterprise_node_id": enterprise_data.id, | |
| "enterprise_slug": ctx.enterprise_name, | |
| } | |
| response = client.post("/graphql", json=data).json() | |
| enterprise_object = (response.get("data") or {}).get("enterprise", {}) | |
| saml_provider = (enterprise_object.get("ownerInfo") or {}).get("samlIdentityProvider") | |
| if not saml_provider: | |
| logger.warning( | |
| "No enterprise SAML provider returned for enterprise '%s'", | |
| ctx.enterprise_name, | |
| ) | |
| return | |
| yield { | |
| **saml_provider, | |
| "enterprise_node_id": enterprise_data.id, | |
| "enterprise_slug": ctx.enterprise_name, | |
| } | |
| client = ctx.sso_client | |
| if not client: | |
| logger.info( | |
| "Skipping enterprise_saml_provider for enterprise '%s': no SSO client configured", | |
| ctx.enterprise_name, | |
| ) | |
| return | |
| data = { | |
| "query": ENTERPRISE_SAML_PROVIDER_QUERY, | |
| "variables": {"slug": ctx.enterprise_name}, | |
| } | |
| try: | |
| response = client.post("/graphql", json=data).json() | |
| except Exception as e: | |
| logger.error( | |
| f"Error in resource 'enterprise_saml_provider' processing enterprise '{ctx.enterprise_name}': {e}", | |
| extra={"resource": "enterprise_saml_provider", "phase": "resource_iteration"}, | |
| ) | |
| return | |
| enterprise_object = (response.get("data") or {}).get("enterprise", {}) | |
| saml_provider = (enterprise_object.get("ownerInfo") or {}).get("samlIdentityProvider") | |
| if not saml_provider: | |
| logger.warning( | |
| "No enterprise SAML provider returned for enterprise '%s'", | |
| ctx.enterprise_name, | |
| ) | |
| return | |
| yield { | |
| **saml_provider, | |
| "enterprise_node_id": enterprise_data.id, | |
| "enterprise_slug": ctx.enterprise_name, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhound_github/resources/enterprise.py` around lines 345 - 372, Add
error handling around the SSO GraphQL request in enterprise_saml_provider: the
unguarded client.post("/graphql", json=data).json() call can raise on non-2xx or
network failures and should degrade gracefully like enterprise(). Wrap the
POST/JSON fetch in a try/except, log a warning or error with ctx.enterprise_name
and the exception details, then return early when the SSO-backed lookup fails;
keep the existing saml_provider parsing and yield logic unchanged when the
request succeeds.
- add normalized SAML models for: - `SAML_ServiceProvider` - `SAML_Issuer` - `SAML_AssertionConsumerService` - wire normalized SAML resources into both organization and enterprise collection flows - unify enterprise and organization SAML handling onto shared models instead of separate enterprise-only variants - emit `SAML_Implements` edges from `GH_SamlIdentityProvider` to `SAML_ServiceProvider` - emit `SAML_TrustsIssuer` and `SAML_HasAssertionConsumerService` from dedicated normalized models - add `SAML_HasAccount` edges from the GitHub SAML service provider to mapped GitHub users when SAML account linkage exists - enrich `GH_SamlIdentityProvider` output with provider details such as issuer, SSO URL, signing configuration, certificate data, and foreign environment context - improve `GH_ExternalIdentity` mapping logic so foreign IdP matches can be constrained by provider-specific tenant/environment properties - support both organization and enterprise scope metadata on normalized SAML nodes - remove duplicated enterprise-specific SAML and external identity model implementations in favor of the shared path
jaredcatkinson
force-pushed
the
fix/BED-8788-enterprise-sso-pat
branch
from
c6970d1 to
2ef1c87
Compare
- replace alert-number-only naming with repository-aware names - set alert display names using the secret type when available - make secret scanning alert nodes easier to distinguish in graph views and search results
- move extension metadata into the package so OpenHound can discover it correctly - remove the repo-root `extension.yaml` - update SAML provider lookup behavior to align with the current collected schema - keep external identity resolution dependent on issuer and SSO URL derived context
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhound_github/resources/enterprise.py (1)
439-477: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd error handling around the
enterprise_external_identitypagination call.Like the previously-flagged
enterprise_organizationsandenterprise_saml_providergaps, this new pagination call (client.paginate(...)) has no try/except. A transient/permission error on the SSO client will propagate unhandled and fail the whole resource, unlike the graceful-degradation pattern used elsewhere in this file and inorganization.py'ssaml_provider/external_identities.🛡️ Suggested fix
- for page_data in client.paginate( - "/graphql", - method="POST", - json=data, - paginator=paginator, - data_selector="data", - ): - for enterprise_object in page_data: + try: + pages = client.paginate( + "/graphql", + method="POST", + json=data, + paginator=paginator, + data_selector="data", + ) + except Exception as e: + logger.error( + f"Error in resource 'enterprise_external_identity' processing enterprise '{ctx.enterprise_name}': {e}", + extra={"resource": "enterprise_external_identity", "phase": "resource_iteration"}, + ) + return + + for page_data in pages: + for enterprise_object in page_data:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/resources/enterprise.py` around lines 439 - 477, Wrap the `client.paginate(...)` iteration in `enterprise_external_identity` with the same targeted exception handling used by the existing `enterprise_organizations`, `enterprise_saml_provider`, or `organization.py` SAML flows. Log the pagination failure with enterprise context and return gracefully so transient or permission errors do not fail the entire resource, while preserving normal page processing.
🧹 Nitpick comments (7)
src/openhound_github/resources/enterprise.py (1)
478-489: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRemove the unused SAML-provider fields
ExternalIdentityonly consumesenvironment_slug; the other keys are ignored here and never surface in the graph. Drop them unless you plan to add matching fields to the model/schema.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/resources/enterprise.py` around lines 478 - 489, Update the external identity construction loop to yield only the SAML-provider field consumed by ExternalIdentity: retain environment_slug and remove environment_node_id, environment_name, saml_provider_id, saml_provider_issuer, and saml_provider_sso_url.src/openhound_github/models/__init__.py (1)
96-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
"ExternalIdentity"entry in__all__.Harmless but confusing — appears at both line 99 and line 101.
🧹 Proposed fix
"SamlAssertionConsumerService", "SamlProvider", "SamlServiceProvider", - "ExternalIdentity", "SamlIssuer", "ExternalIdentity",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/__init__.py` around lines 96 - 101, Remove the duplicate "ExternalIdentity" entry from the __all__ list in src/openhound_github/models/__init__.py, keeping a single export entry alongside the other model symbols.src/openhound_github/models/external_identity.py (2)
202-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
idshadows the Python builtin.Minor style nit flagged by Ruff.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/external_identity.py` at line 202, Rename the local variable id in the external identity unpacking assignment to a non-shadowing name, and update its uses within the surrounding function accordingly.Source: Linters/SAST tools
293-301: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThread
environment_typethrough here instead of inferring it fromenvironment_node_id.idp_for_environment(...)already has the SAML provider row; include the explicitenvironment_typecolumn and reuse it soservice_provider_node_idstays aligned with the normalized SAML nodes andSAML_HAS_ACCOUNTdoesn’t point at the wrong target.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/external_identity.py` around lines 293 - 301, Update ExternalIdentity.service_provider_node_id to obtain and reuse the explicit environment_type from idp_for_environment(...)’s SAML provider row, rather than inferring it from environment_node_id. Thread the environment_type column through the provider lookup and preserve the existing None behavior when required environment data is unavailable, ensuring the generated node ID matches normalized SAML nodes.src/openhound_github/models/saml_provider.py (1)
89-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate foreign-IdP detection logic, and its result is discarded.
detect_foreign_environmentduplicatesExternalIdentity.detect_foreign_idp(inexternal_identity.py) with divergent naming for the same IdPs ("PingOneUser"/"AZUser"/"Okta_User"here vs"pingone"/"entra"/"okta"there). Inas_node(line 105) the returnedforeign_idp_typeis discarded (_), so this duplication currently has no functional effect, but the divergence is a maintenance hazard if either side is later relied upon. Consider extracting a single shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/saml_provider.py` around lines 89 - 101, Remove the duplicate IdP detection logic from SAMLProvider.detect_foreign_environment and reuse the shared ExternalIdentity.detect_foreign_idp helper instead. Update as_node to preserve and use the helper’s canonical IdP values rather than discarding the returned foreign_idp_type, or remove the unused wrapper if no caller requires it.src/openhound_github/models/saml_service_provider.py (1)
90-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused unpacked variable
acs_url.Ruff flags
acs_urlas unused; onlysp_entity_idis consumed.🧹 Proposed fix
- acs_url, sp_entity_id = self.saml_route + _acs_url, sp_entity_id = self.saml_route🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/saml_service_provider.py` around lines 90 - 93, Update the as_node property’s saml_route unpacking to avoid binding the unused acs_url value while preserving the existing use of sp_entity_id and subsequent scope_type logic.Source: Linters/SAST tools
src/openhound_github/kinds/edges.py (1)
143-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove duplicate
SAML_TRUSTS_ISSUERassignment.Line 148 duplicates line 147.
🧹 Proposed fix
SAML_TRUSTS_ISSUER = "SAML_TrustsIssuer" -SAML_TRUSTS_ISSUER = "SAML_TrustsIssuer" SAML_ISSUES_ASSERTIONS_TO = "SAML_IssuesAssertionsTo"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/kinds/edges.py` around lines 143 - 155, Remove the duplicate SAML_TRUSTS_ISSUER assignment in the SAML edges constants, retaining a single definition with the existing value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhound_github/models/external_identity.py`:
- Around line 211-292: Update the match_key selection in the SYNCED_TO_GH_USER
block of _maps_to_user_edges to use "login" for Okta, matching the MAPS_TO_USER
block, while preserving "email" for PingOne and "name" as the default. Prefer
reusing a shared match-key computation if practical so both edge blocks cannot
diverge.
- Around line 273-278: Escape the IdP-supplied foreign_username before
interpolating it into the Cypher query assigned to q, while preserving the
existing uppercase comparison and query behavior. Apply the escaping at the
value used by the WHERE n.name condition so embedded single quotes cannot alter
the composed query.
---
Outside diff comments:
In `@src/openhound_github/resources/enterprise.py`:
- Around line 439-477: Wrap the `client.paginate(...)` iteration in
`enterprise_external_identity` with the same targeted exception handling used by
the existing `enterprise_organizations`, `enterprise_saml_provider`, or
`organization.py` SAML flows. Log the pagination failure with enterprise context
and return gracefully so transient or permission errors do not fail the entire
resource, while preserving normal page processing.
---
Nitpick comments:
In `@src/openhound_github/kinds/edges.py`:
- Around line 143-155: Remove the duplicate SAML_TRUSTS_ISSUER assignment in the
SAML edges constants, retaining a single definition with the existing value.
In `@src/openhound_github/models/__init__.py`:
- Around line 96-101: Remove the duplicate "ExternalIdentity" entry from the
__all__ list in src/openhound_github/models/__init__.py, keeping a single export
entry alongside the other model symbols.
In `@src/openhound_github/models/external_identity.py`:
- Line 202: Rename the local variable id in the external identity unpacking
assignment to a non-shadowing name, and update its uses within the surrounding
function accordingly.
- Around line 293-301: Update ExternalIdentity.service_provider_node_id to
obtain and reuse the explicit environment_type from idp_for_environment(...)’s
SAML provider row, rather than inferring it from environment_node_id. Thread the
environment_type column through the provider lookup and preserve the existing
None behavior when required environment data is unavailable, ensuring the
generated node ID matches normalized SAML nodes.
In `@src/openhound_github/models/saml_provider.py`:
- Around line 89-101: Remove the duplicate IdP detection logic from
SAMLProvider.detect_foreign_environment and reuse the shared
ExternalIdentity.detect_foreign_idp helper instead. Update as_node to preserve
and use the helper’s canonical IdP values rather than discarding the returned
foreign_idp_type, or remove the unused wrapper if no caller requires it.
In `@src/openhound_github/models/saml_service_provider.py`:
- Around line 90-93: Update the as_node property’s saml_route unpacking to avoid
binding the unused acs_url value while preserving the existing use of
sp_entity_id and subsequent scope_type logic.
In `@src/openhound_github/resources/enterprise.py`:
- Around line 478-489: Update the external identity construction loop to yield
only the SAML-provider field consumed by ExternalIdentity: retain
environment_slug and remove environment_node_id, environment_name,
saml_provider_id, saml_provider_issuer, and saml_provider_sso_url.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25442157-24b2-4c32-833e-a43fb94ded43
📒 Files selected for processing (12)
src/openhound_github/kinds/edges.pysrc/openhound_github/kinds/nodes.pysrc/openhound_github/models/__init__.pysrc/openhound_github/models/enterprise_external_identity.pysrc/openhound_github/models/enterprise_saml_provider.pysrc/openhound_github/models/external_identity.pysrc/openhound_github/models/saml_assertion_consumer_service.pysrc/openhound_github/models/saml_issuer.pysrc/openhound_github/models/saml_provider.pysrc/openhound_github/models/saml_service_provider.pysrc/openhound_github/resources/enterprise.pysrc/openhound_github/resources/organization.py
💤 Files with no reviewable changes (2)
- src/openhound_github/models/enterprise_saml_provider.py
- src/openhound_github/models/enterprise_external_identity.py
| q = ( | ||
| f"MATCH p=()<-[:GH_SyncedToEnvironment]-(:GH_SamlIdentityProvider)" | ||
| f"-[:GH_HasExternalIdentity]->(:GH_ExternalIdentity)" | ||
| f"-[:GH_MapsToUser]->(n) " | ||
| f"WHERE n.objectid = '{gh_id}' OR n.name = '{foreign_username.upper()}' RETURN p" | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether other query_composition/query_* string builders in this codebase escape single quotes.
rg -n "query_composition|query_mapped_users|query_environments" --type=py -C2Repository: SpecterOps/openhound-github
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file structure first, then inspect the relevant section.
ast-grep outline src/openhound_github/models/external_identity.py --view expanded || true
echo '--- lines around the reported query ---'
sed -n '240,320p' src/openhound_github/models/external_identity.py
echo '--- search for similar Cypher query construction in this file and related models ---'
rg -n "MATCH p=|RETURN p|foreign_username|gh_id|Cypher|cypher|session\.run|query_" src/openhound_github/models -C 2 || trueRepository: SpecterOps/openhound-github
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- find query_composition definition and usages ---'
rg -n "query_composition" src -C 2
echo '--- inspect the edge/property types around query_composition ---'
sed -n '1,240p' src/openhound_github/models/edge.py
echo '--- inspect the external identity model around the composed query ---'
sed -n '210,300p' src/openhound_github/models/external_identity.py
echo '--- inspect any code that executes composed queries ---'
rg -n "query_composition|composed=True|RETURN p" src/openhound_github -C 2 | sed -n '1,240p'Repository: SpecterOps/openhound-github
Length of output: 2824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- graph.py definition of edge properties ---'
sed -n '1,120p' src/openhound_github/graph.py
echo '--- locate all query_composition field reads and composed-edge handling ---'
rg -n "\.query_composition|query_composition\s*=|composed\s*=\s*True|composed" src/openhound_github -C 3
echo '--- inspect the query builder helpers used in other composed edges for escaping/parameterization patterns ---'
sed -n '1,260p' src/openhound_github/models/org_secret.py
sed -n '200,310p' src/openhound_github/models/secret_scanning_alert.pyRepository: SpecterOps/openhound-github
Length of output: 16302
Escape foreign_username before building this composed Cypher query. foreign_username.upper() is interpolated directly into WHERE n.name = '...', so an IdP-supplied value containing ' can alter the query. src/openhound_github/models/external_identity.py:273-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhound_github/models/external_identity.py` around lines 273 - 278,
Escape the IdP-supplied foreign_username before interpolating it into the Cypher
query assigned to q, while preserving the existing uppercase comparison and
query behavior. Apply the escaping at the value used by the WHERE n.name
condition so embedded single quotes cannot alter the composed query.
… mapping - move shared SAML parsing and node-id construction into saml_helpers - reuse shared helpers across SAML provider, issuer, ACS, and external identity models - stop inferring service provider environment type from node id prefixes - remove duplicated foreign IdP matching logic in external identity edges - escape composed Cypher username matching for synced identity queries
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/openhound_github/resources/enterprise.py (1)
350-372: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing error handling around PAT-backed SSO GraphQL calls.
client.post(...)(Line 364) andclient.paginate(...)(Lines 462-468) — both usingctx.sso_client— are unwrapped, unlikeenterprise()in this same file, which catches exceptions and logs gracefully. A non-2xx response (insufficient PAT scope, transient failure) will raise and crash the whole enterprise SAML/identity pipeline instead of degrading gracefully. Theclient.postcase at Line 364 was already flagged in a prior review round and remains unresolved in this version; theclient.paginatecall at Lines 462-468 has the identical hazard.🛡️ Suggested fix
- response = client.post("/graphql", json=data).json() + try: + response = client.post("/graphql", json=data).json() + except Exception as e: + logger.error( + f"Error in resource 'enterprise_saml_provider' processing enterprise '{ctx.enterprise_name}': {e}", + extra={"resource": "enterprise_saml_provider", "phase": "resource_iteration"}, + ) + return enterprise_object = (response.get("data") or {}).get("enterprise", {})- for page_data in client.paginate( - "/graphql", - method="POST", - json=data, - paginator=paginator, - data_selector="data", - ): + try: + pages = list(client.paginate( + "/graphql", + method="POST", + json=data, + paginator=paginator, + data_selector="data", + )) + except Exception as e: + logger.error( + f"Error in resource 'enterprise_external_identity' processing enterprise '{ctx.enterprise_name}': {e}", + extra={"resource": "enterprise_external_identity", "phase": "resource_iteration"}, + ) + return + for page_data in pages:Also applies to: 462-468
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/resources/enterprise.py` around lines 350 - 372, Wrap the PAT-backed SSO requests in enterprise_saml_provider and the corresponding identity-provider pagination flow around client.post and client.paginate with the same exception handling pattern used by enterprise(). Log the failure with enterprise context and return safely so missing scopes or transient GraphQL errors skip these resources without crashing the pipeline.
🧹 Nitpick comments (2)
src/openhound_github/resources/organization.py (1)
1736-1738: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the nonexistent argument from the docstring.
The docstring documents a
saml (SamlProvider)argument, but the function signature only acceptsctx.♻️ Proposed fix
Args: - saml (SamlProvider): The SAML provider to extract identities for. ctx (SourceContext): The shared context containing the REST client and organization name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/resources/organization.py` around lines 1736 - 1738, Remove the `saml (SamlProvider)` entry from the function docstring’s Args section, leaving only documentation for the actual `ctx` parameter.src/openhound_github/models/saml_helpers.py (1)
20-22: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
urllib.parseto safely extract the domain.Extracting the domain via
.split("/")[2]will throw anIndexErrorif thesso_urlis malformed and lacks a protocol scheme likehttps://. Usingurllib.parse.urlparseis safer and more idiomatic.♻️ Proposed refactor
Add the import at the top of the file:
+from urllib.parse import urlparseAnd update the Okta domain extraction logic:
if issuer.startswith("http://www.okta.com/"): - domain = (sso_url or "").split("/")[2] if sso_url else None + domain = urlparse(sso_url).netloc if sso_url else None return "okta", domain🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhound_github/models/saml_helpers.py` around lines 20 - 22, Update the Okta handling in the issuer-detection function to use urllib.parse.urlparse for extracting the SSO hostname instead of indexing sso_url.split("/"). Preserve the existing None behavior when sso_url is absent and safely handle malformed URLs without raising IndexError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/openhound_github/resources/enterprise.py`:
- Around line 350-372: Wrap the PAT-backed SSO requests in
enterprise_saml_provider and the corresponding identity-provider pagination flow
around client.post and client.paginate with the same exception handling pattern
used by enterprise(). Log the failure with enterprise context and return safely
so missing scopes or transient GraphQL errors skip these resources without
crashing the pipeline.
---
Nitpick comments:
In `@src/openhound_github/models/saml_helpers.py`:
- Around line 20-22: Update the Okta handling in the issuer-detection function
to use urllib.parse.urlparse for extracting the SSO hostname instead of indexing
sso_url.split("/"). Preserve the existing None behavior when sso_url is absent
and safely handle malformed URLs without raising IndexError.
In `@src/openhound_github/resources/organization.py`:
- Around line 1736-1738: Remove the `saml (SamlProvider)` entry from the
function docstring’s Args section, leaving only documentation for the actual
`ctx` parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c192d69-44cd-4a99-bb72-3b887c5b11c9
📒 Files selected for processing (22)
src/openhound_github/extension.yamlsrc/openhound_github/graphql.pysrc/openhound_github/kinds/edges.pysrc/openhound_github/kinds/nodes.pysrc/openhound_github/lookup.pysrc/openhound_github/models/__init__.pysrc/openhound_github/models/enterprise_external_identity.pysrc/openhound_github/models/enterprise_saml_provider.pysrc/openhound_github/models/external_identity.pysrc/openhound_github/models/repository.pysrc/openhound_github/models/saml_assertion_consumer_service.pysrc/openhound_github/models/saml_helpers.pysrc/openhound_github/models/saml_issuer.pysrc/openhound_github/models/saml_provider.pysrc/openhound_github/models/saml_service_provider.pysrc/openhound_github/models/secret_scanning_alert.pysrc/openhound_github/models/workflow.pysrc/openhound_github/models/workflow_job.pysrc/openhound_github/models/workflow_step.pysrc/openhound_github/resources/enterprise.pysrc/openhound_github/resources/organization.pysrc/openhound_github/transforms.py
💤 Files with no reviewable changes (2)
- src/openhound_github/models/enterprise_external_identity.py
- src/openhound_github/models/enterprise_saml_provider.py
🚧 Files skipped from review as they are similar to previous changes (7)
- src/openhound_github/kinds/nodes.py
- src/openhound_github/models/saml_issuer.py
- src/openhound_github/models/init.py
- src/openhound_github/models/saml_assertion_consumer_service.py
- src/openhound_github/models/saml_service_provider.py
- src/openhound_github/kinds/edges.py
- src/openhound_github/models/external_identity.py
- use urllib.parse.urlparse to safely extract Okta SSO hostnames - remove the stale saml argument entry from external_identities docstring
1 participant
Summary by CodeRabbit