From 72cffd89c90f5dae2d348bb3d6e51613644da736 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Fri, 17 Jul 2026 13:54:50 +0530 Subject: [PATCH 1/5] 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 --- .../okta/sdk/resource/common/PagedList.java | 32 ++-- .../custom_templates/ApiClient.mustache | 53 +++++++ .../client/ApiClientJacksonMixinTest.java | 138 ++++++++++++++++++ .../sdk/resource/common/PagedListTest.java | 90 ++++++++++++ src/swagger/api.yaml | 4 + 5 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 api/src/test/java/com/okta/sdk/resource/client/ApiClientJacksonMixinTest.java create mode 100644 api/src/test/java/com/okta/sdk/resource/common/PagedListTest.java diff --git a/api/src/main/java/com/okta/sdk/resource/common/PagedList.java b/api/src/main/java/com/okta/sdk/resource/common/PagedList.java index 16427f2d17c..9f4698ec159 100644 --- a/api/src/main/java/com/okta/sdk/resource/common/PagedList.java +++ b/api/src/main/java/com/okta/sdk/resource/common/PagedList.java @@ -67,27 +67,29 @@ public String getAfter() { public static T constructPagedList(HttpResponse response, T value) { Assert.notNull(response); Assert.isTrue(value instanceof List); - Header[] linkHeaders = response.getHeaders("link"); - if (linkHeaders == null || linkHeaders.length == 0) { + if (value instanceof PagedList) { return value; } String nextPage = null; String self = null; - for (Header link : linkHeaders) { - String[] parts = link.getValue().split("; *"); - String url = parts[0] - .replaceAll("<", "") - .replaceAll(">", ""); - String rel = parts[1]; - if (rel.equals("rel=\"next\"")) { - nextPage = url; - } else if (rel.equals("rel=\"self\"")) { - self = url; + Header[] linkHeaders = response.getHeaders("link"); + if (linkHeaders != null) { + for (Header link : linkHeaders) { + String[] parts = link.getValue().split("; *"); + String url = parts[0] + .replaceAll("<", "") + .replaceAll(">", ""); + String rel = parts[1]; + if (rel.equals("rel=\"next\"")) { + nextPage = url; + } else if (rel.equals("rel=\"self\"")) { + self = url; + } } } - if (nextPage == null && self == null) { - return value; - } + // Always wrap in a PagedList, even when there's no Link header (e.g. a filtered + // query that fits on a single page), so callers get a consistent return type. + // getAfter()/hasMoreItems() already report "no more pages" when nextPage is null. return (T) new PagedList((List) value, self, nextPage, null); } diff --git a/api/src/main/resources/custom_templates/ApiClient.mustache b/api/src/main/resources/custom_templates/ApiClient.mustache index c3f2cbb8deb..9d25f21af89 100644 --- a/api/src/main/resources/custom_templates/ApiClient.mustache +++ b/api/src/main/resources/custom_templates/ApiClient.mustache @@ -20,6 +20,10 @@ import org.slf4j.LoggerFactory; import com.fasterxml.jackson.annotation.*; import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; +import com.fasterxml.jackson.databind.ser.PropertyWriter; +import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter; +import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; {{#joda}} import com.fasterxml.jackson.datatype.joda.JodaModule; {{/joda}} @@ -125,6 +129,43 @@ public class ApiClient{{#jsr310}} extends JavaTimeFormatter{{/jsr310}} { private static final Logger log = LoggerFactory.getLogger(ApiClient.class); +/** + * Some generated models (e.g. ListJwk200ResponseInner, SamlAttributeStatement) declare + * {@literal @}JsonSubTypes entries pointing at classes that don't actually extend them, because their + * source schemas use oneOf/anyOf + discriminator without allOf-based inheritance. Jackson requires real + * Java inheritance for polymorphic resolution, so deserializing these throws InvalidTypeIdException. + * These classes already contain the union of all their branches' properties as flat fields, so disabling + * polymorphic resolution loses no data. + */ +@JsonTypeInfo(use = JsonTypeInfo.Id.NONE) +private interface NoPolymorphicTypeInfoMixin { +} + +/** + * Required properties on generated models get {@literal @}JsonInclude(ALWAYS) so create payloads always + * include them. But Application subtypes (e.g. OpenIdConnectApplication) reuse the same class for partial + * PUT updates, where ALWAYS forces unrelated required fields (credentials, name, settings) to serialize as + * literal null, overwriting them server-side. Jackson property filters run before per-property + * JsonInclude, so this filter restores NON_NULL behavior for Application and all its subtypes. + */ +@JsonFilter("applicationNullFieldFilter") +private interface ApplicationNullFieldFilterMixin { +} + +private static final class SkipNullPropertyFilter extends SimpleBeanPropertyFilter { + @Override + public void serializeAsField(Object pojo, com.fasterxml.jackson.core.JsonGenerator jgen, + SerializerProvider provider, PropertyWriter writer) throws Exception { + if (writer instanceof BeanPropertyWriter && ((BeanPropertyWriter) writer).get(pojo) == null) { + if (!jgen.canOmitFields()) { + writer.serializeAsOmittedField(pojo, jgen, provider); + } + return; + } + super.serializeAsField(pojo, jgen, provider, writer); + } +} + private Map defaultHeaderMap = new HashMap(); private Map defaultCookieMap = new HashMap(); private String basePath = "{{{basePath}}}"; @@ -195,6 +236,18 @@ protected List servers = new ArrayList objectMapper.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING); objectMapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING); objectMapper.enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY); + + // OKTA-1227472: disable broken polymorphic type resolution on models whose spec-declared + // discriminator subtypes don't actually extend them. + objectMapper.addMixIn(com.okta.sdk.resource.model.ListJwk200ResponseInner.class, NoPolymorphicTypeInfoMixin.class); + objectMapper.addMixIn(com.okta.sdk.resource.model.SamlAttributeStatement.class, NoPolymorphicTypeInfoMixin.class); + + // OKTA-1218351: restore NON_NULL serialization on Application and its subtypes, overriding the + // per-property JsonInclude(ALWAYS) generated for their required properties. + objectMapper.addMixIn(com.okta.sdk.resource.model.Application.class, ApplicationNullFieldFilterMixin.class); + objectMapper.setFilterProvider(new SimpleFilterProvider() + .addFilter("applicationNullFieldFilter", new SkipNullPropertyFilter()) + .setFailOnUnknownId(false)); {{#joda}} objectMapper.registerModule(new JodaModule()); {{/joda}} diff --git a/api/src/test/java/com/okta/sdk/resource/client/ApiClientJacksonMixinTest.java b/api/src/test/java/com/okta/sdk/resource/client/ApiClientJacksonMixinTest.java new file mode 100644 index 00000000000..a3cfcc8ef70 --- /dev/null +++ b/api/src/test/java/com/okta/sdk/resource/client/ApiClientJacksonMixinTest.java @@ -0,0 +1,138 @@ +/* + * Copyright 2026-Present Okta, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.okta.sdk.resource.client; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.okta.sdk.cache.Cache; +import com.okta.sdk.cache.CacheManager; +import com.okta.sdk.resource.model.ApplicationVisibility; +import com.okta.sdk.resource.model.ApplicationVisibilityHide; +import com.okta.sdk.resource.model.ListJwk200ResponseInner; +import com.okta.sdk.resource.model.OpenIdConnectApplication; +import com.okta.sdk.resource.model.SamlAttributeStatement; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.testng.annotations.Test; + +import java.util.List; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +/** + * Unit tests for the Jackson mixins registered in {@link ApiClient}'s default {@code ObjectMapper}. + */ +public class ApiClientJacksonMixinTest { + + // A minimal no-op CacheManager, so this test doesn't need the okta-sdk-impl module (which would + // introduce a circular dependency back onto this api module) just to construct an ApiClient. + private static final CacheManager NOOP_CACHE_MANAGER = new CacheManager() { + @Override + public Cache getCache(String name) { + return new Cache() { + @Override + public V get(K key) { + return null; + } + + @Override + public V put(K key, V value) { + return null; + } + + @Override + public V remove(K key) { + return null; + } + }; + } + }; + + private final ObjectMapper objectMapper = + new ApiClient(HttpClients.createDefault(), NOOP_CACHE_MANAGER).getObjectMapper(); + + /** + * OKTA-1227472: ListJwk200ResponseInner declares @JsonSubTypes entries for classes that don't actually + * extend it, which used to throw InvalidTypeIdException. The mixin disables polymorphic resolution so + * the flat class (which already has every branch's properties) is used directly. + */ + @Test + public void deserializeListJwkResponse_withMixedSigAndEncEntries_doesNotThrow() throws Exception { + String json = "[" + + "{\"kid\":\"kid1\",\"status\":\"ACTIVE\",\"kty\":\"RSA\",\"use\":\"sig\",\"id\":\"pks1\"}," + + "{\"e\":\"AQAB\",\"kty\":\"RSA\",\"n\":\"mkC6\",\"use\":\"enc\"}" + + "]"; + + List keys = objectMapper.readValue(json, + new TypeReference>() { }); + + assertEquals(keys.size(), 2); + assertEquals(keys.get(0).getKid(), "kid1"); + assertEquals(keys.get(1).getE(), "AQAB"); + } + + /** + * OKTA-1227472: same defect on SamlAttributeStatement (EXPRESSION/GROUP anyOf without allOf inheritance). + */ + @Test + public void deserializeSamlAttributeStatement_withExpressionAndGroupEntries_doesNotThrow() throws Exception { + String json = "[" + + "{\"type\":\"EXPRESSION\",\"name\":\"email\",\"values\":[\"user.email\"]}," + + "{\"type\":\"GROUP\",\"filterType\":\"STARTS_WITH\",\"filterValue\":\"Team\"}" + + "]"; + + List statements = objectMapper.readValue(json, + new TypeReference>() { }); + + assertEquals(statements.size(), 2); + assertEquals(statements.get(0).getType(), SamlAttributeStatement.TypeEnum.EXPRESSION); + assertEquals(statements.get(0).getName(), "email"); + assertEquals(statements.get(1).getType(), SamlAttributeStatement.TypeEnum.GROUP); + assertEquals(statements.get(1).getFilterValue(), "Team"); + } + + /** + * OKTA-1218351: OpenIdConnectApplication marks credentials/name/settings as required, which generates + * @JsonInclude(ALWAYS) on those properties. A partial update (only visibility set) must not serialize + * them as literal null, or the API overwrites/rejects the update. + */ + @Test + public void serializePartialOpenIdConnectApplication_omitsNullRequiredFields() throws Exception { + OpenIdConnectApplication app = new OpenIdConnectApplication(); + ApplicationVisibilityHide hide = new ApplicationVisibilityHide().web(true).iOS(true); + app.setVisibility(new ApplicationVisibility().hide(hide).autoSubmitToolbar(true)); + + String json = objectMapper.writeValueAsString(app); + + assertFalse(json.contains("\"credentials\""), "credentials should be omitted, got: " + json); + assertFalse(json.contains("\"name\""), "name should be omitted, got: " + json); + assertFalse(json.contains("\"settings\""), "settings should be omitted, got: " + json); + assertTrue(json.contains("\"visibility\""), "visibility should be present, got: " + json); + } + + @Test + public void serializeFullOpenIdConnectApplication_stillIncludesRequiredFields() throws Exception { + OpenIdConnectApplication app = new OpenIdConnectApplication(); + app.setName(OpenIdConnectApplication.NameEnum.OIDC_CLIENT); + + String json = objectMapper.writeValueAsString(app); + + assertNotNull(json); + assertTrue(json.contains("\"name\""), "name should be present when set, got: " + json); + } +} diff --git a/api/src/test/java/com/okta/sdk/resource/common/PagedListTest.java b/api/src/test/java/com/okta/sdk/resource/common/PagedListTest.java new file mode 100644 index 00000000000..c4f6df57fd5 --- /dev/null +++ b/api/src/test/java/com/okta/sdk/resource/common/PagedListTest.java @@ -0,0 +1,90 @@ +/* + * Copyright 2026-Present Okta, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.okta.sdk.resource.common; + +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.message.BasicHttpResponse; +import org.testng.annotations.Test; + +import java.util.Arrays; +import java.util.List; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +/** + * Unit tests for {@link PagedList}, in particular {@link PagedList#constructPagedList}. + * + * OKTA-1217985: SDK list methods must always return a {@link PagedList}, even when a filter (e.g. {@code q}) + * narrows the result to a single page and the response has no {@code Link} header. Consumers that cast the + * result to {@code PagedList} would otherwise get a {@code ClassCastException}. + */ +public class PagedListTest { + + @Test + public void constructPagedList_withNoLinkHeader_returnsPagedListNotPlainList() { + HttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK); + + List value = Arrays.asList("a", "b"); + Object result = PagedList.constructPagedList(response, value); + + assertTrue(result instanceof PagedList, "expected a PagedList even without a Link header"); + PagedList pagedList = (PagedList) result; + assertEquals(pagedList.size(), 2); + assertFalse(pagedList.hasMoreItems()); + } + + @Test + public void constructPagedList_withNextLinkHeader_returnsPagedListWithNextPage() { + BasicHttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK); + response.addHeader("link", "; rel=\"next\""); + + List value = Arrays.asList("a", "b", "c"); + Object result = PagedList.constructPagedList(response, value); + + assertTrue(result instanceof PagedList); + PagedList pagedList = (PagedList) result; + assertEquals(pagedList.size(), 3); + assertTrue(pagedList.hasMoreItems()); + assertEquals(pagedList.getAfter(), "abc123"); + } + + @Test + public void constructPagedList_withSelfLinkOnly_returnsPagedListWithNoMoreItems() { + BasicHttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK); + response.addHeader("link", "; rel=\"self\""); + + List value = Arrays.asList("a"); + Object result = PagedList.constructPagedList(response, value); + + assertTrue(result instanceof PagedList); + PagedList pagedList = (PagedList) result; + assertEquals(pagedList.getSelf(), "https://example.okta.com/api/v1/users?after=xyz"); + assertFalse(pagedList.hasMoreItems()); + } + + @Test + public void constructPagedList_withAlreadyPagedList_returnsSameInstance() { + HttpResponse response = new BasicHttpResponse(HttpStatus.SC_OK); + PagedList existing = new PagedList<>(Arrays.asList("a"), null, null, null); + + Object result = PagedList.constructPagedList(response, existing); + + assertTrue(result == existing); + } +} diff --git a/src/swagger/api.yaml b/src/swagger/api.yaml index 27e1d5068f9..2cc1d957870 100644 --- a/src/swagger/api.yaml +++ b/src/swagger/api.yaml @@ -73296,6 +73296,10 @@ components: type: integer description: 'Indicates if a password must contain at least one upper case letter: `0` indicates no, `1` indicates yes' default: 1 + maxConsecutiveCharacters: + type: integer + description: 'The maximum number of consecutive characters allowed in a password: `0` indicates no limit' + default: 0 oelStatement: type: string description: Use an [Expression Language](https://developer.okta.com/docs/reference/okta-expression-language-in-identity-engine/) expression to block a word from being used in a password. You can only block one word per expression. Use the `OR` operator to connect multiple expressions to block multiple words. From c4a0ed1611b57766432a9b0966a0baa5d5077995 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Fri, 17 Jul 2026 13:55:14 +0530 Subject: [PATCH 2/5] 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 while the API returns a list of objects, causing a MismatchedInputException. Declared items: {} so it generates List. (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 --- .../GroupProfileDeserializer.java | 2 +- .../GroupProfileDeserializerTest.java | 82 +++++++++++++++++++ pom.xml | 2 +- src/swagger/api.yaml | 1 + 4 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 impl/src/test/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializerTest.java diff --git a/impl/src/main/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializer.java b/impl/src/main/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializer.java index 0ed79dfc06e..1f6163fb720 100644 --- a/impl/src/main/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializer.java +++ b/impl/src/main/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializer.java @@ -80,7 +80,7 @@ public GroupProfile deserialize(JsonParser jp, DeserializationContext ctxt) thro break; default: - break; + groupProfile.getAdditionalProperties().put(key, value); } } diff --git a/impl/src/test/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializerTest.java b/impl/src/test/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializerTest.java new file mode 100644 index 00000000000..62de53081dc --- /dev/null +++ b/impl/src/test/java/com/okta/sdk/impl/deserializer/GroupProfileDeserializerTest.java @@ -0,0 +1,82 @@ +/* + * Copyright 2026-Present Okta, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.okta.sdk.impl.deserializer; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.okta.sdk.resource.model.GroupProfile; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +/** + * Unit tests for {@link GroupProfileDeserializer}. + * + * GH-1642: custom group schema attributes (e.g. {@code MaxUsersCount}) were silently dropped instead of + * being surfaced via {@link GroupProfile#getAdditionalProperties()}, because the deserializer's + * {@code default} case discarded any property it didn't explicitly recognize. + */ +public class GroupProfileDeserializerTest { + + private ObjectMapper objectMapper; + + @BeforeMethod + public void setUp() { + objectMapper = new ObjectMapper(); + SimpleModule module = new SimpleModule(); + module.addDeserializer(GroupProfile.class, new GroupProfileDeserializer()); + objectMapper.registerModule(module); + } + + @Test + public void testDeserialize_withCustomAttribute_isSurfacedInAdditionalProperties() throws JsonProcessingException { + String json = "{\"name\":\"Engineering\",\"description\":\"Eng team\",\"MaxUsersCount\":42}"; + + GroupProfile profile = objectMapper.readValue(json, GroupProfile.class); + + assertEquals(profile.getName(), "Engineering"); + assertEquals(profile.getDescription(), "Eng team"); + assertNotNull(profile.getAdditionalProperties()); + assertEquals(profile.getAdditionalProperties().get("MaxUsersCount"), 42); + } + + @Test + public void testDeserialize_withMultipleCustomAttributes_allSurfaced() throws JsonProcessingException { + String json = "{\"name\":\"Sales\",\"region\":\"EMEA\",\"costCenter\":\"CC-100\",\"isVip\":true}"; + + GroupProfile profile = objectMapper.readValue(json, GroupProfile.class); + + assertEquals(profile.getName(), "Sales"); + assertEquals(profile.getAdditionalProperties().get("region"), "EMEA"); + assertEquals(profile.getAdditionalProperties().get("costCenter"), "CC-100"); + assertEquals(profile.getAdditionalProperties().get("isVip"), true); + } + + @Test + public void testDeserialize_withOnlyKnownFields_hasEmptyAdditionalProperties() throws JsonProcessingException { + String json = "{\"name\":\"NoCustomAttrs\"}"; + + GroupProfile profile = objectMapper.readValue(json, GroupProfile.class); + + assertEquals(profile.getName(), "NoCustomAttrs"); + assertTrue(profile.getAdditionalProperties().isEmpty()); + } +} diff --git a/pom.xml b/pom.xml index 4a9b95f1e62..db1cb338d5b 100644 --- a/pom.xml +++ b/pom.xml @@ -36,7 +36,7 @@ 2.18.2 2.4 - 1.79 + 1.84 0.12.6 5.5.1 24.0.1 diff --git a/src/swagger/api.yaml b/src/swagger/api.yaml index 2cc1d957870..095199665fa 100644 --- a/src/swagger/api.yaml +++ b/src/swagger/api.yaml @@ -69180,6 +69180,7 @@ components: type: array readOnly: true nullable: true + items: {} LogSeverity: description: Indicates how severe the event is type: string From bc649b9d7be0376a32c3a9517a6a77bfeb5e0191 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Fri, 17 Jul 2026 15:20:52 +0530 Subject: [PATCH 3/5] 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 --- .circleci/config.yml | 8 +++- .../sdk/tests/it/OrgSettingGeneralIT.groovy | 47 +++++++++++-------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c4692185666..fd7178e8a95 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -110,7 +110,13 @@ workflows: "Circle CI Tests": jobs: - jdk11 - - jdk21 + - jdk21: + # jdk11 and jdk21 run the same integration test suite against the same shared + # live test org (no per-job isolation). Running them concurrently lets one job's + # writes to org-singleton resources (e.g. OrgSetting) clobber the other's, + # causing flaky read-after-write assertion failures. Serialize them. + requires: + - jdk11 - platform-helpers/job-semgrep-scan: name: "Scan with Semgrep" context: diff --git a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy index e3ae52e05f3..45255166c8b 100644 --- a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy +++ b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy @@ -63,6 +63,14 @@ class OrgSettingGeneralIT extends ITSupport { // Initialize API orgSettingGeneralApi = new OrgSettingGeneralApi(getClient()) + // OrgSetting is a singleton per org. IT jobs for different JDKs run this same test + // against the same shared live test org, so a fixed literal value here could be + // clobbered by (or read back from) a concurrently-running job. Use a per-run unique + // value so this run's writes/reads can never be confused with another run's. + def runId = UUID.randomUUID().toString() + def partialUpdateWebsite = "http://www.test-sdk-integration-${runId}.com" + def fullReplaceWebsite = "http://www.test-sdk-full-replace-${runId}.com" + // Track original values for cleanup def originalSettings = null @@ -103,13 +111,13 @@ class OrgSettingGeneralIT extends ITSupport { logger.debug("\n2. POST /api/v1/org (Partial update - modify website only)") def partialUpdate = new OrgSetting() - .website("http://www.test-sdk-integration.com") - + .website(partialUpdateWebsite) + def updatedSettings = orgSettingGeneralApi.updateOrgSettings(partialUpdate) - + assertThat "Updated settings should not be null", updatedSettings, notNullValue() - assertThat "Website should be updated", - updatedSettings.website, equalTo("http://www.test-sdk-integration.com") + assertThat "Website should be updated", + updatedSettings.website, equalTo(partialUpdateWebsite) assertThat "Company name should remain unchanged", updatedSettings.companyName, equalTo(originalSettings.companyName) assertThat "Org ID should remain unchanged", @@ -126,8 +134,8 @@ class OrgSettingGeneralIT extends ITSupport { def verifyPartialUpdate = orgSettingGeneralApi.getOrgSettings() - assertThat "Website should be persisted", - verifyPartialUpdate.website, equalTo("http://www.test-sdk-integration.com") + assertThat "Website should be persisted", + verifyPartialUpdate.website, equalTo(partialUpdateWebsite) assertThat "Other fields should be unchanged", verifyPartialUpdate.companyName, equalTo(originalSettings.companyName) @@ -138,18 +146,19 @@ class OrgSettingGeneralIT extends ITSupport { // ======================================== logger.debug("\n4. PUT /api/v1/org (Full replace - update multiple fields)") + def fullReplaceSupportUrl = "http://support.test-sdk-${runId}.com" def fullReplace = new OrgSetting() .companyName(originalSettings.companyName) - .website("http://www.test-sdk-full-replace.com") - .endUserSupportHelpURL("http://support.test-sdk.com") - + .website(fullReplaceWebsite) + .endUserSupportHelpURL(fullReplaceSupportUrl) + def replacedSettings = orgSettingGeneralApi.replaceOrgSettings(fullReplace) - + assertThat "Replaced settings should not be null", replacedSettings, notNullValue() - assertThat "Website should be updated", - replacedSettings.website, equalTo("http://www.test-sdk-full-replace.com") - assertThat "Support URL should be updated", - replacedSettings.endUserSupportHelpURL, equalTo("http://support.test-sdk.com") + assertThat "Website should be updated", + replacedSettings.website, equalTo(fullReplaceWebsite) + assertThat "Support URL should be updated", + replacedSettings.endUserSupportHelpURL, equalTo(fullReplaceSupportUrl) logger.debug(" Full replace successful:") logger.debug(" - Website: {}", replacedSettings.website) @@ -165,16 +174,16 @@ class OrgSettingGeneralIT extends ITSupport { // Poll until the replace is visible before asserting. def verifyFullReplace = orgSettingGeneralApi.getOrgSettings() int replaceRetries = 20 - for (int i = 0; i < replaceRetries && verifyFullReplace.website != "http://www.test-sdk-full-replace.com"; i++) { + for (int i = 0; i < replaceRetries && verifyFullReplace.website != fullReplaceWebsite; i++) { Thread.sleep(1000) verifyFullReplace = orgSettingGeneralApi.getOrgSettings() } assertThat "All fields should be persisted", verifyFullReplace, notNullValue() assertThat "Website should match", - verifyFullReplace.website, equalTo("http://www.test-sdk-full-replace.com") - assertThat "Support URL should match", - verifyFullReplace.endUserSupportHelpURL, equalTo("http://support.test-sdk.com") + verifyFullReplace.website, equalTo(fullReplaceWebsite) + assertThat "Support URL should match", + verifyFullReplace.endUserSupportHelpURL, equalTo(fullReplaceSupportUrl) logger.debug(" Verified full replace persisted correctly") From f05f4a35402b3301fa57b8d02d38dd6b6d5513d3 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Mon, 20 Jul 2026 10:48:20 +0530 Subject: [PATCH 4/5] 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 bc649b9d7be, 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 --- .circleci/config.yml | 8 +------- .../com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy | 9 ++++++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index fd7178e8a95..c4692185666 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -110,13 +110,7 @@ workflows: "Circle CI Tests": jobs: - jdk11 - - jdk21: - # jdk11 and jdk21 run the same integration test suite against the same shared - # live test org (no per-job isolation). Running them concurrently lets one job's - # writes to org-singleton resources (e.g. OrgSetting) clobber the other's, - # causing flaky read-after-write assertion failures. Serialize them. - requires: - - jdk11 + - jdk21 - platform-helpers/job-semgrep-scan: name: "Scan with Semgrep" context: diff --git a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy index 45255166c8b..c4b35b36231 100644 --- a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy +++ b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/OrgSettingGeneralIT.groovy @@ -67,9 +67,12 @@ class OrgSettingGeneralIT extends ITSupport { // against the same shared live test org, so a fixed literal value here could be // clobbered by (or read back from) a concurrently-running job. Use a per-run unique // value so this run's writes/reads can never be confused with another run's. + // .toString() is required: Groovy string interpolation produces a GString, and + // GString.equals(String) is always false even when the text matches, which would + // make every equalTo() assertion below fail despite identical-looking values. def runId = UUID.randomUUID().toString() - def partialUpdateWebsite = "http://www.test-sdk-integration-${runId}.com" - def fullReplaceWebsite = "http://www.test-sdk-full-replace-${runId}.com" + def partialUpdateWebsite = "http://www.test-sdk-integration-${runId}.com".toString() + def fullReplaceWebsite = "http://www.test-sdk-full-replace-${runId}.com".toString() // Track original values for cleanup def originalSettings = null @@ -146,7 +149,7 @@ class OrgSettingGeneralIT extends ITSupport { // ======================================== logger.debug("\n4. PUT /api/v1/org (Full replace - update multiple fields)") - def fullReplaceSupportUrl = "http://support.test-sdk-${runId}.com" + def fullReplaceSupportUrl = "http://support.test-sdk-${runId}.com".toString() def fullReplace = new OrgSetting() .companyName(originalSettings.companyName) .website(fullReplaceWebsite) From 494c65cde1bb5e0d6624998a1bdcbfb834ed2363 Mon Sep 17 00:00:00 2001 From: Prachi Pandey Date: Mon, 20 Jul 2026 12:35:50 +0530 Subject: [PATCH 5/5] 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 --- .../it/ApplicationSSOPublicKeysIT.groovy | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/ApplicationSSOPublicKeysIT.groovy b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/ApplicationSSOPublicKeysIT.groovy index 923956835c8..64eeb484b12 100644 --- a/integration-tests/src/test/groovy/com/okta/sdk/tests/it/ApplicationSSOPublicKeysIT.groovy +++ b/integration-tests/src/test/groovy/com/okta/sdk/tests/it/ApplicationSSOPublicKeysIT.groovy @@ -179,12 +179,35 @@ class ApplicationSSOPublicKeysIT extends ITSupport { app.credentials(credentials) app.signOnMode(ApplicationSignOnMode.OPENID_CONNECT) - OpenIdConnectApplication createdApp = applicationApi.createApplication(app, true, null) as OpenIdConnectApplication + OpenIdConnectApplication createdApp = createApplicationWithRetry(app) registerForCleanup(createdApp) return createdApp.getId() } + /** + * The SDK's built-in HTTP retry strategy (OktaHttpRequestRetryStrategy) only retries + * 429/503/504 - a bare 500 "Internal Server Error" from the backend during app creation + * is occasionally transient but isn't covered by that policy. Retry a few times here + * before giving up, since this call runs in @BeforeClass and a spurious 500 would fail + * the whole test class. + */ + private OpenIdConnectApplication createApplicationWithRetry(OpenIdConnectApplication app) { + int maxAttempts = 3 + for (int attempt = 1; attempt <= maxAttempts; attempt++) { + try { + return applicationApi.createApplication(app, true, null) as OpenIdConnectApplication + } catch (ApiException e) { + if (e.code != 500 || attempt == maxAttempts) { + throw e + } + logger.warn("createApplication returned 500 (attempt {}/{}), retrying...", attempt, maxAttempts) + Thread.sleep(2000L * attempt) + } + } + throw new IllegalStateException("unreachable") + } + /** * Generates an RSA key pair and returns the public key components for JWK creation. */