K8SPSMDB-1236 Check for custom users name uniqueness (#1813)

Author: gkech

pkg/controller/perconaservermongodb/custom_users.go

@@ -51,10 +51,6 @@ func (r *ReconcilePerconaServerMongoDB) reconcileCustomUsers(ctx context.Context
 
 	handleRoles(ctx, cr, mongoCli)
 
-	if len(cr.Spec.Users) == 0 {
-		return nil
-	}
-
 	err = handleUsers(ctx, cr, mongoCli, r.client)
 	if err != nil {
 		return errors.Wrap(err, "handle users")
@@ -66,39 +62,24 @@ func (r *ReconcilePerconaServerMongoDB) reconcileCustomUsers(ctx context.Context
 func handleUsers(ctx context.Context, cr *api.PerconaServerMongoDB, mongoCli mongo.Client, client client.Client) error {
 	log := logf.FromContext(ctx)
 
-	sysUsersSecret := corev1.Secret{}
-	err := client.Get(ctx,
-		types.NamespacedName{
-			Namespace: cr.Namespace,
-			Name:      api.InternalUserSecretName(cr),
-		},
-		&sysUsersSecret,
-	)
-	if err != nil && !k8serrors.IsNotFound(err) {
-		return errors.Wrap(err, "get internal sys users secret")
+	if len(cr.Spec.Users) == 0 {
+		return nil
 	}
 
-	sysUserNames := sysUserNames(sysUsersSecret)
+	systemUserNames, err := fetchSystemUserNames(ctx, cr, client)
+	if err != nil {
+		return err
+	}
 
-	for _, user := range cr.Spec.Users {
-		if _, ok := sysUserNames[user.Name]; ok {
-			log.Error(nil, "creating user with reserved user name is forbidden", "user", user.Name)
-			continue
-		}
+	uniqueUserNames := make(map[string]struct{}, len(cr.Spec.Users))
 
-		if len(user.Roles) == 0 {
-			log.Error(nil, "user must have at least one role", "user", user.Name)
+	for _, user := range cr.Spec.Users {
+		err := validateUser(&user, systemUserNames, uniqueUserNames)
+		if err != nil {
+			log.Error(err, "invalid user", "user", user)
 			continue
 		}
 
-		if user.DB == "" {
-			user.DB = "admin"
-		}
-
-		if user.PasswordSecretRef != nil && user.PasswordSecretRef.Key == "" {
-			user.PasswordSecretRef.Key = "password"
-		}
-
 		userInfo, err := mongoCli.GetUserInfo(ctx, user.Name, user.DB)
 		if err != nil {
 			log.Error(err, "get user info")
@@ -150,6 +131,49 @@ func handleUsers(ctx context.Context, cr *api.PerconaServerMongoDB, mongoCli mon
 	return nil
 }
 
+func validateUser(user *api.User, sysUserNames, uniqueUserNames map[string]struct{}) error {
+	if sysUserNames == nil || uniqueUserNames == nil {
+		return errors.New("invalid sys or unique usernames config")
+	}
+
+	if _, reserved := sysUserNames[user.Name]; reserved {
+		return fmt.Errorf("creating user with reserved user name %s is forbidden", user.Name)
+	}
+
+	if _, exists := uniqueUserNames[user.Name]; exists {
+		return fmt.Errorf("username %s should be unique", user.Name)
+	}
+	uniqueUserNames[user.Name] = struct{}{}
+
+	if len(user.Roles) == 0 {
+		return fmt.Errorf("user %s must have at least one role", user.Name)
+	}
+
+	if user.DB == "" {
+		user.DB = "admin"
+	}
+
+	if user.PasswordSecretRef != nil && user.PasswordSecretRef.Key == "" {
+		user.PasswordSecretRef.Key = "password"
+	}
+
+	return nil
+}
+
+func fetchSystemUserNames(ctx context.Context, cr *api.PerconaServerMongoDB, client client.Client) (map[string]struct{}, error) {
+	sysUsersSecret := corev1.Secret{}
+	err := client.Get(ctx, types.NamespacedName{
+		Namespace: cr.Namespace,
+		Name:      api.InternalUserSecretName(cr),
+	}, &sysUsersSecret)
+
+	if err != nil && !k8serrors.IsNotFound(err) {
+		return nil, errors.Wrap(err, "get internal sys users secret")
+	}
+
+	return sysUserNames(sysUsersSecret), nil
+}
+
 func handleRoles(ctx context.Context, cr *api.PerconaServerMongoDB, cli mongo.Client) {
 	log := logf.FromContext(ctx)
 	if len(cr.Spec.Roles) == 0 {
@@ -268,15 +292,15 @@ func toMongoRoleModel(role api.Role) (*mongo.Role, error) {
 	return mr, nil
 }
 
-// sysUserNames returns a set of system user names from the sysUsersSecret.
+// sysUserNames returns a set of system usernames from the sysUsersSecret.
 func sysUserNames(sysUsersSecret corev1.Secret) map[string]struct{} {
-	sysUserNames := make(map[string]struct{}, len(sysUsersSecret.Data))
+	names := make(map[string]struct{}, len(sysUsersSecret.Data))
 	for k, v := range sysUsersSecret.Data {
 		if strings.Contains(k, "_USER") {
-			sysUserNames[string(v)] = struct{}{}
+			names[string(v)] = struct{}{}
 		}
 	}
-	return sysUserNames
+	return names
 }
 
 func updatePass(

pkg/controller/perconaservermongodb/custom_users_test.go

@@ -3,6 +3,10 @@ package perconaservermongodb
 import (
 	"testing"
 
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+
+	api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
 	"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo"
 )
 
@@ -232,3 +236,66 @@ func TestRolesChanged(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateUser(t *testing.T) {
+
+	tests := map[string]struct {
+		user            *api.User
+		actualUser      *api.User
+		sysUserNames    map[string]struct{}
+		uniqueUserNames map[string]struct{}
+		expectedErr     error
+	}{
+		"invalid input for sysUserNames and uniqueUserNames": {
+			user:        &api.User{Name: "john", Roles: []api.UserRole{{Name: "rolename", DB: "testdb"}}, DB: "testdb"},
+			expectedErr: errors.New("invalid sys or unique usernames config"),
+		},
+		"valid non-existing username": {
+			user:            &api.User{Name: "john", Roles: []api.UserRole{{Name: "rolename", DB: "testdb"}}, DB: "testdb"},
+			actualUser:      &api.User{Name: "john", Roles: []api.UserRole{{Name: "rolename", DB: "testdb"}}, DB: "testdb"},
+			sysUserNames:    map[string]struct{}{},
+			uniqueUserNames: map[string]struct{}{},
+		},
+		"valid non-existing username, missing db and password secret ref": {
+			user: &api.User{Name: "john", Roles: []api.UserRole{{Name: "rolename"}}, PasswordSecretRef: &api.SecretKeySelector{}},
+			actualUser: &api.User{
+				Name:              "john",
+				Roles:             []api.UserRole{{Name: "rolename"}},
+				DB:                "admin",
+				PasswordSecretRef: &api.SecretKeySelector{Key: "password"},
+			},
+			sysUserNames:    map[string]struct{}{},
+			uniqueUserNames: map[string]struct{}{},
+		},
+		"sys reserved username": {
+			user:            &api.User{Name: "root", Roles: []api.UserRole{{Name: "rolename", DB: "testdb"}}, DB: "testdb"},
+			sysUserNames:    map[string]struct{}{"root": {}},
+			uniqueUserNames: map[string]struct{}{},
+			expectedErr:     errors.New("creating user with reserved user name root is forbidden"),
+		},
+		"not unique username": {
+			user:            &api.User{Name: "useradmin", Roles: []api.UserRole{{Name: "rolename", DB: "testdb"}}, DB: "testdb"},
+			sysUserNames:    map[string]struct{}{},
+			uniqueUserNames: map[string]struct{}{"useradmin": {}},
+			expectedErr:     errors.New("username useradmin should be unique"),
+		},
+		"no roles defined": {
+			user:            &api.User{Name: "john", Roles: []api.UserRole{}, DB: "testdb"},
+			sysUserNames:    map[string]struct{}{},
+			uniqueUserNames: map[string]struct{}{},
+			expectedErr:     errors.New("user john must have at least one role"),
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			err := validateUser(tt.user, tt.sysUserNames, tt.uniqueUserNames)
+			if tt.expectedErr != nil {
+				assert.EqualError(t, err, tt.expectedErr.Error())
+			} else {
+				assert.Equal(t, tt.user, tt.actualUser)
+				assert.NoError(t, err)
+			}
+		})
+	}
+}