Skip to content

Commit 85a0559

Browse files
authored
💥 [BREAKING] enable TLS if api key specified (#2126)
* enable TLS if api key specified * test fix, explicitly disable TLS * make TLSDisabled and TLS mutually exclusive. Note in API key creation methods that api key can enable TLS * address nit
1 parent 3ce6004 commit 85a0559

File tree

4 files changed

+104
-1
lines changed

4 files changed

+104
-1
lines changed

‎client/client.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,6 +1514,8 @@ func HistoryFromJSON(r io.Reader, options HistoryJSONOptions) (*historypb.Histor
15141514
// Note, this uses a fixed header value for authentication. Many users that want
15151515
// to rotate this value without reconnecting should use
15161516
// [NewAPIKeyDynamicCredentials].
1517+
//
1518+
// Note, TLS is auto-enabled when API key is provided and TLS is not explicitly set/disabled.
15171519
func NewAPIKeyStaticCredentials(apiKey string) Credentials {
15181520
return internal.NewAPIKeyStaticCredentials(apiKey)
15191521
}
@@ -1528,6 +1530,8 @@ func NewAPIKeyStaticCredentials(apiKey string) Credentials {
15281530
// "Authorization" header with "Bearer " + the given function result. If the
15291531
// resulting string is non-empty, it will overwrite any "Authorization" header
15301532
// that may be on the context or from existing header provider.
1533+
//
1534+
// Note, TLS is auto-enabled when API key is provided and TLS is not explicitly set/disabled.
15311535
func NewAPIKeyDynamicCredentials(apiKeyCallback func(context.Context) (string, error)) Credentials {
15321536
return internal.NewAPIKeyDynamicCredentials(apiKeyCallback)
15331537
}

‎internal/client.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,12 @@ type (
542542
// TLS configures connection level security credentials.
543543
TLS *tls.Config
544544

545+
// TLSDisabled explicitly disables TLS. When true, TLS will not be used even
546+
// if API key credentials are provided (which would normally auto-enable TLS).
547+
// This is not recommended for production use as it sends credentials in plaintext.
548+
// This option is mutually exclusive with TLS - an error will be returned if both are set.
549+
TLSDisabled bool
550+
545551
// Authority specifies the value to be used as the :authority pseudo-header.
546552
// This value only used when TLS is nil.
547553
Authority string
@@ -990,6 +996,11 @@ func newClient(ctx context.Context, options ClientOptions, existing *WorkflowCli
990996
options.Logger.Info("No logger configured for temporal client. Created default one.")
991997
}
992998

999+
// Validate mutually exclusive TLS options
1000+
if options.ConnectionOptions.TLS != nil && options.ConnectionOptions.TLSDisabled {
1001+
return nil, fmt.Errorf("cannot set both TLS and TLSDisabled in ConnectionOptions")
1002+
}
1003+
9931004
if options.Credentials != nil {
9941005
if err := options.Credentials.applyToOptions(&options.ConnectionOptions); err != nil {
9951006
return nil, err
@@ -1210,7 +1221,13 @@ func NewAPIKeyDynamicCredentials(apiKeyCallback func(context.Context) (string, e
12101221
return apiKeyCredentials(apiKeyCallback)
12111222
}
12121223

1213-
func (apiKeyCredentials) applyToOptions(*ConnectionOptions) error { return nil }
1224+
func (apiKeyCredentials) applyToOptions(opts *ConnectionOptions) error {
1225+
// Auto-enable TLS when API key is provided and TLS is not explicitly set/disabled
1226+
if opts.TLS == nil && !opts.TLSDisabled {
1227+
opts.TLS = &tls.Config{}
1228+
}
1229+
return nil
1230+
}
12141231

12151232
func (a apiKeyCredentials) gRPCInterceptor() grpc.UnaryClientInterceptor { return a.gRPCIntercept }
12161233

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package internal
2+
3+
import (
4+
"context"
5+
"crypto/tls"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestAPIKeyCredentials_TLSEnabledByDefaultWhenAPIKeyProvided(t *testing.T) {
12+
creds := NewAPIKeyStaticCredentials("test-api-key")
13+
opts := &ConnectionOptions{}
14+
15+
err := creds.applyToOptions(opts)
16+
require.NoError(t, err)
17+
18+
// TLS should be auto-enabled when api_key is provided and tls not explicitly set
19+
require.NotNil(t, opts.TLS)
20+
}
21+
22+
func TestAPIKeyCredentials_TLSCanBeExplicitlyDisabledWithAPIKey(t *testing.T) {
23+
// Test that TLS can be explicitly disabled using TLSDisabled field
24+
creds := NewAPIKeyStaticCredentials("test-api-key")
25+
opts := &ConnectionOptions{
26+
TLSDisabled: true,
27+
}
28+
29+
err := creds.applyToOptions(opts)
30+
require.NoError(t, err)
31+
32+
// TLS should remain nil when TLSDisabled is true
33+
require.Nil(t, opts.TLS)
34+
}
35+
36+
func TestConnectionOptions_TLSDisabledByDefaultWithoutAPIKey(t *testing.T) {
37+
// Test that TLS is disabled by default when no API key is provided
38+
opts := &ConnectionOptions{}
39+
40+
// Without API key credentials, TLS should remain nil
41+
require.Nil(t, opts.TLS)
42+
}
43+
44+
func TestAPIKeyCredentials_ExplicitTLSConfigPreservedWithAPIKey(t *testing.T) {
45+
// Test that explicit TLS configuration is preserved when API key is provided
46+
tlsConfig := &tls.Config{
47+
ServerName: "test-domain",
48+
MinVersion: tls.VersionTLS12,
49+
}
50+
51+
creds := NewAPIKeyStaticCredentials("test-api-key")
52+
opts := &ConnectionOptions{
53+
TLS: tlsConfig,
54+
}
55+
56+
err := creds.applyToOptions(opts)
57+
require.NoError(t, err)
58+
59+
// Explicit TLS config should be preserved
60+
require.NotNil(t, opts.TLS)
61+
require.Equal(t, "test-domain", opts.TLS.ServerName)
62+
require.Equal(t, uint16(tls.VersionTLS12), opts.TLS.MinVersion)
63+
}
64+
65+
func TestConnectionOptions_TLSAndTLSDisabledMutuallyExclusive(t *testing.T) {
66+
// Test that setting both TLS and TLSDisabled returns an error
67+
_, err := NewClient(context.Background(), ClientOptions{
68+
HostPort: "localhost:7233",
69+
ConnectionOptions: ConnectionOptions{
70+
TLS: &tls.Config{},
71+
TLSDisabled: true,
72+
},
73+
})
74+
require.Error(t, err)
75+
require.Contains(t, err.Error(), "cannot set both TLS and TLSDisabled")
76+
}

‎internal/grpc_dialer_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,9 @@ func TestCredentialsAPIKey(t *testing.T) {
446446
client, err := DialClient(context.Background(), ClientOptions{
447447
HostPort: srv.addr,
448448
Credentials: NewAPIKeyStaticCredentials("my-api-key"),
449+
ConnectionOptions: ConnectionOptions{
450+
TLSDisabled: true, // Test server doesn't use TLS
451+
},
449452
})
450453
require.NoError(t, err)
451454
defer client.Close()
@@ -484,6 +487,9 @@ func TestCredentialsAPIKey(t *testing.T) {
484487
Credentials: NewAPIKeyDynamicCredentials(func(ctx context.Context) (string, error) {
485488
return "my-callback-api-key", nil
486489
}),
490+
ConnectionOptions: ConnectionOptions{
491+
TLSDisabled: true,
492+
},
487493
})
488494
require.NoError(t, err)
489495
defer client.Close()

0 commit comments

Comments
 (0)