From 4f89ca6ff5253605a47fcd422701dd49981e146a Mon Sep 17 00:00:00 2001 From: Rodrigo Valin Date: Fri, 4 Feb 2022 15:34:54 +0000 Subject: [PATCH 1/2] Fix Secret creation with username with _. --- pkg/authentication/scram/scram.go | 4 +++- pkg/authentication/scram/scram_test.go | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/authentication/scram/scram.go b/pkg/authentication/scram/scram.go index 92e1fd21d..24d29d9e9 100644 --- a/pkg/authentication/scram/scram.go +++ b/pkg/authentication/scram/scram.go @@ -3,6 +3,7 @@ package scram import ( "encoding/base64" "fmt" + "strings" "github.com/pkg/errors" @@ -339,5 +340,6 @@ func convertMongoDBUserToAutomationConfigUser(secretGetUpdateCreateDeleter secre // GetConnectionStringSecretName returns the name of the secret where the operator stores the connection string for current user func (u User) GetConnectionStringSecretName(mdb Configurable) string { - return fmt.Sprintf("%s-%s-%s", mdb.NamespacedName().Name, u.Database, u.Username) + username := strings.ReplaceAll(u.Username, "_", "-") + return fmt.Sprintf("%s-%s-%s", mdb.NamespacedName().Name, u.Database, username) } diff --git a/pkg/authentication/scram/scram_test.go b/pkg/authentication/scram/scram_test.go index c43c41647..595c3976d 100644 --- a/pkg/authentication/scram/scram_test.go +++ b/pkg/authentication/scram/scram_test.go @@ -48,10 +48,17 @@ func newMockedSecretGetUpdateCreateDeleter(secrets ...corev1.Secret) secret.GetU } return mockSecretGetUpdateCreateDeleter } + func notFoundError() error { return &errors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonNotFound}} } +func TestUsernameCanHaveAnUnderscore(t *testing.T) { + user := buildMongoDBUser("name_with_underscores") + mdb := buildConfigurable("mdb") + assert.Equal(t, "mdb-admin-name-with-underscores-user", user.GetConnectionStringSecretName(mdb)) +} + func TestReadExistingCredentials(t *testing.T) { mdbObjectKey := types.NamespacedName{Name: "mdb-0", Namespace: "default"} user := buildMongoDBUser("mdbuser-0") @@ -74,7 +81,6 @@ func TestReadExistingCredentials(t *testing.T) { _, _, err := readExistingCredentials(newMockedSecretGetUpdateCreateDeleter(scramCredsSecret), mdbObjectKey, "different-username") assert.Error(t, err) }) - } func TestComputeScramCredentials_ComputesSameStoredAndServerKey_WithSameSalt(t *testing.T) { @@ -136,7 +142,6 @@ func TestEnsureScramCredentials(t *testing.T) { assert.NotEmpty(t, scram256Creds.ServerKey) assert.Equal(t, 15000, scram256Creds.IterationCount) }) - } func TestConvertMongoDBUserToAutomationConfigUser(t *testing.T) { From 3acfc4f0e76ca8919ddedb309f6f421285501fa2 Mon Sep 17 00:00:00 2001 From: Rodrigo Valin Date: Mon, 7 Feb 2022 10:08:23 +0000 Subject: [PATCH 2/2] More rigorous conversion. --- pkg/authentication/scram/scram.go | 23 ++++++++++++++++++++--- pkg/authentication/scram/scram_test.go | 10 +++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/pkg/authentication/scram/scram.go b/pkg/authentication/scram/scram.go index 24d29d9e9..d54b85ffb 100644 --- a/pkg/authentication/scram/scram.go +++ b/pkg/authentication/scram/scram.go @@ -3,6 +3,7 @@ package scram import ( "encoding/base64" "fmt" + "regexp" "strings" "github.com/pkg/errors" @@ -338,8 +339,24 @@ func convertMongoDBUserToAutomationConfigUser(secretGetUpdateCreateDeleter secre return acUser, nil } -// GetConnectionStringSecretName returns the name of the secret where the operator stores the connection string for current user +// GetConnectionStringSecretName returns the name of the secret where the +// operator stores the connection string for current user. func (u User) GetConnectionStringSecretName(mdb Configurable) string { - username := strings.ReplaceAll(u.Username, "_", "-") - return fmt.Sprintf("%s-%s-%s", mdb.NamespacedName().Name, u.Database, username) + return fmt.Sprintf("%s-%s-%s", mdb.NamespacedName().Name, u.Database, normalizeUsername(u.Username)) +} + +// normalizeUsername returns a string that conforms to RFC-1123, by replacing +// non-allowed characters with `-`. +// +// The MongoDB username can contain the chars in `acceptedChars` variable, as +// documented here: https://docs.mongodb.com/manual/reference/connection-string/. +func normalizeUsername(username string) string { + acceptedChars := `[:\/\?#\[\]@_]` + re := regexp.MustCompile(acceptedChars) + username = re.ReplaceAllString(username, "-") + + // Remove duplicate `-` resulting from contiguous non-allowed chars. + re = regexp.MustCompile(`\-+`) + username = re.ReplaceAllString(username, "-") + return strings.Trim(username, "-") } diff --git a/pkg/authentication/scram/scram_test.go b/pkg/authentication/scram/scram_test.go index 595c3976d..93f168a57 100644 --- a/pkg/authentication/scram/scram_test.go +++ b/pkg/authentication/scram/scram_test.go @@ -53,10 +53,14 @@ func notFoundError() error { return &errors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonNotFound}} } -func TestUsernameCanHaveAnUnderscore(t *testing.T) { - user := buildMongoDBUser("name_with_underscores") +func TestUsernameIsTransformedAndValid(t *testing.T) { + user := buildMongoDBUser("name_with@weird?chars") mdb := buildConfigurable("mdb") - assert.Equal(t, "mdb-admin-name-with-underscores-user", user.GetConnectionStringSecretName(mdb)) + assert.Equal(t, "mdb-admin-name-with-weird-chars-user", user.GetConnectionStringSecretName(mdb)) +} + +func TestUsernameCanHaveAn(t *testing.T) { + assert.Equal(t, "normalize-username-with-no-allowed-chars-only", normalizeUsername("?_normalize/_-username/?@with/[]?no]?/:allowed:chars[only?")) } func TestReadExistingCredentials(t *testing.T) {