From 4c98da0d29810e4713b3ecf6a00fc60cdf2ac202 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 31 May 2023 20:50:22 +1000 Subject: [PATCH] test(configuration): add some additional coverage (#5485) Signed-off-by: James Elliott --- internal/configuration/decode_hooks.go | 8 +- internal/configuration/decode_hooks_test.go | 301 ++++++++++++++++-- .../configuration/koanf_callbacks_test.go | 83 +++++ internal/configuration/schema/types_test.go | 15 + internal/configuration/types.go | 2 +- 5 files changed, 384 insertions(+), 25 deletions(-) diff --git a/internal/configuration/decode_hooks.go b/internal/configuration/decode_hooks.go index 2bcfb614c..b07be7412 100644 --- a/internal/configuration/decode_hooks.go +++ b/internal/configuration/decode_hooks.go @@ -634,11 +634,11 @@ func StringToPasswordDigestHookFunc() mapstructure.DecodeHookFuncType { var result *schema.PasswordDigest - if !strings.HasPrefix(dataStr, "$") { - dataStr = fmt.Sprintf(plaintext.EncodingFmt, plaintext.AlgIdentifierPlainText, dataStr) - } - if dataStr != "" { + if !strings.HasPrefix(dataStr, "$") { + dataStr = fmt.Sprintf(plaintext.EncodingFmt, plaintext.AlgIdentifierPlainText, dataStr) + } + if result, err = schema.DecodePasswordDigest(dataStr); err != nil { return nil, fmt.Errorf(errFmtDecodeHookCouldNotParse, dataStr, prefixType, expectedType.String(), err) } diff --git a/internal/configuration/decode_hooks_test.go b/internal/configuration/decode_hooks_test.go index 6a3b9d0df..403481709 100644 --- a/internal/configuration/decode_hooks_test.go +++ b/internal/configuration/decode_hooks_test.go @@ -4,8 +4,10 @@ import ( "bytes" "crypto/ecdsa" "crypto/rsa" + "crypto/tls" "crypto/x509" "encoding/pem" + "fmt" "math" "net/mail" "net/url" @@ -786,24 +788,6 @@ func TestStringToRegexpFuncPointers(t *testing.T) { } func TestStringToAddressHookFunc(t *testing.T) { - mustParseAddress := func(a string) (addr schema.Address) { - addrs, err := schema.NewAddress(a) - if err != nil { - panic(err) - } - - return *addrs - } - - mustParseAddressPtr := func(a string) (addr *schema.Address) { - addr, err := schema.NewAddress(a) - if err != nil { - panic(err) - } - - return addr - } - testCases := []struct { name string have any @@ -814,13 +798,13 @@ func TestStringToAddressHookFunc(t *testing.T) { { name: "ShouldDecodeNonPtr", have: "tcp://0.0.0.0:2020", - expected: mustParseAddress("tcp://0.0.0.0:2020"), + expected: MustParseAddress("tcp://0.0.0.0:2020"), decode: true, }, { name: "ShouldDecodePtr", have: "tcp://0.0.0.0:2020", - expected: mustParseAddressPtr("tcp://0.0.0.0:2020"), + expected: MustParseAddressPtr("tcp://0.0.0.0:2020"), decode: true, }, { @@ -872,6 +856,90 @@ func TestStringToAddressHookFunc(t *testing.T) { err: "could not decode 'tcp://&!@^#*&!@#&*@!:2020' to a schema.Address: could not parse string 'tcp://&!@^#*&!@#&*@!:2020' as address: expected format is [://][:]: parse \"tcp://&!@^\": invalid character \"^\" in host name", decode: false, }, + { + name: "ShouldDecodeTCP", + have: "tcp://127.0.0.1", + expected: schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeTCPPtr", + have: "tcp://127.0.0.1", + expected: &schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeUDP", + have: "udp://127.0.0.1", + expected: schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeUDPPtr", + have: "udp://127.0.0.1", + expected: &schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeLDAP", + have: "ldap://127.0.0.1", + expected: schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeLDAPPtr", + have: "ldap://127.0.0.1", + expected: &schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeSMTP", + have: "smtp://127.0.0.1", + expected: schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldDecodeSMTPPtr", + have: "smtp://127.0.0.1", + expected: &schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")}, + err: "", + decode: true, + }, + { + name: "ShouldFailDecodeTCP", + have: "@@@@@@@", + expected: schema.AddressTCP{Address: MustParseAddress("tcp://127.0.0.1")}, + err: "could not decode '@@@@@@@' to a schema.AddressTCP: error validating the address: the url 'tcp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses", + decode: false, + }, + { + name: "ShouldFailDecodeUDP", + have: "@@@@@@@", + expected: schema.AddressUDP{Address: MustParseAddress("udp://127.0.0.1")}, + err: "could not decode '@@@@@@@' to a schema.AddressUDP: error validating the address: the url 'udp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses", + decode: false, + }, + { + name: "ShouldFailDecodeLDAP", + have: "@@@@@@@", + expected: schema.AddressLDAP{Address: MustParseAddress("ldap://127.0.0.1")}, + err: "could not decode '@@@@@@@' to a schema.AddressLDAP: error validating the address: the url 'ldaps://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses", + decode: false, + }, + { + name: "ShouldFailDecodeSMTP", + have: "@@@@@@@", + expected: schema.AddressSMTP{Address: MustParseAddress("smtp://127.0.0.1")}, + err: "could not decode '@@@@@@@' to a schema.AddressSMTP: error validating the address: the url 'smtp://%40%40%40%40%40%40@' appears to have user info but this is not valid for addresses", + decode: false, + }, } hook := configuration.StringToAddressHookFunc() @@ -1095,6 +1163,160 @@ func TestStringToX509CertificateHookFunc(t *testing.T) { } } +func TestStringToPasswordDigestHookFunc(t *testing.T) { + var nilvalue *schema.PasswordDigest + + testCases := []struct { + name string + have any + expected any + err string + decode bool + }{ + { + "ShouldParse", + "$plaintext$example", + MustParsePasswordDigest("$plaintext$example"), + "", + true, + }, + { + "ShouldParsePtr", + "$plaintext$example", + MustParsePasswordDigestPtr("$plaintext$example"), + "", + true, + }, + { + "ShouldNotParseUnknown", + "$abc$example", + schema.PasswordDigest{}, + "could not decode '$abc$example' to a schema.PasswordDigest: provided encoded hash has an invalid identifier: the identifier 'abc' is unknown to the decoder", + false, + }, + { + "ShouldNotParseWrongType", + "$abc$example", + schema.TLSVersion{}, + "", + false, + }, + { + "ShouldNotParseWrongTypePtr", + "$abc$example", + &schema.TLSVersion{}, + "", + false, + }, + { + "ShouldNotParseEmptyString", + "", + schema.PasswordDigest{}, + "could not decode an empty value to a schema.PasswordDigest: must have a non-empty value", + false, + }, + { + "ShouldParseEmptyStringPtr", + "", + nilvalue, + "", + true, + }, + } + + hook := configuration.StringToPasswordDigestHookFunc() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual, err := hook(reflect.TypeOf(tc.have), reflect.TypeOf(tc.expected), tc.have) + if tc.err != "" { + assert.EqualError(t, err, tc.err) + + if !tc.decode { + assert.Nil(t, actual) + } + } else { + assert.NoError(t, err) + + if tc.decode { + assert.Equal(t, tc.expected, actual) + } else { + assert.Equal(t, tc.have, actual) + } + } + }) + } +} + +func TestStringToTLSVersionHookFunc(t *testing.T) { + testCases := []struct { + name string + have any + expected any + err string + decode bool + }{ + { + "ShouldParseTLS1.3", + "TLS1.3", + schema.TLSVersion{Value: tls.VersionTLS13}, + "", + true, + }, + { + "ShouldParseTLS1.3PTR", + "TLS1.3", + &schema.TLSVersion{Value: tls.VersionTLS13}, + "", + true, + }, + { + "ShouldParseTLS1.2", + "TLS1.2", + schema.TLSVersion{Value: tls.VersionTLS12}, + "", + true, + }, + { + "ShouldNotParseInt", + 1, + &schema.TLSVersion{}, + "", + false, + }, + { + "ShouldNotParseNonVersion", + "1", + &schema.TLSVersion{}, + "could not decode '1' to a *schema.TLSVersion: supplied tls version isn't supported", + false, + }, + } + + hook := configuration.StringToTLSVersionHookFunc() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual, err := hook(reflect.TypeOf(tc.have), reflect.TypeOf(tc.expected), tc.have) + if tc.err != "" { + assert.EqualError(t, err, tc.err) + + if !tc.decode { + assert.Nil(t, actual) + } + } else { + assert.NoError(t, err) + + if tc.decode { + assert.Equal(t, tc.expected, actual) + } else { + assert.Equal(t, tc.have, actual) + } + } + }) + } +} + func TestStringToX509CertificateChainHookFunc(t *testing.T) { var nilkey *schema.X509CertificateChain @@ -1411,3 +1633,42 @@ var ( testZero int32 testString = "" ) + +func MustParseAddress(input string) schema.Address { + address, err := schema.NewAddress(input) + if err != nil { + panic(err) + } + + fmt.Println(address.String()) + addr := *address + + return addr +} + +func MustParseAddressPtr(input string) *schema.Address { + address, err := schema.NewAddress(input) + if err != nil { + panic(err) + } + + return address +} + +func MustParsePasswordDigest(input string) schema.PasswordDigest { + digest, err := schema.DecodePasswordDigest(input) + if err != nil { + panic(err) + } + + return *digest +} + +func MustParsePasswordDigestPtr(input string) *schema.PasswordDigest { + digest, err := schema.DecodePasswordDigest(input) + if err != nil { + panic(err) + } + + return digest +} diff --git a/internal/configuration/koanf_callbacks_test.go b/internal/configuration/koanf_callbacks_test.go index 5ea5ec914..59d873062 100644 --- a/internal/configuration/koanf_callbacks_test.go +++ b/internal/configuration/koanf_callbacks_test.go @@ -6,6 +6,7 @@ import ( "runtime" "testing" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -125,3 +126,85 @@ func TestKoanfSecretCallbackShouldErrorOnFSError(t *testing.T) { assert.Len(t, val.Warnings(), 0) assert.EqualError(t, val.Errors()[0], fmt.Sprintf("secrets: error loading secret path %s into key 'theme': file permission error occurred: open %s: permission denied", secret, secret)) } + +func TestKoanfCommandLineWithMappingCallback(t *testing.T) { + testCases := []struct { + name string + have []string + flagName string + flagValue string + mapped string + valid bool + unchanged bool + expectedName string + expectedValue any + }{ + { + "ShouldDecodeStandard", + []string{"--commands", "abc"}, + "commands", + "", + "command.another", + false, + false, + "command.another", + "abc", + }, + { + "ShouldSkipUnchangedKey", + []string{}, + "commands", + "abc", + "command.another", + false, + false, + "", + nil, + }, + { + "ShouldLookupNormalizedKey", + []string{"--log.file-path", "abc"}, + "log.file-path", + "", + "", + true, + false, + "log.file_path", + "abc", + }, + { + "ShouldReturnUnmodified", + []string{"--commands", "abc"}, + "commands", + "", + "", + false, + false, + "", + nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + flagset := pflag.NewFlagSet("test", pflag.ContinueOnError) + + flagset.String(tc.flagName, tc.flagValue, "") + + assert.NoError(t, flagset.Parse(tc.have)) + + mapper := map[string]string{} + + if tc.mapped != "" { + mapper[tc.flagName] = tc.mapped + } + + callback := koanfCommandLineWithMappingCallback(mapper, tc.valid, tc.unchanged) + + actualName, actualValue := callback(flagset.Lookup(tc.flagName)) + + assert.Equal(t, tc.expectedName, actualName) + assert.Equal(t, tc.expectedValue, actualValue) + }) + } +} diff --git a/internal/configuration/schema/types_test.go b/internal/configuration/schema/types_test.go index 8a19edf36..742718acd 100644 --- a/internal/configuration/schema/types_test.go +++ b/internal/configuration/schema/types_test.go @@ -268,6 +268,21 @@ func TestX509CertificateChain(t *testing.T) { assert.Regexp(t, regexp.MustCompile(`^certificate #1 in chain is invalid before 13569465600 but the time is \d+$`), err.Error()) } +func TestPasswordDigest_IsPlainText(t *testing.T) { + digest, err := DecodePasswordDigest("$plaintext$exam") + assert.NoError(t, err) + assert.True(t, digest.IsPlainText()) + + digest = &PasswordDigest{} + + assert.False(t, digest.IsPlainText()) + + digest, err = DecodePasswordDigest("$pbkdf2-sha512$310000$c8p78n7pUMln0jzvd4aK4Q$JNRBzwAo0ek5qKn50cFzzvE9RXV88h1wJn5KGiHrD0YKtZaR/nCb2CJPOsKaPK0hjf.9yHxzQGZziziccp6Yng") + assert.NoError(t, err) + + assert.False(t, digest.IsPlainText()) +} + func MustParseX509CertificateChain(data string) *X509CertificateChain { chain, err := NewX509CertificateChain(data) if err != nil { diff --git a/internal/configuration/types.go b/internal/configuration/types.go index 7a3b1822c..198e829a1 100644 --- a/internal/configuration/types.go +++ b/internal/configuration/types.go @@ -21,7 +21,7 @@ type FileSource struct { filters []FileFilter } -// EnvironmentSource is a configuration configuration.Source which loads values from the environment. +// EnvironmentSource is a configuration.Source which loads values from the environment. type EnvironmentSource struct { koanf *koanf.Koanf prefix string