From a5f8e1510e0c11ae36ba8b2552ee69e96e867ee8 Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Thu, 31 Oct 2024 18:10:58 +0000 Subject: [PATCH 1/3] Enable fine-grained update/delete RBAC enforcement by default (#19988) Change applications resource RBAC to use fine-grained update/delete enforcement by default. This allows us to enforce RBAC on the application itself, separately from the sub-resources related to it. (see also #18124, #20600) Signed-off-by: Matt Finkel --- assets/builtin-policy.csv | 4 +- docs/operator-manual/rbac.md | 15 +-- server/application/application.go | 6 +- server/application/application_test.go | 159 ++++++++++++++++++++++++- 4 files changed, 166 insertions(+), 18 deletions(-) diff --git a/assets/builtin-policy.csv b/assets/builtin-policy.csv index 81c8ca5092cb4..8c0cf2c25bfb1 100644 --- a/assets/builtin-policy.csv +++ b/assets/builtin-policy.csv @@ -17,7 +17,9 @@ p, role:readonly, logs, get, */*, allow p, role:admin, applications, create, */*, allow p, role:admin, applications, update, */*, allow +p, role:admin, applications, update/*, */*, allow p, role:admin, applications, delete, */*, allow +p, role:admin, applications, delete/*, */*, allow p, role:admin, applications, sync, */*, allow p, role:admin, applications, override, */*, allow p, role:admin, applications, action/*, */*, allow @@ -43,4 +45,4 @@ p, role:admin, gpgkeys, delete, *, allow p, role:admin, exec, create, */*, allow g, role:admin, role:readonly -g, admin, role:admin \ No newline at end of file +g, admin, role:admin diff --git a/docs/operator-manual/rbac.md b/docs/operator-manual/rbac.md index 63e71c67f001c..7637447fb334f 100644 --- a/docs/operator-manual/rbac.md +++ b/docs/operator-manual/rbac.md @@ -114,7 +114,7 @@ The `applications` resource is an [Application-Specific Policy](#application-spe #### Fine-grained Permissions for `update`/`delete` action -The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself **and** all of its resources. +The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself but not its resources. It can be desirable to only allow `update` or `delete` on specific resources within an application. To do so, when the action if performed on an application's resource, the `` will have the `////` format. @@ -148,15 +148,12 @@ p, example-user, applications, delete, default/prod-app, deny p, example-user, applications, delete/*/Pod/*/*, default/prod-app, allow ``` -!!! note - - It is not possible to deny fine-grained permissions for a sub-resource if the action was **explicitly allowed on the application**. - For instance, the following policies will **allow** a user to delete the Pod and any other resources in the application: +If we want to explicitly allow updates to the application, but deny updates to any sub-resources: - ```csv - p, example-user, applications, delete, default/prod-app, allow - p, example-user, applications, delete/*/Pod/*/*, default/prod-app, deny - ``` +```csv +p, example-user, applications, update, default/prod-app, allow +p, example-user, applications, update/*, default/prod-app, deny +``` #### The `action` action diff --git a/server/application/application.go b/server/application/application.go index de961c82ea5f7..f6cad80b542f1 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -1333,12 +1333,10 @@ func (s *Server) getAppResources(ctx context.Context, a *appv1.Application) (*ap } func (s *Server) getAppLiveResource(ctx context.Context, action string, q *application.ApplicationResourceRequest) (*appv1.ResourceNode, *rest.Config, *appv1.Application, error) { - a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) - if err != nil && errors.Is(err, permissionDeniedErr) && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) { - // If users dont have permission on the whole applications, maybe they have fine-grained access to the specific resources + if action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate { action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName()) - a, _, err = s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) } + a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, nil, nil, err } diff --git a/server/application/application_test.go b/server/application/application_test.go index a030131f679a9..3b579d8e9b142 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -1669,7 +1669,7 @@ func TestDeleteResourcesRBAC(t *testing.T) { p, test-user, applications, delete, default/test-app, allow `) _, err := appServer.DeleteResource(ctx, &req) - assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) }) t.Run("delete with application permission but deny subresource", func(t *testing.T) { @@ -1678,7 +1678,7 @@ p, test-user, applications, delete, default/test-app, allow p, test-user, applications, delete/*, default/test-app, deny `) _, err := appServer.DeleteResource(ctx, &req) - assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) }) t.Run("delete with subresource", func(t *testing.T) { @@ -1732,7 +1732,7 @@ func TestPatchResourcesRBAC(t *testing.T) { p, test-user, applications, update, default/test-app, allow `) _, err := appServer.PatchResource(ctx, &req) - assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) }) t.Run("patch with application permission but deny subresource", func(t *testing.T) { @@ -1741,7 +1741,7 @@ p, test-user, applications, update, default/test-app, allow p, test-user, applications, update/*, default/test-app, deny `) _, err := appServer.PatchResource(ctx, &req) - assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) }) t.Run("patch with subresource", func(t *testing.T) { @@ -1771,6 +1771,157 @@ p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny }) } +func TestUpdateApplicationRBAC(t *testing.T) { + ctx := context.Background() + // nolint:staticcheck + ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) + testApp := newTestApp() + appServer := newTestAppServer(t, testApp) + appServer.enf.SetDefaultRole("") + testApp.Spec.Project = "" + + appSpecReq := application.ApplicationUpdateSpecRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Spec: &testApp.Spec, + } + + t.Run("can update application spec with update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, */*, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + }) + + t.Run("cannot update application spec with sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + resourceReq := application.ApplicationResourcePatchRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Group: strToPtr("fake.io"), + Kind: strToPtr("PodTest"), + Namespace: strToPtr("fake-ns"), + ResourceName: strToPtr("my-pod-test"), + } + + expectedErrorWhenUpdateAllowed := "rpc error: code = InvalidArgument desc = PodTest fake.io my-pod-test not found as part of application test-app" + + t.Run("can update application spec with update allow, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + t.Run("can update application spec with update allow, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application spec with update deny, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application spec with update deny, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + appReq := application.ApplicationUpdateRequest{ + Application: testApp, + } + + t.Run("update application with generic permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + }) + + t.Run("cannot update application with sub-resource permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + t.Run("can update application with update allow, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + t.Run("can update application with update allow, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application with update deny, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application with update deny, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) +} + func TestSyncAndTerminate(t *testing.T) { ctx := context.Background() appServer := newTestAppServer(t) From bfb52e4ac2da9e4b3c1cdce2781b05e78ca71c4e Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Mon, 4 Nov 2024 18:49:32 +0000 Subject: [PATCH 2/3] Add config setting to preserve V2 update/delete RBAC (#20600) A breaking change was introduced in a previous commit that is planned to be a part of the next major version of Argo CD (v3) where it's okay to introduce breaking changes. We want this feature before we hit v3, so we add a config setting that allows us to explicitly turn this new v3 behavior on in v2. The current v2 behavior is the default, so this change will not affect folks who do not explicitly opt in. This commit to add the gating code is added separately so it will be easy to either cherry pick that pervious commit or revert this one. (see also #18124, #19988) Signed-off-by: Matt Finkel --- docs/operator-manual/rbac.md | 35 ++- server/application/application.go | 11 +- server/application/application_test.go | 301 ++++++++++++++++++++++++- util/settings/settings.go | 15 ++ 4 files changed, 352 insertions(+), 10 deletions(-) diff --git a/docs/operator-manual/rbac.md b/docs/operator-manual/rbac.md index 7637447fb334f..5bf2f56cc6217 100644 --- a/docs/operator-manual/rbac.md +++ b/docs/operator-manual/rbac.md @@ -114,7 +114,7 @@ The `applications` resource is an [Application-Specific Policy](#application-spe #### Fine-grained Permissions for `update`/`delete` action -The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself but not its resources. +The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself **and** all of its resources. It can be desirable to only allow `update` or `delete` on specific resources within an application. To do so, when the action if performed on an application's resource, the `` will have the `////` format. @@ -148,12 +148,35 @@ p, example-user, applications, delete, default/prod-app, deny p, example-user, applications, delete/*/Pod/*/*, default/prod-app, allow ``` -If we want to explicitly allow updates to the application, but deny updates to any sub-resources: +!!! note -```csv -p, example-user, applications, update, default/prod-app, allow -p, example-user, applications, update/*, default/prod-app, deny -``` + It is not possible to deny fine-grained permissions for a sub-resource if the action was **explicitly allowed on the application**. + For instance, the following policies will **allow** a user to delete the Pod and any other resources in the application: + + ```csv + p, example-user, applications, delete, default/prod-app, allow + p, example-user, applications, delete/*/Pod/*, default/prod-app, deny + ``` + +!!! note + + In v3, RBAC will have a breaking change. The `update` and `delete` actions + (without a `/*`) will no longer include sub-resources. This allows you to + explicitly allow or deny access to an application without affecting its + sub-resources. For example, you may want to allow enable/disable of auto-sync + by allowing update on an application, but disallow the editing of deployment + manifests for that application. + + To enable this behavior before v3, you can set the config value + `server.rbac.enablev3` to `true` in the Argo CD ConfigMap argocd-cm. + + Once you do so, you can explicitly allow updates to the application, but deny + updates to any sub-resources: + + ```csv + p, example-user, applications, update, default/prod-app, allow + p, example-user, applications, update/*, default/prod-app, deny + ``` #### The `action` action diff --git a/server/application/application.go b/server/application/application.go index f6cad80b542f1..94349afaedf3b 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -1333,10 +1333,19 @@ func (s *Server) getAppResources(ctx context.Context, a *appv1.Application) (*ap } func (s *Server) getAppLiveResource(ctx context.Context, action string, q *application.ApplicationResourceRequest) (*appv1.ResourceNode, *rest.Config, *appv1.Application, error) { - if action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate { + enableV3, err := s.settingsMgr.GetServerRBACEnableV3() + if err != nil { + return nil, nil, nil, err + } + + if enableV3 && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) { action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName()) } a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) + if !enableV3 && err != nil && errors.Is(err, permissionDeniedErr) && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) { + action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName()) + a, _, err = s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) + } if err != nil { return nil, nil, nil, err } diff --git a/server/application/application_test.go b/server/application/application_test.go index 3b579d8e9b142..62d7a924caa42 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -1650,7 +1650,12 @@ func TestDeleteResourcesRBAC(t *testing.T) { // nolint:staticcheck ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) testApp := newTestApp() - appServer := newTestAppServer(t, testApp) + f := func(enf *rbac.Enforcer) { + _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) + enf.SetDefaultRole("role:admin") + } + argoCM := map[string]string{"server.rbac.enablev3": "true"} + appServer := newTestAppServerWithEnforcerConfigure(t, f, argoCM, testApp) appServer.enf.SetDefaultRole("") req := application.ApplicationResourceDeleteRequest{ @@ -1708,7 +1713,7 @@ p, test-user, applications, delete/fake.io/PodTest/*, default/test-app, deny }) } -func TestPatchResourcesRBAC(t *testing.T) { +func TestDeleteResourcesRBACV2(t *testing.T) { ctx := context.Background() // nolint:staticcheck ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) @@ -1716,6 +1721,74 @@ func TestPatchResourcesRBAC(t *testing.T) { appServer := newTestAppServer(t, testApp) appServer.enf.SetDefaultRole("") + req := application.ApplicationResourceDeleteRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Group: strToPtr("fake.io"), + Kind: strToPtr("PodTest"), + Namespace: strToPtr("fake-ns"), + ResourceName: strToPtr("my-pod-test"), + } + + expectedErrorWhenDeleteAllowed := "rpc error: code = InvalidArgument desc = PodTest fake.io my-pod-test not found as part of application test-app" + + t.Run("delete with application permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, delete, default/test-app, allow +`) + _, err := appServer.DeleteResource(ctx, &req) + assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + }) + + t.Run("delete with application permission but deny subresource", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, delete, default/test-app, allow +p, test-user, applications, delete/*, default/test-app, deny +`) + _, err := appServer.DeleteResource(ctx, &req) + assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + }) + + t.Run("delete with subresource", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, delete/*, default/test-app, allow +`) + _, err := appServer.DeleteResource(ctx, &req) + assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + }) + + t.Run("delete with subresource but deny applications", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, delete, default/test-app, deny +p, test-user, applications, delete/*, default/test-app, allow +`) + _, err := appServer.DeleteResource(ctx, &req) + assert.Equal(t, expectedErrorWhenDeleteAllowed, err.Error()) + }) + + t.Run("delete with specific subresource denied", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, delete/*, default/test-app, allow +p, test-user, applications, delete/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.DeleteResource(ctx, &req) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) +} + +func TestPatchResourcesRBAC(t *testing.T) { + ctx := context.Background() + // nolint:staticcheck + ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) + testApp := newTestApp() + f := func(enf *rbac.Enforcer) { + _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) + enf.SetDefaultRole("role:admin") + } + argoCM := map[string]string{"server.rbac.enablev3": "true"} + appServer := newTestAppServerWithEnforcerConfigure(t, f, argoCM, testApp) + appServer.enf.SetDefaultRole("") + req := application.ApplicationResourcePatchRequest{ Name: &testApp.Name, AppNamespace: &testApp.Namespace, @@ -1771,7 +1844,70 @@ p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny }) } -func TestUpdateApplicationRBAC(t *testing.T) { +func TestPatchResourcesRBACV2(t *testing.T) { + ctx := context.Background() + // nolint:staticcheck + ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) + testApp := newTestApp() + appServer := newTestAppServer(t, testApp) + appServer.enf.SetDefaultRole("") + + req := application.ApplicationResourcePatchRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Group: strToPtr("fake.io"), + Kind: strToPtr("PodTest"), + Namespace: strToPtr("fake-ns"), + ResourceName: strToPtr("my-pod-test"), + } + + expectedErrorWhenUpdateAllowed := "rpc error: code = InvalidArgument desc = PodTest fake.io my-pod-test not found as part of application test-app" + + t.Run("patch with application permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +`) + _, err := appServer.PatchResource(ctx, &req) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("patch with application permission but deny subresource", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/*, default/test-app, deny +`) + _, err := appServer.PatchResource(ctx, &req) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("patch with subresource", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.PatchResource(ctx, &req) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("patch with subresource but deny applications", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.PatchResource(ctx, &req) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("patch with specific subresource denied", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.PatchResource(ctx, &req) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) +} + +func TestUpdateApplicationRBACV2(t *testing.T) { ctx := context.Background() // nolint:staticcheck ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) @@ -1813,6 +1949,165 @@ p, test-user, applications, update/*, default/test-app, allow expectedErrorWhenUpdateAllowed := "rpc error: code = InvalidArgument desc = PodTest fake.io my-pod-test not found as part of application test-app" + t.Run("can update application spec and resource with update allow, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("can update application spec with update allow, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application spec with update deny, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application spec with update deny, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + appReq := application.ApplicationUpdateRequest{ + Application: testApp, + } + + t.Run("update application with generic permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + }) + + t.Run("cannot update application with sub-resource permission", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + t.Run("can update application with update allow, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("can update application with update allow, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, allow +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + require.NoError(t, err) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application with update deny, sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, allow +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, expectedErrorWhenUpdateAllowed, err.Error()) + }) + + t.Run("cannot update application with update deny, sub-resource update deny", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, default/test-app, deny +p, test-user, applications, update/fake.io/PodTest/*, default/test-app, deny +`) + _, err := appServer.Update(ctx, &appReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + _, err = appServer.PatchResource(ctx, &resourceReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) +} + +// this test tests the V3 RBAC functionality +func TestUpdateApplicationRBAC(t *testing.T) { + ctx := context.Background() + // nolint:staticcheck + ctx = context.WithValue(ctx, "claims", &jwt.RegisteredClaims{Subject: "test-user"}) + testApp := newTestApp() + f := func(enf *rbac.Enforcer) { + _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) + enf.SetDefaultRole("role:admin") + } + argoCM := map[string]string{"server.rbac.enablev3": "true"} + appServer := newTestAppServerWithEnforcerConfigure(t, f, argoCM, testApp) + appServer.enf.SetDefaultRole("") + testApp.Spec.Project = "" + + // TODO someohow set v3 + + appSpecReq := application.ApplicationUpdateSpecRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Spec: &testApp.Spec, + } + + t.Run("can update application spec with update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update, */*, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + require.NoError(t, err) + }) + + t.Run("cannot update application spec with sub-resource update allow", func(t *testing.T) { + _ = appServer.enf.SetBuiltinPolicy(` +p, test-user, applications, update/*, default/test-app, allow +`) + _, err := appServer.UpdateSpec(ctx, &appSpecReq) + assert.Equal(t, codes.PermissionDenied.String(), status.Code(err).String()) + }) + + resourceReq := application.ApplicationResourcePatchRequest{ + Name: &testApp.Name, + AppNamespace: &testApp.Namespace, + Group: strToPtr("fake.io"), + Kind: strToPtr("PodTest"), + Namespace: strToPtr("fake-ns"), + ResourceName: strToPtr("my-pod-test"), + } + + expectedErrorWhenUpdateAllowed := "rpc error: code = InvalidArgument desc = PodTest fake.io my-pod-test not found as part of application test-app" + t.Run("can update application spec with update allow, sub-resource update deny", func(t *testing.T) { _ = appServer.enf.SetBuiltinPolicy(` p, test-user, applications, update, default/test-app, allow diff --git a/util/settings/settings.go b/util/settings/settings.go index 516116f0ecd99..adf489c9246af 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -511,6 +511,8 @@ const ( inClusterEnabledKey = "cluster.inClusterEnabled" // settingsServerRBACLogEnforceEnable is the key to configure whether logs RBAC enforcement is enabled settingsServerRBACLogEnforceEnableKey = "server.rbac.log.enforce.enable" + // settingsServerRBACEnableV3Key is the key to configure V3 RBAC enforcement + settingsServerRBACEnableV3Key = "server.rbac.enablev3" // MaxPodLogsToRender the maximum number of pod logs to render settingsMaxPodLogsToRender = "server.maxPodLogsToRender" // helmValuesFileSchemesKey is the key to configure the list of supported helm values file schemas @@ -832,6 +834,19 @@ func (mgr *SettingsManager) GetServerRBACLogEnforceEnable() (bool, error) { return strconv.ParseBool(argoCDCM.Data[settingsServerRBACLogEnforceEnableKey]) } +func (mgr *SettingsManager) GetServerRBACEnableV3() (bool, error) { + argoCDCM, err := mgr.getConfigMap() + if err != nil { + return false, err + } + + if argoCDCM.Data[settingsServerRBACEnableV3Key] == "" { + return false, nil + } + + return strconv.ParseBool(argoCDCM.Data[settingsServerRBACEnableV3Key]) +} + func (mgr *SettingsManager) GetMaxPodLogsToRender() (int64, error) { argoCDCM, err := mgr.getConfigMap() if err != nil { From f258ae5a6da185bbf4e5ee50783d0a62387f5702 Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Tue, 5 Nov 2024 13:33:30 +0000 Subject: [PATCH 3/3] Add ZipRecruiter to USERS.md Signed-off-by: Matt Finkel --- USERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/USERS.md b/USERS.md index e1977028165ca..7a098d3db459c 100644 --- a/USERS.md +++ b/USERS.md @@ -389,4 +389,5 @@ Currently, the following organizations are **officially** using Argo CD: 1. [Yubo](https://www.yubo.live/) 1. [ZDF](https://www.zdf.de/) 1. [Zimpler](https://www.zimpler.com/) +1. [ZipRecuiter](https://www.ziprecruiter.com/) 1. [ZOZO](https://corp.zozo.com/)