fix: disable broken polymorphic resolution on 6 more discriminator schemas (GH issues deep-dive) - #1701
Merged
Merged
Conversation
…tor schemas Deep-dive into the still-open GitHub issues (#1654, #1664, #1693) led to auditing every oneOf/anyOf + discriminator pair in the spec for the same defect class as OKTA-1227472 (subtypes that don't actually extend the declared base type via allOf, so Jackson's @JsonSubTypes throws InvalidTypeIdException). Found and fixed 6 more instances: - AgentJsonSigningKeyRequest / AgentJsonSigningKeyResponse (unreferenced in the spec today, but fixed for correctness/consistency) - ManagedConnection / ManagedConnectionCreatable - PotentialConnection - OrgContactTypeObj - this one is live and reachable from the real listOrgContactTypes endpoint Also adds a regression test confirming GH-1654's likely root cause: a mixed OIDC/SAML Application list, where the SAML app's settings.signOn.attributeStatements uses the SamlAttributeStatement type fixed in OKTA-1227472/#1699, now deserializes without throwing. Before that fix, an org with SAML apps using attribute statements could see listApplications fail on those specific apps. Co-Authored-By: Claude Code
…ixes AgentJsonSigningKeyRequest, AgentJsonSigningKeyResponse, ManagedConnection, ManagedConnectionCreatable, and PotentialConnection got the same @JsonTypeInfo(Id.NONE) mixin fix as OrgContactTypeObj in the previous commit, but only OrgContactTypeObj had a regression test. Add the missing five. Note: ManagedConnection/ManagedConnectionCreatable/PotentialConnection have a separate, pre-existing quirk where their flattened ConnectionTypeEnum only retains one oneOf branch's single-value enum (STS_SERVICE_ACCOUNT) - every other connectionType value falls back to UNKNOWN_DEFAULT_OPEN_API. That's harmless (no exception) but means these tests only assert the polymorphism fix, not full enum fidelity; fully fixing that would require reworking how the spec merges oneOf branches, which is out of scope here. Co-Authored-By: Claude Code
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
2 participants
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.
Issue(s)
Deep-dive triage of the remaining open GitHub issues (https://github.com/okta/okta-sdk-java/issues):
mastervia fix(spec): add discriminator to IdentityProvider.protocol to fix SAML… #1695/fix: Jackson polymorphism/serialization, pagination, and GH-reported bugs (GH-1642, GH-1689, GH-1690) #1699 - see below)master- see below)Description
Triage summary
#1693 - Already fixed on
master(PR #1695 added the missing discriminator toIdentityProviderProtocol, and #1699 shipped further Jackson polymorphism fixes). VerifiedIdentityProviderProtocolDeserializerTest's 14 regression tests (SAML2/OIDC/OAuth2/MTLS round-trips) all pass on currentmaster. No code change needed in this PR; the issue just needs to be closed once released.#1664 - Already satisfied. The deprecated
PaginationUtil/ApiClient#getResponseHeaders()/getAfter()methods are still present and functional (PaginationUtilTestpasses), so the pre-25.0.0 "passafteryourself" workflow continues to work. No removal has happened; nothing to fix.#1654 - No SAML-specific filtering exists anywhere in the SDK's
listApplicationspath (ApplicationApi.listApplicationsis a plain generated call;Application's own@JsonSubTypescorrectly mapsSAML_2_0->SamlApplicationvia real Java inheritance). However, auditing the spec for the same defect class fixed in OKTA-1227472/#1699 (ListJwk200ResponseInner/SamlAttributeStatement:oneOf/anyOf+discriminatorwithoutallOf-based inheritance, causing Jackson's@JsonSubTypesto throwInvalidTypeIdException) turned up a very plausible mechanism: before that fix shipped, a SAML app withsettings.signOn.attributeStatementspopulated would throw while being parsed as part of alistApplicationsresponse array. Depending on how a caller's error/pagination handling reacted to that exception, this could plausibly present as "SAML apps silently missing, only OIDC returned." Added a regression test (deserializeApplicationList_withMixedOidcAndSamlAttributeStatements_doesNotThrow) confirming a mixed OIDC/SAML list with attribute statements now deserializes cleanly. This is very likely already fixed as a side effect of #1699 - will comment on the issue asking the reporter to confirm on the latest release.Bonus finding: 6 more instances of the same discriminator defect
Since OKTA-1227472 and #1693/#1695 were both "a
oneOf/anyOf+discriminatorpair whose subtypes don't actually extend the base viaallOf," I audited every such pair insrc/swagger/api.yamlfor the same pattern. Found and fixed 6 more:AgentJsonSigningKeyRequest/AgentJsonSigningKeyResponse(currently unreferenced by any operation in the spec, fixed for correctness/consistency anyway)ManagedConnection/ManagedConnectionCreatable(reachable from managed-connection list endpoints)PotentialConnection(reachable from potential-connection list endpoints)OrgContactTypeObj- this one is live and reachable from the real, commonly-usedlistOrgContactTypesendpoint - any caller of that endpoint would hitInvalidTypeIdExceptiontodayFixed the same way as the existing
ListJwk200ResponseInner/SamlAttributeStatementmixins:@JsonTypeInfo(use = Id.NONE)registered onApiClient's defaultObjectMapper, disabling the broken polymorphic resolution. Added a regression test forOrgContactTypeObj(BILLING/TECHNICAL) confirming it no longer throws.(Note:
GroupProfilealso has this sameoneOf/discriminatorpattern, but it's inert in practice -GroupProfileDeserializeris a hand-written custom deserializer that bypasses Jackson's annotation-based polymorphism entirely, so it was excluded from this fix as unnecessary.)Category
Signoff