fix: tolerate non-conforming apps across all app read paths (#48) - #85
Open
kstlouis wants to merge 5 commits into
Open
fix: tolerate non-conforming apps across all app read paths (#48)#85kstlouis wants to merge 5 commits into
kstlouis wants to merge 5 commits into
Conversation
list_applications deserialized the whole page into strict SDK models in a single pass, so one record the models reject — a SAML app with a partial settings.signOn, or a provisioning features value outside the SDK enum — raised a validation error that aborted the entire response. A single non-conforming app made the tool unusable on real orgs. Fetch the apps page through the SDK request executor and parse each record individually, falling back to the raw dict (tagged with a warning) when strict deserialization fails. Conforming apps are still returned as typed models, and pagination is unchanged.
Extends the resilient per-record parsing from PR okta#64 (list_applications) to list_group_apps, which went through client.list_assigned_applications_for_group and hit the same failure: the SDK bulk-deserializes the whole page into strict pydantic models, so one App Catalog SAML app (sparse settings.signOn) or custom SWA app (name outside the template enum) aborts the entire response. Now fetches /api/v1/groups/{id}/apps through the request executor and parses each record via _safe_parse_app, falling back to the raw dict with a warning marker. Both the initial fetch and the fetch_all pagination path are covered. Updates the TestListGroupAppsFetchAll suite to mock the executor path and adds a resilience test (one bad SAML record no longer aborts the listing). Refs okta#48
Completes the okta#48 fix. get_application went through the typed client.get_application, which validates the record into a strict SDK model — so an App Catalog SAML app with a partial settings.signOn (or a custom SWA whose name is outside the template enum) raised and the call failed outright, as reported in okta#48. Fetches the record via the request executor (GET /api/v1/apps/{id}) and parses it through _safe_parse_app, returning the raw dict with a warning marker when strict deserialization fails — same approach as list_applications and list_group_apps. Refs okta#48
…utor
The resilient list_applications/list_group_apps path fetches pages through the
request executor, which returns the raw aiohttp response. extract_after_cursor
only understood the typed-SDK shapes (ApiResponse.headers / OktaAPIResponse._next),
so on the executor response it found no cursor and pagination stopped after the
first page — listings silently truncated to one page (e.g. fetch_all returned only
20 items) on accounts/groups with more.
Root cause: Okta returns 'self' and 'next' as SEPARATE Link headers, so a multidict
headers.get('Link') yields only 'self'. aiohttp already parses them into .links
(rel-keyed), which is what the SDK itself uses (OktaAPIResponse.extract_pagination).
extract_after_cursor now reads response.links['next'] first (raw executor response),
then falls back to a Link-header string via .headers / get_headers() / _resp_headers
(ApiResponse and other shapes), then the SDK v2 has_next()/_next path. Adds tests for
the aiohttp .links shape and the header-accessor fallbacks.
Refs okta#48
kstlouis
force-pushed
the
fix/resilient-app-parsing
branch
from
June 29, 2026 22:24
095978c to
128b6ba
Compare
BinoyOza-okta
self-requested a review
July 2, 2026 03:57
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
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
Fixes #48 across all three application read paths. The Okta Python SDK bulk-deserializes app records into strict pydantic models, so a single non-conforming record aborts the entire response. This affects real orgs constantly: App Catalog SAML apps with a partial
settings.signOn, and custom SWA apps whosenamefalls outside thetemplate_swa/template_swa3fieldenum (e.g.appstoreconnect,dockerhub).The fix fetches the raw record(s) through the SDK request executor and parses each one individually, falling back to the raw dict (tagged with a
_deserialization_warningmarker) when strict deserialization fails. Conforming apps are still returned as typed models; pagination is unchanged.What's covered
GET /api/v1/appslist_applicationsGET /api/v1/groups/{id}/appslist_group_appsGET /api/v1/apps/{id}get_application#64 fixed
list_applicationsbut #48 also reportsget_applicationand the group-apps listing failing. This PR incorporates #64 (its commit is preserved with original authorship) and extends the same_safe_parse_appapproach to the other two paths so #48 is fully closed. If #64 is merged first, the first commit here drops out cleanly on rebase.Notes
list_group_appsandget_applicationpreviously went through the typedclient.list_assigned_applications_for_group/client.get_application, which is where the strict validation happened — both now use the request-executor + per-record parse, mirroringlist_applications._safe_parse_app/_camel_case_paramhelpers from fix: tolerate non-conforming apps when listing applications #64 are reused as-is.Tests
tests/test_list_applications.py(from fix: tolerate non-conforming apps when listing applications #64)tests/test_pagination.py::TestListGroupAppsFetchAllupdated to the executor path + a new "one bad SAML record doesn't abort the listing" casetests/test_get_application.py(new) — good record → typed model, non-conforming → raw dict with warning, executor error surfaced,expandpassthroughFull suite: 412 passed.