BED-8788: add PAT-backed enterprise SSO collection and fix repeated enterprise SSO pagination#22
Open
jaredcatkinson wants to merge 1 commit into
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
WalkthroughThis PR adds an optional PAT-based SSO REST client to the enterprise GitHub source, a new GraphQL query for SAML providers, and reworks enterprise resource functions to use direct GraphQL posts and conditional SSO-gated SAML/identity fetching, with new unit tests. ChangesSSO-aware enterprise resources
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Resources as enterprise_resources
participant SamlFn as enterprise_saml_provider
participant SsoClient as ctx.sso_client
participant API as GraphQL API
Resources->>SamlFn: build resource chain
SamlFn->>SamlFn: check ctx.sso_client
alt sso_client missing
SamlFn-->>Resources: log info, return (no yield)
else sso_client configured
SamlFn->>SsoClient: post(ENTERPRISE_SAML_PROVIDER_QUERY)
SsoClient->>API: EnterpriseSAMLProvider(slug)
API-->>SsoClient: ownerInfo.samlIdentityProvider
alt provider missing
SamlFn-->>Resources: log warning, return
else provider present
SamlFn-->>Resources: yield provider with enterprise_node_id, enterprise_slug
end
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
Comment on lines
+70
to
+96
| 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.
Suggested change
| 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.
Comment on lines
+345
to
+372
| 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.
Suggested change
| 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests