fix: relax over-strict Okta SDK models that reject valid API responses - #107
Open
jackvaughanjr wants to merge 1 commit into
Open
fix: relax over-strict Okta SDK models that reject valid API responses#107jackvaughanjr wants to merge 1 commit into
jackvaughanjr wants to merge 1 commit into
Conversation
Consolidates every SDK model compatibility patch into one module,
okta_mcp_server.utils.okta_compat, applied at import time.
Covers four defects where a generated Pydantic model is stricter than
the API it describes, each of which aborts an entire response:
A SamlApplicationSettingsSignOn: five required StrictBool fields the
API omits on apps created from OIN catalog templates.
B Policy.embedded: typed Dict[str, Dict[str, Any]], but Okta returns
scalars inside _embedded (e.g. {"resourceType": "APP"}).
C AuthenticatorEnrollmentPolicyAuthenticatorSettings.key: closed enum
missing smart_card_idp, and brittle to any future authenticator.
D UserTypeCondition.exclude/include: required lists where Okta sends
null. Also RiskDetectionTypesPolicyRuleCondition and
UserIdentifierPolicyRuleCondition.
Also migrates the existing LogSecurityContext.user_behaviors patch out
of system_logs.py so all SDK compatibility lives in one place.
Supersedes the inline patches in PR okta#102 (issues okta#100, okta#101).
Upstream: okta/okta-sdk-python#546 (A), #572 (D); okta#100
(B), okta#101 (C), okta#48 (A).
Regression fixtures are synthetic: example.invalid hosts and fake IDs.
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.
Supersedes #102, which patched two of these four inline in
policies.py. Closing that in favour of this.Four generated Okta SDK models are stricter than the API they describe. Each one aborts an entire response, so a single non-conforming record makes a whole page unreadable. They are all in the pinned
okta==3.4.4dependency, so they cannot be fixed in this repo directly; they have to be relaxed at runtime.SamlApplicationSettingsSignOnStrictBoolfields the API omits on apps created from OIN catalog templates. Breakslist_applications,list_group_apps, andget_application.Policy.embeddedDict[str, Dict[str, Any]], but Okta returns scalars inside_embedded, e.g.{"resourceType": "APP"}. Breakslist_policies(type="ACCESS_POLICY").AuthenticatorEnrollmentPolicyAuthenticatorSettings.keysmart_card_idp. Breakslist_policies(type="MFA_ENROLL")on any tenant using PIV/CAC.UserTypeCondition.exclude/.includenull. Breakslist_policy_rules.Approach
One module,
okta_mcp_server/utils/okta_compat.py, applied at import time fromserver.py. This is the technique already used in this repo forLogSecurityContext.user_behaviorsinsystem_logs.py; that patch is migrated into the new module so all SDK compatibility lives in one place.Each patch is individually wrapped, so a future SDK version that removes a field logs a warning instead of breaking startup. Each carries the upstream issue that would make it removable.
Two deliberate choices worth review:
keyis relaxed toOptional[str]rather than adding one enum member. Addingsmart_card_idpalone would break again on the next authenticator Okta ships. This matches the approach in fix: relax two SDK model gaps blocking list_policies on Okta for Government tenants #102.extra="allow"and no model is blanket-relaxed. Request models keep their required-field validation, where a local error is more useful than an opaque API 400.Pydantic v2 detail: each subclass builds its own
model_fieldsat class-creation time, so patchingPolicyalone does not fixAccessPolicy. The patch names subclasses explicitly and rewrites__annotations__so it works regardless of import order. There is a test that runs both orders in subprocesses.Testing
26 tests, 10 synthetic fixtures (
example.invalidhosts, fake IDs). Each defect has a test that fails before the fix with the exact upstream error signature and passes after. Control fixtures assert the happy path is unchanged: a populatedassertionSigned: truestaysTrue, an explicit{"exclude": [], "include": []}stays[]rather than becomingNone, andSAML_2_0still routes toSamlApplication.Missing booleans deserialize to
None, notFalse. For a security review, "not reported" and "not signed" must not be confusable.Full suite: 555 passed, up from 529 on
main.Upstream
UserTypeCondition.exclude/.includerejectnull, but the API returnsnull— one policy rule aborts the wholelist_policy_rulespage okta-sdk-python#572 (D, open)v3.4.4 is the latest release, so there is no version bump that resolves any of these.