From b7762eef541644bf09c688f3cd8293382eeaecb6 Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Tue, 2 Dec 2025 09:13:04 -0800 Subject: [PATCH 1/3] enable TLS if api key provided --- .../serviceclient/ServiceStubsOptions.java | 15 ++++- .../ServiceStubsOptionsTest.java | 57 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index 4f63566654..c45b5f4d88 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -419,6 +419,7 @@ public static class Builder> { private ManagedChannel channel; private SslContext sslContext; private boolean enableHttps; + private boolean enableHttpsExplicitlySet; private String target; private Consumer> channelInitializer; private Duration healthCheckAttemptTimeout; @@ -435,6 +436,7 @@ public static class Builder> { private Collection grpcMetadataProviders; private Collection grpcClientInterceptors; private Scope metricsScope; + private boolean apiKeyProvided; protected Builder() {} @@ -443,6 +445,7 @@ protected Builder(ServiceStubsOptions options) { this.target = options.target; this.channelInitializer = options.channelInitializer; this.enableHttps = options.enableHttps; + this.enableHttpsExplicitlySet = true; this.sslContext = options.sslContext; this.healthCheckAttemptTimeout = options.healthCheckAttemptTimeout; this.healthCheckTimeout = options.healthCheckTimeout; @@ -542,6 +545,7 @@ public T setSslContext(SslContext sslContext) { */ public T setEnableHttps(boolean enableHttps) { this.enableHttps = enableHttps; + this.enableHttpsExplicitlySet = true; return self(); } @@ -613,6 +617,7 @@ public T addGrpcMetadataProvider(GrpcMetadataProvider grpcMetadataProvider) { * @return {@code this} */ public T addApiKey(AuthorizationTokenSupplier apiKey) { + this.apiKeyProvided = true; addGrpcMetadataProvider( new AuthorizationGrpcMetadataProvider(() -> "Bearer " + apiKey.supply())); return self(); @@ -851,6 +856,14 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { Collection grpcClientInterceptors = MoreObjects.firstNonNull(this.grpcClientInterceptors, Collections.emptyList()); + // Auto-enable TLS when API key is provided and TLS is not explicitly set + boolean enableHttps; + if (this.enableHttpsExplicitlySet) { + enableHttps = this.enableHttps; + } else { + enableHttps = this.apiKeyProvided; + } + Scope metricsScope = this.metricsScope != null ? this.metricsScope : new NoopScope(); Duration healthCheckAttemptTimeout = this.healthCheckAttemptTimeout != null @@ -865,7 +878,7 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { this.channel, target, this.channelInitializer, - this.enableHttps, + enableHttps, this.sslContext, healthCheckAttemptTimeout, healthCheckTimeout, diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java new file mode 100644 index 0000000000..7e8d7f7c8e --- /dev/null +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -0,0 +1,57 @@ +package io.temporal.serviceclient; + +import static org.junit.Assert.*; + +import org.junit.Test; + +public class ServiceStubsOptionsTest { + + @Test + public void testTLSEnabledByDefaultWhenAPIKeyProvided() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .addApiKey(() -> "test-api-key") + .validateAndBuildWithDefaults(); + + assertTrue(options.getEnableHttps()); + } + + @Test + public void testTLSCanBeExplicitlyDisabledWithAPIKey() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .addApiKey(() -> "test-api-key") + .setEnableHttps(false) + .validateAndBuildWithDefaults(); + + assertFalse(options.getEnableHttps()); + } + + @Test + public void testExplicitTLSDisableBeforeAPIKeyStillDisables() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setEnableHttps(false) + .addApiKey(() -> "test-api-key") + .validateAndBuildWithDefaults(); + + // Explicit TLS=false should take precedence regardless of order + assertFalse(options.getEnableHttps()); + } + + @Test + public void testExplicitTLSDisableAfterAPIKeyStillDisables() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .addApiKey(() -> "test-api-key") + .setEnableHttps(false) + .validateAndBuildWithDefaults(); + + // Explicit TLS=false should take precedence regardless of order + assertFalse(options.getEnableHttps()); + } +} From 36fae11004f23adfeb52b9d23693c6d356b7d23b Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Wed, 3 Dec 2025 14:35:20 -0800 Subject: [PATCH 2/3] remove explicit tls enabled flag, use Boolean instead of boolean instead --- .../serviceclient/ServiceStubsOptions.java | 19 +++---- .../ServiceStubsOptionsTest.java | 57 ++++++++++++++++--- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index c45b5f4d88..a5595f1014 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -418,8 +418,7 @@ public String toString() { public static class Builder> { private ManagedChannel channel; private SslContext sslContext; - private boolean enableHttps; - private boolean enableHttpsExplicitlySet; + private Boolean enableHttps; private String target; private Consumer> channelInitializer; private Duration healthCheckAttemptTimeout; @@ -445,7 +444,6 @@ protected Builder(ServiceStubsOptions options) { this.target = options.target; this.channelInitializer = options.channelInitializer; this.enableHttps = options.enableHttps; - this.enableHttpsExplicitlySet = true; this.sslContext = options.sslContext; this.healthCheckAttemptTimeout = options.healthCheckAttemptTimeout; this.healthCheckTimeout = options.healthCheckTimeout; @@ -545,7 +543,6 @@ public T setSslContext(SslContext sslContext) { */ public T setEnableHttps(boolean enableHttps) { this.enableHttps = enableHttps; - this.enableHttpsExplicitlySet = true; return self(); } @@ -808,7 +805,7 @@ public ServiceStubsOptions build() { this.channel, this.target, this.channelInitializer, - this.enableHttps, + this.enableHttps != null ? this.enableHttps : false, this.sslContext, this.healthCheckAttemptTimeout, this.healthCheckTimeout, @@ -842,7 +839,7 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { "Only one of the 'sslContext' or 'channel' options can be set at a time"); } - if (this.enableHttps && this.channel != null) { + if (Boolean.TRUE.equals(this.enableHttps) && this.channel != null) { throw new IllegalStateException( "Only one of the 'enableHttps' or 'channel' options can be set at a time"); } @@ -856,12 +853,12 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { Collection grpcClientInterceptors = MoreObjects.firstNonNull(this.grpcClientInterceptors, Collections.emptyList()); - // Auto-enable TLS when API key is provided and TLS is not explicitly set - boolean enableHttps; - if (this.enableHttpsExplicitlySet) { + // Resolve enableHttps: explicit value, auto-enable with API key, or default false + boolean enableHttps = false; + if (this.enableHttps != null) { enableHttps = this.enableHttps; - } else { - enableHttps = this.apiKeyProvided; + } else if (this.apiKeyProvided && this.sslContext == null && this.channel == null) { + enableHttps = true; } Scope metricsScope = this.metricsScope != null ? this.metricsScope : new NoopScope(); diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java index 7e8d7f7c8e..1e7d7571cc 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -1,7 +1,10 @@ package io.temporal.serviceclient; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import io.grpc.ManagedChannel; +import io.grpc.netty.shaded.io.netty.handler.ssl.SslContext; import org.junit.Test; public class ServiceStubsOptionsTest { @@ -18,24 +21,25 @@ public void testTLSEnabledByDefaultWhenAPIKeyProvided() { } @Test - public void testTLSCanBeExplicitlyDisabledWithAPIKey() { + public void testExplicitTLSDisableBeforeAPIKeyStillDisables() { ServiceStubsOptions options = WorkflowServiceStubsOptions.newBuilder() .setTarget("localhost:7233") - .addApiKey(() -> "test-api-key") .setEnableHttps(false) + .addApiKey(() -> "test-api-key") .validateAndBuildWithDefaults(); + // Explicit TLS=false should take precedence regardless of order assertFalse(options.getEnableHttps()); } @Test - public void testExplicitTLSDisableBeforeAPIKeyStillDisables() { + public void testExplicitTLSDisableAfterAPIKeyStillDisables() { ServiceStubsOptions options = WorkflowServiceStubsOptions.newBuilder() .setTarget("localhost:7233") - .setEnableHttps(false) .addApiKey(() -> "test-api-key") + .setEnableHttps(false) .validateAndBuildWithDefaults(); // Explicit TLS=false should take precedence regardless of order @@ -43,15 +47,54 @@ public void testExplicitTLSDisableBeforeAPIKeyStillDisables() { } @Test - public void testExplicitTLSDisableAfterAPIKeyStillDisables() { + public void testTLSDisabledByDefaultWithoutAPIKey() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + assertFalse(options.getEnableHttps()); + } + + @Test + public void testExplicitTLSEnableWithoutAPIKey() { + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setEnableHttps(true) + .validateAndBuildWithDefaults(); + + assertTrue(options.getEnableHttps()); + } + + @Test + public void testTLSNotAutoEnabledWhenSslContextProvided() { + // When user provides custom sslContext, they're handling TLS themselves + // so enableHttps should not be auto-enabled + SslContext sslContext = mock(SslContext.class); ServiceStubsOptions options = WorkflowServiceStubsOptions.newBuilder() .setTarget("localhost:7233") .addApiKey(() -> "test-api-key") - .setEnableHttps(false) + .setSslContext(sslContext) + .validateAndBuildWithDefaults(); + + // enableHttps stays false because sslContext handles TLS + assertFalse(options.getEnableHttps()); + assertNotNull(options.getSslContext()); + } + + @Test + public void testTLSNotAutoEnabledWhenCustomChannelProvided() { + // When user provides custom channel, they're managing connection themselves + // so enableHttps should not be auto-enabled + ManagedChannel channel = mock(ManagedChannel.class); + ServiceStubsOptions options = + WorkflowServiceStubsOptions.newBuilder() + .setChannel(channel) + .addApiKey(() -> "test-api-key") .validateAndBuildWithDefaults(); - // Explicit TLS=false should take precedence regardless of order assertFalse(options.getEnableHttps()); } } From 0e6459d402f6481cca3aad3901faf0bc8b8ba1f8 Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Mon, 15 Dec 2025 10:59:41 -0500 Subject: [PATCH 3/3] make enableHttps a Boolean --- .../serviceclient/ServiceStubsOptions.java | 64 +++++++++---- .../ServiceStubsOptionsTest.java | 92 +++++++++++++++---- 2 files changed, 118 insertions(+), 38 deletions(-) diff --git a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java index a5595f1014..0c3d8ae3ad 100644 --- a/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java +++ b/temporal-serviceclient/src/main/java/io/temporal/serviceclient/ServiceStubsOptions.java @@ -39,8 +39,14 @@ public class ServiceStubsOptions { protected final @Nullable Consumer> channelInitializer; - /** Indicates whether basic HTTPS/SSL/TLS should be enabled * */ - protected final boolean enableHttps; + /** + * Indicates whether basic HTTPS/SSL/TLS should be enabled. Null means not explicitly set by the + * user, allowing runtime derivation of the effective value. + */ + protected final Boolean enableHttps; + + /** Indicates whether an API key was provided, used for automatic TLS enablement */ + protected final boolean apiKeyProvided; /** The user provided context for SSL/TLS over gRPC * */ protected final SslContext sslContext; @@ -113,6 +119,7 @@ public class ServiceStubsOptions { this.target = that.target; this.channelInitializer = that.channelInitializer; this.enableHttps = that.enableHttps; + this.apiKeyProvided = that.apiKeyProvided; this.sslContext = that.sslContext; this.healthCheckAttemptTimeout = that.healthCheckAttemptTimeout; this.systemInfoTimeout = that.systemInfoTimeout; @@ -134,7 +141,8 @@ public class ServiceStubsOptions { ManagedChannel channel, String target, @Nullable Consumer> channelInitializer, - boolean enableHttps, + Boolean enableHttps, + boolean apiKeyProvided, SslContext sslContext, Duration healthCheckAttemptTimeout, Duration healthCheckTimeout, @@ -154,6 +162,7 @@ public class ServiceStubsOptions { this.target = target; this.channelInitializer = channelInitializer; this.enableHttps = enableHttps; + this.apiKeyProvided = apiKeyProvided; this.sslContext = sslContext; this.healthCheckAttemptTimeout = healthCheckAttemptTimeout; this.healthCheckTimeout = healthCheckTimeout; @@ -202,11 +211,24 @@ public Consumer> getChannelInitializer() { } /** - * @return if gRPC should use SSL/TLS; Ignored and assumed {@code true} if {@link - * #getSslContext()} is set + * Returns whether gRPC should use SSL/TLS. This method computes the effective value based on: + * + *
    + *
  • If explicitly set via {@link Builder#setEnableHttps(boolean)}, returns that value + *
  • If an API key was provided and no custom SSL context or channel is set, returns {@code + * true} (auto-enabled) + *
  • Otherwise returns {@code false} + *
+ * + *

Note: When {@link #getSslContext()} is set, TLS is handled by the SSL context regardless of + * this value. + * + * @return if gRPC should use SSL/TLS */ public boolean getEnableHttps() { - return enableHttps; + return (enableHttps != null) + ? enableHttps + : (apiKeyProvided && sslContext == null && channel == null); } /** @@ -325,12 +347,13 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ServiceStubsOptions that = (ServiceStubsOptions) o; - return enableHttps == that.enableHttps + return apiKeyProvided == that.apiKeyProvided && enableKeepAlive == that.enableKeepAlive && keepAlivePermitWithoutStream == that.keepAlivePermitWithoutStream && Objects.equals(channel, that.channel) && Objects.equals(target, that.target) && Objects.equals(channelInitializer, that.channelInitializer) + && Objects.equals(enableHttps, that.enableHttps) && Objects.equals(sslContext, that.sslContext) && Objects.equals(healthCheckAttemptTimeout, that.healthCheckAttemptTimeout) && Objects.equals(healthCheckTimeout, that.healthCheckTimeout) @@ -353,6 +376,7 @@ public int hashCode() { target, channelInitializer, enableHttps, + apiKeyProvided, sslContext, healthCheckAttemptTimeout, healthCheckTimeout, @@ -444,6 +468,7 @@ protected Builder(ServiceStubsOptions options) { this.target = options.target; this.channelInitializer = options.channelInitializer; this.enableHttps = options.enableHttps; + this.apiKeyProvided = options.apiKeyProvided; this.sslContext = options.sslContext; this.healthCheckAttemptTimeout = options.healthCheckAttemptTimeout; this.healthCheckTimeout = options.healthCheckTimeout; @@ -456,8 +481,15 @@ protected Builder(ServiceStubsOptions options) { this.connectionBackoffResetFrequency = options.connectionBackoffResetFrequency; this.grpcReconnectFrequency = options.grpcReconnectFrequency; this.headers = options.headers; - this.grpcMetadataProviders = options.grpcMetadataProviders; - this.grpcClientInterceptors = options.grpcClientInterceptors; + // Make mutable copies of collections to allow adding more items + this.grpcMetadataProviders = + options.grpcMetadataProviders != null && !options.grpcMetadataProviders.isEmpty() + ? new ArrayList<>(options.grpcMetadataProviders) + : null; + this.grpcClientInterceptors = + options.grpcClientInterceptors != null && !options.grpcClientInterceptors.isEmpty() + ? new ArrayList<>(options.grpcClientInterceptors) + : null; this.metricsScope = options.metricsScope; } @@ -805,7 +837,8 @@ public ServiceStubsOptions build() { this.channel, this.target, this.channelInitializer, - this.enableHttps != null ? this.enableHttps : false, + this.enableHttps, + this.apiKeyProvided, this.sslContext, this.healthCheckAttemptTimeout, this.healthCheckTimeout, @@ -853,14 +886,6 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { Collection grpcClientInterceptors = MoreObjects.firstNonNull(this.grpcClientInterceptors, Collections.emptyList()); - // Resolve enableHttps: explicit value, auto-enable with API key, or default false - boolean enableHttps = false; - if (this.enableHttps != null) { - enableHttps = this.enableHttps; - } else if (this.apiKeyProvided && this.sslContext == null && this.channel == null) { - enableHttps = true; - } - Scope metricsScope = this.metricsScope != null ? this.metricsScope : new NoopScope(); Duration healthCheckAttemptTimeout = this.healthCheckAttemptTimeout != null @@ -875,7 +900,8 @@ public ServiceStubsOptions validateAndBuildWithDefaults() { this.channel, target, this.channelInitializer, - enableHttps, + this.enableHttps, + this.apiKeyProvided, this.sslContext, healthCheckAttemptTimeout, healthCheckTimeout, diff --git a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java index 1e7d7571cc..ccadbffe81 100644 --- a/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java +++ b/temporal-serviceclient/src/test/java/io/temporal/serviceclient/ServiceStubsOptionsTest.java @@ -1,10 +1,7 @@ package io.temporal.serviceclient; import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; -import io.grpc.ManagedChannel; -import io.grpc.netty.shaded.io.netty.handler.ssl.SslContext; import org.junit.Test; public class ServiceStubsOptionsTest { @@ -68,33 +65,90 @@ public void testExplicitTLSEnableWithoutAPIKey() { } @Test - public void testTLSNotAutoEnabledWhenSslContextProvided() { - // When user provides custom sslContext, they're handling TLS themselves - // so enableHttps should not be auto-enabled - SslContext sslContext = mock(SslContext.class); - ServiceStubsOptions options = + public void testBuilderFromOptionsPreservesDefaultTLSBehavior() { + ServiceStubsOptions options1 = WorkflowServiceStubsOptions.newBuilder() .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + assertFalse(options1.getEnableHttps()); + + ServiceStubsOptions options2 = + WorkflowServiceStubsOptions.newBuilder(options1) .addApiKey(() -> "test-api-key") - .setSslContext(sslContext) .validateAndBuildWithDefaults(); - // enableHttps stays false because sslContext handles TLS - assertFalse(options.getEnableHttps()); - assertNotNull(options.getSslContext()); + assertTrue( + "TLS should auto-enable when API key is added to builder from options that had default TLS behavior", + options2.getEnableHttps()); } @Test - public void testTLSNotAutoEnabledWhenCustomChannelProvided() { - // When user provides custom channel, they're managing connection themselves - // so enableHttps should not be auto-enabled - ManagedChannel channel = mock(ManagedChannel.class); - ServiceStubsOptions options = + public void testBuilderFromOptionsWithExplicitTLSDisableStaysDisabled() { + ServiceStubsOptions options1 = WorkflowServiceStubsOptions.newBuilder() - .setChannel(channel) + .setTarget("localhost:7233") + .setEnableHttps(false) + .validateAndBuildWithDefaults(); + + assertFalse(options1.getEnableHttps()); + + ServiceStubsOptions options2 = + WorkflowServiceStubsOptions.newBuilder(options1) .addApiKey(() -> "test-api-key") .validateAndBuildWithDefaults(); - assertFalse(options.getEnableHttps()); + assertFalse( + "TLS should stay disabled when explicitly set to false, even with API key", + options2.getEnableHttps()); + } + + @Test + public void testBuilderFromOptionsWithExplicitTLSEnableStaysEnabled() { + ServiceStubsOptions options1 = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setEnableHttps(true) + .validateAndBuildWithDefaults(); + + assertTrue(options1.getEnableHttps()); + + ServiceStubsOptions options2 = + WorkflowServiceStubsOptions.newBuilder(options1).validateAndBuildWithDefaults(); + + assertTrue("TLS should stay enabled when explicitly set to true", options2.getEnableHttps()); + } + + @Test + public void testSpringBootStyleAutoTLSWithApiKey() { + ServiceStubsOptions options1 = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("my-namespace.tmprl.cloud:7233") + .addApiKey(() -> "my-api-key") + .validateAndBuildWithDefaults(); + + assertTrue( + "TLS should auto-enable when API key is provided without explicit TLS setting", + options1.getEnableHttps()); + + ServiceStubsOptions options2 = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .setEnableHttps(false) + .addApiKey(() -> "my-api-key") + .validateAndBuildWithDefaults(); + + assertFalse( + "TLS should stay disabled when explicitly set to false, even with API key", + options2.getEnableHttps()); + + ServiceStubsOptions options3 = + WorkflowServiceStubsOptions.newBuilder() + .setTarget("localhost:7233") + .validateAndBuildWithDefaults(); + + assertFalse( + "TLS should be disabled when no API key and no explicit TLS setting", + options3.getEnableHttps()); } }