fix: Handle unknown Application signOnMode values gracefully#511
fix: Handle unknown Application signOnMode values gracefully#511BinoyOza-okta wants to merge 1 commit intomasterfrom
Conversation
This commit introduces a post-processing mechanism to handle Application objects with signOnMode values that are not defined in the ApplicationSignOnMode enum, preventing deserialization errors when new sign-on modes are introduced by Okta. Changes: - Added post_process_application.py script with custom deserialization logic - Implements x-post-process-function vendor extension support - Updated api.yaml to reference post-processing for Application schema - Modified model_generic.mustache to invoke post-processing when defined - Added OTHER enum value to ApplicationSignOnMode for unknown modes The solution uses OpenAPI Generator's vendor extension mechanism (x-post-process-function) to apply custom deserialization logic specifically to the Application model without affecting other models in the SDK. When an Application response contains an unknown signOnMode: 1. Discriminator resolution falls back to 'OTHER' mapping 2. Post-processing preserves the original signOnMode value 3. Application object is returned with the actual value from API response 4. SDK remains forward-compatible with new Okta sign-on modes This prevents breaking changes when Okta introduces new application types like MFA_AS_SERVICE while maintaining type safety for known values. Fixes: Deserialization errors for applications with unknown signOnMode values
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| 'BasicAuthApplication', | ||
| 'BookmarkApplication', | ||
| 'BrowserPluginApplication', | ||
| 'Application' |
There was a problem hiding this comment.
Missing trailing comma — Python silently concatenates adjacent string literals, so this becomes 'ApplicationOpenIdConnectApplication' as a single list element. OpenIdConnectApplication is silently dropped from the preload group, which can break discriminator resolution for OIDC apps.
| 'Application' | |
| 'Application', |
| from okta.models.browser_plugin_application import BrowserPluginApplication | ||
| from okta.models.application import Application | ||
| from okta.models.open_id_connect_application import OpenIdConnectApplication | ||
| from okta.models.application import Application |
There was a problem hiding this comment.
Duplicate import — from okta.models.application import Application is already imported on line 53. Remove this duplicate.
| embedded: Optional[ApplicationEmbedded] = Field(default=None, alias="_embedded") | ||
| links: Optional[ApplicationLinks] = Field(default=None, alias="_links") | ||
| # Store the original sign-on mode value when it's not in the enum | ||
| _original_sign_on_mode: Optional[str] = None |
There was a problem hiding this comment.
Not idiomatic Pydantic v2 — underscore-prefixed fields are silently ignored by Pydantic (not in model_fields, not validated, not serialized), but the intent is unclear to readers. Use PrivateAttr to make this explicit:
| _original_sign_on_mode: Optional[str] = None | |
| _original_sign_on_mode: Optional[str] = PrivateAttr(default=None) |
Also add PrivateAttr to the pydantic import at the top of the file.
| if object_type == cls.__name__: | ||
| return cls.model_validate(obj) | ||
| return models.OpenIdConnectApplication.from_dict(obj) | ||
| if object_type == "Application": |
There was a problem hiding this comment.
Unreachable dead code — the first if object_type == "Application": block (line 391) always returns, so this second block can never be reached. This is a copy-paste artifact from adding the OpenIdConnectApplication entry. Remove lines 415–419 entirely.
| mapped_class = cls.__discriminator_value_class_map.get(discriminator_value) | ||
| if mapped_class: | ||
| return mapped_class | ||
| # If not in mapping, return base class (will be handled by post-processing for Application) |
There was a problem hiding this comment.
Incorrect comment — post-processing only applies to Application, not ActionProvider (or any of the other 21 models with this same change). Also, silently swallowing unknown discriminator values here could mask real API schema changes. Consider logging a warning instead of silently falling back.
| # If not in mapping, return base class (will be handled by post-processing for Application) | |
| # Unknown discriminator value — fall back to base class to avoid crashing |
| SECURE_PASSWORD_STORE = "SECURE_PASSWORD_STORE" | ||
| WS_FEDERATION = "WS_FEDERATION" | ||
| MFA_AS_SERVICE = "MFA_AS_SERVICE" | ||
| OTHER = "OTHER" |
There was a problem hiding this comment.
OTHER is an SDK-internal sentinel — Okta's API never returns this value. Exposing it in the public enum misleads consumers and could cause failures if passed in write operations (e.g. PUT /api/v1/apps/{id}). Add a docstring note making the internal-only nature clear:
| OTHER = "OTHER" | |
| OTHER = "OTHER" # SDK-internal sentinel for unknown sign-on modes; never send this value to the Okta API |
| echo "=========================================" | ||
| echo "Post-processing Application model..." | ||
| echo "=========================================" | ||
| python3 post_process_application.py |
There was a problem hiding this comment.
The changes this script applies are already manually committed to application.py in this PR. The script's own check_if_already_processed() guard will detect this and skip — making it a no-op on every generate.sh run. Additionally, the regex patterns are brittle; any generator formatting change will cause silent partial failure.
Pick one source of truth:
- If manual edits are canonical → remove the script invocation from
generate.sh - If the script is canonical → remove the manual edits from the committed
application.pyand harden the script with strict exit codes
aniket-okta
left a comment
aniket-okta
left a comment
There was a problem hiding this comment.
Review Summary
The core roundtrip logic (unknown signOnMode → store original → restore on serialise) works correctly per my local testing. However, there are two blocking bugs and several quality issues that need to be addressed before this can merge.
7 inline comments have been posted covering:
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🔴 Critical | okta/models/__init__.py:44 |
Missing comma — Python silently concatenates 'Application' + 'OpenIdConnectApplication' into one string |
| 2 | 🔴 Critical | okta/models/application.py:55 |
Duplicate self-import — from okta.models.application import Application already imported above |
| 3 | 🟡 Major | okta/models/application.py:113 |
_original_sign_on_mode should use Pydantic PrivateAttr(default=None) instead of a public field |
| 4 | 🟡 Major | okta/models/application.py:415 |
Unreachable dead code — second if object_type == "Application": block never executes |
| 5 | 🟡 Major | okta/models/action_provider.py:76 |
Comment says "post-processing for Application" but this pattern was applied to all 22 discriminator models |
| 6 | 🟡 Major | okta/models/application_sign_on_mode.py:56 |
OTHER is SDK-internal; it must not round-trip as a literal string to the Okta API |
| 7 | 🟡 Major | openapi/generate.sh:15 |
post_process_application.py is a no-op (changes already manually applied); its regex patterns are also brittle |
Please address the two 🔴 critical bugs at minimum before re-requesting a review.
2 participants
Handle Unknown Application SignOnMode Values
Problem
SDK throws deserialization errors when applications have unknown
signOnModevalues (e.g.,MFA_AS_SERVICE).Solution
Implements post-processing using OpenAPI Generator vendor extensions to preserve unknown signOnMode values.
Key Changes
x-post-process-functionvendor extensionHow It Works
Benefits
✅ Forward compatibility
✅ Application-specific solution
✅ Preserves data integrity
Testing
Verified
list_applications()works with MFA_AS_SERVICE and other unknown modes.Commit: bef284d