diff --git a/docs/operator-manual/rbac.md b/docs/operator-manual/rbac.md index e7ad426a85955b..3636cbffe75dc5 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. @@ -138,12 +138,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 f6cad80b542f18..94349afaedf3bc 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 3b579d8e9b1420..62d7a924caa428 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 516116f0ecd993..adf489c9246af2 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 {