diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index f4264e53ce..bebc2c5658 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -29,6 +29,8 @@ import ( "sigs.k8s.io/yaml" ) +var delimiters = []string{",", "|", ";", ":", "#", "\t"} + func Contains(list []string, s string) bool { for _, v := range list { if v == s { @@ -242,6 +244,20 @@ func FormatLabels(m map[string]string) string { return labels.FormatLabels(m) } +// FindAvailableDelimiter returns the first delimiter from a predefined list (comma, pipe, semicolon, +// colon, hash, tab) that is not present in the given string. This is useful for selecting a safe +// separator character when joining values that may themselves contain common delimiters. +// Returns an empty string if all candidate delimiters are already present in the input. +func FindAvailableDelimiter(s string) string { + for _, delimiter := range delimiters { + if !strings.Contains(s, delimiter) { + return delimiter + } + } + + return "" +} + // Whitelists the host. // Sample: Whitelist("che.yourcompany.com") -> ".yourcompany.com" func Whitelist(hostname string) (value string) { diff --git a/pkg/deploy/server/server_configmap.go b/pkg/deploy/server/server_configmap.go index 4ad150e124..17124564aa 100644 --- a/pkg/deploy/server/server_configmap.go +++ b/pkg/deploy/server/server_configmap.go @@ -170,7 +170,9 @@ func (s *CheServerReconciler) getConfigMapData(ctx *chetypes.DeployContext) (che maps.Copy(cheEnv, ctx.CheCluster.Spec.Components.CheServer.ExtraProperties) // Updates `CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION__<...>` - s.updateAdvancedAuthorizationEnv(ctx, cheEnv) + if err := s.updateAdvancedAuthorizationEnv(ctx, cheEnv); err != nil { + return nil, err + } // Updates `CHE_INFRA_KUBERNETES_USER__CLUSTER__ROLES` s.updateUserClusterRoleEnv(ctx, cheEnv) @@ -183,25 +185,43 @@ func (s *CheServerReconciler) getConfigMapData(ctx *chetypes.DeployContext) (che return cheEnv, nil } -func (s *CheServerReconciler) updateAdvancedAuthorizationEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) { +func (s *CheServerReconciler) updateAdvancedAuthorizationEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) error { if ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization != nil { + delimiter := cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER"] + if delimiter == "" { + allFields := strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowUsers, "") + + strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyUsers, "") + + strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowGroups, "") + + strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyGroups, "") + + delimiter = utils.FindAvailableDelimiter(allFields) + } + + if delimiter == "" { + return fmt.Errorf("failed to find an available delimiter for advanced authorization values") + } + cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS"] = strings.Join( ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowUsers, - ",", + delimiter, ) cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__USERS"] = strings.Join( ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyUsers, - ",", + delimiter, ) cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__GROUPS"] = strings.Join( ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowGroups, - ",", + delimiter, ) cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__GROUPS"] = strings.Join( ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyGroups, - ",", + delimiter, ) + + cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER"] = delimiter } + + return nil } func (s *CheServerReconciler) updateUserClusterRoleEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) { diff --git a/pkg/deploy/server/server_configmap_test.go b/pkg/deploy/server/server_configmap_test.go index 54b2ebf0a5..a64772f45c 100644 --- a/pkg/deploy/server/server_configmap_test.go +++ b/pkg/deploy/server/server_configmap_test.go @@ -403,3 +403,147 @@ func TestGetConfigMapDataWithUserClusterRoles(t *testing.T) { }) } } + +func TestUpdateAdvancedAuthorizationEnv(t *testing.T) { + type testCase struct { + name string + cheCluster *chev2.CheCluster + expectedData map[string]string + errorExpected bool + } + + testCases := []testCase{ + { + name: "Test advanced authorization with comma (default) delimiter", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + Name: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Networking: chev2.CheClusterSpecNetworking{ + Auth: chev2.Auth{ + AdvancedAuthorization: &chev2.AdvancedAuthorization{ + AllowUsers: []string{"user1", "user2"}, + }, + }, + }, + }, + }, + expectedData: map[string]string{ + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": "user1,user2", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": ",", + }, + }, + { + name: "Test advanced authorization picks alternative delimiter", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + Name: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Networking: chev2.CheClusterSpecNetworking{ + Auth: chev2.Auth{ + AdvancedAuthorization: &chev2.AdvancedAuthorization{ + AllowUsers: []string{"user1,a", "user2,b"}, + DenyUsers: []string{}, + AllowGroups: []string{}, + DenyGroups: []string{}, + }, + }, + }, + }, + }, + expectedData: map[string]string{ + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": "user1,a|user2,b", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__USERS": "", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__GROUPS": "", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__GROUPS": "", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": "|", + }, + }, + { + name: "Test advanced authorization nil does nothing", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + Name: "eclipse-che", + }, + }, + expectedData: map[string]string{}, + }, + { + name: "Test advanced authorization fails when no delimiter available", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + Name: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Networking: chev2.CheClusterSpecNetworking{ + Auth: chev2.Auth{ + AdvancedAuthorization: &chev2.AdvancedAuthorization{ + AllowUsers: []string{",|;:#\t"}, + DenyUsers: []string{}, + AllowGroups: []string{}, + DenyGroups: []string{}, + }, + }, + }, + }, + }, + errorExpected: true, + }, + { + name: "Test advanced authorization picks up configured delimiter", + cheCluster: &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + Name: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + Components: chev2.CheClusterComponents{ + CheServer: chev2.CheServer{ + ExtraProperties: map[string]string{ + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": "@", + }, + }, + }, + Networking: chev2.CheClusterSpecNetworking{ + Auth: chev2.Auth{ + AdvancedAuthorization: &chev2.AdvancedAuthorization{ + AllowUsers: []string{",|;:#\t", ",|;:#\t"}, + DenyUsers: []string{}, + AllowGroups: []string{}, + DenyGroups: []string{}, + }, + }, + }, + }, + }, + expectedData: map[string]string{ + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": ",|;:#\t@,|;:#\t", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": "@", + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + ctx := test.NewCtxBuilder().WithCheCluster(testCase.cheCluster).Build() + + serverReconciler := NewCheServerReconciler() + cheEnv, err := serverReconciler.getConfigMapData(ctx) + + if testCase.errorExpected { + assert.Error(t, err) + } else { + assert.NoError(t, err) + for k, v := range testCase.expectedData { + assert.Equal(t, v, cheEnv[k], "key: %s", k) + } + } + }) + } +}