Skip to content

Commit 34039db

Browse files
aarmamaarmam
committed
fix: codereview suggestions
1 parent 94ced6a commit 34039db

File tree

4 files changed

+144
-79
lines changed

4 files changed

+144
-79
lines changed

‎consent/strategy_default.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ type DefaultStrategy struct {
7474
}
7575

7676
func NewStrategy(r InternalRegistry, c *config.Provider) *DefaultStrategy {
77-
tlsClientConfig, err := c.TLSClientConfig()
77+
tlsClientConfig, err := c.TLSClientConfigWithDefaultFallback(config.KeyPrefixClientBackChannelLogout)
7878
if err != nil {
7979
r.Logger().WithError(err).Fatalf("Unable to setup backchannel logout request client TLS configuration.")
8080
}

‎driver/config/tls.go

+39-10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package config
22

33
import (
44
"crypto/tls"
5+
"fmt"
56
"strings"
67

78
"github.com/hashicorp/go-secure-stdlib/tlsutil"
@@ -24,9 +25,33 @@ const (
2425
KeyTLSCertPath = "serve." + KeySuffixTLSCertPath
2526
KeyTLSKeyPath = "serve." + KeySuffixTLSKeyPath
2627

27-
KeyClientTLSCipherSuites = "client.tls.cipher_suites"
28-
KeyClientTLSMinVer = "client.tls.min_version"
29-
KeyClientTLSMaxVer = "client.tls.max_version"
28+
KeySuffixClientTLSCipherSuites = "tls.cipher_suites"
29+
KeySuffixClientTLSMinVer = "tls.min_version"
30+
KeySuffixClientTLSMaxVer = "tls.max_version"
31+
)
32+
33+
type ClientInterface interface {
34+
Key(suffix string) string
35+
}
36+
37+
func (iface *clientPrefix) Key(suffix string) string {
38+
return fmt.Sprintf("%s.%s", iface.prefix, suffix)
39+
}
40+
41+
type clientPrefix struct {
42+
prefix string
43+
}
44+
45+
var (
46+
KeyPrefixClientDefault ClientInterface = &clientPrefix{
47+
prefix: "client.default",
48+
}
49+
KeyPrefixClientBackChannelLogout ClientInterface = &clientPrefix{
50+
prefix: "client.back_channel_logout",
51+
}
52+
KeyPrefixClientRefreshTokenHook ClientInterface = &clientPrefix{
53+
prefix: "client.refresh_token_hook",
54+
}
3055
)
3156

3257
type TLSConfig interface {
@@ -55,29 +80,33 @@ func (p *Provider) TLS(iface ServeInterface) TLSConfig {
5580
}
5681
}
5782

58-
func (p *Provider) TLSClientConfig() (*tls.Config, error) {
83+
func (p *Provider) TLSClientConfigDefault() (*tls.Config, error) {
84+
return p.TLSClientConfigWithDefaultFallback(KeyPrefixClientDefault)
85+
}
86+
87+
func (p *Provider) TLSClientConfigWithDefaultFallback(iface ClientInterface) (*tls.Config, error) {
5988
tlsClientConfig := new(tls.Config)
6089

61-
if p.p.Exists(KeyClientTLSCipherSuites) {
62-
keyCipherSuites := p.p.Strings(KeyClientTLSCipherSuites)
90+
if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSCipherSuites)) || p.p.Exists(iface.Key(KeySuffixClientTLSCipherSuites)) {
91+
keyCipherSuites := p.p.StringsF(iface.Key(KeySuffixClientTLSCipherSuites), p.p.Strings(KeyPrefixClientDefault.Key(KeySuffixClientTLSCipherSuites)))
6392
cipherSuites, err := tlsutil.ParseCiphers(strings.Join(keyCipherSuites[:], ","))
6493
if err != nil {
6594
return nil, errors.WithMessage(err, "Unable to setup client TLS configuration")
6695
}
6796
tlsClientConfig.CipherSuites = cipherSuites
6897
}
6998

70-
if p.p.Exists(KeyClientTLSMinVer) {
71-
keyMinVer := p.p.String(KeyClientTLSMinVer)
99+
if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSMinVer)) || p.p.Exists(iface.Key(KeySuffixClientTLSMinVer)) {
100+
keyMinVer := p.p.StringF(iface.Key(KeySuffixClientTLSMinVer), p.p.String(KeyPrefixClientDefault.Key(KeySuffixClientTLSMinVer)))
72101
if tlsMinVer, found := tlsutil.TLSLookup[keyMinVer]; !found {
73102
return nil, errors.Errorf("Unable to setup client TLS configuration. Invalid minimum TLS version: %s", keyMinVer)
74103
} else {
75104
tlsClientConfig.MinVersion = tlsMinVer
76105
}
77106
}
78107

79-
if p.p.Exists(KeyClientTLSMaxVer) {
80-
keyMaxVer := p.p.String(KeyClientTLSMaxVer)
108+
if p.p.Exists(KeyPrefixClientDefault.Key(KeySuffixClientTLSMaxVer)) || p.p.Exists(iface.Key(KeySuffixClientTLSMaxVer)) {
109+
keyMaxVer := p.p.StringF(iface.Key(KeySuffixClientTLSMaxVer), p.p.String(KeyPrefixClientDefault.Key(KeySuffixClientTLSMaxVer)))
81110
if tlsMaxVer, found := tlsutil.TLSLookup[keyMaxVer]; !found {
82111
return nil, errors.Errorf("Unable to setup client TLS configuration. Invalid maximum TLS version: %s", keyMaxVer)
83112
} else {

‎driver/config/tls_test.go

+26-12
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import (
1313

1414
func TestTLSClientConfig_CipherSuite(t *testing.T) {
1515
l := logrusx.New("", "")
16-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}))
16+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}))
1717

18-
tlsClientConfig, err := c.TLSClientConfig()
18+
tlsClientConfig, err := c.TLSClientConfigDefault()
1919
assert.NoError(t, err)
2020
cipherSuites := tlsClientConfig.CipherSuites
2121

@@ -26,47 +26,61 @@ func TestTLSClientConfig_CipherSuite(t *testing.T) {
2626

2727
func TestTLSClientConfig_InvalidCipherSuite(t *testing.T) {
2828
l := logrusx.New("", "")
29-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_INVALID_CIPHER_SUITE"}))
29+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.cipher_suites", []string{"TLS_AES_128_GCM_SHA256", "TLS_INVALID_CIPHER_SUITE"}))
3030

31-
_, err := c.TLSClientConfig()
31+
_, err := c.TLSClientConfigDefault()
3232

3333
assert.EqualError(t, err, "Unable to setup client TLS configuration: unsupported cipher \"TLS_INVALID_CIPHER_SUITE\"")
3434
}
3535

3636
func TestTLSClientConfig_MinVersion(t *testing.T) {
3737
l := logrusx.New("", "")
38-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.min_version", "tls13"))
38+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.min_version", "tls13"))
3939

40-
tlsClientConfig, err := c.TLSClientConfig()
40+
tlsClientConfig, err := c.TLSClientConfigDefault()
4141

4242
assert.NoError(t, err)
4343
assert.Equal(t, uint16(tls.VersionTLS13), tlsClientConfig.MinVersion)
4444
}
4545

4646
func TestTLSClientConfig_InvalidMinVersion(t *testing.T) {
4747
l := logrusx.New("", "")
48-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.min_version", "tlsx"))
48+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.min_version", "tlsx"))
4949

50-
_, err := c.TLSClientConfig()
50+
_, err := c.TLSClientConfigDefault()
5151

5252
assert.EqualError(t, err, "Unable to setup client TLS configuration. Invalid minimum TLS version: tlsx")
5353
}
5454

5555
func TestTLSClientConfig_MaxVersion(t *testing.T) {
5656
l := logrusx.New("", "")
57-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.max_version", "tls10"))
57+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.max_version", "tls10"))
5858

59-
tlsClientConfig, err := c.TLSClientConfig()
59+
tlsClientConfig, err := c.TLSClientConfigDefault()
6060

6161
assert.NoError(t, err)
6262
assert.Equal(t, uint16(tls.VersionTLS10), tlsClientConfig.MaxVersion)
6363
}
6464

6565
func TestTLSClientConfig_InvalidMaxTlsVersion(t *testing.T) {
6666
l := logrusx.New("", "")
67-
c := MustNew(context.TODO(), l, configx.WithValue("client.tls.max_version", "tlsx"))
67+
c := MustNew(context.TODO(), l, configx.WithValue("client.default.tls.max_version", "tlsx"))
6868

69-
_, err := c.TLSClientConfig()
69+
_, err := c.TLSClientConfigDefault()
7070

7171
assert.EqualError(t, err, "Unable to setup client TLS configuration. Invalid maximum TLS version: tlsx")
7272
}
73+
74+
func TestTLSClientConfig_WithDefaultFallback(t *testing.T) {
75+
l := logrusx.New("", "")
76+
c := MustNew(context.TODO(), l)
77+
c.MustSet("client.default.tls.min_version", "tls11")
78+
c.MustSet("client.default.tls.max_version", "tls12")
79+
c.MustSet("client.back_channel_logout.tls.max_version", "tls13")
80+
81+
tlsClientConfig, err := c.TLSClientConfigWithDefaultFallback(KeyPrefixClientBackChannelLogout)
82+
83+
assert.NoError(t, err)
84+
assert.Equal(t, uint16(tls.VersionTLS11), tlsClientConfig.MinVersion)
85+
assert.Equal(t, uint16(tls.VersionTLS13), tlsClientConfig.MaxVersion)
86+
}

‎spec/config.json

+78-56
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@
217217
"1h"
218218
]
219219
},
220-
"tls_config": {
220+
"server_tls_config": {
221221
"type": "object",
222222
"description": "Configures HTTPS (HTTP over TLS). If configured, the server automatically supports HTTP/2.",
223223
"properties": {
@@ -246,6 +246,67 @@
246246
}
247247
}
248248
},
249+
"client_tls_config": {
250+
"type": "object",
251+
"additionalProperties": false,
252+
"description": "Configures http client TLS settings.",
253+
"properties": {
254+
"min_version": {
255+
"type": "string",
256+
"description": "Minimum supported TLS version.",
257+
"examples": [
258+
"tls10",
259+
"tls11",
260+
"tls12",
261+
"tls13"
262+
]
263+
},
264+
"max_version": {
265+
"type": "string",
266+
"description": "Maximum supported TLS version.",
267+
"examples": [
268+
"tls10",
269+
"tls11",
270+
"tls12",
271+
"tls13"
272+
]
273+
},
274+
"cipher_suites": {
275+
"type": "array",
276+
"description": "A list of supported cipher suites.",
277+
"items": {
278+
"type": "string"
279+
},
280+
"examples": [
281+
"TLS_RSA_WITH_RC4_128_SHA",
282+
"TLS_RSA_WITH_3DES_EDE_CBC_SHA",
283+
"TLS_RSA_WITH_AES_128_CBC_SHA",
284+
"TLS_RSA_WITH_AES_256_CBC_SHA",
285+
"TLS_RSA_WITH_AES_128_CBC_SHA256",
286+
"TLS_RSA_WITH_AES_128_GCM_SHA256",
287+
"TLS_RSA_WITH_AES_256_GCM_SHA384",
288+
"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA",
289+
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
290+
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
291+
"TLS_ECDHE_RSA_WITH_RC4_128_SHA",
292+
"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA",
293+
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
294+
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
295+
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
296+
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
297+
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
298+
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
299+
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
300+
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
301+
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
302+
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
303+
"TLS_AES_128_GCM_SHA256",
304+
"TLS_AES_256_GCM_SHA384",
305+
"TLS_CHACHA20_POLY1305_SHA256"
306+
]
307+
}
308+
}
309+
},
249310
"grantJwt": {
250311
"type": "object",
251312
"additionalProperties": false,
@@ -361,7 +422,7 @@
361422
}
362423
},
363424
"tls": {
364-
"$ref": "#/definitions/tls_config"
425+
"$ref": "#/definitions/server_tls_config"
365426
}
366427
}
367428
},
@@ -407,7 +468,7 @@
407468
"tls": {
408469
"allOf": [
409470
{
410-
"$ref": "#/definitions/tls_config"
471+
"$ref": "#/definitions/server_tls_config"
411472
},
412473
{
413474
"type": "object",
@@ -424,7 +485,7 @@
424485
}
425486
},
426487
"tls": {
427-
"$ref": "#/definitions/tls_config"
488+
"$ref": "#/definitions/server_tls_config"
428489
},
429490
"cookies": {
430491
"type": "object",
@@ -452,58 +513,19 @@
452513
}
453514
}
454515
},
455-
"client.tls": {
456-
"type": "object",
457-
"additionalProperties": false,
458-
"description": "Configures http client TLS settings.",
459-
"properties": {
460-
"min_version": {
461-
"type": "string",
462-
"description": "Minimum supported TLS version.",
463-
"examples": [
464-
"tls10","tls11","tls12","tls13"
465-
]
466-
},
467-
"max_version": {
468-
"type": "string",
469-
"description": "Maximum supported TLS version.",
470-
"examples": [
471-
"tls10","tls11","tls12","tls13"
472-
]
473-
},
474-
"cipher_suites": {
475-
"type": "array",
476-
"description": "A list of supported cipher suites.",
477-
"items": {
478-
"type": "string"
479-
},
480-
"examples": [
481-
"TLS_RSA_WITH_RC4_128_SHA",
482-
"TLS_RSA_WITH_3DES_EDE_CBC_SHA",
483-
"TLS_RSA_WITH_AES_128_CBC_SHA",
484-
"TLS_RSA_WITH_AES_256_CBC_SHA",
485-
"TLS_RSA_WITH_AES_128_CBC_SHA256",
486-
"TLS_RSA_WITH_AES_128_GCM_SHA256",
487-
"TLS_RSA_WITH_AES_256_GCM_SHA384",
488-
"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA",
489-
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
490-
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
491-
"TLS_ECDHE_RSA_WITH_RC4_128_SHA",
492-
"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA",
493-
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
494-
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
495-
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
496-
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
497-
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
498-
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
499-
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
500-
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
501-
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
502-
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
503-
"TLS_AES_128_GCM_SHA256",
504-
"TLS_AES_256_GCM_SHA384",
505-
"TLS_CHACHA20_POLY1305_SHA256"
506-
]
516+
"client": {
517+
"default": {
518+
"tls": {
519+
"$ref": "#/definitions/client_tls_config"
520+
}
521+
},
522+
"back_channel_logout": {
523+
"tls": {
524+
"allOf": [
525+
{
526+
"$ref": "#/definitions/client_tls_config"
527+
}
528+
]
507529
}
508530
}
509531
},

0 commit comments

Comments
 (0)