From cb85877a38ec775238d2a6a237570ecbb3c5e8b7 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 13:20:22 +0300 Subject: [PATCH 1/3] Fix note-level CodeQL findings across core, oauth2 and doc - OAuth2Session: fix the missing space in the "ID token could not be decoded" message, report a non-numeric expires_in as an OAuth2 server error instead of a raw NumberFormatException, and stop shadowing the accessTokenResponse field - ICAPFilter: name the offending field when connect_timeout or read_timeout is not a number - Principal: parse the websocket TTL system property once, falling back to the default with a warning when it is not a number - LdapClient: drop finalize(), which never runs on the singleton - Heaplets, JsonValues: replace the deprecated Class.newInstance() - OpenApiMockResponseHandler: replace the deprecated ApiResponses.getDefault() - MockDataGenerator: generate coordinates as numbers instead of parsing locale-formatted strings - SampleApplication: use SSLContextConfigurator.createSSLContext(true) and print the usage on a non-numeric port argument --- .../openig/handler/MockDataGenerator.java | 4 +- .../handler/OpenApiMockResponseHandler.java | 2 +- .../org/forgerock/openig/heap/Heaplets.java | 2 +- .../org/forgerock/openig/ldap/LdapClient.java | 10 +-- .../org/forgerock/openig/util/JsonValues.java | 7 ++- .../openig/filter/ICAPFilter.java | 33 +++++++++- .../openig/websocket/Principal.java | 22 ++++++- .../openig/filter/ICAPFilterTest.java | 63 +++++++++++++++++++ .../openig/websocket/PrincipalTest.java | 38 +++++++++++ .../openig/doc/SampleApplication.java | 21 ++++--- .../filter/oauth2/client/OAuth2Session.java | 16 +++-- .../oauth2/client/OAuth2SessionTest.java | 25 ++++++++ 12 files changed, 213 insertions(+), 30 deletions(-) create mode 100644 openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java create mode 100644 openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java diff --git a/openig-core/src/main/java/org/forgerock/openig/handler/MockDataGenerator.java b/openig-core/src/main/java/org/forgerock/openig/handler/MockDataGenerator.java index 46411dd50..8c675ccaf 100644 --- a/openig-core/src/main/java/org/forgerock/openig/handler/MockDataGenerator.java +++ b/openig-core/src/main/java/org/forgerock/openig/handler/MockDataGenerator.java @@ -292,9 +292,9 @@ private static Object generateByFieldName(final String fieldName, if (key.equals("port")) return coerce(FAKER.number().numberBetween(1024, 65535), type, schema); if (key.equals("latitude")) - return coerce(Double.parseDouble(FAKER.address().latitude().replace(",", ".")), type, schema); + return coerce(FAKER.number().randomDouble(6, -90, 90), type, schema); if (key.equals("longitude")) - return coerce(Double.parseDouble(FAKER.address().longitude().replace(",", ".")), type, schema); + return coerce(FAKER.number().randomDouble(6, -180, 180), type, schema); // --- Booleans --- if ("boolean".equals(type)) { diff --git a/openig-core/src/main/java/org/forgerock/openig/handler/OpenApiMockResponseHandler.java b/openig-core/src/main/java/org/forgerock/openig/handler/OpenApiMockResponseHandler.java index 89741d520..66a97dbff 100644 --- a/openig-core/src/main/java/org/forgerock/openig/handler/OpenApiMockResponseHandler.java +++ b/openig-core/src/main/java/org/forgerock/openig/handler/OpenApiMockResponseHandler.java @@ -266,7 +266,7 @@ static Schema bestResponseSchema(final Operation operation) { } // default - return schemaFromResponse(responses.getDefault()); + return schemaFromResponse(responses.get(ApiResponses.DEFAULT)); } @SuppressWarnings("rawtypes") diff --git a/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java b/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java index 0e5536b95..fb429e2b5 100644 --- a/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java +++ b/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java @@ -81,7 +81,7 @@ public static Heaplet getHeaplet(Class c) { // we're directly pointing to the Heaplet if (Heaplet.class.isAssignableFrom(c)) { try { - heaplet = c.asSubclass(Heaplet.class).newInstance(); + heaplet = c.asSubclass(Heaplet.class).getDeclaredConstructor().newInstance(); } catch (Exception e) { logger.warn("An error occurred while trying to instantiate {} as a Heaplet", c.getName(), e); // Ignored diff --git a/openig-core/src/main/java/org/forgerock/openig/ldap/LdapClient.java b/openig-core/src/main/java/org/forgerock/openig/ldap/LdapClient.java index e7aa49879..86069629b 100644 --- a/openig-core/src/main/java/org/forgerock/openig/ldap/LdapClient.java +++ b/openig-core/src/main/java/org/forgerock/openig/ldap/LdapClient.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.openig.ldap; @@ -192,13 +193,4 @@ private ConnectionFactory getConnectionFactory(final String host, final int port } return factory; } - - @Override - protected void finalize() throws Throwable { - for (ConnectionFactory factory : factories.values()) { - factory.close(); - } - factories.clear(); - super.finalize(); - } } diff --git a/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java b/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java index 711bd38c9..2806ff9bb 100644 --- a/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java +++ b/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; import java.net.URL; import java.util.List; import java.util.Map; @@ -152,8 +153,10 @@ public static T asNewInstance(JsonValue value, Class type) { throw new JsonValueException(value, "expecting " + type.getName()); } try { - return (T) c.newInstance(); - } catch (ExceptionInInitializerError | InstantiationException | IllegalAccessException e) { + return (T) c.getDeclaredConstructor().newInstance(); + } catch (InvocationTargetException e) { + throw new JsonValueException(value, e.getTargetException()); + } catch (ExceptionInInitializerError | ReflectiveOperationException e) { throw new JsonValueException(value, e); } } diff --git a/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java b/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java index 9fc74bbe4..69f8f858d 100644 --- a/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java +++ b/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java @@ -1,3 +1,19 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2022-2026 3A Systems, LLC. + */ + package org.openidentityplatform.openig.filter; import java.io.IOException; @@ -11,6 +27,8 @@ import org.forgerock.http.protocol.Request; import org.forgerock.http.protocol.Response; import org.forgerock.http.protocol.Status; +import org.forgerock.json.JsonValue; +import org.forgerock.json.JsonValueException; import org.forgerock.openig.heap.GenericHeaplet; import org.forgerock.openig.heap.HeapException; import org.forgerock.services.context.Context; @@ -56,8 +74,8 @@ public void start() throws HeapException { try { final URI uri=new URI(server); filter.icap=new ICAPClient(uri.getHost(), uri.getPort()>0?uri.getPort():1344); - filter.icap.setConnectTimeout(Integer.parseInt(config.get("connect_timeout").defaultTo("5000").as(expression(String.class)).eval())); - filter.icap.setReadTimeout(Integer.parseInt(config.get("read_timeout").defaultTo("15000").as(expression(String.class)).eval())); + filter.icap.setConnectTimeout(timeout("connect_timeout", "5000")); + filter.icap.setReadTimeout(timeout("read_timeout", "15000")); filter.service=config.get("service").defaultTo("").as(expression(String.class)).eval(); filter.rewrite=Boolean.parseBoolean(config.get("rewrite").defaultTo("true").as(expression(String.class)).eval()); logger.info("start {} connect_timeout={} read_timeout={}",uri,filter.icap.getConnectTimeout(),filter.icap.getReadTimeout()); @@ -66,6 +84,17 @@ public void start() throws HeapException { } } + /** Reads a timeout (in milliseconds) from the config, naming the field when the value is not a number. */ + private int timeout(String name, String defaultValue) { + final JsonValue value=config.get(name).defaultTo(defaultValue); + final String evaluated=value.as(expression(String.class)).eval(); + try { + return Integer.parseInt(evaluated); + } catch (NumberFormatException e) { + throw new JsonValueException(value, "Expecting a number of milliseconds, got: "+evaluated, e); + } + } + @Override public void destroy() { super.destroy(); diff --git a/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java b/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java index c365de104..97ba8c8aa 100644 --- a/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java +++ b/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java @@ -22,8 +22,11 @@ import org.forgerock.http.protocol.Status; import org.forgerock.http.servlet.HttpFrameworkServlet; import org.forgerock.services.context.Context; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class Principal implements java.security.Principal { + private static final Logger logger=LoggerFactory.getLogger(Principal.class); final Context context; @@ -42,8 +45,25 @@ public String getName() { long lastAuthorize=0; Status res=null; + /** System property holding the re-authorization interval, in seconds. */ + static final String TTL_PROPERTY="org.openidentityplatform.openig.websocket.ttl"; + static final long DEFAULT_TTL_SECONDS=180; + private static final long TTL_MILLIS=ttlMillis(System.getProperty(TTL_PROPERTY)); + + /** Parses the re-authorization interval, falling back to the default when the value is not a number. */ + static long ttlMillis(String seconds) { + if (seconds!=null) { + try { + return Long.parseLong(seconds.trim())*1000; + } catch (NumberFormatException e) { + logger.warn("Ignoring invalid value \"{}\" of system property {}, using {} seconds", seconds, TTL_PROPERTY, DEFAULT_TTL_SECONDS); + } + } + return DEFAULT_TTL_SECONDS*1000; + } + public Status authorize() throws InterruptedException, ExecutionException { - final long ttl=Long.parseLong(System.getProperty("org.openidentityplatform.openig.websocket.ttl", "180"))*1000; + final long ttl=TTL_MILLIS; if (res==null || System.currentTimeMillis()-lastAuthorize>=ttl) { synchronized (this) { if (res==null || (System.currentTimeMillis()-lastAuthorize)>=ttl) { diff --git a/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java b/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java new file mode 100644 index 000000000..8a3da7ad3 --- /dev/null +++ b/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java @@ -0,0 +1,63 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.openidentityplatform.openig.filter; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.forgerock.json.JsonValue.field; +import static org.forgerock.json.JsonValue.json; +import static org.forgerock.json.JsonValue.object; + +import org.forgerock.json.JsonValueException; +import org.forgerock.openig.heap.HeapUtilsTest; +import org.forgerock.openig.heap.Name; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +@SuppressWarnings("javadoc") +public class ICAPFilterTest { + + @Test + public void shouldConfigureTimeouts() throws Exception { + ICAPFilter filter = (ICAPFilter) new ICAPFilter.Heaplet().create( + Name.of("icap"), + json(object(field("server", "icap://localhost:1344"), + field("connect_timeout", "1000"), + field("read_timeout", "2000"))), + HeapUtilsTest.buildDefaultHeap()); + + assertThat(filter.icap.getConnectTimeout()).isEqualTo(1000); + assertThat(filter.icap.getReadTimeout()).isEqualTo(2000); + } + + @DataProvider + public static Object[][] timeoutFields() { + return new Object[][] { { "connect_timeout" }, { "read_timeout" } }; + } + + @Test(dataProvider = "timeoutFields") + public void shouldRejectNonNumericTimeoutNamingTheField(String name) { + assertThatThrownBy(() -> new ICAPFilter.Heaplet().create( + Name.of("icap"), + json(object(field("server", "icap://localhost:1344"), + field(name, "soon"))), + HeapUtilsTest.buildDefaultHeap())) + .isInstanceOf(JsonValueException.class) + .hasMessageContaining(name) + .hasMessageContaining("soon"); + } +} diff --git a/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java new file mode 100644 index 000000000..14f25440c --- /dev/null +++ b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java @@ -0,0 +1,38 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.openidentityplatform.openig.websocket; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.Test; + +@SuppressWarnings("javadoc") +public class PrincipalTest { + + @Test + public void shouldUseConfiguredTtlInSeconds() { + assertThat(Principal.ttlMillis("5")).isEqualTo(5_000L); + assertThat(Principal.ttlMillis(" 42 ")).isEqualTo(42_000L); + } + + @Test + public void shouldFallBackToDefaultTtlWhenPropertyIsMissingOrInvalid() { + assertThat(Principal.ttlMillis(null)).isEqualTo(Principal.DEFAULT_TTL_SECONDS * 1000); + assertThat(Principal.ttlMillis("soon")).isEqualTo(Principal.DEFAULT_TTL_SECONDS * 1000); + assertThat(Principal.ttlMillis("")).isEqualTo(Principal.DEFAULT_TTL_SECONDS * 1000); + } +} diff --git a/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java b/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java index 978f40cbb..b0b3e716a 100644 --- a/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java +++ b/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.openig.doc; @@ -84,9 +85,14 @@ public static void main(String[] args) { } if (args.length > 0) { - port = Integer.parseInt(args[0]); - if (args.length >= 2) { - sslPort = Integer.parseInt(args[1]); + try { + port = Integer.parseInt(args[0]); + if (args.length >= 2) { + sslPort = Integer.parseInt(args[1]); + } + } catch (NumberFormatException e) { + System.out.println("Port numbers must be integers: " + e.getMessage() + "\n" + usage); + System.exit(-1); } if (args.length == 3) { openamUrl = args[2]; @@ -238,16 +244,17 @@ private static SSLEngineConfigurator createSslConfiguration() { return null; } - if (sslContextConfigurator.validateConfiguration(true)) { + try { SSLEngineConfigurator sslEngineConfigurator = - new SSLEngineConfigurator(sslContextConfigurator.createSSLContext()); + new SSLEngineConfigurator(sslContextConfigurator.createSSLContext(true)); sslEngineConfigurator.setClientMode(false); sslEngineConfigurator.setNeedClientAuth(false); sslEngineConfigurator.setWantClientAuth(false); return sslEngineConfigurator; + } catch (SSLContextConfigurator.GenericStoreException e) { + LOGGER.log(Level.INFO, "Failed to build a valid HTTPS configuration.", e); + return null; } - LOGGER.info("Failed to build a valid HTTPS configuration."); - return null; } /** diff --git a/openig-oauth2/src/main/java/org/forgerock/openig/filter/oauth2/client/OAuth2Session.java b/openig-oauth2/src/main/java/org/forgerock/openig/filter/oauth2/client/OAuth2Session.java index e536e63fb..a683aa56d 100644 --- a/openig-oauth2/src/main/java/org/forgerock/openig/filter/oauth2/client/OAuth2Session.java +++ b/openig-oauth2/src/main/java/org/forgerock/openig/filter/oauth2/client/OAuth2Session.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.openig.filter.oauth2.client; @@ -55,7 +56,7 @@ private static SignedJwt extractIdToken(final JsonValue accessTokenResponse) return JWT_DECODER.reconstructJwt(idToken, SignedJwt.class); } catch (final JwtReconstructionException e) { throw new OAuth2ErrorException(E_SERVER_ERROR, - "Authorization call-back failed because the OpenID Connect ID token" + "Authorization call-back failed because the OpenID Connect ID token " + "could not be decoded", e); } } @@ -183,7 +184,7 @@ OAuth2Session stateAuthorized(final JsonValue newAccessTokenResponse) final Map mergedAccessTokenResponse = new LinkedHashMap<>(accessTokenResponse.asMap()); mergedAccessTokenResponse.putAll(newAccessTokenResponse.asMap()); - JsonValue accessTokenResponse = + final JsonValue mergedResponse = new JsonValue(Collections.unmodifiableMap(mergedAccessTokenResponse)); // Compute effective scopes. @@ -201,7 +202,12 @@ OAuth2Session stateAuthorized(final JsonValue newAccessTokenResponse) if (expires.isNotNull()) { long expiresIn; if (expires.isString()) { - expiresIn = Long.valueOf(expires.asString()); + try { + expiresIn = Long.parseLong(expires.asString()); + } catch (NumberFormatException e) { + throw new OAuth2ErrorException(OAuth2Error.E_SERVER_ERROR, + "'expires_in' field value is not a number: " + expires.asString(), e); + } } else if (expires.isNumber()) { expiresIn = expires.asNumber().longValue(); } else { @@ -211,10 +217,10 @@ OAuth2Session stateAuthorized(final JsonValue newAccessTokenResponse) expiresAt = expiresIn + now(); } // Decode the ID token for OpenID Connect interactions. - final SignedJwt idToken = extractIdToken(accessTokenResponse); + final SignedJwt idToken = extractIdToken(mergedResponse); return new OAuth2Session(time, clientRegistrationName, clientEndpoint, null, actualScopes, - accessTokenResponse, idToken, expiresAt); + mergedResponse, idToken, expiresAt); } OAuth2Session stateAuthorizing(final String clientRegistrationName, diff --git a/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java b/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java index c186ad2fe..45a667ecc 100644 --- a/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java +++ b/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java @@ -12,11 +12,13 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.openig.filter.oauth2.client; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.data.MapEntry.entry; import static org.forgerock.json.JsonValue.field; import static org.forgerock.json.JsonValue.json; @@ -177,6 +179,29 @@ public void shouldStateAuthorizedWithExpiresInExpressedAsStringSucceed() throws assertThat(session.getExpiresIn()).isEqualTo(3599L); } + @Test + public void shouldStateAuthorizedWithNonNumericExpiresInFailWithOAuth2Error() { + JsonValue accessTokenResponse = json(object(field("access_token", "at"), + field("token_type", "tt"), + field("expires_in", "soon"), + field("id_token", idToken) + )); + assertThatThrownBy(() -> OAuth2Session.stateNew(time).stateAuthorized(accessTokenResponse)) + .isInstanceOf(OAuth2ErrorException.class) + .hasMessageContaining("expires_in"); + } + + @Test + public void shouldStateAuthorizedWithUndecodableIdTokenFailWithReadableMessage() { + JsonValue accessTokenResponse = json(object(field("access_token", "at"), + field("token_type", "tt"), + field("id_token", "not-a-jwt") + )); + assertThatThrownBy(() -> OAuth2Session.stateNew(time).stateAuthorized(accessTokenResponse)) + .isInstanceOf(OAuth2ErrorException.class) + .hasMessageContaining("ID token could not be decoded"); + } + @Test public void shouldStateAuthorizedWithNullExpiresInSucceed() throws Exception { From 88e07ac38e6217cd209a578c4c529aa1197a3c18 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 22 Oct 2026 15:08:01 +0300 Subject: [PATCH 2/3] Address review of the CodeQL note fixes - Principal: check the re-authorization interval through expired(now) and pin in a test that the TTL is read once at class load - JsonValues.asNewInstance, Heaplets.getHeaplet: let an Error thrown by the constructor propagate, as Class.newInstance() did, instead of wrapping it or logging it away - Tests: assert the server_error code for a non-numeric expires_in, the ICAP default timeouts, the coordinate ranges in MockDataGenerator and the default-response fallback of bestResponseSchema --- .../org/forgerock/openig/heap/Heaplets.java | 18 +++++++- .../org/forgerock/openig/util/JsonValues.java | 6 ++- .../openig/websocket/Principal.java | 12 ++++-- .../openig/handler/MockDataGeneratorTest.java | 14 +++++++ .../OpenApiMockResponseHandlerTest.java | 8 ++++ .../forgerock/openig/heap/HeapletsTest.java | 41 +++++++++++++++++++ .../forgerock/openig/util/JsonValuesTest.java | 27 ++++++++++++ .../openig/filter/ICAPFilterTest.java | 11 +++++ .../openig/websocket/PrincipalTest.java | 20 +++++++++ .../oauth2/client/OAuth2SessionTest.java | 4 +- 10 files changed, 154 insertions(+), 7 deletions(-) diff --git a/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java b/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java index fb429e2b5..9ffd9909a 100644 --- a/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java +++ b/openig-core/src/main/java/org/forgerock/openig/heap/Heaplets.java @@ -20,6 +20,7 @@ import static java.util.Collections.*; +import java.lang.reflect.InvocationTargetException; import java.util.List; import org.forgerock.http.util.Loader; @@ -81,7 +82,7 @@ public static Heaplet getHeaplet(Class c) { // we're directly pointing to the Heaplet if (Heaplet.class.isAssignableFrom(c)) { try { - heaplet = c.asSubclass(Heaplet.class).getDeclaredConstructor().newInstance(); + heaplet = instantiate(c.asSubclass(Heaplet.class)); } catch (Exception e) { logger.warn("An error occurred while trying to instantiate {} as a Heaplet", c.getName(), e); // Ignored @@ -89,4 +90,19 @@ public static Heaplet getHeaplet(Class c) { } return heaplet; } + + /** + * Invokes the no-arg constructor, letting an {@link Error} thrown by it propagate as + * {@code Class.newInstance()} did. + */ + private static Heaplet instantiate(final Class c) throws ReflectiveOperationException { + try { + return c.getDeclaredConstructor().newInstance(); + } catch (InvocationTargetException e) { + if (e.getTargetException() instanceof Error) { + throw (Error) e.getTargetException(); + } + throw e; + } + } } diff --git a/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java b/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java index 2806ff9bb..c49d11505 100644 --- a/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java +++ b/openig-core/src/main/java/org/forgerock/openig/util/JsonValues.java @@ -155,7 +155,11 @@ public static T asNewInstance(JsonValue value, Class type) { try { return (T) c.getDeclaredConstructor().newInstance(); } catch (InvocationTargetException e) { - throw new JsonValueException(value, e.getTargetException()); + final Throwable target = e.getTargetException(); + if (target instanceof Error) { + throw (Error) target; + } + throw new JsonValueException(value, target); } catch (ExceptionInInitializerError | ReflectiveOperationException e) { throw new JsonValueException(value, e); } diff --git a/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java b/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java index 97ba8c8aa..90cc75fa6 100644 --- a/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java +++ b/openig-core/src/main/java/org/openidentityplatform/openig/websocket/Principal.java @@ -48,7 +48,7 @@ public String getName() { /** System property holding the re-authorization interval, in seconds. */ static final String TTL_PROPERTY="org.openidentityplatform.openig.websocket.ttl"; static final long DEFAULT_TTL_SECONDS=180; - private static final long TTL_MILLIS=ttlMillis(System.getProperty(TTL_PROPERTY)); + static final long TTL_MILLIS=ttlMillis(System.getProperty(TTL_PROPERTY)); /** Parses the re-authorization interval, falling back to the default when the value is not a number. */ static long ttlMillis(String seconds) { @@ -62,11 +62,15 @@ static long ttlMillis(String seconds) { return DEFAULT_TTL_SECONDS*1000; } + /** Tells whether the cached authorization result has to be refreshed at the given time. */ + boolean expired(long now) { + return res==null || now-lastAuthorize>=TTL_MILLIS; + } + public Status authorize() throws InterruptedException, ExecutionException { - final long ttl=TTL_MILLIS; - if (res==null || System.currentTimeMillis()-lastAuthorize>=ttl) { + if (expired(System.currentTimeMillis())) { synchronized (this) { - if (res==null || (System.currentTimeMillis()-lastAuthorize)>=ttl) { + if (expired(System.currentTimeMillis())) { res=HttpFrameworkServlet.getGatewayHttpFrameworkServlet().handler.handle(context, request).get().getStatus(); lastAuthorize=System.currentTimeMillis(); } diff --git a/openig-core/src/test/java/org/forgerock/openig/handler/MockDataGeneratorTest.java b/openig-core/src/test/java/org/forgerock/openig/handler/MockDataGeneratorTest.java index 6baf6a3d1..435902866 100644 --- a/openig-core/src/test/java/org/forgerock/openig/handler/MockDataGeneratorTest.java +++ b/openig-core/src/test/java/org/forgerock/openig/handler/MockDataGeneratorTest.java @@ -188,6 +188,20 @@ public void generate_normalisesFieldNameForLookup() { assertThat(MockDataGenerator.generate("FIRSTNAME", schema)).isInstanceOf(String.class); } + @Test + public void generate_returnsCoordinatesInRange() { + final Schema schema = new Schema<>(); + schema.setType("number"); + for (int i = 0; i < 100; i++) { + final Object latitude = MockDataGenerator.generate("latitude", schema); + final Object longitude = MockDataGenerator.generate("longitude", schema); + assertThat(latitude).isInstanceOf(Number.class); + assertThat(longitude).isInstanceOf(Number.class); + assertThat(((Number) latitude).doubleValue()).isBetween(-90.0, 90.0); + assertThat(((Number) longitude).doubleValue()).isBetween(-180.0, 180.0); + } + } + // ----------------------------------------------------------------------- // Numeric generation with constraints // ----------------------------------------------------------------------- diff --git a/openig-core/src/test/java/org/forgerock/openig/handler/OpenApiMockResponseHandlerTest.java b/openig-core/src/test/java/org/forgerock/openig/handler/OpenApiMockResponseHandlerTest.java index 66a637ff8..66af6f916 100644 --- a/openig-core/src/test/java/org/forgerock/openig/handler/OpenApiMockResponseHandlerTest.java +++ b/openig-core/src/test/java/org/forgerock/openig/handler/OpenApiMockResponseHandlerTest.java @@ -270,6 +270,14 @@ public void bestResponseSchema_falls_to201_when200IsAbsent() { assertThat(schema.getDescription()).isEqualTo("schema-201"); } + @Test + public void bestResponseSchema_fallsBackToDefaultResponse() { + final Operation op = buildOperation("default"); + final Schema schema = OpenApiMockResponseHandler.bestResponseSchema(op); + assertThat(schema).isNotNull(); + assertThat(schema.getDescription()).isEqualTo("schema-default"); + } + @Test public void bestResponseSchema_returnsNull_whenNoResponseSchema() { final Operation op = new Operation(); diff --git a/openig-core/src/test/java/org/forgerock/openig/heap/HeapletsTest.java b/openig-core/src/test/java/org/forgerock/openig/heap/HeapletsTest.java index 99bf4e4b1..4c39d0500 100644 --- a/openig-core/src/test/java/org/forgerock/openig/heap/HeapletsTest.java +++ b/openig-core/src/test/java/org/forgerock/openig/heap/HeapletsTest.java @@ -23,6 +23,7 @@ import static org.forgerock.json.JsonValue.object; import static org.forgerock.openig.heap.Keys.TEMPORARY_STORAGE_HEAP_KEY; +import org.forgerock.json.JsonValue; import org.forgerock.openig.heap.domain.Author; import org.forgerock.openig.heap.domain.AuthorHeaplet; import org.forgerock.openig.heap.domain.Book; @@ -141,4 +142,44 @@ public Object create() throws HeapException { return null; } } + + @Test + public void shouldIgnoreGivenHeapletClassWhoseConstructorThrowsAnException() throws Exception { + assertThat(Heaplets.getHeaplet(ThrowingHeaplet.class)).isNull(); + } + + @Test(expectedExceptions = NoClassDefFoundError.class) + public void shouldPropagateErrorThrownByGivenHeapletClassConstructor() throws Exception { + Heaplets.getHeaplet(FailingHeaplet.class); + } + + public static class ThrowingHeaplet implements Heaplet { + public ThrowingHeaplet() { + throw new IllegalStateException("boom"); + } + + @Override + public Object create(Name name, JsonValue config, Heap heap) { + return null; + } + + @Override + public void destroy() { + } + } + + public static class FailingHeaplet implements Heaplet { + public FailingHeaplet() { + throw new NoClassDefFoundError("boom"); + } + + @Override + public Object create(Name name, JsonValue config, Heap heap) { + return null; + } + + @Override + public void destroy() { + } + } } diff --git a/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java b/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java index d5793f0f9..06b268230 100644 --- a/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java +++ b/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java @@ -18,6 +18,7 @@ package org.forgerock.openig.util; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.forgerock.json.JsonValue.array; import static org.forgerock.json.JsonValue.field; import static org.forgerock.json.JsonValue.json; @@ -25,6 +26,7 @@ import static org.forgerock.json.JsonValueFunctions.listOf; import static org.forgerock.openig.el.Bindings.bindings; import static org.forgerock.openig.util.JsonValues.asFunction; +import static org.forgerock.openig.util.JsonValues.asNewInstance; import static org.forgerock.openig.util.JsonValues.evaluated; import static org.forgerock.openig.util.JsonValues.firstOf; import static org.forgerock.openig.util.JsonValues.getWithDeprecation; @@ -41,6 +43,7 @@ import org.forgerock.json.JsonException; import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; +import org.forgerock.json.JsonValueException; import org.forgerock.openig.el.Bindings; import org.forgerock.openig.el.ExpressionException; import org.forgerock.openig.heap.Heap; @@ -320,4 +323,28 @@ public static Object[][] asFunctionProvider() { }; //@Checkstyle:on } + + @Test + public void shouldAsNewInstanceWrapExceptionThrownByConstructor() throws Exception { + assertThatThrownBy(() -> asNewInstance(json(ThrowingConstructor.class.getName()), Object.class)) + .isInstanceOf(JsonValueException.class) + .hasCauseInstanceOf(IllegalStateException.class); + } + + @Test(expectedExceptions = NoClassDefFoundError.class) + public void shouldAsNewInstancePropagateErrorThrownByConstructor() throws Exception { + asNewInstance(json(FailingConstructor.class.getName()), Object.class); + } + + public static class ThrowingConstructor { + public ThrowingConstructor() { + throw new IllegalStateException("boom"); + } + } + + public static class FailingConstructor { + public FailingConstructor() { + throw new NoClassDefFoundError("boom"); + } + } } diff --git a/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java b/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java index 8a3da7ad3..edb98ed23 100644 --- a/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java +++ b/openig-core/src/test/java/org/openidentityplatform/openig/filter/ICAPFilterTest.java @@ -44,6 +44,17 @@ public void shouldConfigureTimeouts() throws Exception { assertThat(filter.icap.getReadTimeout()).isEqualTo(2000); } + @Test + public void shouldApplyDefaultTimeouts() throws Exception { + ICAPFilter filter = (ICAPFilter) new ICAPFilter.Heaplet().create( + Name.of("icap"), + json(object(field("server", "icap://localhost:1344"))), + HeapUtilsTest.buildDefaultHeap()); + + assertThat(filter.icap.getConnectTimeout()).isEqualTo(5000); + assertThat(filter.icap.getReadTimeout()).isEqualTo(15000); + } + @DataProvider public static Object[][] timeoutFields() { return new Object[][] { { "connect_timeout" }, { "read_timeout" } }; diff --git a/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java index 14f25440c..2e6bb29b9 100644 --- a/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java +++ b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import org.forgerock.http.protocol.Status; import org.testng.annotations.Test; @SuppressWarnings("javadoc") @@ -35,4 +36,23 @@ public void shouldFallBackToDefaultTtlWhenPropertyIsMissingOrInvalid() { assertThat(Principal.ttlMillis("soon")).isEqualTo(Principal.DEFAULT_TTL_SECONDS * 1000); assertThat(Principal.ttlMillis("")).isEqualTo(Principal.DEFAULT_TTL_SECONDS * 1000); } + + @Test + public void shouldReauthorizeWithTtlReadAtClassLoad() { + final Principal principal = new Principal(null, null); + principal.res = Status.OK; + principal.lastAuthorize = 1_000L; + // Changing the property after class load must not affect the interval + final String previous = System.setProperty(Principal.TTL_PROPERTY, "0"); + try { + assertThat(principal.expired(1_000L + Principal.TTL_MILLIS - 1)).isFalse(); + assertThat(principal.expired(1_000L + Principal.TTL_MILLIS)).isTrue(); + } finally { + if (previous == null) { + System.clearProperty(Principal.TTL_PROPERTY); + } else { + System.setProperty(Principal.TTL_PROPERTY, previous); + } + } + } } diff --git a/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java b/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java index 45a667ecc..4a4579726 100644 --- a/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java +++ b/openig-oauth2/src/test/java/org/forgerock/openig/filter/oauth2/client/OAuth2SessionTest.java @@ -25,6 +25,7 @@ import static org.forgerock.json.JsonValue.object; import static org.mockito.Mockito.when; +import org.forgerock.http.oauth2.OAuth2Error; import org.forgerock.json.JsonValue; import org.forgerock.util.time.TimeService; import org.mockito.Mock; @@ -187,7 +188,8 @@ public void shouldStateAuthorizedWithNonNumericExpiresInFailWithOAuth2Error() { field("id_token", idToken) )); assertThatThrownBy(() -> OAuth2Session.stateNew(time).stateAuthorized(accessTokenResponse)) - .isInstanceOf(OAuth2ErrorException.class) + .isInstanceOfSatisfying(OAuth2ErrorException.class, e -> + assertThat(e.getOAuth2Error().getError()).isEqualTo(OAuth2Error.E_SERVER_ERROR)) .hasMessageContaining("expires_in"); } From d5133bac61ff500ca99198956824db616ae0b566 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 22 Oct 2026 20:42:20 +0300 Subject: [PATCH 3/3] Address the second review of the CodeQL note fixes - PrincipalTest: pin that authorize() serves the cached result with the TTL read at class load, and that a missing result is always refreshed - SampleApplication: build the SSL context without the javax.net.ssl.* system properties, so a missing key store provider cannot leave the HTTPS listener without a key - JsonValuesTest: pin that an ExceptionInInitializerError raised by the constructor propagates from asNewInstance - ICAPFilter: say that the timeout must fit in an int --- .../openig/filter/ICAPFilter.java | 4 ++-- .../forgerock/openig/util/JsonValuesTest.java | 23 +++++++++++++++++++ .../openig/websocket/PrincipalTest.java | 23 +++++++++++++++++++ .../openig/doc/SampleApplication.java | 4 +++- 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java b/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java index 69f8f858d..3b97f31f7 100644 --- a/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java +++ b/openig-core/src/main/java/org/openidentityplatform/openig/filter/ICAPFilter.java @@ -84,14 +84,14 @@ public void start() throws HeapException { } } - /** Reads a timeout (in milliseconds) from the config, naming the field when the value is not a number. */ + /** Reads a timeout (in milliseconds) from the config, naming the field when the value is not an int. */ private int timeout(String name, String defaultValue) { final JsonValue value=config.get(name).defaultTo(defaultValue); final String evaluated=value.as(expression(String.class)).eval(); try { return Integer.parseInt(evaluated); } catch (NumberFormatException e) { - throw new JsonValueException(value, "Expecting a number of milliseconds, got: "+evaluated, e); + throw new JsonValueException(value, "Expecting a number of milliseconds that fits in an int, got: "+evaluated, e); } } diff --git a/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java b/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java index 06b268230..58d201639 100644 --- a/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java +++ b/openig-core/src/test/java/org/forgerock/openig/util/JsonValuesTest.java @@ -336,6 +336,11 @@ public void shouldAsNewInstancePropagateErrorThrownByConstructor() throws Except asNewInstance(json(FailingConstructor.class.getName()), Object.class); } + @Test(expectedExceptions = ExceptionInInitializerError.class) + public void shouldAsNewInstancePropagateInitializerErrorRaisedByConstructor() throws Exception { + asNewInstance(json(ConstructorTouchingBrokenClass.class.getName()), Object.class); + } + public static class ThrowingConstructor { public ThrowingConstructor() { throw new IllegalStateException("boom"); @@ -347,4 +352,22 @@ public FailingConstructor() { throw new NoClassDefFoundError("boom"); } } + + public static class ConstructorTouchingBrokenClass { + public ConstructorTouchingBrokenClass() { + BrokenStaticInitializer.touch(); + } + } + + /** + * Only the first access raises {@link ExceptionInInitializerError}, later ones get a + * {@link NoClassDefFoundError}: keep this class private to the test above. + */ + static class BrokenStaticInitializer { + static final int VALUE = Integer.parseInt("boom"); + + static int touch() { + return VALUE; + } + } } diff --git a/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java index 2e6bb29b9..abb3581e4 100644 --- a/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java +++ b/openig-core/src/test/java/org/openidentityplatform/openig/websocket/PrincipalTest.java @@ -55,4 +55,27 @@ public void shouldReauthorizeWithTtlReadAtClassLoad() { } } } + + @Test + public void shouldAuthorizeFromCacheWithTtlReadAtClassLoad() throws Exception { + final Principal principal = new Principal(null, null); + principal.res = Status.OK; + principal.lastAuthorize = System.currentTimeMillis(); + // A per-call read of "0" would re-authorize through the gateway servlet instead of the cache + final String previous = System.setProperty(Principal.TTL_PROPERTY, "0"); + try { + assertThat(principal.authorize()).isSameAs(Status.OK); + } finally { + if (previous == null) { + System.clearProperty(Principal.TTL_PROPERTY); + } else { + System.setProperty(Principal.TTL_PROPERTY, previous); + } + } + } + + @Test + public void shouldExpireWithoutCachedResult() { + assertThat(new Principal(null, null).expired(1_000L)).isTrue(); + } } diff --git a/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java b/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java index b0b3e716a..13ef810f5 100644 --- a/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java +++ b/openig-doc/src/main/java/org/forgerock/openig/doc/SampleApplication.java @@ -232,7 +232,9 @@ static void stop(final HttpServer httpServer) { * or null if no valid SSL configuration could be set up. */ private static SSLEngineConfigurator createSslConfiguration() { - SSLContextConfigurator sslContextConfigurator = new SSLContextConfigurator(); + // Ignore the javax.net.ssl.* system properties: the sample brings its own key store, + // and a missing key store provider would leave the HTTPS listener without a key. + SSLContextConfigurator sslContextConfigurator = new SSLContextConfigurator(false); try { // Cannot read the key store as a file inside a jar, // so get the key store as a byte array.