diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index abd2735710e72..b1986437936d2 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -2084,6 +2084,12 @@ func isValidResource(resource string) bool { return validResources[resource] } +func isValidObject(proj string, object string) bool { + // match against [/]/ + objectRegexp, err := regexp.Compile(fmt.Sprintf(`^%s(/[*\w-.]+)?/[*\w-.]+$`, regexp.QuoteMeta(proj))) + return objectRegexp.MatchString(object) && err == nil +} + func validatePolicy(proj string, role string, policy string) error { policyComponents := strings.Split(policy, ",") if len(policyComponents) != 6 || strings.Trim(policyComponents[0], " ") != "p" { @@ -2107,9 +2113,8 @@ func validatePolicy(proj string, role string, policy string) error { } // object object := strings.Trim(policyComponents[4], " ") - objectRegexp, err := regexp.Compile(fmt.Sprintf(`^%s/[*\w-.]+$`, regexp.QuoteMeta(proj))) - if err != nil || !objectRegexp.MatchString(object) { - return status.Errorf(codes.InvalidArgument, "invalid policy rule '%s': object must be of form '%s/*' or '%s/', not '%s'", policy, proj, proj, object) + if !isValidObject(proj, object) { + return status.Errorf(codes.InvalidArgument, "invalid policy rule '%s': object must be of form '%s/*', '%s[/]/' or '%s/', not '%s'", policy, proj, proj, proj, object) } // effect effect := strings.Trim(policyComponents[5], " ") diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index 2374f5fb503e6..fb78e4d93cbc4 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -3082,6 +3082,69 @@ func TestOrphanedResourcesMonitorSettings_IsWarn(t *testing.T) { assert.True(t, settings.IsWarn()) } +func Test_isValidPolicy(t *testing.T) { + policyTests := []struct { + name string + policy string + isValid bool + }{ + { + name: "policy with full wildcard", + policy: "some-project/*", + isValid: true, + }, + { + name: "policy with specified project and application", + policy: "some-project/some-application", + isValid: true, + }, + { + name: "policy with full wildcard namespace and application", + policy: "some-project/*/*", + isValid: true, + }, + { + name: "policy with wildcard namespace and specified application", + policy: "some-project/*/some-application", + isValid: true, + }, + { + name: "policy with specified namespace and wildcard application", + policy: "some-project/some-namespace/*", + isValid: true, + }, + { + name: "policy with wildcard prefix namespace and specified application", + policy: "some-project/some-name*/some-application", + isValid: true, + }, + { + name: "policy with specified namespace and wildcard prefixed application", + policy: "some-project/some-namespace/some-app*", + isValid: true, + }, + { + name: "policy with valid namespace and application", + policy: "some-project/some-namespace/some-application", + isValid: true, + }, + { + name: "policy with invalid namespace character", + policy: "some-project/some~namespace/some-application", + isValid: false, + }, + { + name: "policy with invalid application character", + policy: "some-project/some-namespace/some^application", + isValid: false, + }, + } + + for _, policyTest := range policyTests { + assert.Equal(t, policyTest.isValid, isValidObject("some-project", policyTest.policy), policyTest.name) + } +} + func Test_validatePolicy_projIsNotRegex(t *testing.T) { // Make sure the "." in "some.project" isn't treated as the regex wildcard. err := validatePolicy("some.project", "org-admin", "p, proj:some.project:org-admin, applications, *, some-project/*, allow") diff --git a/server/project/project_test.go b/server/project/project_test.go index caf0df9f3ebac..c970c8b20b4d3 100644 --- a/server/project/project_test.go +++ b/server/project/project_test.go @@ -585,7 +585,7 @@ p, role:admin, projects, update, *, allow`) projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projWithRole), enforcer, sync.NewKeyLock(), nil, nil, projInformer, settingsMgr, argoDB) request := &project.ProjectUpdateRequest{Project: projWithRole} _, err := projectServer.Update(context.Background(), request) - assert.Contains(t, err.Error(), "object must be of form 'test/*' or 'test/'") + assert.Contains(t, err.Error(), "object must be of form 'test/*', 'test[/]/' or 'test/'") }) t.Run("TestValidateProjectIncorrectProjectInRoleFailure", func(t *testing.T) {