Skip to content

Commit eb5d681

Browse files
prachi-oktaprachi-okta
authored
fix: Jackson polymorphism/serialization, pagination, and GH-reported bugs (GH-1642, GH-1689, GH-1690) (#1699)
* fix(client): correct Jackson polymorphism, null serialization, and pagination type consistency - Disable broken @JsonSubTypes-based polymorphic resolution on ListJwk200ResponseInner/SamlAttributeStatement, whose spec-declared discriminator subtypes don't actually extend them, causing InvalidTypeIdException on deserialization. - Restore NON_NULL serialization on Application and its subtypes via a JsonFilter mixin, overriding the @JsonInclude(ALWAYS) generated for their required properties, which was forcing partial PUT updates to null out unrelated fields like credentials/name/settings. - Always wrap list responses in PagedList, even without a Link header (e.g. a filtered query that fits on one page), so callers get a consistent return type instead of an occasional ClassCastException. - Add maxConsecutiveCharacters to PasswordPolicyPasswordSettingsComplexity, which was missing from the spec and silently dropped on write. Co-Authored-By: Claude Code * fix: surface custom group profile attributes, correct userBehaviors type, bump bouncycastle - GroupProfileDeserializer's default case for unrecognized keys was discarding them instead of routing them into additionalProperties, so custom group schema attributes were silently dropped on read. Fixed to match the existing UserProfileDeserializer/ OktaUserGroupProfileDeserializer pattern. (GH-1642) - LogSecurityContext.userBehaviors was missing an items type, so the generator defaulted to List<String> while the API returns a list of objects, causing a MismatchedInputException. Declared items: {} so it generates List<Object>. (GH-1689) - Bump bcprov-jdk18on/bcpkix-jdk18on 1.79 -> 1.84 to address reported CVEs; commons-lang3 was already at 3.18.0. (GH-1690) Co-Authored-By: Claude Code * fix(ci): stop jdk11/jdk21 from racing on the shared org-singleton OrgSetting IT jdk11 and jdk21 run the identical integration test suite against the same shared live test org with no per-job isolation, and previously ran concurrently in the CircleCI workflow. OrgSettingGeneralIT mutates OrgSetting, which is a singleton per org, so one job's write could be clobbered by (or read back as) the other job's concurrent write to the same fields, causing flaky read-after-write assertion failures that no amount of retrying could fix. - Make jdk21 require jdk11 so the two jobs no longer run concurrently against the shared org. - Belt-and-suspenders: give OrgSettingGeneralIT's lifecycle test unique per-run values (UUID-suffixed) for the fields it writes, so even concurrent runs can no longer be confused by each other's writes. Co-Authored-By: Claude Code * fix(it): fix GString equality bug in OrgSettingGeneralIT, revert jdk21 serialization The real root cause of the flaky assertion was a Groovy GString/String equality bug in my own previous fix, not job concurrency: string interpolation ("...${runId}...") produces a GString, and GString.equals(String) is always false even when the text matches - CI logs showed the "Expected" and "was" values as byte-for-byte identical strings while the assertion still failed. - Add .toString() to the three unique per-run values so they're real Strings, fixing the equalTo() comparisons. - Revert the jdk21->jdk11 CircleCI dependency added in bc649b9, since it's no longer needed and unnecessarily halves CI coverage whenever jdk11 fails for any reason; jdk11/jdk21 run independently again. Co-Authored-By: Claude Code * fix(it): retry transient 500 on createApplication in ApplicationSSOPublicKeysIT.setup CI observed a one-off ApiException{code=500, errorCode=E0000009} from createApplication while provisioning the test OIDC app in @BeforeClass. The SDK's built-in retry strategy (OktaHttpRequestRetryStrategy) only retries 429/503/504, not a bare 500, and this call runs in @BeforeClass so a single spurious 500 fails the entire test class. Add a small bounded retry (3 attempts, linear backoff) scoped to this one call. Co-Authored-By: Claude Code
1 parent 30a3e0f commit eb5d681

10 files changed

Lines changed: 442 additions & 37 deletions

File tree

‎api/src/main/java/com/okta/sdk/resource/common/PagedList.java‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,27 +67,29 @@ public String getAfter() {
6767
public static <T> T constructPagedList(HttpResponse response, T value) {
6868
Assert.notNull(response);
6969
Assert.isTrue(value instanceof List);
70-
Header[] linkHeaders = response.getHeaders("link");
71-
if (linkHeaders == null || linkHeaders.length == 0) {
70+
if (value instanceof PagedList) {
7271
return value;
7372
}
7473
String nextPage = null;
7574
String self = null;
76-
for (Header link : linkHeaders) {
77-
String[] parts = link.getValue().split("; *");
78-
String url = parts[0]
79-
.replaceAll("<", "")
80-
.replaceAll(">", "");
81-
String rel = parts[1];
82-
if (rel.equals("rel=\"next\"")) {
83-
nextPage = url;
84-
} else if (rel.equals("rel=\"self\"")) {
85-
self = url;
75+
Header[] linkHeaders = response.getHeaders("link");
76+
if (linkHeaders != null) {
77+
for (Header link : linkHeaders) {
78+
String[] parts = link.getValue().split("; *");
79+
String url = parts[0]
80+
.replaceAll("<", "")
81+
.replaceAll(">", "");
82+
String rel = parts[1];
83+
if (rel.equals("rel=\"next\"")) {
84+
nextPage = url;
85+
} else if (rel.equals("rel=\"self\"")) {
86+
self = url;
87+
}
8688
}
8789
}
88-
if (nextPage == null && self == null) {
89-
return value;
90-
}
90+
// Always wrap in a PagedList, even when there's no Link header (e.g. a filtered
91+
// query that fits on a single page), so callers get a consistent return type.
92+
// getAfter()/hasMoreItems() already report "no more pages" when nextPage is null.
9193
return (T) new PagedList((List) value, self, nextPage, null);
9294
}
9395

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import org.slf4j.LoggerFactory;
2020

2121
import com.fasterxml.jackson.annotation.*;
2222
import com.fasterxml.jackson.databind.*;
23+
import com.fasterxml.jackson.databind.ser.BeanPropertyWriter;
24+
import com.fasterxml.jackson.databind.ser.PropertyWriter;
25+
import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter;
26+
import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
2327
{{#joda}}
2428
import com.fasterxml.jackson.datatype.joda.JodaModule;
2529
{{/joda}}
@@ -125,6 +129,43 @@ public class ApiClient{{#jsr310}} extends JavaTimeFormatter{{/jsr310}} {
125129
126130
private static final Logger log = LoggerFactory.getLogger(ApiClient.class);
127131
132+
/**
133+
* Some generated models (e.g. ListJwk200ResponseInner, SamlAttributeStatement) declare
134+
* {@literal @}JsonSubTypes entries pointing at classes that don't actually extend them, because their
135+
* source schemas use oneOf/anyOf + discriminator without allOf-based inheritance. Jackson requires real
136+
* Java inheritance for polymorphic resolution, so deserializing these throws InvalidTypeIdException.
137+
* These classes already contain the union of all their branches' properties as flat fields, so disabling
138+
* polymorphic resolution loses no data.
139+
*/
140+
@JsonTypeInfo(use = JsonTypeInfo.Id.NONE)
141+
private interface NoPolymorphicTypeInfoMixin {
142+
}
143+
144+
/**
145+
* Required properties on generated models get {@literal @}JsonInclude(ALWAYS) so create payloads always
146+
* include them. But Application subtypes (e.g. OpenIdConnectApplication) reuse the same class for partial
147+
* PUT updates, where ALWAYS forces unrelated required fields (credentials, name, settings) to serialize as
148+
* literal null, overwriting them server-side. Jackson property filters run before per-property
149+
* JsonInclude, so this filter restores NON_NULL behavior for Application and all its subtypes.
150+
*/
151+
@JsonFilter("applicationNullFieldFilter")
152+
private interface ApplicationNullFieldFilterMixin {
153+
}
154+
155+
private static final class SkipNullPropertyFilter extends SimpleBeanPropertyFilter {
156+
@Override
157+
public void serializeAsField(Object pojo, com.fasterxml.jackson.core.JsonGenerator jgen,
158+
SerializerProvider provider, PropertyWriter writer) throws Exception {
159+
if (writer instanceof BeanPropertyWriter && ((BeanPropertyWriter) writer).get(pojo) == null) {
160+
if (!jgen.canOmitFields()) {
161+
writer.serializeAsOmittedField(pojo, jgen, provider);
162+
}
163+
return;
164+
}
165+
super.serializeAsField(pojo, jgen, provider, writer);
166+
}
167+
}
168+
128169
private Map<String, String> defaultHeaderMap = new HashMap<String, String>();
129170
private Map<String, String> defaultCookieMap = new HashMap<String, String>();
130171
private String basePath = "{{{basePath}}}";
@@ -195,6 +236,18 @@ protected List<ServerConfiguration> servers = new ArrayList<ServerConfiguration>
195236
objectMapper.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING);
196237
objectMapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING);
197238
objectMapper.enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);
239+
240+
// OKTA-1227472: disable broken polymorphic type resolution on models whose spec-declared
241+
// discriminator subtypes don't actually extend them.
242+
objectMapper.addMixIn(com.okta.sdk.resource.model.ListJwk200ResponseInner.class, NoPolymorphicTypeInfoMixin.class);
243+
objectMapper.addMixIn(com.okta.sdk.resource.model.SamlAttributeStatement.class, NoPolymorphicTypeInfoMixin.class);
244+
245+
// OKTA-1218351: restore NON_NULL serialization on Application and its subtypes, overriding the
246+
// per-property JsonInclude(ALWAYS) generated for their required properties.
247+
objectMapper.addMixIn(com.okta.sdk.resource.model.Application.class, ApplicationNullFieldFilterMixin.class);
248+
objectMapper.setFilterProvider(new SimpleFilterProvider()
249+
.addFilter("applicationNullFieldFilter", new SkipNullPropertyFilter())
250+
.setFailOnUnknownId(false));
198251
{{#joda}}
199252
objectMapper.registerModule(new JodaModule());
200253
{{/joda}}

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* Copyright 2026-Present Okta, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.okta.sdk.resource.client;
17+
18+
import com.fasterxml.jackson.core.type.TypeReference;
19+
import com.fasterxml.jackson.databind.ObjectMapper;
20+
import com.okta.sdk.cache.Cache;
21+
import com.okta.sdk.cache.CacheManager;
22+
import com.okta.sdk.resource.model.ApplicationVisibility;
23+
import com.okta.sdk.resource.model.ApplicationVisibilityHide;
24+
import com.okta.sdk.resource.model.ListJwk200ResponseInner;
25+
import com.okta.sdk.resource.model.OpenIdConnectApplication;
26+
import com.okta.sdk.resource.model.SamlAttributeStatement;
27+
import org.apache.hc.client5.http.impl.classic.HttpClients;
28+
import org.testng.annotations.Test;
29+
30+
import java.util.List;
31+
32+
import static org.testng.Assert.assertEquals;
33+
import static org.testng.Assert.assertFalse;
34+
import static org.testng.Assert.assertNotNull;
35+
import static org.testng.Assert.assertTrue;
36+
37+
/**
38+
* Unit tests for the Jackson mixins registered in {@link ApiClient}'s default {@code ObjectMapper}.
39+
*/
40+
public class ApiClientJacksonMixinTest {
41+
42+
// A minimal no-op CacheManager, so this test doesn't need the okta-sdk-impl module (which would
43+
// introduce a circular dependency back onto this api module) just to construct an ApiClient.
44+
private static final CacheManager NOOP_CACHE_MANAGER = new CacheManager() {
45+
@Override
46+
public <K, V> Cache<K, V> getCache(String name) {
47+
return new Cache<K, V>() {
48+
@Override
49+
public V get(K key) {
50+
return null;
51+
}
52+
53+
@Override
54+
public V put(K key, V value) {
55+
return null;
56+
}
57+
58+
@Override
59+
public V remove(K key) {
60+
return null;
61+
}
62+
};
63+
}
64+
};
65+
66+
private final ObjectMapper objectMapper =
67+
new ApiClient(HttpClients.createDefault(), NOOP_CACHE_MANAGER).getObjectMapper();
68+
69+
/**
70+
* OKTA-1227472: ListJwk200ResponseInner declares @JsonSubTypes entries for classes that don't actually
71+
* extend it, which used to throw InvalidTypeIdException. The mixin disables polymorphic resolution so
72+
* the flat class (which already has every branch's properties) is used directly.
73+
*/
74+
@Test
75+
public void deserializeListJwkResponse_withMixedSigAndEncEntries_doesNotThrow() throws Exception {
76+
String json = "["
77+
+ "{\"kid\":\"kid1\",\"status\":\"ACTIVE\",\"kty\":\"RSA\",\"use\":\"sig\",\"id\":\"pks1\"},"
78+
+ "{\"e\":\"AQAB\",\"kty\":\"RSA\",\"n\":\"mkC6\",\"use\":\"enc\"}"
79+
+ "]";
80+
81+
List<ListJwk200ResponseInner> keys = objectMapper.readValue(json,
82+
new TypeReference<List<ListJwk200ResponseInner>>() { });
83+
84+
assertEquals(keys.size(), 2);
85+
assertEquals(keys.get(0).getKid(), "kid1");
86+
assertEquals(keys.get(1).getE(), "AQAB");
87+
}
88+
89+
/**
90+
* OKTA-1227472: same defect on SamlAttributeStatement (EXPRESSION/GROUP anyOf without allOf inheritance).
91+
*/
92+
@Test
93+
public void deserializeSamlAttributeStatement_withExpressionAndGroupEntries_doesNotThrow() throws Exception {
94+
String json = "["
95+
+ "{\"type\":\"EXPRESSION\",\"name\":\"email\",\"values\":[\"user.email\"]},"
96+
+ "{\"type\":\"GROUP\",\"filterType\":\"STARTS_WITH\",\"filterValue\":\"Team\"}"
97+
+ "]";
98+
99+
List<SamlAttributeStatement> statements = objectMapper.readValue(json,
100+
new TypeReference<List<SamlAttributeStatement>>() { });
101+
102+
assertEquals(statements.size(), 2);
103+
assertEquals(statements.get(0).getType(), SamlAttributeStatement.TypeEnum.EXPRESSION);
104+
assertEquals(statements.get(0).getName(), "email");
105+
assertEquals(statements.get(1).getType(), SamlAttributeStatement.TypeEnum.GROUP);
106+
assertEquals(statements.get(1).getFilterValue(), "Team");
107+
}
108+
109+
/**
110+
* OKTA-1218351: OpenIdConnectApplication marks credentials/name/settings as required, which generates
111+
* @JsonInclude(ALWAYS) on those properties. A partial update (only visibility set) must not serialize
112+
* them as literal null, or the API overwrites/rejects the update.
113+
*/
114+
@Test
115+
public void serializePartialOpenIdConnectApplication_omitsNullRequiredFields() throws Exception {
116+
OpenIdConnectApplication app = new OpenIdConnectApplication();
117+
ApplicationVisibilityHide hide = new ApplicationVisibilityHide().web(true).iOS(true);
118+
app.setVisibility(new ApplicationVisibility().hide(hide).autoSubmitToolbar(true));
119+
120+
String json = objectMapper.writeValueAsString(app);
121+
122+
assertFalse(json.contains("\"credentials\""), "credentials should be omitted, got: " + json);
123+
assertFalse(json.contains("\"name\""), "name should be omitted, got: " + json);
124+
assertFalse(json.contains("\"settings\""), "settings should be omitted, got: " + json);
125+
assertTrue(json.contains("\"visibility\""), "visibility should be present, got: " + json);
126+
}
127+
128+
@Test
129+
public void serializeFullOpenIdConnectApplication_stillIncludesRequiredFields() throws Exception {
130+
OpenIdConnectApplication app = new OpenIdConnectApplication();
131+
app.setName(OpenIdConnectApplication.NameEnum.OIDC_CLIENT);
132+
133+
String json = objectMapper.writeValueAsString(app);
134+
135+
assertNotNull(json);
136+
assertTrue(json.contains("\"name\""), "name should be present when set, got: " + json);
137+
}
138+
}

‎api/src/test/java/com/okta/sdk/resource/common/PagedListTest.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Copyright 2026-Present Okta, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.okta.sdk.resource.common;
17+
18+
import org.apache.hc.core5.http.HttpResponse;
19+
import org.apache.hc.core5.http.HttpStatus;
20+
import org.apache.hc.core5.http.message.BasicHttpResponse;
21+
import org.testng.annotations.Test;
22+
23+
import java.util.Arrays;
24+
import java.util.List;
25+
26+
import static org.testng.Assert.assertEquals;
27+
import static org.testng.Assert.assertFalse;
28+
import static org.testng.Assert.assertTrue;
29+
30+
/**
31+
* Unit tests for {@link PagedList}, in particular {@link PagedList#constructPagedList}.
32+
*
33+
* OKTA-1217985: SDK list methods must always return a {@link PagedList}, even when a filter (e.g. {@code q})
34+
* narrows the result to a single page and the response has no {@code Link} header. Consumers that cast the
35+
* result to {@code PagedList} would otherwise get a {@code ClassCastException}.
36+
*/
37+
public class PagedListTest {
38+
39+
@Test
40+
public void constructPagedList_withNoLinkHeader_returnsPagedListNotPlainList() {
41+
HttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK);
42+
43+
List<String> value = Arrays.asList("a", "b");
44+
Object result = PagedList.constructPagedList(response, value);
45+
46+
assertTrue(result instanceof PagedList, "expected a PagedList even without a Link header");
47+
PagedList<?> pagedList = (PagedList<?>) result;
48+
assertEquals(pagedList.size(), 2);
49+
assertFalse(pagedList.hasMoreItems());
50+
}
51+
52+
@Test
53+
public void constructPagedList_withNextLinkHeader_returnsPagedListWithNextPage() {
54+
BasicHttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK);
55+
response.addHeader("link", "<https://example.okta.com/api/v1/users?after=abc123>; rel=\"next\"");
56+
57+
List<String> value = Arrays.asList("a", "b", "c");
58+
Object result = PagedList.constructPagedList(response, value);
59+
60+
assertTrue(result instanceof PagedList);
61+
PagedList<?> pagedList = (PagedList<?>) result;
62+
assertEquals(pagedList.size(), 3);
63+
assertTrue(pagedList.hasMoreItems());
64+
assertEquals(pagedList.getAfter(), "abc123");
65+
}
66+
67+
@Test
68+
public void constructPagedList_withSelfLinkOnly_returnsPagedListWithNoMoreItems() {
69+
BasicHttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK);
70+
response.addHeader("link", "<https://example.okta.com/api/v1/users?after=xyz>; rel=\"self\"");
71+
72+
List<String> value = Arrays.asList("a");
73+
Object result = PagedList.constructPagedList(response, value);
74+
75+
assertTrue(result instanceof PagedList);
76+
PagedList<?> pagedList = (PagedList<?>) result;
77+
assertEquals(pagedList.getSelf(), "https://example.okta.com/api/v1/users?after=xyz");
78+
assertFalse(pagedList.hasMoreItems());
79+
}
80+
81+
@Test
82+
public void constructPagedList_withAlreadyPagedList_returnsSameInstance() {
83+
HttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK);
84+
PagedList<String> existing = new PagedList<>(Arrays.asList("a"), null, null, null);
85+
86+
Object result = PagedList.constructPagedList(response, existing);
87+
88+
assertTrue(result == existing);
89+
}
90+
}

‎impl/src/main/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public GroupProfile deserialize(JsonParser jp, DeserializationContext ctxt) thro
8080
break;
8181

8282
default:
83-
break;
83+
groupProfile.getAdditionalProperties().put(key, value);
8484
}
8585
}
8686

0 commit comments

Comments
 (0)