Skip to content

Commit 2ff9c2c

Browse files
THardy98THardy98
authored
💥 [BREAKING] enable TLS if api key provided (#2740)
* enable TLS if api key provided * remove explicit tls enabled flag, use Boolean instead of boolean instead * make enableHttps a Boolean
1 parent a39d10c commit 2ff9c2c

File tree

2 files changed

+201
-11
lines changed

2 files changed

+201
-11
lines changed

‎temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,14 @@ public class ServiceStubsOptions {
3939

4040
protected final @Nullable Consumer<ManagedChannelBuilder<?>> channelInitializer;
4141

42-
/** Indicates whether basic HTTPS/SSL/TLS should be enabled * */
43-
protected final boolean enableHttps;
42+
/**
43+
* Indicates whether basic HTTPS/SSL/TLS should be enabled. Null means not explicitly set by the
44+
* user, allowing runtime derivation of the effective value.
45+
*/
46+
protected final Boolean enableHttps;
47+
48+
/** Indicates whether an API key was provided, used for automatic TLS enablement */
49+
protected final boolean apiKeyProvided;
4450

4551
/** The user provided context for SSL/TLS over gRPC * */
4652
protected final SslContext sslContext;
@@ -113,6 +119,7 @@ public class ServiceStubsOptions {
113119
this.target = that.target;
114120
this.channelInitializer = that.channelInitializer;
115121
this.enableHttps = that.enableHttps;
122+
this.apiKeyProvided = that.apiKeyProvided;
116123
this.sslContext = that.sslContext;
117124
this.healthCheckAttemptTimeout = that.healthCheckAttemptTimeout;
118125
this.systemInfoTimeout = that.systemInfoTimeout;
@@ -134,7 +141,8 @@ public class ServiceStubsOptions {
134141
ManagedChannel channel,
135142
String target,
136143
@Nullable Consumer<ManagedChannelBuilder<?>> channelInitializer,
137-
boolean enableHttps,
144+
Boolean enableHttps,
145+
boolean apiKeyProvided,
138146
SslContext sslContext,
139147
Duration healthCheckAttemptTimeout,
140148
Duration healthCheckTimeout,
@@ -154,6 +162,7 @@ public class ServiceStubsOptions {
154162
this.target = target;
155163
this.channelInitializer = channelInitializer;
156164
this.enableHttps = enableHttps;
165+
this.apiKeyProvided = apiKeyProvided;
157166
this.sslContext = sslContext;
158167
this.healthCheckAttemptTimeout = healthCheckAttemptTimeout;
159168
this.healthCheckTimeout = healthCheckTimeout;
@@ -202,11 +211,24 @@ public Consumer<ManagedChannelBuilder<?>> getChannelInitializer() {
202211
}
203212

204213
/**
205-
* @return if gRPC should use SSL/TLS; Ignored and assumed {@code true} if {@link
206-
* #getSslContext()} is set
214+
* Returns whether gRPC should use SSL/TLS. This method computes the effective value based on:
215+
*
216+
* <ul>
217+
* <li>If explicitly set via {@link Builder#setEnableHttps(boolean)}, returns that value
218+
* <li>If an API key was provided and no custom SSL context or channel is set, returns {@code
219+
* true} (auto-enabled)
220+
* <li>Otherwise returns {@code false}
221+
* </ul>
222+
*
223+
* <p>Note: When {@link #getSslContext()} is set, TLS is handled by the SSL context regardless of
224+
* this value.
225+
*
226+
* @return if gRPC should use SSL/TLS
207227
*/
208228
public boolean getEnableHttps() {
209-
return enableHttps;
229+
return (enableHttps != null)
230+
? enableHttps
231+
: (apiKeyProvided && sslContext == null && channel == null);
210232
}
211233

212234
/**
@@ -325,12 +347,13 @@ public boolean equals(Object o) {
325347
if (this == o) return true;
326348
if (o == null || getClass() != o.getClass()) return false;
327349
ServiceStubsOptions that = (ServiceStubsOptions) o;
328-
return enableHttps == that.enableHttps
350+
return apiKeyProvided == that.apiKeyProvided
329351
&& enableKeepAlive == that.enableKeepAlive
330352
&& keepAlivePermitWithoutStream == that.keepAlivePermitWithoutStream
331353
&& Objects.equals(channel, that.channel)
332354
&& Objects.equals(target, that.target)
333355
&& Objects.equals(channelInitializer, that.channelInitializer)
356+
&& Objects.equals(enableHttps, that.enableHttps)
334357
&& Objects.equals(sslContext, that.sslContext)
335358
&& Objects.equals(healthCheckAttemptTimeout, that.healthCheckAttemptTimeout)
336359
&& Objects.equals(healthCheckTimeout, that.healthCheckTimeout)
@@ -353,6 +376,7 @@ public int hashCode() {
353376
target,
354377
channelInitializer,
355378
enableHttps,
379+
apiKeyProvided,
356380
sslContext,
357381
healthCheckAttemptTimeout,
358382
healthCheckTimeout,
@@ -418,7 +442,7 @@ public String toString() {
418442
public static class Builder<T extends Builder<T>> {
419443
private ManagedChannel channel;
420444
private SslContext sslContext;
421-
private boolean enableHttps;
445+
private Boolean enableHttps;
422446
private String target;
423447
private Consumer<ManagedChannelBuilder<?>> channelInitializer;
424448
private Duration healthCheckAttemptTimeout;
@@ -435,6 +459,7 @@ public static class Builder<T extends Builder<T>> {
435459
private Collection<GrpcMetadataProvider> grpcMetadataProviders;
436460
private Collection<ClientInterceptor> grpcClientInterceptors;
437461
private Scope metricsScope;
462+
private boolean apiKeyProvided;
438463

439464
protected Builder() {}
440465

@@ -443,6 +468,7 @@ protected Builder(ServiceStubsOptions options) {
443468
this.target = options.target;
444469
this.channelInitializer = options.channelInitializer;
445470
this.enableHttps = options.enableHttps;
471+
this.apiKeyProvided = options.apiKeyProvided;
446472
this.sslContext = options.sslContext;
447473
this.healthCheckAttemptTimeout = options.healthCheckAttemptTimeout;
448474
this.healthCheckTimeout = options.healthCheckTimeout;
@@ -455,8 +481,15 @@ protected Builder(ServiceStubsOptions options) {
455481
this.connectionBackoffResetFrequency = options.connectionBackoffResetFrequency;
456482
this.grpcReconnectFrequency = options.grpcReconnectFrequency;
457483
this.headers = options.headers;
458-
this.grpcMetadataProviders = options.grpcMetadataProviders;
459-
this.grpcClientInterceptors = options.grpcClientInterceptors;
484+
// Make mutable copies of collections to allow adding more items
485+
this.grpcMetadataProviders =
486+
options.grpcMetadataProviders != null && !options.grpcMetadataProviders.isEmpty()
487+
? new ArrayList<>(options.grpcMetadataProviders)
488+
: null;
489+
this.grpcClientInterceptors =
490+
options.grpcClientInterceptors != null && !options.grpcClientInterceptors.isEmpty()
491+
? new ArrayList<>(options.grpcClientInterceptors)
492+
: null;
460493
this.metricsScope = options.metricsScope;
461494
}
462495

@@ -613,6 +646,7 @@ public T addGrpcMetadataProvider(GrpcMetadataProvider grpcMetadataProvider) {
613646
* @return {@code this}
614647
*/
615648
public T addApiKey(AuthorizationTokenSupplier apiKey) {
649+
this.apiKeyProvided = true;
616650
addGrpcMetadataProvider(
617651
new AuthorizationGrpcMetadataProvider(() -> "Bearer " + apiKey.supply()));
618652
return self();
@@ -804,6 +838,7 @@ public ServiceStubsOptions build() {
804838
this.target,
805839
this.channelInitializer,
806840
this.enableHttps,
841+
this.apiKeyProvided,
807842
this.sslContext,
808843
this.healthCheckAttemptTimeout,
809844
this.healthCheckTimeout,
@@ -837,7 +872,7 @@ public ServiceStubsOptions validateAndBuildWithDefaults() {
837872
"Only one of the 'sslContext' or 'channel' options can be set at a time");
838873
}
839874

840-
if (this.enableHttps && this.channel != null) {
875+
if (Boolean.TRUE.equals(this.enableHttps) && this.channel != null) {
841876
throw new IllegalStateException(
842877
"Only one of the 'enableHttps' or 'channel' options can be set at a time");
843878
}
@@ -866,6 +901,7 @@ public ServiceStubsOptions validateAndBuildWithDefaults() {
866901
target,
867902
this.channelInitializer,
868903
this.enableHttps,
904+
this.apiKeyProvided,
869905
this.sslContext,
870906
healthCheckAttemptTimeout,
871907
healthCheckTimeout,

‎temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package io.temporal.serviceclient;
2+
3+
import static org.junit.Assert.*;
4+
5+
import org.junit.Test;
6+
7+
public class ServiceStubsOptionsTest {
8+
9+
@Test
10+
public void testTLSEnabledByDefaultWhenAPIKeyProvided() {
11+
ServiceStubsOptions options =
12+
WorkflowServiceStubsOptions.newBuilder()
13+
.setTarget("localhost:7233")
14+
.addApiKey(() -> "test-api-key")
15+
.validateAndBuildWithDefaults();
16+
17+
assertTrue(options.getEnableHttps());
18+
}
19+
20+
@Test
21+
public void testExplicitTLSDisableBeforeAPIKeyStillDisables() {
22+
ServiceStubsOptions options =
23+
WorkflowServiceStubsOptions.newBuilder()
24+
.setTarget("localhost:7233")
25+
.setEnableHttps(false)
26+
.addApiKey(() -> "test-api-key")
27+
.validateAndBuildWithDefaults();
28+
29+
// Explicit TLS=false should take precedence regardless of order
30+
assertFalse(options.getEnableHttps());
31+
}
32+
33+
@Test
34+
public void testExplicitTLSDisableAfterAPIKeyStillDisables() {
35+
ServiceStubsOptions options =
36+
WorkflowServiceStubsOptions.newBuilder()
37+
.setTarget("localhost:7233")
38+
.addApiKey(() -> "test-api-key")
39+
.setEnableHttps(false)
40+
.validateAndBuildWithDefaults();
41+
42+
// Explicit TLS=false should take precedence regardless of order
43+
assertFalse(options.getEnableHttps());
44+
}
45+
46+
@Test
47+
public void testTLSDisabledByDefaultWithoutAPIKey() {
48+
ServiceStubsOptions options =
49+
WorkflowServiceStubsOptions.newBuilder()
50+
.setTarget("localhost:7233")
51+
.validateAndBuildWithDefaults();
52+
53+
assertFalse(options.getEnableHttps());
54+
}
55+
56+
@Test
57+
public void testExplicitTLSEnableWithoutAPIKey() {
58+
ServiceStubsOptions options =
59+
WorkflowServiceStubsOptions.newBuilder()
60+
.setTarget("localhost:7233")
61+
.setEnableHttps(true)
62+
.validateAndBuildWithDefaults();
63+
64+
assertTrue(options.getEnableHttps());
65+
}
66+
67+
@Test
68+
public void testBuilderFromOptionsPreservesDefaultTLSBehavior() {
69+
ServiceStubsOptions options1 =
70+
WorkflowServiceStubsOptions.newBuilder()
71+
.setTarget("localhost:7233")
72+
.validateAndBuildWithDefaults();
73+
74+
assertFalse(options1.getEnableHttps());
75+
76+
ServiceStubsOptions options2 =
77+
WorkflowServiceStubsOptions.newBuilder(options1)
78+
.addApiKey(() -> "test-api-key")
79+
.validateAndBuildWithDefaults();
80+
81+
assertTrue(
82+
"TLS should auto-enable when API key is added to builder from options that had default TLS behavior",
83+
options2.getEnableHttps());
84+
}
85+
86+
@Test
87+
public void testBuilderFromOptionsWithExplicitTLSDisableStaysDisabled() {
88+
ServiceStubsOptions options1 =
89+
WorkflowServiceStubsOptions.newBuilder()
90+
.setTarget("localhost:7233")
91+
.setEnableHttps(false)
92+
.validateAndBuildWithDefaults();
93+
94+
assertFalse(options1.getEnableHttps());
95+
96+
ServiceStubsOptions options2 =
97+
WorkflowServiceStubsOptions.newBuilder(options1)
98+
.addApiKey(() -> "test-api-key")
99+
.validateAndBuildWithDefaults();
100+
101+
assertFalse(
102+
"TLS should stay disabled when explicitly set to false, even with API key",
103+
options2.getEnableHttps());
104+
}
105+
106+
@Test
107+
public void testBuilderFromOptionsWithExplicitTLSEnableStaysEnabled() {
108+
ServiceStubsOptions options1 =
109+
WorkflowServiceStubsOptions.newBuilder()
110+
.setTarget("localhost:7233")
111+
.setEnableHttps(true)
112+
.validateAndBuildWithDefaults();
113+
114+
assertTrue(options1.getEnableHttps());
115+
116+
ServiceStubsOptions options2 =
117+
WorkflowServiceStubsOptions.newBuilder(options1).validateAndBuildWithDefaults();
118+
119+
assertTrue("TLS should stay enabled when explicitly set to true", options2.getEnableHttps());
120+
}
121+
122+
@Test
123+
public void testSpringBootStyleAutoTLSWithApiKey() {
124+
ServiceStubsOptions options1 =
125+
WorkflowServiceStubsOptions.newBuilder()
126+
.setTarget("my-namespace.tmprl.cloud:7233")
127+
.addApiKey(() -> "my-api-key")
128+
.validateAndBuildWithDefaults();
129+
130+
assertTrue(
131+
"TLS should auto-enable when API key is provided without explicit TLS setting",
132+
options1.getEnableHttps());
133+
134+
ServiceStubsOptions options2 =
135+
WorkflowServiceStubsOptions.newBuilder()
136+
.setTarget("localhost:7233")
137+
.setEnableHttps(false)
138+
.addApiKey(() -> "my-api-key")
139+
.validateAndBuildWithDefaults();
140+
141+
assertFalse(
142+
"TLS should stay disabled when explicitly set to false, even with API key",
143+
options2.getEnableHttps());
144+
145+
ServiceStubsOptions options3 =
146+
WorkflowServiceStubsOptions.newBuilder()
147+
.setTarget("localhost:7233")
148+
.validateAndBuildWithDefaults();
149+
150+
assertFalse(
151+
"TLS should be disabled when no API key and no explicit TLS setting",
152+
options3.getEnableHttps());
153+
}
154+
}

0 commit comments

Comments
 (0)