Skip to content

Commit 48af9bd

Browse files
test(security): add positive-path test for insecure TLS double-gate
Code review on PR #157 caught that the existing tests only assert the SAFE/DEFAULT path and the env-var helper's UNSET behaviour. They never exercise the positive path where BOTH gates are present and the insecure TrustManager actually activates — the one combination that ships a trust-all SSLContext and permissive HostnameVerifier. This commit adds four tests to close the gap: - envVarHelperShouldBeTrueWhenSetToTrue / ...WhenSetToOne Confirm isInsecureTlsEnvVarEnabled() honours both accepted values. - shouldActivateInsecurePathWhenBothGatesArePresent The positive-path regression. Builds an OkHttpClient with insecureSkipVerify(true) AND AXONFLOW_INSECURE_TLS=true, then asserts: 1. hostnameVerifier class name does NOT contain "OkHostnameVerifier" (i.e. the synthetic permissive lambda is installed), 2. that verifier returns true for an arbitrary hostname, 3. sslSocketFactory() is NOT the same instance as the one a default-path client produces (trust-all SSLContext is wired in). - shouldKeepDefaultPathWhenOnlyEnvVarIsSet Complementary negative path — env var alone, without the builder flag, must keep OkHttp's default OkHostnameVerifier. Together with the existing "neither flag set" test, this proves the gate is symmetric: both must be present to flip behaviour. Env-var injection is done with junit-pioneer 2.2.0's @SetEnvironmentVariable, which scopes the variable to a single test method via reflective access into java.base/java.util. The required JDK 17+ --add-opens flags are added to the surefire argLine alongside the existing -Dnet.bytebuddy.experimental=true. Full suite: 1228/1228 green.
1 parent b65ef0e commit 48af9bd

2 files changed

Lines changed: 88 additions & 1 deletion

File tree

pom.xml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757

5858
<!-- Test Dependency Versions -->
5959
<junit.version>5.10.2</junit.version>
60+
<junit-pioneer.version>2.2.0</junit-pioneer.version>
6061
<mockito.version>5.11.0</mockito.version>
6162
<wiremock.version>3.4.2</wiremock.version>
6263
<assertj.version>3.27.7</assertj.version>
@@ -128,6 +129,13 @@
128129
<version>${junit.version}</version>
129130
<scope>test</scope>
130131
</dependency>
132+
<!-- junit-pioneer: provides @SetEnvironmentVariable for in-process env-var injection -->
133+
<dependency>
134+
<groupId>org.junit-pioneer</groupId>
135+
<artifactId>junit-pioneer</artifactId>
136+
<version>${junit-pioneer.version}</version>
137+
<scope>test</scope>
138+
</dependency>
131139
<dependency>
132140
<groupId>org.mockito</groupId>
133141
<artifactId>mockito-core</artifactId>
@@ -183,7 +191,8 @@
183191
<artifactId>maven-surefire-plugin</artifactId>
184192
<version>${maven-surefire-plugin.version}</version>
185193
<configuration>
186-
<argLine>@{argLine} -Dnet.bytebuddy.experimental=true</argLine>
194+
<!-- add-opens flags below are required by junit-pioneer's @SetEnvironmentVariable on JDK 17+ -->
195+
<argLine>@{argLine} -Dnet.bytebuddy.experimental=true --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED</argLine>
187196
<skipTests>${skipUnitTests}</skipTests>
188197
<includes>
189198
<include>**/*Test.java</include>

src/test/java/com/getaxonflow/sdk/util/HttpClientFactoryTest.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import okhttp3.OkHttpClient;
2323
import org.junit.jupiter.api.DisplayName;
2424
import org.junit.jupiter.api.Test;
25+
import org.junitpioneer.jupiter.SetEnvironmentVariable;
2526

2627
@DisplayName("HttpClientFactory")
2728
class HttpClientFactoryTest {
@@ -100,4 +101,81 @@ void envVarHelperShouldBeFalseWhenUnset() {
100101
+ "has the env var set and the insecure path could activate")
101102
.isFalse();
102103
}
104+
105+
@Test
106+
@DisplayName("env-var gate helper should be true when AXONFLOW_INSECURE_TLS=true")
107+
@SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
108+
void envVarHelperShouldBeTrueWhenSetToTrue() {
109+
assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
110+
.as("helper must return true when env var is set to 'true'")
111+
.isTrue();
112+
}
113+
114+
@Test
115+
@DisplayName("env-var gate helper should be true when AXONFLOW_INSECURE_TLS=1")
116+
@SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "1")
117+
void envVarHelperShouldBeTrueWhenSetToOne() {
118+
assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
119+
.as("helper must return true when env var is set to '1'")
120+
.isTrue();
121+
}
122+
123+
@Test
124+
@DisplayName(
125+
"should activate insecure TLS path when BOTH insecureSkipVerify(true) AND env var are set")
126+
@SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
127+
void shouldActivateInsecurePathWhenBothGatesArePresent() {
128+
// Positive-path regression: this is the single combination where the trust-all path
129+
// is intentionally activated. Both gates MUST be required; either alone is insufficient
130+
// (covered by sibling tests). If either gate stops being required, this test will still
131+
// pass — the gating behaviour is asserted in the negative-path tests.
132+
AxonFlowConfig config =
133+
AxonFlowConfig.builder().agentUrl("https://localhost:8080").insecureSkipVerify(true).build();
134+
135+
OkHttpClient client = HttpClientFactory.create(config);
136+
137+
assertThat(client).isNotNull();
138+
139+
// 1. Hostname verifier MUST be the permissive lambda, NOT OkHttp's default OkHostnameVerifier.
140+
// The permissive verifier in HttpClientFactory is `(hostname, session) -> true`, which is
141+
// a synthetic lambda class — its name will NOT contain "OkHostnameVerifier".
142+
assertThat(client.hostnameVerifier().getClass().getName())
143+
.as("permissive hostname verifier must be installed when both gates are set")
144+
.doesNotContain("OkHostnameVerifier");
145+
146+
// 2. Functional check: the verifier must accept any hostname/session pair.
147+
assertThat(client.hostnameVerifier().verify("any.host.example", null))
148+
.as("permissive hostname verifier must return true for arbitrary hostnames")
149+
.isTrue();
150+
151+
// 3. SSL socket factory must be the trust-all-configured one (i.e. NOT the JVM default
152+
// that OkHttp lazily constructs from the system trust store). We assert non-null and
153+
// that calling sslSocketFactory() does not trigger OkHttp's default construction path
154+
// by comparing against a freshly built default client.
155+
AxonFlowConfig defaultConfig =
156+
AxonFlowConfig.builder().agentUrl("https://localhost:8080").build();
157+
OkHttpClient defaultClient = HttpClientFactory.create(defaultConfig);
158+
assertThat(client.sslSocketFactory())
159+
.as("insecure-path SSL socket factory must differ from the default-path factory")
160+
.isNotSameAs(defaultClient.sslSocketFactory());
161+
}
162+
163+
@Test
164+
@DisplayName(
165+
"should NOT activate insecure TLS path when env var is set but builder flag is false")
166+
@SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
167+
void shouldKeepDefaultPathWhenOnlyEnvVarIsSet() {
168+
// Complementary negative path: env var alone, without insecureSkipVerify(true), must
169+
// keep the default safe TrustManager and OkHostnameVerifier. The double-gate is symmetric:
170+
// BOTH must be present — neither alone activates the insecure path.
171+
AxonFlowConfig config =
172+
AxonFlowConfig.builder().agentUrl("https://localhost:8080").build();
173+
174+
OkHttpClient client = HttpClientFactory.create(config);
175+
176+
assertThat(client).isNotNull();
177+
assertThat(client.hostnameVerifier().getClass().getName())
178+
.as("default hostname verifier must remain when only env var is set")
179+
.contains("OkHostnameVerifier");
180+
}
103181
}

0 commit comments

Comments
 (0)