Skip to content

Commit 8176d82

Browse files
authored
fix: disable broken polymorphic resolution on 6 more discriminator schemas (GH issues deep-dive) (#1701)
* fix: disable broken polymorphic resolution on 6 more oneOf/discriminator 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 * test: add missing coverage for the 5 Agent/Connection discriminator fixes 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
1 parent 2dee8e4 commit 8176d82

2 files changed

Lines changed: 172 additions & 1 deletion

File tree

api/src/main/resources/custom_templates/ApiClient.mustache

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,19 @@ protected List<ServerConfiguration> servers = new ArrayList<ServerConfiguration>
238238
objectMapper.enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);
239239
240240
// OKTA-1227472: disable broken polymorphic type resolution on models whose spec-declared
241-
// discriminator subtypes don't actually extend them.
241+
// discriminator subtypes don't actually extend them. Found via an audit of every
242+
// oneOf/anyOf + discriminator pair in the spec for this same defect class - these six
243+
// have the identical problem (ListJwk200ResponseInner/SamlAttributeStatement above were
244+
// the first two found; OrgContactTypeObj in particular breaks the real, commonly-used
245+
// listOrgContactTypes endpoint).
242246
objectMapper.addMixIn(com.okta.sdk.resource.model.ListJwk200ResponseInner.class, NoPolymorphicTypeInfoMixin.class);
243247
objectMapper.addMixIn(com.okta.sdk.resource.model.SamlAttributeStatement.class, NoPolymorphicTypeInfoMixin.class);
248+
objectMapper.addMixIn(com.okta.sdk.resource.model.AgentJsonSigningKeyRequest.class, NoPolymorphicTypeInfoMixin.class);
249+
objectMapper.addMixIn(com.okta.sdk.resource.model.AgentJsonSigningKeyResponse.class, NoPolymorphicTypeInfoMixin.class);
250+
objectMapper.addMixIn(com.okta.sdk.resource.model.ManagedConnection.class, NoPolymorphicTypeInfoMixin.class);
251+
objectMapper.addMixIn(com.okta.sdk.resource.model.ManagedConnectionCreatable.class, NoPolymorphicTypeInfoMixin.class);
252+
objectMapper.addMixIn(com.okta.sdk.resource.model.OrgContactTypeObj.class, NoPolymorphicTypeInfoMixin.class);
253+
objectMapper.addMixIn(com.okta.sdk.resource.model.PotentialConnection.class, NoPolymorphicTypeInfoMixin.class);
244254
245255
// OKTA-1218351: restore NON_NULL serialization on Application and its subtypes, overriding the
246256
// per-property JsonInclude(ALWAYS) generated for their required properties.

api/src/test/java/com/okta/sdk/resource/client/ApiClientJacksonMixinTest.java

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,19 @@
1919
import com.fasterxml.jackson.databind.ObjectMapper;
2020
import com.okta.sdk.cache.Cache;
2121
import com.okta.sdk.cache.CacheManager;
22+
import com.okta.sdk.resource.model.AgentJsonSigningKeyRequest;
23+
import com.okta.sdk.resource.model.AgentJsonSigningKeyResponse;
24+
import com.okta.sdk.resource.model.Application;
2225
import com.okta.sdk.resource.model.ApplicationVisibility;
2326
import com.okta.sdk.resource.model.ApplicationVisibilityHide;
2427
import com.okta.sdk.resource.model.ListJwk200ResponseInner;
28+
import com.okta.sdk.resource.model.ManagedConnection;
29+
import com.okta.sdk.resource.model.ManagedConnectionCreatable;
2530
import com.okta.sdk.resource.model.OpenIdConnectApplication;
31+
import com.okta.sdk.resource.model.OrgContactType;
32+
import com.okta.sdk.resource.model.OrgContactTypeObj;
33+
import com.okta.sdk.resource.model.PotentialConnection;
34+
import com.okta.sdk.resource.model.SamlApplication;
2635
import com.okta.sdk.resource.model.SamlAttributeStatement;
2736
import org.apache.hc.client5.http.impl.classic.HttpClients;
2837
import org.testng.annotations.Test;
@@ -135,4 +144,156 @@ public void serializeFullOpenIdConnectApplication_stillIncludesRequiredFields()
135144
assertNotNull(json);
136145
assertTrue(json.contains("\"name\""), "name should be present when set, got: " + json);
137146
}
147+
148+
/**
149+
* GH-1654: reported that listApplications only returned OIDC apps, no SAML apps. A SAML app whose
150+
* settings.signOn.attributeStatements uses the (now-fixed) broken SamlAttributeStatement type would
151+
* throw InvalidTypeIdException while parsing the response array - depending on how a caller's
152+
* pagination/error handling reacted to that, it could plausibly look like SAML apps were being
153+
* silently dropped. Confirms a mixed OIDC/SAML list - with attribute statements populated - now
154+
* deserializes cleanly end-to-end via Application's own (structurally correct) polymorphism.
155+
*/
156+
@Test
157+
public void deserializeApplicationList_withMixedOidcAndSamlAttributeStatements_doesNotThrow() throws Exception {
158+
String json = "["
159+
+ "{\"signOnMode\":\"OPENID_CONNECT\",\"label\":\"oidc-app\",\"id\":\"0oa1\"},"
160+
+ "{\"signOnMode\":\"SAML_2_0\",\"label\":\"saml-app\",\"id\":\"0oa2\",\"settings\":{\"signOn\":{"
161+
+ "\"attributeStatements\":["
162+
+ "{\"type\":\"EXPRESSION\",\"name\":\"email\",\"values\":[\"user.email\"]},"
163+
+ "{\"type\":\"GROUP\",\"filterType\":\"STARTS_WITH\",\"filterValue\":\"Team\"}"
164+
+ "]}}}"
165+
+ "]";
166+
167+
List<Application> apps = objectMapper.readValue(json, new TypeReference<List<Application>>() { });
168+
169+
assertEquals(apps.size(), 2);
170+
assertTrue(apps.get(0) instanceof OpenIdConnectApplication, "expected OpenIdConnectApplication, got " + apps.get(0).getClass());
171+
assertTrue(apps.get(1) instanceof SamlApplication, "expected SamlApplication, got " + apps.get(1).getClass());
172+
173+
SamlApplication samlApp = (SamlApplication) apps.get(1);
174+
List<SamlAttributeStatement> statements = samlApp.getSettings().getSignOn().getAttributeStatements();
175+
assertEquals(statements.size(), 2);
176+
assertEquals(statements.get(0).getType(), SamlAttributeStatement.TypeEnum.EXPRESSION);
177+
assertEquals(statements.get(1).getType(), SamlAttributeStatement.TypeEnum.GROUP);
178+
}
179+
180+
/**
181+
* Found via an audit of every oneOf/anyOf + discriminator pair in the spec for the same defect class
182+
* as OKTA-1227472: OrgContactTypeObj declares BILLING/TECHNICAL subtypes that don't extend it, breaking
183+
* the real listOrgContactTypes endpoint.
184+
*/
185+
@Test
186+
public void deserializeOrgContactTypeObj_withBillingAndTechnicalEntries_doesNotThrow() throws Exception {
187+
String json = "["
188+
+ "{\"contactType\":\"BILLING\"},"
189+
+ "{\"contactType\":\"TECHNICAL\"}"
190+
+ "]";
191+
192+
List<OrgContactTypeObj> contacts = objectMapper.readValue(json, new TypeReference<List<OrgContactTypeObj>>() { });
193+
194+
assertEquals(contacts.size(), 2);
195+
assertEquals(contacts.get(0).getContactType(), OrgContactType.BILLING);
196+
assertEquals(contacts.get(1).getContactType(), OrgContactType.TECHNICAL);
197+
}
198+
199+
/**
200+
* Same audit finding as OrgContactTypeObj: AgentJsonSigningKeyRequest declares RSA/EC subtypes that
201+
* don't extend it. Currently unreferenced by any operation in the spec, fixed for consistency.
202+
*/
203+
@Test
204+
public void deserializeAgentJsonSigningKeyRequest_withRsaAndEcEntries_doesNotThrow() throws Exception {
205+
String json = "["
206+
+ "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"mkC6\",\"use\":\"sig\",\"alg\":\"RS256\"},"
207+
+ "{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"abc\",\"y\":\"def\",\"use\":\"sig\",\"alg\":\"ES256\"}"
208+
+ "]";
209+
210+
List<AgentJsonSigningKeyRequest> keys = objectMapper.readValue(json,
211+
new TypeReference<List<AgentJsonSigningKeyRequest>>() { });
212+
213+
assertEquals(keys.size(), 2);
214+
assertEquals(keys.get(0).getE(), "AQAB");
215+
assertEquals(keys.get(1).getCrv().getValue(), "P-256");
216+
}
217+
218+
/**
219+
* Same audit finding, response-side counterpart of AgentJsonSigningKeyRequest.
220+
*/
221+
@Test
222+
public void deserializeAgentJsonSigningKeyResponse_withRsaAndEcEntries_doesNotThrow() throws Exception {
223+
String json = "["
224+
+ "{\"kty\":\"RSA\",\"e\":\"AQAB\",\"n\":\"mkC6\",\"use\":\"sig\",\"alg\":\"RS256\",\"id\":\"key1\"},"
225+
+ "{\"kty\":\"EC\",\"crv\":\"P-256\",\"x\":\"abc\",\"y\":\"def\",\"use\":\"sig\",\"alg\":\"ES256\",\"id\":\"key2\"}"
226+
+ "]";
227+
228+
List<AgentJsonSigningKeyResponse> keys = objectMapper.readValue(json,
229+
new TypeReference<List<AgentJsonSigningKeyResponse>>() { });
230+
231+
assertEquals(keys.size(), 2);
232+
assertEquals(keys.get(0).getId(), "key1");
233+
assertEquals(keys.get(1).getCrv().getValue(), "P-256");
234+
}
235+
236+
/**
237+
* Same audit finding: ManagedConnection declares 4 connectionType subtypes that don't extend it.
238+
* Reachable from the managed-connection-list endpoint.
239+
*
240+
* Separate, pre-existing quirk unrelated to this fix: each oneOf branch declares its own
241+
* single-value connectionType enum (e.g. just "IDENTITY_ASSERTION_APP_INSTANCE"), and the flat
242+
* merged class ends up keeping only the last-merged branch's enum ("STS_SERVICE_ACCOUNT") - every
243+
* other value falls back to UNKNOWN_DEFAULT_OPEN_API (harmless, since
244+
* READ_UNKNOWN_ENUM_VALUES_AS_NULL-style fallback is already relied on elsewhere; it doesn't throw).
245+
* This test only asserts the polymorphism fix - that deserialization doesn't throw - not full type
246+
* fidelity, which is a separate, wider issue with how the generator merges oneOf enum properties.
247+
*/
248+
@Test
249+
public void deserializeManagedConnection_withDifferentConnectionTypes_doesNotThrow() throws Exception {
250+
String json = "["
251+
+ "{\"connectionType\":\"IDENTITY_ASSERTION_APP_INSTANCE\",\"id\":\"conn1\"},"
252+
+ "{\"connectionType\":\"STS_SERVICE_ACCOUNT\",\"id\":\"conn2\"}"
253+
+ "]";
254+
255+
List<ManagedConnection> connections = objectMapper.readValue(json,
256+
new TypeReference<List<ManagedConnection>>() { });
257+
258+
assertEquals(connections.size(), 2);
259+
assertEquals(connections.get(0).getId(), "conn1");
260+
assertEquals(connections.get(1).getConnectionType(), ManagedConnection.ConnectionTypeEnum.STS_SERVICE_ACCOUNT);
261+
}
262+
263+
/**
264+
* Same audit finding, "creatable" (request-body) counterpart of ManagedConnection. See the enum
265+
* fidelity caveat on {@link #deserializeManagedConnection_withDifferentConnectionTypes_doesNotThrow}.
266+
*/
267+
@Test
268+
public void deserializeManagedConnectionCreatable_withDifferentConnectionTypes_doesNotThrow() throws Exception {
269+
String json = "["
270+
+ "{\"connectionType\":\"IDENTITY_ASSERTION_CUSTOM_AS\"},"
271+
+ "{\"connectionType\":\"STS_SERVICE_ACCOUNT\"}"
272+
+ "]";
273+
274+
List<ManagedConnectionCreatable> connections = objectMapper.readValue(json,
275+
new TypeReference<List<ManagedConnectionCreatable>>() { });
276+
277+
assertEquals(connections.size(), 2);
278+
assertEquals(connections.get(1).getConnectionType(), ManagedConnectionCreatable.ConnectionTypeEnum.STS_SERVICE_ACCOUNT);
279+
}
280+
281+
/**
282+
* Same audit finding: PotentialConnection is a near-duplicate of ManagedConnection with the identical
283+
* defect (same 4 subtypes, same discriminator). See the enum fidelity caveat on
284+
* {@link #deserializeManagedConnection_withDifferentConnectionTypes_doesNotThrow}.
285+
*/
286+
@Test
287+
public void deserializePotentialConnection_withDifferentConnectionTypes_doesNotThrow() throws Exception {
288+
String json = "["
289+
+ "{\"connectionType\":\"IDENTITY_ASSERTION_APP_INSTANCE\"},"
290+
+ "{\"connectionType\":\"STS_SERVICE_ACCOUNT\"}"
291+
+ "]";
292+
293+
List<PotentialConnection> connections = objectMapper.readValue(json,
294+
new TypeReference<List<PotentialConnection>>() { });
295+
296+
assertEquals(connections.size(), 2);
297+
assertEquals(connections.get(1).getConnectionType(), PotentialConnection.ConnectionTypeEnum.STS_SERVICE_ACCOUNT);
298+
}
138299
}

0 commit comments

Comments
 (0)