From f690386921e790f7aef230dd00bfb7298347b067 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 5 Apr 2024 16:50:18 +0200 Subject: [PATCH 01/16] InheritedFleetWorkspacePermissions validation - Validate the user have enough permission to create/update the rules defined in InheritedFleetWorkspacePermissions.ResourceRules - Validate the user have enough permission to create/pdate the rules that are generated based on the InheritedFleetWorkspacePermissions.WorkspaceVerbs --- docs.md | 16 ++ pkg/auth/globalrole.go | 25 +++ pkg/codegen/main.go | 3 + .../management.cattle.io/v3/interface.go | 5 + .../rbac.authorization.k8s.io/v1/objects.go | 106 +++++++++++ .../v3/globalrole/setup_test.go | 31 +++- .../v3/globalrole/validator.go | 44 ++++- .../v3/globalrole/validator_test.go | 133 +++++++++++++- .../v1/clusterrole/ClusterRole.md | 5 + .../v1/clusterrole/validator.go | 84 +++++++++ .../v1/clusterrole/validator_test.go | 172 +++++++++++++++++ .../clusterrolebinding/ClusterRoleBinding.md | 5 + .../v1/clusterrolebinding/validator.go | 85 +++++++++ .../v1/clusterrolebinding/validator_test.go | 173 ++++++++++++++++++ pkg/server/handlers.go | 8 +- 15 files changed, 889 insertions(+), 6 deletions(-) create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/ClusterRole.md create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/ClusterRoleBinding.md create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go create mode 100644 pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go diff --git a/docs.md b/docs.md index aef944d0d..4210ce1b4 100644 --- a/docs.md +++ b/docs.md @@ -345,6 +345,22 @@ When a UserAttribute is updated, the following checks take place: # rbac.authorization.k8s.io/v1 +## ClusterRole + +### Validation Checks + +#### Invalid Fields - Update +Users cannot update or remove the following label after it has been added: +- authz.management.cattle.io/gr-owner + +## ClusterRoleBinding + +### Validation Checks + +#### Invalid Fields - Update +Users cannot update or remove the following label after it has been added: +- authz.management.cattle.io/grb-owner + ## Role ### Validation Checks diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 35084de30..eaeafad32 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -41,6 +41,31 @@ func (g *GlobalRoleResolver) GlobalRulesFromRole(gr *v3.GlobalRole) []rbacv1.Pol return gr.Rules } +// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local. +func (g *GlobalRoleResolver) FleetWorkspaceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { + // no rules on a nil role + if gr == nil { + return nil + } + return gr.InheritedFleetWorkspacePermissions.ResourceRules +} + +// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local. +func (g *GlobalRoleResolver) FleetWorkspaceVerbsRuleFromRole(gr *v3.GlobalRole, workspaceNames []string) []rbacv1.PolicyRule { + if gr == nil { + return nil + } + + return []rbacv1.PolicyRule{ + { + Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"fleetworkspaces"}, + ResourceNames: workspaceNames, + }, + } +} + // ClusterRulesFromRole finds all rules which this gr gives on downstream clusters. func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.PolicyRule, error) { if gr == nil { diff --git a/pkg/codegen/main.go b/pkg/codegen/main.go index a6a70ba6c..a729ec43e 100644 --- a/pkg/codegen/main.go +++ b/pkg/codegen/main.go @@ -45,6 +45,7 @@ func main() { v3.Node{}, v3.Project{}, v3.ClusterProxyConfig{}, + v3.FleetWorkspace{}, }, }, "provisioning.cattle.io": { @@ -89,6 +90,8 @@ func main() { Types: []interface{}{ &rbacv1.Role{}, &rbacv1.RoleBinding{}, + &rbacv1.ClusterRole{}, + &rbacv1.ClusterRoleBinding{}, }, }}); err != nil { fmt.Printf("ERROR: %v\n", err) diff --git a/pkg/generated/controllers/management.cattle.io/v3/interface.go b/pkg/generated/controllers/management.cattle.io/v3/interface.go index a584f3beb..ffa7f9084 100644 --- a/pkg/generated/controllers/management.cattle.io/v3/interface.go +++ b/pkg/generated/controllers/management.cattle.io/v3/interface.go @@ -34,6 +34,7 @@ type Interface interface { Cluster() ClusterController ClusterProxyConfig() ClusterProxyConfigController ClusterRoleTemplateBinding() ClusterRoleTemplateBindingController + FleetWorkspace() FleetWorkspaceController GlobalRole() GlobalRoleController GlobalRoleBinding() GlobalRoleBindingController Node() NodeController @@ -65,6 +66,10 @@ func (v *version) ClusterRoleTemplateBinding() ClusterRoleTemplateBindingControl return generic.NewController[*v3.ClusterRoleTemplateBinding, *v3.ClusterRoleTemplateBindingList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "ClusterRoleTemplateBinding"}, "clusterroletemplatebindings", true, v.controllerFactory) } +func (v *version) FleetWorkspace() FleetWorkspaceController { + return generic.NewNonNamespacedController[*v3.FleetWorkspace, *v3.FleetWorkspaceList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "FleetWorkspace"}, "fleetworkspaces", v.controllerFactory) +} + func (v *version) GlobalRole() GlobalRoleController { return generic.NewNonNamespacedController[*v3.GlobalRole, *v3.GlobalRoleList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "GlobalRole"}, "globalroles", v.controllerFactory) } diff --git a/pkg/generated/objects/rbac.authorization.k8s.io/v1/objects.go b/pkg/generated/objects/rbac.authorization.k8s.io/v1/objects.go index d9802bc28..6d4cf6132 100644 --- a/pkg/generated/objects/rbac.authorization.k8s.io/v1/objects.go +++ b/pkg/generated/objects/rbac.authorization.k8s.io/v1/objects.go @@ -113,3 +113,109 @@ func RoleBindingFromRequest(request *admissionv1.AdmissionRequest) (*v1.RoleBind return object, nil } + +// ClusterRoleOldAndNewFromRequest gets the old and new ClusterRole objects, respectively, from the webhook request. +// If the request is a Delete operation, then the new object is the zero value for ClusterRole. +// Similarly, if the request is a Create operation, then the old object is the zero value for ClusterRole. +func ClusterRoleOldAndNewFromRequest(request *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error) { + if request == nil { + return nil, nil, fmt.Errorf("nil request") + } + + object := &v1.ClusterRole{} + oldObject := &v1.ClusterRole{} + + if request.Operation != admissionv1.Delete { + err := json.Unmarshal(request.Object.Raw, object) + if err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal request object: %w", err) + } + } + + if request.Operation == admissionv1.Create { + return oldObject, object, nil + } + + err := json.Unmarshal(request.OldObject.Raw, oldObject) + if err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal request oldObject: %w", err) + } + + return oldObject, object, nil +} + +// ClusterRoleFromRequest returns a ClusterRole object from the webhook request. +// If the operation is a Delete operation, then the old object is returned. +// Otherwise, the new object is returned. +func ClusterRoleFromRequest(request *admissionv1.AdmissionRequest) (*v1.ClusterRole, error) { + if request == nil { + return nil, fmt.Errorf("nil request") + } + + object := &v1.ClusterRole{} + raw := request.Object.Raw + + if request.Operation == admissionv1.Delete { + raw = request.OldObject.Raw + } + + err := json.Unmarshal(raw, object) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal request object: %w", err) + } + + return object, nil +} + +// ClusterRoleBindingOldAndNewFromRequest gets the old and new ClusterRoleBinding objects, respectively, from the webhook request. +// If the request is a Delete operation, then the new object is the zero value for ClusterRoleBinding. +// Similarly, if the request is a Create operation, then the old object is the zero value for ClusterRoleBinding. +func ClusterRoleBindingOldAndNewFromRequest(request *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, *v1.ClusterRoleBinding, error) { + if request == nil { + return nil, nil, fmt.Errorf("nil request") + } + + object := &v1.ClusterRoleBinding{} + oldObject := &v1.ClusterRoleBinding{} + + if request.Operation != admissionv1.Delete { + err := json.Unmarshal(request.Object.Raw, object) + if err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal request object: %w", err) + } + } + + if request.Operation == admissionv1.Create { + return oldObject, object, nil + } + + err := json.Unmarshal(request.OldObject.Raw, oldObject) + if err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal request oldObject: %w", err) + } + + return oldObject, object, nil +} + +// ClusterRoleBindingFromRequest returns a ClusterRoleBinding object from the webhook request. +// If the operation is a Delete operation, then the old object is returned. +// Otherwise, the new object is returned. +func ClusterRoleBindingFromRequest(request *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, error) { + if request == nil { + return nil, fmt.Errorf("nil request") + } + + object := &v1.ClusterRoleBinding{} + raw := request.Object.Raw + + if request.Operation == admissionv1.Delete { + raw = request.OldObject.Raw + } + + err := json.Unmarshal(raw, object) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal request object: %w", err) + } + + return object, nil +} diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 7d9643ea1..e67777505 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/labels" "testing" "github.com/golang/mock/gomock" @@ -42,7 +43,7 @@ var ( }, }, } - clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR} + clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR, readFleetWorkspacesCR} clusterRoleBindings = []*v1.ClusterRoleBinding{ { @@ -74,6 +75,12 @@ var ( }, }, }, + { + Subjects: []v1.Subject{ + {Kind: v1.UserKind, Name: testUser}, + }, + RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: readFleetWorkspacesCR.Name}, + }, } baseRT = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -128,6 +135,13 @@ var ( APIGroups: []string{"*"}, Resources: []string{"*"}, } + ruleReadFleetWorkspaces = v1.PolicyRule{ + Verbs: []string{"GET"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"fleetworkspaces"}, + ResourceNames: []string{"fleet-default"}, + } + adminCR = &v1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: "admin-role", @@ -138,6 +152,10 @@ var ( ObjectMeta: metav1.ObjectMeta{Name: "read-pods"}, Rules: []v1.PolicyRule{ruleReadPods}, } + readFleetWorkspacesCR = &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "read-fleetworkspaces"}, + Rules: []v1.PolicyRule{ruleReadFleetWorkspaces}, + } roleTemplate = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -172,6 +190,7 @@ type testState struct { rtCacheMock *fake.MockNonNamespacedCacheInterface[*v3.RoleTemplate] grCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRole] grbCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRoleBinding] + fwCacheMock *fake.MockNonNamespacedCacheInterface[*v3.FleetWorkspace] sarMock *k8fake.FakeSubjectAccessReviews resolver validation.AuthorizationRuleResolver } @@ -259,12 +278,21 @@ func newDefaultState(t *testing.T) testState { rtCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) grCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) grbCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) + fwCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.FleetWorkspace](ctrl) grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() rtCacheMock.EXPECT().Get(baseRT.Name).Return(&baseRT, nil).AnyTimes() + fwCacheMock.EXPECT().List(labels.Everything()).Return([]*v3.FleetWorkspace{ + { + ObjectMeta: metav1.ObjectMeta{Name: "fleet-default"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "fleet-local"}, + }, + }, nil).AnyTimes() k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} @@ -273,6 +301,7 @@ func newDefaultState(t *testing.T) testState { rtCacheMock: rtCacheMock, grCacheMock: grCacheMock, grbCacheMock: grbCacheMock, + fwCacheMock: fwCacheMock, sarMock: fakeSAR, resolver: resolver, } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index 38e6e6c2c..660ddb5dd 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -8,12 +8,14 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + mgmtv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" "github.com/rancher/webhook/pkg/resolvers" "github.com/rancher/webhook/pkg/resources/common" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1" @@ -34,12 +36,13 @@ const ( // NewValidator returns a new validator used for validation globalRoles. func NewValidator(ruleResolver validation.AuthorizationRuleResolver, grbResolver *resolvers.GRBClusterRuleResolver, - sar authorizationv1.SubjectAccessReviewInterface) *Validator { + sar authorizationv1.SubjectAccessReviewInterface, fwCache mgmtv3.FleetWorkspaceCache) *Validator { return &Validator{ admitter: admitter{ resolver: ruleResolver, grbResolver: grbResolver, sar: sar, + fwCache: fwCache, }, } } @@ -73,6 +76,7 @@ type admitter struct { resolver validation.AuthorizationRuleResolver grbResolver *resolvers.GRBClusterRuleResolver sar authorizationv1.SubjectAccessReviewInterface + fwCache mgmtv3.FleetWorkspaceCache } // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. @@ -127,6 +131,12 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp returnError = errors.Join(returnError, common.ValidateRules(rules, true, nsrPath.Child(index))) } + // Validate fleet workspace rules + if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { + fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR) + fwrPath := fldPath.Child("inheritedFleetWorkspacePermissions").Child("resourceRules") + returnError = errors.Join(returnError, common.ValidateRules(fleetWorkspaceRules, true, fwrPath)) + } if returnError != nil { return admission.ResponseBadRequest(returnError.Error()), nil @@ -154,6 +164,22 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseAllowed(), nil } } + // Check for escalations in fleet workspace rules + if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { + fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR) + workspaceNames, err := a.allFleetWorkspaceNamesExceptLocal() + if err != nil { + return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err) + } + for _, w := range workspaceNames { + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceRules, a.resolver, w)) + if escalateChecker.HasVerb() { + return admission.ResponseAllowed(), nil + } + } + fleetWorkspaceVerbs := a.grbResolver.GlobalRoleResolver.FleetWorkspaceVerbsRuleFromRole(newGR, workspaceNames) + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceVerbs, a.resolver, "")) + } if returnError != nil { return admission.ResponseFailedEscalation(fmt.Sprintf("errors due to escalation: %v", returnError)), nil } @@ -161,6 +187,22 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseAllowed(), nil } +// allFleetWorkspaceNamesExceptLocal returns all workspace names except fleet-local +func (a *admitter) allFleetWorkspaceNamesExceptLocal() ([]string, error) { + workspaces, err := a.fwCache.List(labels.Everything()) + if err != nil { + return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err) + } + var workspaceNames []string + for _, w := range workspaces { + if w.Name != "fleet-local" { + workspaceNames = append(workspaceNames, w.Name) + } + } + + return workspaceNames, nil +} + // validateDelete checks if a global role can be deleted and returns the appropriate response. func validateDelete(oldRole *v3.GlobalRole, fldPath *field.Path) (*admissionv1.AdmissionResponse, error) { if oldRole.Builtin { diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index 9e83463b6..703e5b5cc 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -717,6 +717,135 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "allowed InheritedFleetWorkspacePermissions", + args: args{ + username: testUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleReadPods, + } + baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ + "GET", + } + return baseGR + }, + stateSetup: func(state testState) { + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: true, + }, + { + name: "not allowed InheritedFleetWorkspacePermissions.ResourceRules", + args: args{ + username: testUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleAdmin, + } + baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ + "GET", + } + return baseGR + }, + stateSetup: func(state testState) { + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: false, + }, + { + name: "not allowed InheritedFleetWorkspacePermissions.WorkspaceVerbs", + args: args{ + username: testUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleReadPods, + } + baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ + "*", + } + return baseGR + }, + stateSetup: func(state testState) { + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: false, + }, + { + name: "InheritedFleetWorkspacePermissions rules contains PolicyRule that is invalid", + args: args{ + username: adminUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{}, + }, + } + return baseGR + }, + }, + allowed: false, + }, + { + name: "update in InheritedFleetWorkspacePermissions with privilege escalation", + args: args{ + username: testUser, + oldGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleReadPods, + } + return baseGR + }, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleAdmin, + } + return baseGR + }, + stateSetup: func(state testState) { + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + allowed: false, + }, + { + name: "update in InheritedFleetWorkspacePermissions with privilege escalation, escalate allowed", + args: args{ + username: testUser, + oldGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleReadPods, + } + return baseGR + }, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ + ruleAdmin, + } + return baseGR + }, + stateSetup: func(state testState) { + setSarResponse(true, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + allowed: true, + }, } for _, test := range tests { @@ -728,7 +857,7 @@ func TestAdmit(t *testing.T) { test.args.stateSetup(state) } grbResolver := state.createBaseGRBResolver() - admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters() + admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock, state.fwCacheMock).Admitters() assert.Len(t, admitters, 1) req := createGRRequest(t, test) @@ -746,7 +875,7 @@ func TestAdmit(t *testing.T) { func Test_UnexpectedErrors(t *testing.T) { t.Parallel() resolver, _ := validation.NewTestRuleResolver(nil, nil, nil, nil) - validator := globalrole.NewValidator(resolver, nil, nil) + validator := globalrole.NewValidator(resolver, nil, nil, nil) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") test := testCase{ diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/ClusterRole.md b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/ClusterRole.md new file mode 100644 index 000000000..e02222c25 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/ClusterRole.md @@ -0,0 +1,5 @@ +## Validation Checks + +### Invalid Fields - Update +Users cannot update or remove the following label after it has been added: +- authz.management.cattle.io/gr-owner \ No newline at end of file diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go new file mode 100644 index 000000000..c921e2821 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go @@ -0,0 +1,84 @@ +package clusterrole + +import ( + "fmt" + + "github.com/rancher/webhook/pkg/admission" + objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" + "github.com/rancher/webhook/pkg/resources/common" + admissionv1 "k8s.io/api/admission/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/trace" +) + +const ( + grOwnerLabel = "authz.management.cattle.io/gr-owner" +) + +// Validator implements admission.ValidatingAdmissionHandler. +type Validator struct { + admitter admitter +} + +// NewValidator returns a new validator for roles. +func NewValidator() *Validator { + return &Validator{ + admitter: admitter{}, + } +} + +// GVR returns the GroupVersionKind for this CRD. +func (v *Validator) GVR() schema.GroupVersionResource { + return schema.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "clusterroles", + } +} + +// Operations returns list of operations handled by this validator. +func (v *Validator) Operations() []admissionregistrationv1.OperationType { + return []admissionregistrationv1.OperationType{ + admissionregistrationv1.Update, + } +} + +// ValidatingWebhook returns the ValidatingWebhook used for this CRD. +func (v *Validator) ValidatingWebhook(clientConfig admissionregistrationv1.WebhookClientConfig) []admissionregistrationv1.ValidatingWebhook { + webhook := admission.NewDefaultValidatingWebhook(v, clientConfig, admissionregistrationv1.ClusterScope, v.Operations()) + webhook.ObjectSelector = &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: grOwnerLabel, + Operator: metav1.LabelSelectorOpExists, + }, + }, + } + return []admissionregistrationv1.ValidatingWebhook{*webhook} +} + +// Admitters returns the admitter objects used to validate roles. +func (v *Validator) Admitters() []admission.Admitter { + return []admission.Admitter{&v.admitter} +} + +type admitter struct{} + +// Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. +func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { + listTrace := trace.New("clusterRoleValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) + defer listTrace.LogIfLong(admission.SlowTraceDuration) + + oldRole, newRole, err := objectsv1.ClusterRoleOldAndNewFromRequest(&request.AdmissionRequest) + if err != nil { + return nil, err + } + + if common.IsModifyingLabel(oldRole.Labels, newRole.Labels, grOwnerLabel) { + return admission.ResponseBadRequest(fmt.Sprintf("cannot modify or remove label %s", grOwnerLabel)), nil + } + + return admission.ResponseAllowed(), nil +} diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go new file mode 100644 index 000000000..6fcdb3953 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go @@ -0,0 +1,172 @@ +package clusterrole + +import ( + "context" + "encoding/json" + "testing" + + "github.com/rancher/webhook/pkg/admission" + "github.com/stretchr/testify/require" + admissionv1 "k8s.io/api/admission/v1" + v1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var ( + gvk = metav1.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "Role"} + gvr = metav1.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "roles"} +) + +func TestAdmit(t *testing.T) { + t.Parallel() + + defaultRole := &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + } + emptyRole := &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + } + roleWithOwnerLabel := &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + grOwnerLabel: "gr-owner", + }, + }, + } + roleWithNewOwnerLabel := &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + grOwnerLabel: "new-owner", + }, + }, + } + + type args struct { + oldRole *v1.ClusterRole + newRole *v1.ClusterRole + } + type test struct { + name string + args args + allowed bool + wantErr bool + } + tests := []test{ + { + name: "updating with 2 nil labels maps allowed", + args: args{ + oldRole: defaultRole.DeepCopy(), + newRole: defaultRole.DeepCopy(), + }, + allowed: true, + }, + { + name: "updating with 2 empty labels maps allowed", + args: args{ + oldRole: emptyRole.DeepCopy(), + newRole: emptyRole.DeepCopy(), + }, + allowed: true, + }, + { + name: "updating labels other than grOwner allowed", + args: args{ + oldRole: roleWithOwnerLabel.DeepCopy(), + newRole: &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + "new-label": "test-value", + grOwnerLabel: "gr-owner", + }, + }, + }, + }, + allowed: true, + }, + { + name: "adding gr-owner label allowed", + args: args{ + oldRole: defaultRole.DeepCopy(), + newRole: roleWithNewOwnerLabel.DeepCopy(), + }, + allowed: true, + }, + { + name: "adding gr-owner label to empty labels map allowed", + args: args{ + oldRole: emptyRole.DeepCopy(), + newRole: roleWithNewOwnerLabel.DeepCopy(), + }, + allowed: true, + }, + { + name: "modifying gr-owner label not allowed", + args: args{ + oldRole: roleWithOwnerLabel.DeepCopy(), + newRole: roleWithNewOwnerLabel.DeepCopy(), + }, + allowed: false, + }, + { + name: "removing gr-owner label not allowed", + args: args{ + oldRole: roleWithOwnerLabel.DeepCopy(), + newRole: defaultRole.DeepCopy(), + }, + allowed: false, + }, + { + name: "replacing labels with empty map not allowed", + args: args{ + oldRole: roleWithOwnerLabel.DeepCopy(), + newRole: emptyRole.DeepCopy(), + }, + allowed: false, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + req := &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "1", + Kind: gvk, + Resource: gvr, + RequestKind: &gvk, + RequestResource: &gvr, + Operation: admissionv1.Update, + Object: runtime.RawExtension{}, + OldObject: runtime.RawExtension{}, + }, + Context: context.Background(), + } + var err error + req.Object.Raw, err = json.Marshal(test.args.newRole) + require.NoError(t, err) + req.OldObject.Raw, err = json.Marshal(test.args.oldRole) + require.NoError(t, err) + + admitter := NewValidator().Admitters() + + response, err := admitter[0].Admit(req) + if test.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equalf(t, test.allowed, response.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, response.Allowed, response.Result) + + }) + } +} diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/ClusterRoleBinding.md b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/ClusterRoleBinding.md new file mode 100644 index 000000000..60d8d0588 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/ClusterRoleBinding.md @@ -0,0 +1,5 @@ +## Validation Checks + +### Invalid Fields - Update +Users cannot update or remove the following label after it has been added: +- authz.management.cattle.io/grb-owner \ No newline at end of file diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go new file mode 100644 index 000000000..06801a786 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go @@ -0,0 +1,85 @@ +package clusterrolebinding + +import ( + "fmt" + + "github.com/rancher/webhook/pkg/admission" + objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" + "github.com/rancher/webhook/pkg/resources/common" + admissionv1 "k8s.io/api/admission/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/trace" +) + +const ( + grbOwnerLabel = "authz.management.cattle.io/grb-owner" +) + +// Validator implements admission.ValidatingAdmissionHandler. +type Validator struct { + admitter admitter +} + +// NewValidator returns a new validator for clusterrolebindings. +func NewValidator() *Validator { + return &Validator{ + admitter: admitter{}, + } +} + +// GVR returns the GroupVersionKind for this CRD. +func (v *Validator) GVR() schema.GroupVersionResource { + return schema.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "clusterrolebindings", + } +} + +// Operations returns list of operations handled by this validator. +func (v *Validator) Operations() []admissionregistrationv1.OperationType { + return []admissionregistrationv1.OperationType{ + admissionregistrationv1.Update, + } +} + +// ValidatingWebhook returns the ValidatingWebhook used for this CRD. +func (v *Validator) ValidatingWebhook(clientConfig admissionregistrationv1.WebhookClientConfig) []admissionregistrationv1.ValidatingWebhook { + webhook := admission.NewDefaultValidatingWebhook(v, clientConfig, admissionregistrationv1.ClusterScope, v.Operations()) + webhook.ObjectSelector = &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: grbOwnerLabel, + Operator: metav1.LabelSelectorOpExists, + }, + }, + } + + return []admissionregistrationv1.ValidatingWebhook{*webhook} +} + +// Admitters returns the admitter objects used to validate roles. +func (v *Validator) Admitters() []admission.Admitter { + return []admission.Admitter{&v.admitter} +} + +type admitter struct{} + +// Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. +func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { + listTrace := trace.New("clusterRolebindingValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) + defer listTrace.LogIfLong(admission.SlowTraceDuration) + + oldRoleBinding, newRoleBinding, err := objectsv1.ClusterRoleBindingOldAndNewFromRequest(&request.AdmissionRequest) + if err != nil { + return nil, err + } + + if common.IsModifyingLabel(oldRoleBinding.Labels, newRoleBinding.Labels, grbOwnerLabel) { + return admission.ResponseBadRequest(fmt.Sprintf("cannot modify or remove label %s", grbOwnerLabel)), nil + } + + return admission.ResponseAllowed(), nil +} diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go new file mode 100644 index 000000000..91596e134 --- /dev/null +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go @@ -0,0 +1,173 @@ +package clusterrolebinding + +import ( + "context" + "encoding/json" + "testing" + + "github.com/rancher/webhook/pkg/admission" + "github.com/stretchr/testify/require" + admissionv1 "k8s.io/api/admission/v1" + v1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var ( + gvk = metav1.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "RoleBinding"} + gvr = metav1.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "rolebindings"} +) + +func TestAdmit(t *testing.T) { + t.Parallel() + + defaultRoleBinding := &v1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + } + emptyLabelsRoleBinding := &v1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: "default", + }, + } + roleBindingWithOwnerLabel := &v1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + grbOwnerLabel: "grb-owner", + }, + }, + } + roleBindingWithNewOwnerLabel := &v1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + grbOwnerLabel: "new-owner", + }, + }, + } + + type args struct { + oldRB *v1.ClusterRoleBinding + newRB *v1.ClusterRoleBinding + } + type test struct { + name string + args args + allowed bool + wantErr bool + } + tests := []test{ + { + name: "updating with 2 nil labels maps allowed", + args: args{ + oldRB: defaultRoleBinding.DeepCopy(), + newRB: defaultRoleBinding.DeepCopy(), + }, + allowed: true, + }, + { + name: "updating with 2 empty labels maps allowed", + args: args{ + oldRB: emptyLabelsRoleBinding.DeepCopy(), + newRB: emptyLabelsRoleBinding.DeepCopy(), + }, + allowed: true, + }, + { + name: "updating labels other than grbOwner allowed", + args: args{ + oldRB: roleBindingWithOwnerLabel.DeepCopy(), + newRB: &v1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Labels: map[string]string{ + "new-label": "test-value", + grbOwnerLabel: "grb-owner", + }, + }, + }, + }, + allowed: true, + }, + { + name: "adding grb-owner label allowed", + args: args{ + oldRB: defaultRoleBinding.DeepCopy(), + newRB: roleBindingWithOwnerLabel.DeepCopy(), + }, + allowed: true, + }, + { + name: "adding grb-owner label to empty labels map allowed", + args: args{ + oldRB: emptyLabelsRoleBinding.DeepCopy(), + newRB: roleBindingWithOwnerLabel.DeepCopy(), + }, + allowed: true, + }, + { + name: "modifying grb-owner label not allowed", + args: args{ + oldRB: roleBindingWithOwnerLabel.DeepCopy(), + newRB: roleBindingWithNewOwnerLabel.DeepCopy(), + }, + allowed: false, + }, + { + name: "removing grb-owner label not allowed", + args: args{ + oldRB: roleBindingWithOwnerLabel.DeepCopy(), + newRB: defaultRoleBinding.DeepCopy(), + }, + allowed: false, + }, + { + name: "replacing labels with empty map not allowed", + args: args{ + oldRB: roleBindingWithOwnerLabel.DeepCopy(), + newRB: emptyLabelsRoleBinding.DeepCopy(), + }, + allowed: false, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + req := &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "1", + Kind: gvk, + Resource: gvr, + RequestKind: &gvk, + RequestResource: &gvr, + Operation: admissionv1.Update, + Object: runtime.RawExtension{}, + OldObject: runtime.RawExtension{}, + }, + Context: context.Background(), + } + var err error + req.Object.Raw, err = json.Marshal(test.args.newRB) + require.NoError(t, err) + req.OldObject.Raw, err = json.Marshal(test.args.oldRB) + require.NoError(t, err) + + admitter := NewValidator().Admitters() + + response, err := admitter[0].Admit(req) + if test.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equalf(t, test.allowed, response.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, response.Allowed, response.Result) + + }) + } +} diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index 053b20a70..1f29a0ffc 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -21,6 +21,8 @@ import ( "github.com/rancher/webhook/pkg/resources/management.cattle.io/v3/setting" "github.com/rancher/webhook/pkg/resources/management.cattle.io/v3/userattribute" provisioningCluster "github.com/rancher/webhook/pkg/resources/provisioning.cattle.io/v1/cluster" + "github.com/rancher/webhook/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole" + "github.com/rancher/webhook/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding" "github.com/rancher/webhook/pkg/resources/rbac.authorization.k8s.io/v1/role" "github.com/rancher/webhook/pkg/resources/rbac.authorization.k8s.io/v1/rolebinding" "github.com/rancher/webhook/pkg/resources/rke-machine-config.cattle.io/v1/machineconfig" @@ -42,7 +44,7 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle prtbResolver := resolvers.NewPRTBRuleResolver(clients.Management.ProjectRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) grbResolver := resolvers.NewGRBClusterRuleResolver(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) psact := podsecurityadmissionconfigurationtemplate.NewValidator(clients.Management.Cluster().Cache(), clients.Provisioning.Cluster().Cache()) - globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) + globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.FleetWorkspace().Cache()) globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) prtbs := projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.Cluster().Cache(), clients.Management.Project().Cache()) crtbs := clusterroletemplatebinding.NewValidator(crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.GlobalRoleBinding().Cache(), clients.Management.Cluster().Cache()) @@ -54,8 +56,10 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle rolebindings := rolebinding.NewValidator() setting := setting.NewValidator() userAttribute := userattribute.NewValidator() + clusterRoles := clusterrole.NewValidator() + clusterRoleBindings := clusterrolebinding.NewValidator() - handlers = append(handlers, psact, globalRoles, globalRoleBindings, prtbs, crtbs, roleTemplates, secrets, nodeDriver, projects, roles, rolebindings, clusterProxyConfigs, userAttribute, setting) + handlers = append(handlers, psact, globalRoles, globalRoleBindings, prtbs, crtbs, roleTemplates, secrets, nodeDriver, projects, roles, rolebindings, clusterRoles, clusterRoleBindings, clusterProxyConfigs, userAttribute, setting) } return handlers, nil } From 2a1fd7168998e712760c098c85414f07708cc3f5 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 5 Apr 2024 17:23:38 +0200 Subject: [PATCH 02/16] Add missing generated file --- .../management.cattle.io/v3/fleetworkspace.go | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go diff --git a/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go b/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go new file mode 100644 index 000000000..a883afc36 --- /dev/null +++ b/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go @@ -0,0 +1,208 @@ +/* +Copyright 2024 Rancher Labs, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by codegen. DO NOT EDIT. + +package v3 + +import ( + "context" + "sync" + "time" + + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + "github.com/rancher/wrangler/v2/pkg/apply" + "github.com/rancher/wrangler/v2/pkg/condition" + "github.com/rancher/wrangler/v2/pkg/generic" + "github.com/rancher/wrangler/v2/pkg/kv" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// FleetWorkspaceController interface for managing FleetWorkspace resources. +type FleetWorkspaceController interface { + generic.NonNamespacedControllerInterface[*v3.FleetWorkspace, *v3.FleetWorkspaceList] +} + +// FleetWorkspaceClient interface for managing FleetWorkspace resources in Kubernetes. +type FleetWorkspaceClient interface { + generic.NonNamespacedClientInterface[*v3.FleetWorkspace, *v3.FleetWorkspaceList] +} + +// FleetWorkspaceCache interface for retrieving FleetWorkspace resources in memory. +type FleetWorkspaceCache interface { + generic.NonNamespacedCacheInterface[*v3.FleetWorkspace] +} + +// FleetWorkspaceStatusHandler is executed for every added or modified FleetWorkspace. Should return the new status to be updated +type FleetWorkspaceStatusHandler func(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) (v3.FleetWorkspaceStatus, error) + +// FleetWorkspaceGeneratingHandler is the top-level handler that is executed for every FleetWorkspace event. It extends FleetWorkspaceStatusHandler by a returning a slice of child objects to be passed to apply.Apply +type FleetWorkspaceGeneratingHandler func(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) ([]runtime.Object, v3.FleetWorkspaceStatus, error) + +// RegisterFleetWorkspaceStatusHandler configures a FleetWorkspaceController to execute a FleetWorkspaceStatusHandler for every events observed. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterFleetWorkspaceStatusHandler(ctx context.Context, controller FleetWorkspaceController, condition condition.Cond, name string, handler FleetWorkspaceStatusHandler) { + statusHandler := &fleetWorkspaceStatusHandler{ + client: controller, + condition: condition, + handler: handler, + } + controller.AddGenericHandler(ctx, name, generic.FromObjectHandlerToHandler(statusHandler.sync)) +} + +// RegisterFleetWorkspaceGeneratingHandler configures a FleetWorkspaceController to execute a FleetWorkspaceGeneratingHandler for every events observed, passing the returned objects to the provided apply.Apply. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterFleetWorkspaceGeneratingHandler(ctx context.Context, controller FleetWorkspaceController, apply apply.Apply, + condition condition.Cond, name string, handler FleetWorkspaceGeneratingHandler, opts *generic.GeneratingHandlerOptions) { + statusHandler := &fleetWorkspaceGeneratingHandler{ + FleetWorkspaceGeneratingHandler: handler, + apply: apply, + name: name, + gvk: controller.GroupVersionKind(), + } + if opts != nil { + statusHandler.opts = *opts + } + controller.OnChange(ctx, name, statusHandler.Remove) + RegisterFleetWorkspaceStatusHandler(ctx, controller, condition, name, statusHandler.Handle) +} + +type fleetWorkspaceStatusHandler struct { + client FleetWorkspaceClient + condition condition.Cond + handler FleetWorkspaceStatusHandler +} + +// sync is executed on every resource addition or modification. Executes the configured handlers and sends the updated status to the Kubernetes API +func (a *fleetWorkspaceStatusHandler) sync(key string, obj *v3.FleetWorkspace) (*v3.FleetWorkspace, error) { + if obj == nil { + return obj, nil + } + + origStatus := obj.Status.DeepCopy() + obj = obj.DeepCopy() + newStatus, err := a.handler(obj, obj.Status) + if err != nil { + // Revert to old status on error + newStatus = *origStatus.DeepCopy() + } + + if a.condition != "" { + if errors.IsConflict(err) { + a.condition.SetError(&newStatus, "", nil) + } else { + a.condition.SetError(&newStatus, "", err) + } + } + if !equality.Semantic.DeepEqual(origStatus, &newStatus) { + if a.condition != "" { + // Since status has changed, update the lastUpdatedTime + a.condition.LastUpdated(&newStatus, time.Now().UTC().Format(time.RFC3339)) + } + + var newErr error + obj.Status = newStatus + newObj, newErr := a.client.UpdateStatus(obj) + if err == nil { + err = newErr + } + if newErr == nil { + obj = newObj + } + } + return obj, err +} + +type fleetWorkspaceGeneratingHandler struct { + FleetWorkspaceGeneratingHandler + apply apply.Apply + opts generic.GeneratingHandlerOptions + gvk schema.GroupVersionKind + name string + seen sync.Map +} + +// Remove handles the observed deletion of a resource, cascade deleting every associated resource previously applied +func (a *fleetWorkspaceGeneratingHandler) Remove(key string, obj *v3.FleetWorkspace) (*v3.FleetWorkspace, error) { + if obj != nil { + return obj, nil + } + + obj = &v3.FleetWorkspace{} + obj.Namespace, obj.Name = kv.RSplit(key, "/") + obj.SetGroupVersionKind(a.gvk) + + if a.opts.UniqueApplyForResourceVersion { + a.seen.Delete(key) + } + + return nil, generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects() +} + +// Handle executes the configured FleetWorkspaceGeneratingHandler and pass the resulting objects to apply.Apply, finally returning the new status of the resource +func (a *fleetWorkspaceGeneratingHandler) Handle(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) (v3.FleetWorkspaceStatus, error) { + if !obj.DeletionTimestamp.IsZero() { + return status, nil + } + + objs, newStatus, err := a.FleetWorkspaceGeneratingHandler(obj, status) + if err != nil { + return newStatus, err + } + if !a.isNewResourceVersion(obj) { + return newStatus, nil + } + + err = generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects(objs...) + if err != nil { + return newStatus, err + } + a.storeResourceVersion(obj) + return newStatus, nil +} + +// isNewResourceVersion detects if a specific resource version was already successfully processed. +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *fleetWorkspaceGeneratingHandler) isNewResourceVersion(obj *v3.FleetWorkspace) bool { + if !a.opts.UniqueApplyForResourceVersion { + return true + } + + // Apply once per resource version + key := obj.Namespace + "/" + obj.Name + previous, ok := a.seen.Load(key) + return !ok || previous != obj.ResourceVersion +} + +// storeResourceVersion keeps track of the latest resource version of an object for which Apply was executed +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *fleetWorkspaceGeneratingHandler) storeResourceVersion(obj *v3.FleetWorkspace) { + if !a.opts.UniqueApplyForResourceVersion { + return + } + + key := obj.Namespace + "/" + obj.Name + a.seen.Store(key, obj.ResourceVersion) +} From 377cc019f063df6a2eac971588cadf3da60432ba Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 5 Apr 2024 17:50:37 +0200 Subject: [PATCH 03/16] Fix golangci-lint --- pkg/resources/management.cattle.io/v3/globalrole/setup_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index e67777505..300992285 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -4,9 +4,10 @@ import ( "context" "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/labels" "testing" + "k8s.io/apimachinery/pkg/labels" + "github.com/golang/mock/gomock" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" From d08ae0d9d5823ee7260910dbb63c3fe9ecda174f Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 11 Apr 2024 09:41:26 +0200 Subject: [PATCH 04/16] Move fleetworkspace validation to GlobalRoleResolver --- docs.md | 2 +- pkg/auth/globalrole.go | 37 +--- pkg/codegen/main.go | 1 - .../management.cattle.io/v3/fleetworkspace.go | 208 ------------------ .../management.cattle.io/v3/interface.go | 5 - .../v3/globalrole/GlobalRole.md | 2 +- .../v3/globalrole/setup_test.go | 38 +--- .../v3/globalrole/validator.go | 42 +--- .../v3/globalrole/validator_test.go | 4 +- .../v1/clusterrole/validator.go | 14 +- .../v1/clusterrole/validator_test.go | 36 ++- .../v1/clusterrolebinding/validator.go | 16 +- .../v1/clusterrolebinding/validator_test.go | 31 ++- pkg/server/handlers.go | 2 +- 14 files changed, 102 insertions(+), 336 deletions(-) delete mode 100644 pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go diff --git a/docs.md b/docs.md index 4210ce1b4..72a2b17d0 100644 --- a/docs.md +++ b/docs.md @@ -135,7 +135,7 @@ Rules without verbs, resources, or apigroups are not permitted. The `rules` incl Escalation checks are bypassed if a user has the `escalate` verb on the GlobalRole that they are attempting to update or create. This can also be given through a wildcard permission (i.e. the `*` verb also gives `escalate`). -Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles`. +Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles` and the rules in `inheritedFleetWorkspacePermissions`. Users can only grant rules in the `NamespacedRules` field with rights less than or equal to those they currently possess. This works on a per namespace basis, meaning that the user must have the permission in the namespace specified. The `Rules` field apply to every namespace, which means a user can create `NamespacedRules` in any namespace that are equal to or less than the `Rules` they currently possess. diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index eaeafad32..2437be15d 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -41,32 +41,7 @@ func (g *GlobalRoleResolver) GlobalRulesFromRole(gr *v3.GlobalRole) []rbacv1.Pol return gr.Rules } -// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local. -func (g *GlobalRoleResolver) FleetWorkspaceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { - // no rules on a nil role - if gr == nil { - return nil - } - return gr.InheritedFleetWorkspacePermissions.ResourceRules -} - -// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local. -func (g *GlobalRoleResolver) FleetWorkspaceVerbsRuleFromRole(gr *v3.GlobalRole, workspaceNames []string) []rbacv1.PolicyRule { - if gr == nil { - return nil - } - - return []rbacv1.PolicyRule{ - { - Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs, - APIGroups: []string{"management.cattle.io"}, - Resources: []string{"fleetworkspaces"}, - ResourceNames: workspaceNames, - }, - } -} - -// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters. +// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters and fleet workspaces except local. func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.PolicyRule, error) { if gr == nil { return nil, nil @@ -90,6 +65,16 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P } rules = append(rules, templateRules...) } + if gr.InheritedFleetWorkspacePermissions.ResourceRules != nil { + rules = append(rules, gr.InheritedFleetWorkspacePermissions.ResourceRules...) + } + if gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { + rules = append(rules, rbacv1.PolicyRule{ + Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"fleetworkspaces"}, + }) + } return rules, nil } diff --git a/pkg/codegen/main.go b/pkg/codegen/main.go index a729ec43e..929b99c48 100644 --- a/pkg/codegen/main.go +++ b/pkg/codegen/main.go @@ -45,7 +45,6 @@ func main() { v3.Node{}, v3.Project{}, v3.ClusterProxyConfig{}, - v3.FleetWorkspace{}, }, }, "provisioning.cattle.io": { diff --git a/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go b/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go deleted file mode 100644 index a883afc36..000000000 --- a/pkg/generated/controllers/management.cattle.io/v3/fleetworkspace.go +++ /dev/null @@ -1,208 +0,0 @@ -/* -Copyright 2024 Rancher Labs, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Code generated by codegen. DO NOT EDIT. - -package v3 - -import ( - "context" - "sync" - "time" - - v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" - "github.com/rancher/wrangler/v2/pkg/apply" - "github.com/rancher/wrangler/v2/pkg/condition" - "github.com/rancher/wrangler/v2/pkg/generic" - "github.com/rancher/wrangler/v2/pkg/kv" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" -) - -// FleetWorkspaceController interface for managing FleetWorkspace resources. -type FleetWorkspaceController interface { - generic.NonNamespacedControllerInterface[*v3.FleetWorkspace, *v3.FleetWorkspaceList] -} - -// FleetWorkspaceClient interface for managing FleetWorkspace resources in Kubernetes. -type FleetWorkspaceClient interface { - generic.NonNamespacedClientInterface[*v3.FleetWorkspace, *v3.FleetWorkspaceList] -} - -// FleetWorkspaceCache interface for retrieving FleetWorkspace resources in memory. -type FleetWorkspaceCache interface { - generic.NonNamespacedCacheInterface[*v3.FleetWorkspace] -} - -// FleetWorkspaceStatusHandler is executed for every added or modified FleetWorkspace. Should return the new status to be updated -type FleetWorkspaceStatusHandler func(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) (v3.FleetWorkspaceStatus, error) - -// FleetWorkspaceGeneratingHandler is the top-level handler that is executed for every FleetWorkspace event. It extends FleetWorkspaceStatusHandler by a returning a slice of child objects to be passed to apply.Apply -type FleetWorkspaceGeneratingHandler func(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) ([]runtime.Object, v3.FleetWorkspaceStatus, error) - -// RegisterFleetWorkspaceStatusHandler configures a FleetWorkspaceController to execute a FleetWorkspaceStatusHandler for every events observed. -// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution -func RegisterFleetWorkspaceStatusHandler(ctx context.Context, controller FleetWorkspaceController, condition condition.Cond, name string, handler FleetWorkspaceStatusHandler) { - statusHandler := &fleetWorkspaceStatusHandler{ - client: controller, - condition: condition, - handler: handler, - } - controller.AddGenericHandler(ctx, name, generic.FromObjectHandlerToHandler(statusHandler.sync)) -} - -// RegisterFleetWorkspaceGeneratingHandler configures a FleetWorkspaceController to execute a FleetWorkspaceGeneratingHandler for every events observed, passing the returned objects to the provided apply.Apply. -// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution -func RegisterFleetWorkspaceGeneratingHandler(ctx context.Context, controller FleetWorkspaceController, apply apply.Apply, - condition condition.Cond, name string, handler FleetWorkspaceGeneratingHandler, opts *generic.GeneratingHandlerOptions) { - statusHandler := &fleetWorkspaceGeneratingHandler{ - FleetWorkspaceGeneratingHandler: handler, - apply: apply, - name: name, - gvk: controller.GroupVersionKind(), - } - if opts != nil { - statusHandler.opts = *opts - } - controller.OnChange(ctx, name, statusHandler.Remove) - RegisterFleetWorkspaceStatusHandler(ctx, controller, condition, name, statusHandler.Handle) -} - -type fleetWorkspaceStatusHandler struct { - client FleetWorkspaceClient - condition condition.Cond - handler FleetWorkspaceStatusHandler -} - -// sync is executed on every resource addition or modification. Executes the configured handlers and sends the updated status to the Kubernetes API -func (a *fleetWorkspaceStatusHandler) sync(key string, obj *v3.FleetWorkspace) (*v3.FleetWorkspace, error) { - if obj == nil { - return obj, nil - } - - origStatus := obj.Status.DeepCopy() - obj = obj.DeepCopy() - newStatus, err := a.handler(obj, obj.Status) - if err != nil { - // Revert to old status on error - newStatus = *origStatus.DeepCopy() - } - - if a.condition != "" { - if errors.IsConflict(err) { - a.condition.SetError(&newStatus, "", nil) - } else { - a.condition.SetError(&newStatus, "", err) - } - } - if !equality.Semantic.DeepEqual(origStatus, &newStatus) { - if a.condition != "" { - // Since status has changed, update the lastUpdatedTime - a.condition.LastUpdated(&newStatus, time.Now().UTC().Format(time.RFC3339)) - } - - var newErr error - obj.Status = newStatus - newObj, newErr := a.client.UpdateStatus(obj) - if err == nil { - err = newErr - } - if newErr == nil { - obj = newObj - } - } - return obj, err -} - -type fleetWorkspaceGeneratingHandler struct { - FleetWorkspaceGeneratingHandler - apply apply.Apply - opts generic.GeneratingHandlerOptions - gvk schema.GroupVersionKind - name string - seen sync.Map -} - -// Remove handles the observed deletion of a resource, cascade deleting every associated resource previously applied -func (a *fleetWorkspaceGeneratingHandler) Remove(key string, obj *v3.FleetWorkspace) (*v3.FleetWorkspace, error) { - if obj != nil { - return obj, nil - } - - obj = &v3.FleetWorkspace{} - obj.Namespace, obj.Name = kv.RSplit(key, "/") - obj.SetGroupVersionKind(a.gvk) - - if a.opts.UniqueApplyForResourceVersion { - a.seen.Delete(key) - } - - return nil, generic.ConfigureApplyForObject(a.apply, obj, &a.opts). - WithOwner(obj). - WithSetID(a.name). - ApplyObjects() -} - -// Handle executes the configured FleetWorkspaceGeneratingHandler and pass the resulting objects to apply.Apply, finally returning the new status of the resource -func (a *fleetWorkspaceGeneratingHandler) Handle(obj *v3.FleetWorkspace, status v3.FleetWorkspaceStatus) (v3.FleetWorkspaceStatus, error) { - if !obj.DeletionTimestamp.IsZero() { - return status, nil - } - - objs, newStatus, err := a.FleetWorkspaceGeneratingHandler(obj, status) - if err != nil { - return newStatus, err - } - if !a.isNewResourceVersion(obj) { - return newStatus, nil - } - - err = generic.ConfigureApplyForObject(a.apply, obj, &a.opts). - WithOwner(obj). - WithSetID(a.name). - ApplyObjects(objs...) - if err != nil { - return newStatus, err - } - a.storeResourceVersion(obj) - return newStatus, nil -} - -// isNewResourceVersion detects if a specific resource version was already successfully processed. -// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions -func (a *fleetWorkspaceGeneratingHandler) isNewResourceVersion(obj *v3.FleetWorkspace) bool { - if !a.opts.UniqueApplyForResourceVersion { - return true - } - - // Apply once per resource version - key := obj.Namespace + "/" + obj.Name - previous, ok := a.seen.Load(key) - return !ok || previous != obj.ResourceVersion -} - -// storeResourceVersion keeps track of the latest resource version of an object for which Apply was executed -// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions -func (a *fleetWorkspaceGeneratingHandler) storeResourceVersion(obj *v3.FleetWorkspace) { - if !a.opts.UniqueApplyForResourceVersion { - return - } - - key := obj.Namespace + "/" + obj.Name - a.seen.Store(key, obj.ResourceVersion) -} diff --git a/pkg/generated/controllers/management.cattle.io/v3/interface.go b/pkg/generated/controllers/management.cattle.io/v3/interface.go index ffa7f9084..a584f3beb 100644 --- a/pkg/generated/controllers/management.cattle.io/v3/interface.go +++ b/pkg/generated/controllers/management.cattle.io/v3/interface.go @@ -34,7 +34,6 @@ type Interface interface { Cluster() ClusterController ClusterProxyConfig() ClusterProxyConfigController ClusterRoleTemplateBinding() ClusterRoleTemplateBindingController - FleetWorkspace() FleetWorkspaceController GlobalRole() GlobalRoleController GlobalRoleBinding() GlobalRoleBindingController Node() NodeController @@ -66,10 +65,6 @@ func (v *version) ClusterRoleTemplateBinding() ClusterRoleTemplateBindingControl return generic.NewController[*v3.ClusterRoleTemplateBinding, *v3.ClusterRoleTemplateBindingList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "ClusterRoleTemplateBinding"}, "clusterroletemplatebindings", true, v.controllerFactory) } -func (v *version) FleetWorkspace() FleetWorkspaceController { - return generic.NewNonNamespacedController[*v3.FleetWorkspace, *v3.FleetWorkspaceList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "FleetWorkspace"}, "fleetworkspaces", v.controllerFactory) -} - func (v *version) GlobalRole() GlobalRoleController { return generic.NewNonNamespacedController[*v3.GlobalRole, *v3.GlobalRoleList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "GlobalRole"}, "globalroles", v.controllerFactory) } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/GlobalRole.md b/pkg/resources/management.cattle.io/v3/globalrole/GlobalRole.md index bc891daad..d773f3b0e 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/GlobalRole.md +++ b/pkg/resources/management.cattle.io/v3/globalrole/GlobalRole.md @@ -16,7 +16,7 @@ Rules without verbs, resources, or apigroups are not permitted. The `rules` incl Escalation checks are bypassed if a user has the `escalate` verb on the GlobalRole that they are attempting to update or create. This can also be given through a wildcard permission (i.e. the `*` verb also gives `escalate`). -Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles`. +Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles` and the rules in `inheritedFleetWorkspacePermissions`. Users can only grant rules in the `NamespacedRules` field with rights less than or equal to those they currently possess. This works on a per namespace basis, meaning that the user must have the permission in the namespace specified. The `Rules` field apply to every namespace, which means a user can create `NamespacedRules` in any namespace that are equal to or less than the `Rules` they currently possess. diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 300992285..0e3a885b7 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -6,8 +6,6 @@ import ( "fmt" "testing" - "k8s.io/apimachinery/pkg/labels" - "github.com/golang/mock/gomock" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" @@ -44,7 +42,7 @@ var ( }, }, } - clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR, readFleetWorkspacesCR} + clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR} clusterRoleBindings = []*v1.ClusterRoleBinding{ { @@ -76,12 +74,6 @@ var ( }, }, }, - { - Subjects: []v1.Subject{ - {Kind: v1.UserKind, Name: testUser}, - }, - RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: readFleetWorkspacesCR.Name}, - }, } baseRT = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -107,6 +99,12 @@ var ( }, }, InheritedClusterRoles: []string{baseRT.Name}, + InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleReadPods, + }, + WorkspaceVerbs: []string{"GET"}, + }, } baseGRB = v3.GlobalRoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -136,13 +134,6 @@ var ( APIGroups: []string{"*"}, Resources: []string{"*"}, } - ruleReadFleetWorkspaces = v1.PolicyRule{ - Verbs: []string{"GET"}, - APIGroups: []string{"management.cattle.io"}, - Resources: []string{"fleetworkspaces"}, - ResourceNames: []string{"fleet-default"}, - } - adminCR = &v1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ Name: "admin-role", @@ -153,10 +144,6 @@ var ( ObjectMeta: metav1.ObjectMeta{Name: "read-pods"}, Rules: []v1.PolicyRule{ruleReadPods}, } - readFleetWorkspacesCR = &v1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{Name: "read-fleetworkspaces"}, - Rules: []v1.PolicyRule{ruleReadFleetWorkspaces}, - } roleTemplate = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -191,7 +178,6 @@ type testState struct { rtCacheMock *fake.MockNonNamespacedCacheInterface[*v3.RoleTemplate] grCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRole] grbCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRoleBinding] - fwCacheMock *fake.MockNonNamespacedCacheInterface[*v3.FleetWorkspace] sarMock *k8fake.FakeSubjectAccessReviews resolver validation.AuthorizationRuleResolver } @@ -279,21 +265,12 @@ func newDefaultState(t *testing.T) testState { rtCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl) grCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl) grbCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) - fwCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.FleetWorkspace](ctrl) grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() rtCacheMock.EXPECT().Get(baseRT.Name).Return(&baseRT, nil).AnyTimes() - fwCacheMock.EXPECT().List(labels.Everything()).Return([]*v3.FleetWorkspace{ - { - ObjectMeta: metav1.ObjectMeta{Name: "fleet-default"}, - }, - { - ObjectMeta: metav1.ObjectMeta{Name: "fleet-local"}, - }, - }, nil).AnyTimes() k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} @@ -302,7 +279,6 @@ func newDefaultState(t *testing.T) testState { rtCacheMock: rtCacheMock, grCacheMock: grCacheMock, grbCacheMock: grbCacheMock, - fwCacheMock: fwCacheMock, sarMock: fakeSAR, resolver: resolver, } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index 660ddb5dd..b170752ea 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -8,14 +8,12 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" - mgmtv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" "github.com/rancher/webhook/pkg/resolvers" "github.com/rancher/webhook/pkg/resources/common" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1" @@ -36,13 +34,12 @@ const ( // NewValidator returns a new validator used for validation globalRoles. func NewValidator(ruleResolver validation.AuthorizationRuleResolver, grbResolver *resolvers.GRBClusterRuleResolver, - sar authorizationv1.SubjectAccessReviewInterface, fwCache mgmtv3.FleetWorkspaceCache) *Validator { + sar authorizationv1.SubjectAccessReviewInterface) *Validator { return &Validator{ admitter: admitter{ resolver: ruleResolver, grbResolver: grbResolver, sar: sar, - fwCache: fwCache, }, } } @@ -76,7 +73,6 @@ type admitter struct { resolver validation.AuthorizationRuleResolver grbResolver *resolvers.GRBClusterRuleResolver sar authorizationv1.SubjectAccessReviewInterface - fwCache mgmtv3.FleetWorkspaceCache } // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. @@ -132,8 +128,8 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp nsrPath.Child(index))) } // Validate fleet workspace rules - if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { - fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR) + if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil { + fleetWorkspaceRules := newGR.InheritedFleetWorkspacePermissions.ResourceRules fwrPath := fldPath.Child("inheritedFleetWorkspacePermissions").Child("resourceRules") returnError = errors.Join(returnError, common.ValidateRules(fleetWorkspaceRules, true, fwrPath)) } @@ -164,22 +160,6 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseAllowed(), nil } } - // Check for escalations in fleet workspace rules - if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { - fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR) - workspaceNames, err := a.allFleetWorkspaceNamesExceptLocal() - if err != nil { - return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err) - } - for _, w := range workspaceNames { - returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceRules, a.resolver, w)) - if escalateChecker.HasVerb() { - return admission.ResponseAllowed(), nil - } - } - fleetWorkspaceVerbs := a.grbResolver.GlobalRoleResolver.FleetWorkspaceVerbsRuleFromRole(newGR, workspaceNames) - returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceVerbs, a.resolver, "")) - } if returnError != nil { return admission.ResponseFailedEscalation(fmt.Sprintf("errors due to escalation: %v", returnError)), nil } @@ -187,22 +167,6 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return admission.ResponseAllowed(), nil } -// allFleetWorkspaceNamesExceptLocal returns all workspace names except fleet-local -func (a *admitter) allFleetWorkspaceNamesExceptLocal() ([]string, error) { - workspaces, err := a.fwCache.List(labels.Everything()) - if err != nil { - return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err) - } - var workspaceNames []string - for _, w := range workspaces { - if w.Name != "fleet-local" { - workspaceNames = append(workspaceNames, w.Name) - } - } - - return workspaceNames, nil -} - // validateDelete checks if a global role can be deleted and returns the appropriate response. func validateDelete(oldRole *v3.GlobalRole, fldPath *field.Path) (*admissionv1.AdmissionResponse, error) { if oldRole.Builtin { diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index 703e5b5cc..c40083fbb 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -857,7 +857,7 @@ func TestAdmit(t *testing.T) { test.args.stateSetup(state) } grbResolver := state.createBaseGRBResolver() - admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock, state.fwCacheMock).Admitters() + admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters() assert.Len(t, admitters, 1) req := createGRRequest(t, test) @@ -875,7 +875,7 @@ func TestAdmit(t *testing.T) { func Test_UnexpectedErrors(t *testing.T) { t.Parallel() resolver, _ := validation.NewTestRuleResolver(nil, nil, nil, nil) - validator := globalrole.NewValidator(resolver, nil, nil, nil) + validator := globalrole.NewValidator(resolver, nil, nil) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") test := testCase{ diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go index c921e2821..7ffdd1a7e 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go @@ -3,6 +3,8 @@ package clusterrole import ( "fmt" + v1 "k8s.io/api/rbac/v1" + "github.com/rancher/webhook/pkg/admission" objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" "github.com/rancher/webhook/pkg/resources/common" @@ -25,7 +27,9 @@ type Validator struct { // NewValidator returns a new validator for roles. func NewValidator() *Validator { return &Validator{ - admitter: admitter{}, + admitter: admitter{ + clusterRoleOldAndNewFromRequest: objectsv1.ClusterRoleOldAndNewFromRequest, + }, } } @@ -64,14 +68,18 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type admitter struct{} +type clusterRoleOldAndNewFromRequest func(request *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error) + +type admitter struct { + clusterRoleOldAndNewFromRequest clusterRoleOldAndNewFromRequest +} // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { listTrace := trace.New("clusterRoleValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - oldRole, newRole, err := objectsv1.ClusterRoleOldAndNewFromRequest(&request.AdmissionRequest) + oldRole, newRole, err := a.clusterRoleOldAndNewFromRequest(&request.AdmissionRequest) if err != nil { return nil, err } diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go index 6fcdb3953..e383a1676 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go @@ -3,14 +3,16 @@ package clusterrole import ( "context" "encoding/json" + "errors" "testing" + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/runtime" + "github.com/rancher/webhook/pkg/admission" "github.com/stretchr/testify/require" - admissionv1 "k8s.io/api/admission/v1" v1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" ) var ( @@ -53,10 +55,11 @@ func TestAdmit(t *testing.T) { newRole *v1.ClusterRole } type test struct { - name string - args args - allowed bool - wantErr bool + name string + args args + allowed bool + clusterRoleOldAndNewFromRequest clusterRoleOldAndNewFromRequest + wantErr bool } tests := []test{ { @@ -131,6 +134,16 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "error getting old and new ClusterRole from request", + args: args{ + oldRole: roleWithOwnerLabel.DeepCopy(), + newRole: emptyRole.DeepCopy(), + }, + clusterRoleOldAndNewFromRequest: clusterRoleOldAndNewFromRequestError, + allowed: false, + wantErr: true, + }, } for _, test := range tests { @@ -157,8 +170,11 @@ func TestAdmit(t *testing.T) { req.OldObject.Raw, err = json.Marshal(test.args.oldRole) require.NoError(t, err) - admitter := NewValidator().Admitters() - + validator := NewValidator() + if test.clusterRoleOldAndNewFromRequest != nil { + validator.admitter.clusterRoleOldAndNewFromRequest = test.clusterRoleOldAndNewFromRequest + } + admitter := validator.Admitters() response, err := admitter[0].Admit(req) if test.wantErr { require.Error(t, err) @@ -170,3 +186,7 @@ func TestAdmit(t *testing.T) { }) } } + +func clusterRoleOldAndNewFromRequestError(_ *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error) { + return nil, nil, errors.New("unexpected") +} diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go index 06801a786..e00acf006 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go @@ -3,8 +3,10 @@ package clusterrolebinding import ( "fmt" - "github.com/rancher/webhook/pkg/admission" objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" + v1 "k8s.io/api/rbac/v1" + + "github.com/rancher/webhook/pkg/admission" "github.com/rancher/webhook/pkg/resources/common" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -25,7 +27,9 @@ type Validator struct { // NewValidator returns a new validator for clusterrolebindings. func NewValidator() *Validator { return &Validator{ - admitter: admitter{}, + admitter: admitter{ + clusterRoleBindingOldAndNewFromRequest: objectsv1.ClusterRoleBindingOldAndNewFromRequest, + }, } } @@ -65,14 +69,18 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type admitter struct{} +type clusterRoleBindingOldAndNewFromRequest func(request *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, *v1.ClusterRoleBinding, error) + +type admitter struct { + clusterRoleBindingOldAndNewFromRequest clusterRoleBindingOldAndNewFromRequest +} // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { listTrace := trace.New("clusterRolebindingValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - oldRoleBinding, newRoleBinding, err := objectsv1.ClusterRoleBindingOldAndNewFromRequest(&request.AdmissionRequest) + oldRoleBinding, newRoleBinding, err := a.clusterRoleBindingOldAndNewFromRequest(&request.AdmissionRequest) if err != nil { return nil, err } diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go index 91596e134..759446bc8 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go @@ -3,6 +3,7 @@ package clusterrolebinding import ( "context" "encoding/json" + "errors" "testing" "github.com/rancher/webhook/pkg/admission" @@ -54,10 +55,11 @@ func TestAdmit(t *testing.T) { newRB *v1.ClusterRoleBinding } type test struct { - name string - args args - allowed bool - wantErr bool + name string + args args + allowed bool + clusterRoleBindingOldAndNewFromRequest clusterRoleBindingOldAndNewFromRequest + wantErr bool } tests := []test{ { @@ -132,6 +134,16 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "error getting old and new ClusterRoleBinding from request", + args: args{ + oldRB: roleBindingWithOwnerLabel.DeepCopy(), + newRB: emptyLabelsRoleBinding.DeepCopy(), + }, + allowed: false, + clusterRoleBindingOldAndNewFromRequest: clusterRoleBindingOldAndNewFromRequestError, + wantErr: true, + }, } for _, test := range tests { @@ -158,7 +170,11 @@ func TestAdmit(t *testing.T) { req.OldObject.Raw, err = json.Marshal(test.args.oldRB) require.NoError(t, err) - admitter := NewValidator().Admitters() + validator := NewValidator() + if test.clusterRoleBindingOldAndNewFromRequest != nil { + validator.admitter.clusterRoleBindingOldAndNewFromRequest = test.clusterRoleBindingOldAndNewFromRequest + } + admitter := validator.Admitters() response, err := admitter[0].Admit(req) if test.wantErr { @@ -167,7 +183,10 @@ func TestAdmit(t *testing.T) { } require.NoError(t, err) require.Equalf(t, test.allowed, response.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, response.Allowed, response.Result) - }) } } + +func clusterRoleBindingOldAndNewFromRequestError(_ *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, *v1.ClusterRoleBinding, error) { + return nil, nil, errors.New("unexpected") +} diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index 1f29a0ffc..f65766957 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -44,7 +44,7 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle prtbResolver := resolvers.NewPRTBRuleResolver(clients.Management.ProjectRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) grbResolver := resolvers.NewGRBClusterRuleResolver(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) psact := podsecurityadmissionconfigurationtemplate.NewValidator(clients.Management.Cluster().Cache(), clients.Provisioning.Cluster().Cache()) - globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.FleetWorkspace().Cache()) + globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) prtbs := projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.Cluster().Cache(), clients.Management.Project().Cache()) crtbs := clusterroletemplatebinding.NewValidator(crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.GlobalRoleBinding().Cache(), clients.Management.Cluster().Cache()) From f10e67f6811b77334e168fcbd2857318edc3f404 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 19 Apr 2024 11:29:33 +0200 Subject: [PATCH 05/16] validate fleetworkspace rules in its own resolver --- pkg/auth/globalrole.go | 26 ++++++- pkg/resolvers/grbClusterResolver.go | 70 +++++++++++++------ pkg/resolvers/grbClusterResolver_test.go | 2 +- .../v3/globalrole/setup_test.go | 9 ++- .../v3/globalrole/validator.go | 42 +++++++---- .../v3/globalrole/validator_test.go | 7 +- .../v3/globalrolebinding/setup_test.go | 47 +++++++++++++ .../v3/globalrolebinding/validator.go | 45 ++++++++---- .../v3/globalrolebinding/validator_test.go | 62 ++++++++++++++-- pkg/server/handlers.go | 6 +- 10 files changed, 253 insertions(+), 63 deletions(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 2437be15d..179aa2e25 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -65,17 +65,37 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P } rules = append(rules, templateRules...) } + + return rules, nil +} + +func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { + if gr == nil { + return nil + } + + var rules []rbacv1.PolicyRule if gr.InheritedFleetWorkspacePermissions.ResourceRules != nil { rules = append(rules, gr.InheritedFleetWorkspacePermissions.ResourceRules...) } + + return rules +} + +func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { + if gr == nil { + return nil + } + if gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { - rules = append(rules, rbacv1.PolicyRule{ + return []rbacv1.PolicyRule{{ Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs, APIGroups: []string{"management.cattle.io"}, Resources: []string{"fleetworkspaces"}, - }) + }} } - return rules, nil + + return nil } // GetRoleTemplate allows the caller to retrieve the roleTemplates in use by a given global role. Does not diff --git a/pkg/resolvers/grbClusterResolver.go b/pkg/resolvers/grbClusterResolver.go index e40cabc8e..65fd2b9ee 100644 --- a/pkg/resolvers/grbClusterResolver.go +++ b/pkg/resolvers/grbClusterResolver.go @@ -18,18 +18,46 @@ const ( // GRBClusterRuleResolver implements the rbacv1.AuthorizationRuleResolver interface. Provides rule resolution // for the permissions a GRB gives that apply in a given cluster (or all clusters). type GRBClusterRuleResolver struct { - GlobalRoleBindings v3.GlobalRoleBindingCache - GlobalRoleResolver *auth.GlobalRoleResolver + gbrCache v3.GlobalRoleBindingCache + grResolver *auth.GlobalRoleResolver + ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) } -// New NewGRBClusterRuleResolver returns a new resolver for resolving rules given through GlobalRoleBindings +// New NewGRBClusterRuleResolver returns a new resolver for resolving rules given through gbrCache // which apply to cluster(s). This function can only be called once for each unique instance of grbCache. -func NewGRBClusterRuleResolver(grbCache v3.GlobalRoleBindingCache, grResolver *auth.GlobalRoleResolver) *GRBClusterRuleResolver { +func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.GlobalRoleResolver) (*GRBClusterRuleResolver, *GRBClusterRuleResolver, *GRBClusterRuleResolver) { grbCache.AddIndexer(grbSubjectIndex, grbBySubject) - return &GRBClusterRuleResolver{ - GlobalRoleBindings: grbCache, - GlobalRoleResolver: grResolver, + inheritedClusterRoleResolver := &GRBClusterRuleResolver{ + gbrCache: grbCache, + grResolver: grResolver, + ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { + var err error + var rules []rbacv1.PolicyRule + // the downstream clusters, so we take the local cluster rules from the GlobalRules + if namespace == localCluster { + rules = grResolver.GlobalRulesFromRole(gr) + } else { + rules, err = grResolver.ClusterRulesFromRole(gr) + } + return rules, err + }, } + fleetWorkspaceResourceRulesResolver := &GRBClusterRuleResolver{ + gbrCache: grbCache, + grResolver: grResolver, + ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { + return grResolver.FleetWorkspacePermissionsResourceRulesFromRole(gr), nil + }, + } + fleetWorkspaceVerbsResolver := &GRBClusterRuleResolver{ + gbrCache: grbCache, + grResolver: grResolver, + ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { + return grResolver.FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr), nil + }, + } + + return inheritedClusterRoleResolver, fleetWorkspaceResourceRulesResolver, fleetWorkspaceVerbsResolver } // GetRoleReferenceRules is used to find which rules are granted by a rolebinding/clusterRoleBinding. Since we don't @@ -41,26 +69,32 @@ func (g *GRBClusterRuleResolver) GetRoleReferenceRules(rbacv1.RoleRef, string) ( // RulesFor returns the list of Cluster rules that apply in a given namespace (usually either the namespace of a // specific cluster or "" for all clusters). If an error is returned, the slice of PolicyRules may not be complete, // but contains all retrievable rules. -func (g *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { +func (r *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { visitor := &ruleAccumulator{} - g.VisitRulesFor(user, namespace, visitor.visit) + r.visitRulesForWithRuleResolver(user, namespace, visitor.visit, r.ruleResolver) return visitor.rules, visitor.getError() } // VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. // If visitor() returns false, visiting is short-circuited. This will return different rules for the "local" namespace. -func (g *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { +func (r *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { + r.visitRulesForWithRuleResolver(user, namespace, visitor, r.ruleResolver) +} + +// visitRulesForWithRuleResolver invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. +// If visitor() returns false, visiting is short-circuited. This will return different rules for the "local" namespace. +func (g *GRBClusterRuleResolver) visitRulesForWithRuleResolver(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool, ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error)) { var grbs []*apisv3.GlobalRoleBinding // gather all grbs that apply to this user through group or user assignment for _, group := range user.GetGroups() { - groupGrbs, err := g.GlobalRoleBindings.GetByIndex(grbSubjectIndex, GetGroupKey(group, "")) + groupGrbs, err := g.gbrCache.GetByIndex(grbSubjectIndex, GetGroupKey(group, "")) if err != nil { visitor(nil, nil, err) continue } grbs = append(grbs, groupGrbs...) } - userGrbs, err := g.GlobalRoleBindings.GetByIndex(grbSubjectIndex, GetUserKey(user.GetName(), "")) + userGrbs, err := g.gbrCache.GetByIndex(grbSubjectIndex, GetUserKey(user.GetName(), "")) // don't return here - we may have gotten bindings from the group lookup to evaluate if err != nil { visitor(nil, nil, err) @@ -68,20 +102,16 @@ func (g *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, grbs = append(grbs, userGrbs...) } for _, grb := range grbs { - globalRole, err := g.GlobalRoleResolver.GlobalRoleCache().Get(grb.GlobalRoleName) + globalRole, err := g.grResolver.GlobalRoleCache().Get(grb.GlobalRoleName) if err != nil { visitor(nil, nil, err) continue } var rules []rbacv1.PolicyRule var ruleError error - // rules for the local cluster come from the GlobalRoles bucket - the ClusterRules only apply to - // the downstream clusters, so we take the local cluster rules from the GlobalRules - if namespace == localCluster { - rules = g.GlobalRoleResolver.GlobalRulesFromRole(globalRole) - } else { - rules, ruleError = g.GlobalRoleResolver.ClusterRulesFromRole(globalRole) - } + + rules, ruleError = ruleResolver(namespace, globalRole, g.grResolver) + if !visitRules(nil, rules, ruleError, visitor) { return } diff --git a/pkg/resolvers/grbClusterResolver_test.go b/pkg/resolvers/grbClusterResolver_test.go index ba50688a0..e398c5d1a 100644 --- a/pkg/resolvers/grbClusterResolver_test.go +++ b/pkg/resolvers/grbClusterResolver_test.go @@ -325,7 +325,7 @@ func (g *GRBClusterRuleResolverSuite) TestGRBClusterRuleResolver() { } grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCache, nil), state.grCache) - grbResolver := NewGRBClusterRuleResolver(state.grbCache, grResolver) + grbResolver, _, _ := NewGRRuleResolvers(state.grbCache, grResolver) rules, err := grbResolver.RulesFor(g.userInfo, test.namespace) g.Require().Len(rules, len(test.wantRules)) diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 0e3a885b7..a5929799d 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -284,7 +284,10 @@ func newDefaultState(t *testing.T) testState { } } -func (m *testState) createBaseGRBResolver() *resolvers.GRBClusterRuleResolver { - grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(m.rtCacheMock, nil), m.grCacheMock) - return resolvers.NewGRBClusterRuleResolver(m.grbCacheMock, grResolver) +func (m *testState) createBaseGRResolver() *auth.GlobalRoleResolver { + return auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(m.rtCacheMock, nil), m.grCacheMock) +} + +func (m *testState) createBaseGRBResolvers(grResolver *auth.GlobalRoleResolver) (*resolvers.GRBClusterRuleResolver, *resolvers.GRBClusterRuleResolver, *resolvers.GRBClusterRuleResolver) { + return resolvers.NewGRRuleResolvers(m.grbCacheMock, grResolver) } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index b170752ea..2096acd63 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -8,6 +8,7 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/auth" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" "github.com/rancher/webhook/pkg/resolvers" "github.com/rancher/webhook/pkg/resources/common" @@ -33,13 +34,16 @@ const ( ) // NewValidator returns a new validator used for validation globalRoles. -func NewValidator(ruleResolver validation.AuthorizationRuleResolver, grbResolver *resolvers.GRBClusterRuleResolver, - sar authorizationv1.SubjectAccessReviewInterface) *Validator { +func NewValidator(ruleResolver validation.AuthorizationRuleResolver, icrResolver *resolvers.GRBClusterRuleResolver, fwResolver *resolvers.GRBClusterRuleResolver, + fwVerbsResolver *resolvers.GRBClusterRuleResolver, sar authorizationv1.SubjectAccessReviewInterface, grResolver *auth.GlobalRoleResolver) *Validator { return &Validator{ admitter: admitter{ - resolver: ruleResolver, - grbResolver: grbResolver, - sar: sar, + resolver: ruleResolver, + grResolver: grResolver, + icrResolver: icrResolver, + fwRulesResolver: fwResolver, + fwVerbsResolver: fwVerbsResolver, + sar: sar, }, } } @@ -70,9 +74,12 @@ func (v *Validator) Admitters() []admission.Admitter { } type admitter struct { - resolver validation.AuthorizationRuleResolver - grbResolver *resolvers.GRBClusterRuleResolver - sar authorizationv1.SubjectAccessReviewInterface + resolver validation.AuthorizationRuleResolver + grResolver *auth.GlobalRoleResolver + icrResolver *resolvers.GRBClusterRuleResolver + fwRulesResolver *resolvers.GRBClusterRuleResolver + fwVerbsResolver *resolvers.GRBClusterRuleResolver + sar authorizationv1.SubjectAccessReviewInterface } // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. @@ -119,7 +126,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp } // Validate the global and namespaced rules of the new GR - globalRules := a.grbResolver.GlobalRoleResolver.GlobalRulesFromRole(newGR) + globalRules := a.grResolver.GlobalRulesFromRole(newGR) returnError := common.ValidateRules(globalRules, false, fldPath.Child("rules")) nsrPath := fldPath.Child("namespacedRules") @@ -139,14 +146,15 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp } // Check for escalations in the rules - clusterRules, err := a.grbResolver.GlobalRoleResolver.ClusterRulesFromRole(newGR) + clusterRules, err := a.grResolver.ClusterRulesFromRole(newGR) if err != nil { return nil, fmt.Errorf("unable to resolve rules for new global role: %w", err) } + fwResourceRules := a.grResolver.FleetWorkspacePermissionsResourceRulesFromRole(newGR) + fwWorkspaceVerbsRules := a.grResolver.FleetWorkspacePermissionsWorkspaceVerbsFromRole(newGR) escalateChecker := common.NewCachedVerbChecker(request, newGR.Name, a.sar, gvr, escalateVerb) - - returnError = escalateChecker.IsRulesAllowed(clusterRules, a.grbResolver, "") + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(clusterRules, a.icrResolver, "")) if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } @@ -154,6 +162,14 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwResourceRules, a.fwRulesResolver, "")) + if escalateChecker.HasVerb() { + return admission.ResponseAllowed(), nil + } + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.fwVerbsResolver, "")) + if escalateChecker.HasVerb() { + return admission.ResponseAllowed(), nil + } for namespace, rules := range newGR.NamespacedRules { returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(rules, a.resolver, namespace)) if escalateChecker.HasVerb() { @@ -198,7 +214,7 @@ func (a *admitter) validateInheritedClusterRoles(oldGR *v3.GlobalRole, newGR *v3 var currentRoleTemplates []*v3.RoleTemplate var err error if newGR != nil { - currentRoleTemplates, err = a.grbResolver.GlobalRoleResolver.GetRoleTemplatesForGlobalRole(newGR) + currentRoleTemplates, err = a.grResolver.GetRoleTemplatesForGlobalRole(newGR) if err != nil { if apierrors.IsNotFound(err) { reason := fmt.Sprintf("unable to find all roleTemplates %s", err.Error()) diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index c40083fbb..e64f965e9 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -856,8 +856,9 @@ func TestAdmit(t *testing.T) { if test.args.stateSetup != nil { test.args.stateSetup(state) } - grbResolver := state.createBaseGRBResolver() - admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters() + grResolver := state.createBaseGRResolver() + icrResolver, fwResolver, fwVerbsResolver := state.createBaseGRBResolvers(grResolver) + admitters := globalrole.NewValidator(state.resolver, icrResolver, fwResolver, fwVerbsResolver, state.sarMock, grResolver).Admitters() assert.Len(t, admitters, 1) req := createGRRequest(t, test) @@ -875,7 +876,7 @@ func TestAdmit(t *testing.T) { func Test_UnexpectedErrors(t *testing.T) { t.Parallel() resolver, _ := validation.NewTestRuleResolver(nil, nil, nil, nil) - validator := globalrole.NewValidator(resolver, nil, nil) + validator := globalrole.NewValidator(resolver, nil, nil, nil, nil, nil) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") test := testCase{ diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go index 76b69349f..692db4d64 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go @@ -134,6 +134,10 @@ var ( }, }, InheritedClusterRoles: []string{baseRT.Name}, + InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + ResourceRules: fwResourceRules, + WorkspaceVerbs: fwWorkspaceVerbs, + }, } adminGR = &v3.GlobalRole{ ObjectMeta: metav1.ObjectMeta{ @@ -250,6 +254,49 @@ var ( }, }, } + fwResourceRules = []rbacv1.PolicyRule{ + { + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"gitrepos"}, + Verbs: []string{"get"}, + }, + } + fwWorkspaceVerbs = []string{"GET"} + fwResourceRulesAdmin = []rbacv1.PolicyRule{ + { + APIGroups: []string{"*"}, + Resources: []string{"*"}, + Verbs: []string{"*"}, + }, + } + fwWorkspaceVerbsAdmin = []string{"*"} + fwGR = v3.GlobalRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "namespacedRules-gr", + }, + InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + ResourceRules: fwResourceRules, + WorkspaceVerbs: fwWorkspaceVerbs, + }, + } + fwGRResourceRulesAdmin = v3.GlobalRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "namespacedRules-gr", + }, + InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + ResourceRules: fwResourceRulesAdmin, + WorkspaceVerbs: fwWorkspaceVerbs, + }, + } + fwGRWorkspaceVerbsAdmin = v3.GlobalRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "namespacedRules-gr", + }, + InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + ResourceRules: fwResourceRules, + WorkspaceVerbs: fwWorkspaceVerbsAdmin, + }, + } ) // createGRBRequest will return a new webhookRequest using the given GRBs diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go index cada9a97a..a1c9433d4 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go @@ -8,6 +8,7 @@ import ( v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/auth" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" "github.com/rancher/webhook/pkg/resolvers" "github.com/rancher/webhook/pkg/resources/common" @@ -37,13 +38,17 @@ var ( const bindVerb = "bind" // NewValidator returns a new validator for GlobalRoleBindings. -func NewValidator(resolver rbacvalidation.AuthorizationRuleResolver, grbResolver *resolvers.GRBClusterRuleResolver, - sar authorizationv1.SubjectAccessReviewInterface) *Validator { +func NewValidator(resolver rbacvalidation.AuthorizationRuleResolver, icrResolver *resolvers.GRBClusterRuleResolver, + fwRulesResolver *resolvers.GRBClusterRuleResolver, fwVerbsResolver *resolvers.GRBClusterRuleResolver, + sar authorizationv1.SubjectAccessReviewInterface, grResolver *auth.GlobalRoleResolver) *Validator { return &Validator{ admitter: admitter{ - resolver: resolver, - grbResolver: grbResolver, - sar: sar, + resolver: resolver, + icrResolver: icrResolver, + fwRulesResolver: fwRulesResolver, + fwVerbsResolver: fwVerbsResolver, + sar: sar, + grResolver: grResolver, }, } } @@ -74,9 +79,12 @@ func (v *Validator) Admitters() []admission.Admitter { } type admitter struct { - resolver rbacvalidation.AuthorizationRuleResolver - grbResolver *resolvers.GRBClusterRuleResolver - sar authorizationv1.SubjectAccessReviewInterface + resolver rbacvalidation.AuthorizationRuleResolver + icrResolver *resolvers.GRBClusterRuleResolver + fwRulesResolver *resolvers.GRBClusterRuleResolver + fwVerbsResolver *resolvers.GRBClusterRuleResolver + grResolver *auth.GlobalRoleResolver + sar authorizationv1.SubjectAccessReviewInterface } // Admit handles the webhook admission request sent to this webhook. @@ -96,7 +104,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp fldPath := field.NewPath(gvr.Resource) // Pull the global role for validation. - globalRole, err := a.grbResolver.GlobalRoleResolver.GlobalRoleCache().Get(newGRB.GlobalRoleName) + globalRole, err := a.grResolver.GlobalRoleCache().Get(newGRB.GlobalRoleName) if err != nil { if !apierrors.IsNotFound(err) { return nil, fmt.Errorf("failed to get GlobalRole '%s': %w", newGRB.Name, err) @@ -121,8 +129,10 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return nil, err } - globalRules := a.grbResolver.GlobalRoleResolver.GlobalRulesFromRole(globalRole) - clusterRules, err := a.grbResolver.GlobalRoleResolver.ClusterRulesFromRole(globalRole) + fwResourceRules := a.grResolver.FleetWorkspacePermissionsResourceRulesFromRole(globalRole) + fwWorkspaceVerbsRules := a.grResolver.FleetWorkspacePermissionsWorkspaceVerbsFromRole(globalRole) + globalRules := a.grResolver.GlobalRulesFromRole(globalRole) + clusterRules, err := a.grResolver.ClusterRulesFromRole(globalRole) if err != nil { if apierrors.IsNotFound(err) { reason := fmt.Sprintf("at least one roleTemplate was not found %s", err.Error()) @@ -135,7 +145,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp var returnError error bindChecker := common.NewCachedVerbChecker(request, globalRole.Name, a.sar, globalRoleGvr, bindVerb) - returnError = bindChecker.IsRulesAllowed(clusterRules, a.grbResolver, "") + returnError = bindChecker.IsRulesAllowed(clusterRules, a.icrResolver, "") if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } @@ -143,6 +153,15 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } + returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwResourceRules, a.fwRulesResolver, "")) + if bindChecker.HasVerb() { + return admission.ResponseAllowed(), nil + } + returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.fwVerbsResolver, "")) + if bindChecker.HasVerb() { + return admission.ResponseAllowed(), nil + } + for namespace, rules := range globalRole.NamespacedRules { returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(rules, a.resolver, namespace)) if bindChecker.HasVerb() { @@ -186,7 +205,7 @@ func (a *admitter) validateCreate(newBinding *v3.GlobalRoleBinding, globalRole * // validateGlobalRole validates that the attached global role isn't trying to use a locked RoleTemplate. func (a *admitter) validateGlobalRole(globalRole *v3.GlobalRole, fieldPath *field.Path) error { - roleTemplates, err := a.grbResolver.GlobalRoleResolver.GetRoleTemplatesForGlobalRole(globalRole) + roleTemplates, err := a.grResolver.GetRoleTemplatesForGlobalRole(globalRole) if err != nil { if apierrors.IsNotFound(err) { reason := fmt.Sprintf("unable to find all roleTemplates %s", err.Error()) diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go index 776db7c98..e759ae9b3 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go @@ -730,6 +730,60 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "GR InheritedFleetWorkspacePermissions allowed", + args: args{ + newGRB: func() *v3.GlobalRoleBinding { + return &v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grb", + }, + UserName: testUser, + GlobalRoleName: fwGR.Name, + } + }, + stateSetup: func(ts testState) { + ts.grCacheMock.EXPECT().Get(fwGR.Name).Return(&fwGR, nil) + }, + }, + allowed: true, + }, + { + name: "GR InheritedFleetWorkspacePermissions not allowed because ResourceRules permissions", + args: args{ + newGRB: func() *v3.GlobalRoleBinding { + return &v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grb", + }, + UserName: testUser, + GlobalRoleName: fwGRResourceRulesAdmin.Name, + } + }, + stateSetup: func(ts testState) { + ts.grCacheMock.EXPECT().Get(fwGRResourceRulesAdmin.Name).Return(&fwGRResourceRulesAdmin, nil) + }, + }, + allowed: false, + }, + { + name: "GR InheritedFleetWorkspacePermissions not allowed because WorkspaceVerbs permissions", + args: args{ + newGRB: func() *v3.GlobalRoleBinding { + return &v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grb", + }, + UserName: testUser, + GlobalRoleName: fwGRWorkspaceVerbsAdmin.Name, + } + }, + stateSetup: func(ts testState) { + ts.grCacheMock.EXPECT().Get(fwGRWorkspaceVerbsAdmin.Name).Return(&fwGRWorkspaceVerbsAdmin, nil) + }, + }, + allowed: false, + }, } for _, test := range tests { @@ -741,8 +795,8 @@ func TestAdmit(t *testing.T) { test.args.stateSetup(state) } grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) - grbResolver := resolvers.NewGRBClusterRuleResolver(state.grbCacheMock, grResolver) - admitters := globalrolebinding.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters() + icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(state.grbCacheMock, grResolver) + admitters := globalrolebinding.NewValidator(state.resolver, icrResolver, fwRulesResolver, fwVerbsResolver, state.sarMock, grResolver).Admitters() require.Len(t, admitters, 1) req := createGRBRequest(t, test) @@ -762,8 +816,8 @@ func Test_UnexpectedErrors(t *testing.T) { t.Parallel() state := newDefaultState(t) grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) - grbResolver := resolvers.NewGRBClusterRuleResolver(state.grbCacheMock, grResolver) - validator := globalrolebinding.NewValidator(state.resolver, grbResolver, state.sarMock) + icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(state.grbCacheMock, grResolver) + validator := globalrolebinding.NewValidator(state.resolver, icrResolver, fwRulesResolver, fwVerbsResolver, state.sarMock, grResolver) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") admitter := admitters[0] diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index f65766957..dc85c2326 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -42,10 +42,10 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle clusterProxyConfigs := clusterproxyconfig.NewValidator(clients.Management.ClusterProxyConfig().Cache()) crtbResolver := resolvers.NewCRTBRuleResolver(clients.Management.ClusterRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) prtbResolver := resolvers.NewPRTBRuleResolver(clients.Management.ProjectRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) - grbResolver := resolvers.NewGRBClusterRuleResolver(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) + icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) psact := podsecurityadmissionconfigurationtemplate.NewValidator(clients.Management.Cluster().Cache(), clients.Provisioning.Cluster().Cache()) - globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) - globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, grbResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews()) + globalRoles := globalrole.NewValidator(clients.DefaultResolver, icrResolver, fwRulesResolver, fwVerbsResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) + globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, icrResolver, fwRulesResolver, fwVerbsResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) prtbs := projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.Cluster().Cache(), clients.Management.Project().Cache()) crtbs := clusterroletemplatebinding.NewValidator(crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.GlobalRoleBinding().Cache(), clients.Management.Cluster().Cache()) roleTemplates := roletemplate.NewValidator(clients.DefaultResolver, clients.RoleTemplateResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.GlobalRole().Cache()) From dbb090d6337086893a13842f9e4e2df2ccb50d45 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 19 Apr 2024 12:27:41 +0200 Subject: [PATCH 06/16] validate workspaceVerbs is not empty --- pkg/resolvers/grbClusterResolver.go | 16 ++++++++-------- .../v3/globalrole/setup_test.go | 13 +++++++++---- .../v3/globalrole/validator.go | 7 +++++++ .../v3/globalrole/validator_test.go | 8 ++++++++ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/pkg/resolvers/grbClusterResolver.go b/pkg/resolvers/grbClusterResolver.go index 65fd2b9ee..885a9527b 100644 --- a/pkg/resolvers/grbClusterResolver.go +++ b/pkg/resolvers/grbClusterResolver.go @@ -23,8 +23,8 @@ type GRBClusterRuleResolver struct { ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) } -// New NewGRBClusterRuleResolver returns a new resolver for resolving rules given through gbrCache -// which apply to cluster(s). This function can only be called once for each unique instance of grbCache. +// NewGRRuleResolvers returns resolvers for resolving rules given through GlobalRoleBindings +// which apply to cluster(s). This function can only be called once for each unique instance of GlobalRoleBindings. func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.GlobalRoleResolver) (*GRBClusterRuleResolver, *GRBClusterRuleResolver, *GRBClusterRuleResolver) { grbCache.AddIndexer(grbSubjectIndex, grbBySubject) inheritedClusterRoleResolver := &GRBClusterRuleResolver{ @@ -45,14 +45,14 @@ func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.Glo fleetWorkspaceResourceRulesResolver := &GRBClusterRuleResolver{ gbrCache: grbCache, grResolver: grResolver, - ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { + ruleResolver: func(_ string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { return grResolver.FleetWorkspacePermissionsResourceRulesFromRole(gr), nil }, } fleetWorkspaceVerbsResolver := &GRBClusterRuleResolver{ gbrCache: grbCache, grResolver: grResolver, - ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { + ruleResolver: func(_ string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { return grResolver.FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr), nil }, } @@ -69,16 +69,16 @@ func (g *GRBClusterRuleResolver) GetRoleReferenceRules(rbacv1.RoleRef, string) ( // RulesFor returns the list of Cluster rules that apply in a given namespace (usually either the namespace of a // specific cluster or "" for all clusters). If an error is returned, the slice of PolicyRules may not be complete, // but contains all retrievable rules. -func (r *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { +func (g *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { visitor := &ruleAccumulator{} - r.visitRulesForWithRuleResolver(user, namespace, visitor.visit, r.ruleResolver) + g.visitRulesForWithRuleResolver(user, namespace, visitor.visit, g.ruleResolver) return visitor.rules, visitor.getError() } // VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. // If visitor() returns false, visiting is short-circuited. This will return different rules for the "local" namespace. -func (r *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { - r.visitRulesForWithRuleResolver(user, namespace, visitor, r.ruleResolver) +func (g *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { + g.visitRulesForWithRuleResolver(user, namespace, visitor, g.ruleResolver) } // visitRulesForWithRuleResolver invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index a5929799d..695eeaca4 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -170,6 +170,7 @@ type testCase struct { type args struct { oldGR func() *v3.GlobalRole newGR func() *v3.GlobalRole + rawNewGR []byte stateSetup func(testState) username string } @@ -225,10 +226,14 @@ func createGRRequest(t *testing.T, test testCase) *admission.Request { req.OldObject.Raw, err = json.Marshal(oldGR) assert.NoError(t, err, "Failed to marshal GR while creating request") } - if newGR != nil { - req.Name = newGR.Name - req.Namespace = newGR.Namespace - req.Object.Raw, err = json.Marshal(newGR) + if newGR != nil || test.args.rawNewGR != nil { + if test.args.rawNewGR != nil { + req.Object.Raw = test.args.rawNewGR + } else { + req.Object.Raw, err = json.Marshal(newGR) + req.Name = newGR.Name + req.Namespace = newGR.Namespace + } assert.NoError(t, err, "Failed to marshal GR while creating request") } else { req.Operation = admissionv1.Delete diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index 2096acd63..3e355e7bf 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -140,6 +140,13 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp fwrPath := fldPath.Child("inheritedFleetWorkspacePermissions").Child("resourceRules") returnError = errors.Join(returnError, common.ValidateRules(fleetWorkspaceRules, true, fwrPath)) } + // Validate fleet workspace verbs + if newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { + fleetWorkspaceVerbs := newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs + if len(fleetWorkspaceVerbs) == 0 { + returnError = errors.Join(returnError, fmt.Errorf("InheritedFleetWorkspacePermissions.WorkspaceVerbs can't be empty")) + } + } if returnError != nil { return admission.ResponseBadRequest(returnError.Error()), nil diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index e64f965e9..df8d892b0 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -798,6 +798,14 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "InheritedFleetWorkspacePermissions rules contains empty WorkspaceVerbs", + args: args{ + username: adminUser, + rawNewGR: []byte(`{"kind":"GlobalRole","apiVersion":"management.cattle.io/v3","metadata":{"name":"gr-new","generateName":"gr-","namespace":"c-namespace","uid":"6534e4ef-f07b-4c61-b88d-95a92cce4852","resourceVersion":"1","generation":1,"creationTimestamp":null},"displayName":"Test Global Role","description":"This is a role created for testing.","inheritedFleetWorkspacePermissions":{"resourceRules":[{"verbs":["GET","WATCH"],"apiGroups":["v1"],"resources":["pods"]}], "workspaceVerbs":[]},"status":{}}`), + }, + allowed: false, + }, { name: "update in InheritedFleetWorkspacePermissions with privilege escalation", args: args{ From 6ffd38ceeb2f9db533389932cfec341ab478cea7 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 19 Apr 2024 15:45:13 +0200 Subject: [PATCH 07/16] improve unit test --- .../v1/clusterrole/validator.go | 11 +--- .../v1/clusterrole/validator_test.go | 51 +++++++++---------- .../v1/clusterrolebinding/validator.go | 16 ++---- .../v1/clusterrolebinding/validator_test.go | 45 +++++++++------- 4 files changed, 58 insertions(+), 65 deletions(-) diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go index 7ffdd1a7e..e35221237 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator.go @@ -3,8 +3,6 @@ package clusterrole import ( "fmt" - v1 "k8s.io/api/rbac/v1" - "github.com/rancher/webhook/pkg/admission" objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" "github.com/rancher/webhook/pkg/resources/common" @@ -27,9 +25,7 @@ type Validator struct { // NewValidator returns a new validator for roles. func NewValidator() *Validator { return &Validator{ - admitter: admitter{ - clusterRoleOldAndNewFromRequest: objectsv1.ClusterRoleOldAndNewFromRequest, - }, + admitter: admitter{}, } } @@ -68,10 +64,7 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type clusterRoleOldAndNewFromRequest func(request *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error) - type admitter struct { - clusterRoleOldAndNewFromRequest clusterRoleOldAndNewFromRequest } // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. @@ -79,7 +72,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp listTrace := trace.New("clusterRoleValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - oldRole, newRole, err := a.clusterRoleOldAndNewFromRequest(&request.AdmissionRequest) + oldRole, newRole, err := objectsv1.ClusterRoleOldAndNewFromRequest(&request.AdmissionRequest) if err != nil { return nil, err } diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go index e383a1676..b6b40e454 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrole/validator_test.go @@ -3,7 +3,6 @@ package clusterrole import ( "context" "encoding/json" - "errors" "testing" admissionv1 "k8s.io/api/admission/v1" @@ -55,11 +54,9 @@ func TestAdmit(t *testing.T) { newRole *v1.ClusterRole } type test struct { - name string - args args - allowed bool - clusterRoleOldAndNewFromRequest clusterRoleOldAndNewFromRequest - wantErr bool + name string + args args + allowed bool } tests := []test{ { @@ -134,16 +131,6 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, - { - name: "error getting old and new ClusterRole from request", - args: args{ - oldRole: roleWithOwnerLabel.DeepCopy(), - newRole: emptyRole.DeepCopy(), - }, - clusterRoleOldAndNewFromRequest: clusterRoleOldAndNewFromRequestError, - allowed: false, - wantErr: true, - }, } for _, test := range tests { @@ -171,22 +158,34 @@ func TestAdmit(t *testing.T) { require.NoError(t, err) validator := NewValidator() - if test.clusterRoleOldAndNewFromRequest != nil { - validator.admitter.clusterRoleOldAndNewFromRequest = test.clusterRoleOldAndNewFromRequest - } admitter := validator.Admitters() response, err := admitter[0].Admit(req) - if test.wantErr { - require.Error(t, err) - return - } require.NoError(t, err) require.Equalf(t, test.allowed, response.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, response.Allowed, response.Result) - }) } } -func clusterRoleOldAndNewFromRequestError(_ *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error) { - return nil, nil, errors.New("unexpected") +func TestAdmin_errors(t *testing.T) { + t.Parallel() + + req := &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "1", + Kind: gvk, + Resource: gvr, + RequestKind: &gvk, + RequestResource: &gvr, + Operation: admissionv1.Update, + Object: runtime.RawExtension{}, + OldObject: runtime.RawExtension{}, + }, + Context: context.Background(), + } + req.Object = runtime.RawExtension{} + + validator := NewValidator() + admitter := validator.Admitters() + _, err := admitter[0].Admit(req) + require.Error(t, err, "Admit should fail on bad request object") } diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go index e00acf006..06801a786 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator.go @@ -3,10 +3,8 @@ package clusterrolebinding import ( "fmt" - objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" - v1 "k8s.io/api/rbac/v1" - "github.com/rancher/webhook/pkg/admission" + objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1" "github.com/rancher/webhook/pkg/resources/common" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -27,9 +25,7 @@ type Validator struct { // NewValidator returns a new validator for clusterrolebindings. func NewValidator() *Validator { return &Validator{ - admitter: admitter{ - clusterRoleBindingOldAndNewFromRequest: objectsv1.ClusterRoleBindingOldAndNewFromRequest, - }, + admitter: admitter{}, } } @@ -69,18 +65,14 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type clusterRoleBindingOldAndNewFromRequest func(request *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, *v1.ClusterRoleBinding, error) - -type admitter struct { - clusterRoleBindingOldAndNewFromRequest clusterRoleBindingOldAndNewFromRequest -} +type admitter struct{} // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { listTrace := trace.New("clusterRolebindingValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - oldRoleBinding, newRoleBinding, err := a.clusterRoleBindingOldAndNewFromRequest(&request.AdmissionRequest) + oldRoleBinding, newRoleBinding, err := objectsv1.ClusterRoleBindingOldAndNewFromRequest(&request.AdmissionRequest) if err != nil { return nil, err } diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go index 759446bc8..c065f70d4 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go @@ -3,7 +3,6 @@ package clusterrolebinding import ( "context" "encoding/json" - "errors" "testing" "github.com/rancher/webhook/pkg/admission" @@ -55,11 +54,9 @@ func TestAdmit(t *testing.T) { newRB *v1.ClusterRoleBinding } type test struct { - name string - args args - allowed bool - clusterRoleBindingOldAndNewFromRequest clusterRoleBindingOldAndNewFromRequest - wantErr bool + name string + args args + allowed bool } tests := []test{ { @@ -140,9 +137,7 @@ func TestAdmit(t *testing.T) { oldRB: roleBindingWithOwnerLabel.DeepCopy(), newRB: emptyLabelsRoleBinding.DeepCopy(), }, - allowed: false, - clusterRoleBindingOldAndNewFromRequest: clusterRoleBindingOldAndNewFromRequestError, - wantErr: true, + allowed: false, }, } @@ -171,22 +166,36 @@ func TestAdmit(t *testing.T) { require.NoError(t, err) validator := NewValidator() - if test.clusterRoleBindingOldAndNewFromRequest != nil { - validator.admitter.clusterRoleBindingOldAndNewFromRequest = test.clusterRoleBindingOldAndNewFromRequest - } admitter := validator.Admitters() response, err := admitter[0].Admit(req) - if test.wantErr { - require.Error(t, err) - return - } + require.NoError(t, err) require.Equalf(t, test.allowed, response.Allowed, "Response was incorrectly validated wanted response.Allowed = '%v' got '%v' message=%+v", test.allowed, response.Allowed, response.Result) }) } } -func clusterRoleBindingOldAndNewFromRequestError(_ *admissionv1.AdmissionRequest) (*v1.ClusterRoleBinding, *v1.ClusterRoleBinding, error) { - return nil, nil, errors.New("unexpected") +func TestAdmin_errors(t *testing.T) { + t.Parallel() + + req := &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "1", + Kind: gvk, + Resource: gvr, + RequestKind: &gvk, + RequestResource: &gvr, + Operation: admissionv1.Update, + Object: runtime.RawExtension{}, + OldObject: runtime.RawExtension{}, + }, + Context: context.Background(), + } + req.Object = runtime.RawExtension{} + + validator := NewValidator() + admitter := validator.Admitters() + _, err := admitter[0].Admit(req) + require.Error(t, err, "Admit should fail on bad request object") } From 2499660c8231a0a024c9488b2dd2fb465327eaa9 Mon Sep 17 00:00:00 2001 From: Raul Cabello Martin Date: Tue, 23 Apr 2024 11:14:29 +0200 Subject: [PATCH 08/16] remove unnecessary comment Co-authored-by: Michael Bolot --- pkg/auth/globalrole.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 179aa2e25..7f3277271 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -41,7 +41,7 @@ func (g *GlobalRoleResolver) GlobalRulesFromRole(gr *v3.GlobalRole) []rbacv1.Pol return gr.Rules } -// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters and fleet workspaces except local. +// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters. func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.PolicyRule, error) { if gr == nil { return nil, nil From 1cfd47e512e9a3981eebac175e146b5b817bf9cc Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 23 Apr 2024 11:16:09 +0200 Subject: [PATCH 09/16] don't create intermediary slice --- pkg/auth/globalrole.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 7f3277271..052b508d7 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -74,12 +74,7 @@ func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr * return nil } - var rules []rbacv1.PolicyRule - if gr.InheritedFleetWorkspacePermissions.ResourceRules != nil { - rules = append(rules, gr.InheritedFleetWorkspacePermissions.ResourceRules...) - } - - return rules + return gr.InheritedFleetWorkspacePermissions.ResourceRules } func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { From 2cbaf6c40802d1c9d3b5b75a8251ad14fc31befa Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 23 Apr 2024 12:19:04 +0200 Subject: [PATCH 10/16] aggregate rule resolvers in GRBRuleResolvers --- ...ClusterResolver.go => grbRuleResolvers.go} | 37 ++++++++++++------- ...olver_test.go => grbRuleResolvers_test.go} | 4 +- .../v3/globalrole/setup_test.go | 4 +- .../v3/globalrole/validator.go | 29 ++++++--------- .../v3/globalrole/validator_test.go | 6 +-- .../v3/globalrolebinding/validator.go | 29 ++++++--------- .../v3/globalrolebinding/validator_test.go | 8 ++-- pkg/server/handlers.go | 6 +-- 8 files changed, 62 insertions(+), 61 deletions(-) rename pkg/resolvers/{grbClusterResolver.go => grbRuleResolvers.go} (73%) rename pkg/resolvers/{grbClusterResolver_test.go => grbRuleResolvers_test.go} (98%) diff --git a/pkg/resolvers/grbClusterResolver.go b/pkg/resolvers/grbRuleResolvers.go similarity index 73% rename from pkg/resolvers/grbClusterResolver.go rename to pkg/resolvers/grbRuleResolvers.go index 885a9527b..a7b30cc8a 100644 --- a/pkg/resolvers/grbClusterResolver.go +++ b/pkg/resolvers/grbRuleResolvers.go @@ -15,19 +15,26 @@ const ( localCluster = "local" ) -// GRBClusterRuleResolver implements the rbacv1.AuthorizationRuleResolver interface. Provides rule resolution +// GRBRuleResolvers contains three rule resolvers for: InheritedClusterRules, FleetWorkspaceRules, FleetWorkspaceVerbs. +type GRBRuleResolvers struct { + ICRResolver *GRBRuleResolver + FWRulesResolver *GRBRuleResolver + FWVerbsResolver *GRBRuleResolver +} + +// GRBRuleResolver implements the rbacv1.AuthorizationRuleResolver interface. Provides rule resolution // for the permissions a GRB gives that apply in a given cluster (or all clusters). -type GRBClusterRuleResolver struct { +type GRBRuleResolver struct { gbrCache v3.GlobalRoleBindingCache grResolver *auth.GlobalRoleResolver ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) } -// NewGRRuleResolvers returns resolvers for resolving rules given through GlobalRoleBindings -// which apply to cluster(s). This function can only be called once for each unique instance of GlobalRoleBindings. -func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.GlobalRoleResolver) (*GRBClusterRuleResolver, *GRBClusterRuleResolver, *GRBClusterRuleResolver) { +// NewGRBRuleResolvers returns resolvers for resolving rules given through GlobalRoleBindings +// which apply to cluster(s). This function can only be called once for each unique instance of grbCache. +func NewGRBRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.GlobalRoleResolver) *GRBRuleResolvers { grbCache.AddIndexer(grbSubjectIndex, grbBySubject) - inheritedClusterRoleResolver := &GRBClusterRuleResolver{ + inheritedClusterRoleResolver := &GRBRuleResolver{ gbrCache: grbCache, grResolver: grResolver, ruleResolver: func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { @@ -42,14 +49,14 @@ func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.Glo return rules, err }, } - fleetWorkspaceResourceRulesResolver := &GRBClusterRuleResolver{ + fleetWorkspaceResourceRulesResolver := &GRBRuleResolver{ gbrCache: grbCache, grResolver: grResolver, ruleResolver: func(_ string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { return grResolver.FleetWorkspacePermissionsResourceRulesFromRole(gr), nil }, } - fleetWorkspaceVerbsResolver := &GRBClusterRuleResolver{ + fleetWorkspaceVerbsResolver := &GRBRuleResolver{ gbrCache: grbCache, grResolver: grResolver, ruleResolver: func(_ string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error) { @@ -57,19 +64,23 @@ func NewGRRuleResolvers(grbCache v3.GlobalRoleBindingCache, grResolver *auth.Glo }, } - return inheritedClusterRoleResolver, fleetWorkspaceResourceRulesResolver, fleetWorkspaceVerbsResolver + return &GRBRuleResolvers{ + ICRResolver: inheritedClusterRoleResolver, + FWVerbsResolver: fleetWorkspaceVerbsResolver, + FWRulesResolver: fleetWorkspaceResourceRulesResolver, + } } // GetRoleReferenceRules is used to find which rules are granted by a rolebinding/clusterRoleBinding. Since we don't // use these primitives to refer to the globalRoles, this function returns an empty slice. -func (g *GRBClusterRuleResolver) GetRoleReferenceRules(rbacv1.RoleRef, string) ([]rbacv1.PolicyRule, error) { +func (g *GRBRuleResolver) GetRoleReferenceRules(rbacv1.RoleRef, string) ([]rbacv1.PolicyRule, error) { return []rbacv1.PolicyRule{}, nil } // RulesFor returns the list of Cluster rules that apply in a given namespace (usually either the namespace of a // specific cluster or "" for all clusters). If an error is returned, the slice of PolicyRules may not be complete, // but contains all retrievable rules. -func (g *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { +func (g *GRBRuleResolver) RulesFor(user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { visitor := &ruleAccumulator{} g.visitRulesForWithRuleResolver(user, namespace, visitor.visit, g.ruleResolver) return visitor.rules, visitor.getError() @@ -77,13 +88,13 @@ func (g *GRBClusterRuleResolver) RulesFor(user user.Info, namespace string) ([]r // VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. // If visitor() returns false, visiting is short-circuited. This will return different rules for the "local" namespace. -func (g *GRBClusterRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { +func (g *GRBRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { g.visitRulesForWithRuleResolver(user, namespace, visitor, g.ruleResolver) } // visitRulesForWithRuleResolver invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules. // If visitor() returns false, visiting is short-circuited. This will return different rules for the "local" namespace. -func (g *GRBClusterRuleResolver) visitRulesForWithRuleResolver(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool, ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error)) { +func (g *GRBRuleResolver) visitRulesForWithRuleResolver(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool, ruleResolver func(namespace string, gr *apisv3.GlobalRole, grResolver *auth.GlobalRoleResolver) ([]rbacv1.PolicyRule, error)) { var grbs []*apisv3.GlobalRoleBinding // gather all grbs that apply to this user through group or user assignment for _, group := range user.GetGroups() { diff --git a/pkg/resolvers/grbClusterResolver_test.go b/pkg/resolvers/grbRuleResolvers_test.go similarity index 98% rename from pkg/resolvers/grbClusterResolver_test.go rename to pkg/resolvers/grbRuleResolvers_test.go index e398c5d1a..511b59232 100644 --- a/pkg/resolvers/grbClusterResolver_test.go +++ b/pkg/resolvers/grbRuleResolvers_test.go @@ -325,9 +325,9 @@ func (g *GRBClusterRuleResolverSuite) TestGRBClusterRuleResolver() { } grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCache, nil), state.grCache) - grbResolver, _, _ := NewGRRuleResolvers(state.grbCache, grResolver) + grbResolvers := NewGRBRuleResolvers(state.grbCache, grResolver) - rules, err := grbResolver.RulesFor(g.userInfo, test.namespace) + rules, err := grbResolvers.ICRResolver.RulesFor(g.userInfo, test.namespace) g.Require().Len(rules, len(test.wantRules)) for _, rule := range test.wantRules { g.Require().Contains(rules, rule) diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 695eeaca4..5e34849ef 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -293,6 +293,6 @@ func (m *testState) createBaseGRResolver() *auth.GlobalRoleResolver { return auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(m.rtCacheMock, nil), m.grCacheMock) } -func (m *testState) createBaseGRBResolvers(grResolver *auth.GlobalRoleResolver) (*resolvers.GRBClusterRuleResolver, *resolvers.GRBClusterRuleResolver, *resolvers.GRBClusterRuleResolver) { - return resolvers.NewGRRuleResolvers(m.grbCacheMock, grResolver) +func (m *testState) createBaseGRBResolvers(grResolver *auth.GlobalRoleResolver) *resolvers.GRBRuleResolvers { + return resolvers.NewGRBRuleResolvers(m.grbCacheMock, grResolver) } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index 3e355e7bf..b6a486db2 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -34,16 +34,13 @@ const ( ) // NewValidator returns a new validator used for validation globalRoles. -func NewValidator(ruleResolver validation.AuthorizationRuleResolver, icrResolver *resolvers.GRBClusterRuleResolver, fwResolver *resolvers.GRBClusterRuleResolver, - fwVerbsResolver *resolvers.GRBClusterRuleResolver, sar authorizationv1.SubjectAccessReviewInterface, grResolver *auth.GlobalRoleResolver) *Validator { +func NewValidator(ruleResolver validation.AuthorizationRuleResolver, grbResolvers *resolvers.GRBRuleResolvers, sar authorizationv1.SubjectAccessReviewInterface, grResolver *auth.GlobalRoleResolver) *Validator { return &Validator{ admitter: admitter{ - resolver: ruleResolver, - grResolver: grResolver, - icrResolver: icrResolver, - fwRulesResolver: fwResolver, - fwVerbsResolver: fwVerbsResolver, - sar: sar, + resolver: ruleResolver, + grResolver: grResolver, + grbResolvers: grbResolvers, + sar: sar, }, } } @@ -74,12 +71,10 @@ func (v *Validator) Admitters() []admission.Admitter { } type admitter struct { - resolver validation.AuthorizationRuleResolver - grResolver *auth.GlobalRoleResolver - icrResolver *resolvers.GRBClusterRuleResolver - fwRulesResolver *resolvers.GRBClusterRuleResolver - fwVerbsResolver *resolvers.GRBClusterRuleResolver - sar authorizationv1.SubjectAccessReviewInterface + resolver validation.AuthorizationRuleResolver + grResolver *auth.GlobalRoleResolver + grbResolvers *resolvers.GRBRuleResolvers + sar authorizationv1.SubjectAccessReviewInterface } // Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request. @@ -161,7 +156,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp fwWorkspaceVerbsRules := a.grResolver.FleetWorkspacePermissionsWorkspaceVerbsFromRole(newGR) escalateChecker := common.NewCachedVerbChecker(request, newGR.Name, a.sar, gvr, escalateVerb) - returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(clusterRules, a.icrResolver, "")) + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(clusterRules, a.grbResolvers.ICRResolver, "")) if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } @@ -169,11 +164,11 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } - returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwResourceRules, a.fwRulesResolver, "")) + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwResourceRules, a.grbResolvers.FWRulesResolver, "")) if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } - returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.fwVerbsResolver, "")) + returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.grbResolvers.FWVerbsResolver, "")) if escalateChecker.HasVerb() { return admission.ResponseAllowed(), nil } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index df8d892b0..00d2bb9bd 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -865,8 +865,8 @@ func TestAdmit(t *testing.T) { test.args.stateSetup(state) } grResolver := state.createBaseGRResolver() - icrResolver, fwResolver, fwVerbsResolver := state.createBaseGRBResolvers(grResolver) - admitters := globalrole.NewValidator(state.resolver, icrResolver, fwResolver, fwVerbsResolver, state.sarMock, grResolver).Admitters() + grbResolvers := state.createBaseGRBResolvers(grResolver) + admitters := globalrole.NewValidator(state.resolver, grbResolvers, state.sarMock, grResolver).Admitters() assert.Len(t, admitters, 1) req := createGRRequest(t, test) @@ -884,7 +884,7 @@ func TestAdmit(t *testing.T) { func Test_UnexpectedErrors(t *testing.T) { t.Parallel() resolver, _ := validation.NewTestRuleResolver(nil, nil, nil, nil) - validator := globalrole.NewValidator(resolver, nil, nil, nil, nil, nil) + validator := globalrole.NewValidator(resolver, nil, nil, nil) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") test := testCase{ diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go index a1c9433d4..7395f5bd6 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator.go @@ -38,17 +38,14 @@ var ( const bindVerb = "bind" // NewValidator returns a new validator for GlobalRoleBindings. -func NewValidator(resolver rbacvalidation.AuthorizationRuleResolver, icrResolver *resolvers.GRBClusterRuleResolver, - fwRulesResolver *resolvers.GRBClusterRuleResolver, fwVerbsResolver *resolvers.GRBClusterRuleResolver, +func NewValidator(resolver rbacvalidation.AuthorizationRuleResolver, grbResolvers *resolvers.GRBRuleResolvers, sar authorizationv1.SubjectAccessReviewInterface, grResolver *auth.GlobalRoleResolver) *Validator { return &Validator{ admitter: admitter{ - resolver: resolver, - icrResolver: icrResolver, - fwRulesResolver: fwRulesResolver, - fwVerbsResolver: fwVerbsResolver, - sar: sar, - grResolver: grResolver, + resolver: resolver, + grbResolvers: grbResolvers, + sar: sar, + grResolver: grResolver, }, } } @@ -79,12 +76,10 @@ func (v *Validator) Admitters() []admission.Admitter { } type admitter struct { - resolver rbacvalidation.AuthorizationRuleResolver - icrResolver *resolvers.GRBClusterRuleResolver - fwRulesResolver *resolvers.GRBClusterRuleResolver - fwVerbsResolver *resolvers.GRBClusterRuleResolver - grResolver *auth.GlobalRoleResolver - sar authorizationv1.SubjectAccessReviewInterface + resolver rbacvalidation.AuthorizationRuleResolver + grbResolvers *resolvers.GRBRuleResolvers + grResolver *auth.GlobalRoleResolver + sar authorizationv1.SubjectAccessReviewInterface } // Admit handles the webhook admission request sent to this webhook. @@ -145,7 +140,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp var returnError error bindChecker := common.NewCachedVerbChecker(request, globalRole.Name, a.sar, globalRoleGvr, bindVerb) - returnError = bindChecker.IsRulesAllowed(clusterRules, a.icrResolver, "") + returnError = bindChecker.IsRulesAllowed(clusterRules, a.grbResolvers.ICRResolver, "") if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } @@ -153,11 +148,11 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } - returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwResourceRules, a.fwRulesResolver, "")) + returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwResourceRules, a.grbResolvers.FWRulesResolver, "")) if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } - returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.fwVerbsResolver, "")) + returnError = errors.Join(returnError, bindChecker.IsRulesAllowed(fwWorkspaceVerbsRules, a.grbResolvers.FWVerbsResolver, "")) if bindChecker.HasVerb() { return admission.ResponseAllowed(), nil } diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go index e759ae9b3..390395c11 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go @@ -795,8 +795,8 @@ func TestAdmit(t *testing.T) { test.args.stateSetup(state) } grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) - icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(state.grbCacheMock, grResolver) - admitters := globalrolebinding.NewValidator(state.resolver, icrResolver, fwRulesResolver, fwVerbsResolver, state.sarMock, grResolver).Admitters() + gbrResolvers := resolvers.NewGRBRuleResolvers(state.grbCacheMock, grResolver) + admitters := globalrolebinding.NewValidator(state.resolver, gbrResolvers, state.sarMock, grResolver).Admitters() require.Len(t, admitters, 1) req := createGRBRequest(t, test) @@ -816,8 +816,8 @@ func Test_UnexpectedErrors(t *testing.T) { t.Parallel() state := newDefaultState(t) grResolver := auth.NewGlobalRoleResolver(auth.NewRoleTemplateResolver(state.rtCacheMock, nil), state.grCacheMock) - icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(state.grbCacheMock, grResolver) - validator := globalrolebinding.NewValidator(state.resolver, icrResolver, fwRulesResolver, fwVerbsResolver, state.sarMock, grResolver) + gbrResolvers := resolvers.NewGRBRuleResolvers(state.grbCacheMock, grResolver) + validator := globalrolebinding.NewValidator(state.resolver, gbrResolvers, state.sarMock, grResolver) admitters := validator.Admitters() require.Len(t, admitters, 1, "wanted only one admitter") admitter := admitters[0] diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index dc85c2326..ae95058fd 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -42,10 +42,10 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle clusterProxyConfigs := clusterproxyconfig.NewValidator(clients.Management.ClusterProxyConfig().Cache()) crtbResolver := resolvers.NewCRTBRuleResolver(clients.Management.ClusterRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) prtbResolver := resolvers.NewPRTBRuleResolver(clients.Management.ProjectRoleTemplateBinding().Cache(), clients.RoleTemplateResolver) - icrResolver, fwRulesResolver, fwVerbsResolver := resolvers.NewGRRuleResolvers(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) + grbResolvers := resolvers.NewGRBRuleResolvers(clients.Management.GlobalRoleBinding().Cache(), clients.GlobalRoleResolver) psact := podsecurityadmissionconfigurationtemplate.NewValidator(clients.Management.Cluster().Cache(), clients.Provisioning.Cluster().Cache()) - globalRoles := globalrole.NewValidator(clients.DefaultResolver, icrResolver, fwRulesResolver, fwVerbsResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) - globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, icrResolver, fwRulesResolver, fwVerbsResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) + globalRoles := globalrole.NewValidator(clients.DefaultResolver, grbResolvers, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) + globalRoleBindings := globalrolebinding.NewValidator(clients.DefaultResolver, grbResolvers, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.GlobalRoleResolver) prtbs := projectroletemplatebinding.NewValidator(prtbResolver, crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.Cluster().Cache(), clients.Management.Project().Cache()) crtbs := clusterroletemplatebinding.NewValidator(crtbResolver, clients.DefaultResolver, clients.RoleTemplateResolver, clients.Management.GlobalRoleBinding().Cache(), clients.Management.Cluster().Cache()) roleTemplates := roletemplate.NewValidator(clients.DefaultResolver, clients.RoleTemplateResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.GlobalRole().Cache()) From 6a6b8decfece0f3a63a6e1ff92ee79692ec30749 Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 23 Apr 2024 15:35:17 +0200 Subject: [PATCH 11/16] remove duplicated unit test --- .../v1/clusterrolebinding/validator_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go index c065f70d4..2acff81b1 100644 --- a/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go +++ b/pkg/resources/rbac.authorization.k8s.io/v1/clusterrolebinding/validator_test.go @@ -131,14 +131,6 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, - { - name: "error getting old and new ClusterRoleBinding from request", - args: args{ - oldRB: roleBindingWithOwnerLabel.DeepCopy(), - newRB: emptyLabelsRoleBinding.DeepCopy(), - }, - allowed: false, - }, } for _, test := range tests { From 91e65547031c92d37c8d399b9a3053fc8e6d5de1 Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 23 Apr 2024 16:00:08 +0200 Subject: [PATCH 12/16] Add comment in FleetWorkspacePermissionsWorkspaceVerbsFromRole --- pkg/auth/globalrole.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 052b508d7..eafe68f15 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -77,6 +77,10 @@ func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr * return gr.InheritedFleetWorkspacePermissions.ResourceRules } +// FleetWorkspacePermissionsWorkspaceVerbsFromRole finds rules which this GlobalRole gives on the fleetworkspace cluster-wide resources. +// This is assuming a user has permissions in all workspaces (including fleet-local), which is not true. That's fine if we +// use it to evaluate InheritedFleetWorkspacePermissions.WorkspaceVerbs. However, it shouldn't be used in a more generic evaluation +// of permissions on the workspace object. func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { if gr == nil { return nil From 5d11abb6fb5a7e1f38e991597809e4d1f71a2e5c Mon Sep 17 00:00:00 2001 From: raul Date: Mon, 29 Apr 2024 12:56:33 +0200 Subject: [PATCH 13/16] adapt code to FleetWorkspacePermissions change to be a pointer --- pkg/auth/globalrole.go | 4 +- pkg/resolvers/grbRuleResolvers.go | 9 ++- .../v3/globalrole/setup_test.go | 2 +- .../v3/globalrole/validator.go | 4 +- .../v3/globalrole/validator_test.go | 72 +++++++++++-------- 5 files changed, 57 insertions(+), 34 deletions(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index eafe68f15..5e453b846 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -70,7 +70,7 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P } func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { - if gr == nil { + if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } @@ -82,7 +82,7 @@ func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr * // use it to evaluate InheritedFleetWorkspacePermissions.WorkspaceVerbs. However, it shouldn't be used in a more generic evaluation // of permissions on the workspace object. func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { - if gr == nil { + if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } diff --git a/pkg/resolvers/grbRuleResolvers.go b/pkg/resolvers/grbRuleResolvers.go index a7b30cc8a..19c0c0b2a 100644 --- a/pkg/resolvers/grbRuleResolvers.go +++ b/pkg/resolvers/grbRuleResolvers.go @@ -16,9 +16,16 @@ const ( ) // GRBRuleResolvers contains three rule resolvers for: InheritedClusterRules, FleetWorkspaceRules, FleetWorkspaceVerbs. +// InheritedClusterRules grants permissions to all cluster except local. +// FleetWorkspaceRules grants permissions to all fleetworkspaces except local. +// FleetWorkspaceVerbs grants permissions to fleetworkspaces cluster-wide resource except local. +// To ensure that rules are resolved without interference, we require separate resolvers for each of them. type GRBRuleResolvers struct { - ICRResolver *GRBRuleResolver + // ICRResolver resolves rules for GlobalRole rules defined in InheritedClusterRoles. + ICRResolver *GRBRuleResolver + // FWRulesResolver resolves rules for GlobalRole rules defined in InheritedFleetWorkspacePermissions.ResourceRules. FWRulesResolver *GRBRuleResolver + // FWVerbsResolver resolves rules for GlobalRole rules defined in InheritedFleetWorkspacePermissions.WorkspaceVerbs. FWVerbsResolver *GRBRuleResolver } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 5e34849ef..618d53562 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -99,7 +99,7 @@ var ( }, }, InheritedClusterRoles: []string{baseRT.Name}, - InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + InheritedFleetWorkspacePermissions: &v3.FleetWorkspacePermission{ ResourceRules: []v1.PolicyRule{ ruleReadPods, }, diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator.go b/pkg/resources/management.cattle.io/v3/globalrole/validator.go index b6a486db2..aa6097bd5 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator.go @@ -130,13 +130,13 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp nsrPath.Child(index))) } // Validate fleet workspace rules - if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil { + if newGR.InheritedFleetWorkspacePermissions != nil && newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil { fleetWorkspaceRules := newGR.InheritedFleetWorkspacePermissions.ResourceRules fwrPath := fldPath.Child("inheritedFleetWorkspacePermissions").Child("resourceRules") returnError = errors.Join(returnError, common.ValidateRules(fleetWorkspaceRules, true, fwrPath)) } // Validate fleet workspace verbs - if newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { + if newGR.InheritedFleetWorkspacePermissions != nil && newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil { fleetWorkspaceVerbs := newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs if len(fleetWorkspaceVerbs) == 0 { returnError = errors.Join(returnError, fmt.Errorf("InheritedFleetWorkspacePermissions.WorkspaceVerbs can't be empty")) diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index 00d2bb9bd..fe43b330e 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -723,11 +723,13 @@ func TestAdmit(t *testing.T) { username: testUser, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleReadPods, - } - baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ - "GET", + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleReadPods, + }, + WorkspaceVerbs: []string{ + "GET", + }, } return baseGR }, @@ -744,11 +746,13 @@ func TestAdmit(t *testing.T) { username: testUser, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleAdmin, - } - baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ - "GET", + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleAdmin, + }, + WorkspaceVerbs: []string{ + "GET", + }, } return baseGR }, @@ -765,11 +769,13 @@ func TestAdmit(t *testing.T) { username: testUser, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleReadPods, - } - baseGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs = []string{ - "*", + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleReadPods, + }, + WorkspaceVerbs: []string{ + "*", + }, } return baseGR }, @@ -786,11 +792,13 @@ func TestAdmit(t *testing.T) { username: adminUser, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"pods"}, - Verbs: []string{}, + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{}, + }, }, } return baseGR @@ -812,15 +820,19 @@ func TestAdmit(t *testing.T) { username: testUser, oldGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleReadPods, + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleReadPods, + }, } return baseGR }, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleAdmin, + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleAdmin, + }, } return baseGR }, @@ -836,15 +848,19 @@ func TestAdmit(t *testing.T) { username: testUser, oldGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleReadPods, + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleReadPods, + }, } return baseGR }, newGR: func() *v3.GlobalRole { baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions.ResourceRules = []v1.PolicyRule{ - ruleAdmin, + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + ruleAdmin, + }, } return baseGR }, From 08254bbf7c4f4e0e3c71f53b503225ba8d259a82 Mon Sep 17 00:00:00 2001 From: raul Date: Mon, 29 Apr 2024 13:18:10 +0200 Subject: [PATCH 14/16] fix breaking unit test --- .../v3/globalrolebinding/setup_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go index 692db4d64..9b92659f3 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go @@ -134,7 +134,7 @@ var ( }, }, InheritedClusterRoles: []string{baseRT.Name}, - InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + InheritedFleetWorkspacePermissions: &v3.FleetWorkspacePermission{ ResourceRules: fwResourceRules, WorkspaceVerbs: fwWorkspaceVerbs, }, @@ -274,7 +274,7 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "namespacedRules-gr", }, - InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + InheritedFleetWorkspacePermissions: &v3.FleetWorkspacePermission{ ResourceRules: fwResourceRules, WorkspaceVerbs: fwWorkspaceVerbs, }, @@ -283,7 +283,7 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "namespacedRules-gr", }, - InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + InheritedFleetWorkspacePermissions: &v3.FleetWorkspacePermission{ ResourceRules: fwResourceRulesAdmin, WorkspaceVerbs: fwWorkspaceVerbs, }, @@ -292,7 +292,7 @@ var ( ObjectMeta: metav1.ObjectMeta{ Name: "namespacedRules-gr", }, - InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{ + InheritedFleetWorkspacePermissions: &v3.FleetWorkspacePermission{ ResourceRules: fwResourceRules, WorkspaceVerbs: fwWorkspaceVerbsAdmin, }, From 44f5d22a2725fcdc5bb9fa8faf3fb2b2305c28bf Mon Sep 17 00:00:00 2001 From: raul Date: Thu, 13 Jun 2024 11:28:08 +0200 Subject: [PATCH 15/16] Allow Restricted Admin to create GR and GRB with FleetWorkspacePermissions --- pkg/auth/globalrole.go | 22 +++ .../v3/globalrole/setup_test.go | 52 ++++- .../v3/globalrole/validator_test.go | 181 ++++++++++++++++++ .../v3/globalrolebinding/setup_test.go | 52 ++++- .../v3/globalrolebinding/validator_test.go | 19 ++ 5 files changed, 315 insertions(+), 11 deletions(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 5e453b846..42775a951 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -70,6 +70,18 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P } func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { + for _, name := range adminRoles { + if gr.Name == name { + return []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"clusterregistrationtokens", "gitreporestrictions", "clusterregistrations", "clusters", "gitrepos", "bundles", "clustergroups"}, + }, + } + } + } + if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } @@ -82,6 +94,16 @@ func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr * // use it to evaluate InheritedFleetWorkspacePermissions.WorkspaceVerbs. However, it shouldn't be used in a more generic evaluation // of permissions on the workspace object. func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { + for _, name := range adminRoles { + if gr.Name == name { + return []rbacv1.PolicyRule{{ + Verbs: []string{"*"}, + APIGroups: []string{"management.cattle.io"}, + Resources: []string{"fleetworkspaces"}, + }} + } + } + if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 618d53562..47280e87a 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -24,8 +24,9 @@ import ( ) const ( - adminUser = "admin-userid" - testUser = "test-user" + adminUser = "admin-userid" + testUser = "test-user" + restrictedAdminUser = "restricted-admin-userid" ) var ( @@ -42,7 +43,7 @@ var ( }, }, } - clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR} + clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR, restrictedAdminCR} clusterRoleBindings = []*v1.ClusterRoleBinding{ { @@ -51,6 +52,12 @@ var ( }, RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: adminCR.Name}, }, + { + Subjects: []v1.Subject{ + {Kind: v1.UserKind, Name: restrictedAdminUser}, + }, + RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: restrictedAdminCR.Name}, + }, { Subjects: []v1.Subject{ {Kind: v1.UserKind, Name: testUser}, @@ -87,6 +94,24 @@ var ( }, }, } + clusterOwnerRT = v3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-owner", + }, + Rules: []v1.PolicyRule{ + { + APIGroups: []string{ + "*", + }, + Resources: []string{ + "*", + }, + Verbs: []string{ + "*", + }, + }, + }, + } baseGR = v3.GlobalRole{ ObjectMeta: metav1.ObjectMeta{ Name: "base-gr", @@ -106,6 +131,11 @@ var ( WorkspaceVerbs: []string{"GET"}, }, } + restrictedAdminGR = v3.GlobalRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted-admin", + }, + } baseGRB = v3.GlobalRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "base-grb", @@ -113,6 +143,13 @@ var ( GlobalRoleName: baseGR.Name, UserName: testUser, } + restrictedAdminGRB = v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted-admin-grb", + }, + GlobalRoleName: restrictedAdminCR.Name, + UserName: restrictedAdminUser, + } ruleReadPods = v1.PolicyRule{ Verbs: []string{"GET", "WATCH"}, @@ -140,6 +177,12 @@ var ( }, Rules: []v1.PolicyRule{ruleAdmin}, } + restrictedAdminCR = &v1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted-admin", + }, + Rules: []v1.PolicyRule{}, + } readPodsCR = &v1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: "read-pods"}, Rules: []v1.PolicyRule{ruleReadPods}, @@ -272,10 +315,13 @@ func newDefaultState(t *testing.T) testState { grbCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() + grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(restrictedAdminUser, "")).Return([]*v3.GlobalRoleBinding{&restrictedAdminGRB}, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() + rtCacheMock.EXPECT().Get(clusterOwnerRT.Name).Return(&clusterOwnerRT, nil).AnyTimes() rtCacheMock.EXPECT().Get(baseRT.Name).Return(&baseRT, nil).AnyTimes() + rtCacheMock.EXPECT().Get(clusterOwnerRT.Name).Return(&clusterOwnerRT, nil).AnyTimes() k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index fe43b330e..a9609da99 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -870,6 +870,187 @@ func TestAdmit(t *testing.T) { }, allowed: true, }, + { + name: "restricted admin can create GR with InheritedFleetWorkspacePermissions and fleet rules", + args: args{ + username: restrictedAdminUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"get", "list", "create", "delete"}, + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"bundles", "gitrepos"}, + }, + }, + WorkspaceVerbs: []string{ + "get", + "create", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: true, + }, + { + name: "restricted admin can create GR with InheritedFleetWorkspacePermissions and fleet rules and *", + args: args{ + username: restrictedAdminUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"bundles", "gitrepos"}, + }, + }, + WorkspaceVerbs: []string{ + "*", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: true, + }, + { + name: "restricted admin can't create GR with InheritedFleetWorkspacePermissions and pod rules", + args: args{ + username: restrictedAdminUser, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"get", ""}, + APIGroups: []string{""}, + Resources: []string{"pods"}, + }, + }, + WorkspaceVerbs: []string{ + "get", + "create", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: false, + }, + { + name: "restricted admin can update GR with InheritedFleetWorkspacePermissions and fleet rules", + args: args{ + username: restrictedAdminUser, + oldGR: func() *v3.GlobalRole { + return newDefaultGR() + }, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"get", "list", "create", "delete"}, + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"bundles", "gitrepos"}, + }, + }, + WorkspaceVerbs: []string{ + "get", + "create", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: true, + }, + { + name: "restricted admin can update GR with InheritedFleetWorkspacePermissions and fleet rules and *", + args: args{ + username: restrictedAdminUser, + oldGR: func() *v3.GlobalRole { + return newDefaultGR() + }, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"fleet.cattle.io"}, + Resources: []string{"bundles", "gitrepos"}, + }, + }, + WorkspaceVerbs: []string{ + "*", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: true, + }, + { + name: "restricted admin can't update GR with InheritedFleetWorkspacePermissions and pod rules", + args: args{ + username: restrictedAdminUser, + oldGR: func() *v3.GlobalRole { + return newDefaultGR() + }, + newGR: func() *v3.GlobalRole { + baseGR := newDefaultGR() + baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ + ResourceRules: []v1.PolicyRule{ + { + Verbs: []string{"get", ""}, + APIGroups: []string{""}, + Resources: []string{"pods"}, + }, + }, + WorkspaceVerbs: []string{ + "get", + "create", + }, + } + return baseGR + }, + stateSetup: func(state testState) { + state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() + setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) + }, + }, + + allowed: false, + }, } for _, test := range tests { diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go index 9b92659f3..e8614af2e 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go @@ -25,14 +25,15 @@ import ( ) const ( - adminUser = "admin-user" - testUser = "test-user" - noPrivUser = "no-priv-user" - newUser = "newUser-user" - newGroupPrinc = "local://group" - testGroup = "testGroup" - notFoundName = "not-found" - errName = "error-Name" + adminUser = "admin-user" + restrictedAdminUser = "restricted-admin-user" + testUser = "test-user" + noPrivUser = "no-priv-user" + newUser = "newUser-user" + newGroupPrinc = "local://group" + testGroup = "testGroup" + notFoundName = "not-found" + errName = "error-Name" ) type testCase struct { @@ -122,6 +123,24 @@ var ( }, }, } + clusterOwnerRT = v3.RoleTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-owner", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{ + "*", + }, + Resources: []string{ + "*", + }, + Verbs: []string{ + "*", + }, + }, + }, + } baseGR = v3.GlobalRole{ ObjectMeta: metav1.ObjectMeta{ Name: "base-gr", @@ -154,6 +173,13 @@ var ( GlobalRoleName: baseGR.Name, UserName: testUser, } + restrictedAdminGRB = v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "res-grb", + }, + GlobalRoleName: restrictedAdminGR.Name, + UserName: restrictedAdminUser, + } adminRT = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "escalation-rt", @@ -254,6 +280,11 @@ var ( }, }, } + restrictedAdminGR = v3.GlobalRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted-admin", + }, + } fwResourceRules = []rbacv1.PolicyRule{ { APIGroups: []string{"fleet.cattle.io"}, @@ -364,12 +395,17 @@ func newDefaultState(t *testing.T) testState { grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() + grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(restrictedAdminUser, "")).Return([]*v3.GlobalRoleBinding{&restrictedAdminGRB}, nil).AnyTimes() + grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() + grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() grCacheMock.EXPECT().Get(adminGR.Name).Return(adminGR, nil).AnyTimes() grCacheMock.EXPECT().Get(notFoundName).Return(nil, newNotFound(notFoundName)).AnyTimes() grCacheMock.EXPECT().Get("").Return(nil, newNotFound("")).AnyTimes() rtCacheMock.EXPECT().Get(baseRT.Name).Return(&baseRT, nil).AnyTimes() + rtCacheMock.EXPECT().Get(clusterOwnerRT.Name).Return(&clusterOwnerRT, nil).AnyTimes() + k8Fake := &k8testing.Fake{} fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}} diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go index 390395c11..94e3a2502 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go @@ -784,6 +784,25 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, + { + name: "restricted admin can grant GR with InheritedFleetWorkspacePermissions", + args: args{ + username: restrictedAdminUser, + newGRB: func() *v3.GlobalRoleBinding { + return &v3.GlobalRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grb", + }, + UserName: testUser, + GlobalRoleName: fwGR.Name, + } + }, + stateSetup: func(ts testState) { + ts.grCacheMock.EXPECT().Get(fwGR.Name).Return(&fwGR, nil) + }, + }, + allowed: true, + }, } for _, test := range tests { From 70b603aa8d0b9d1ac370d79eb776b417e95c6eb4 Mon Sep 17 00:00:00 2001 From: raul Date: Fri, 14 Jun 2024 17:06:36 +0200 Subject: [PATCH 16/16] Add missing comment to FleetWorkspacePermissionsResourceRulesFromRole --- pkg/auth/globalrole.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index 42775a951..ad2b5bf3b 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -69,6 +69,10 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P return rules, nil } +// FleetWorkspacePermissionsResourceRulesFromRole finds rules which this GlobalRole gives on fleet resources in the workspace backing namespace. +// This is assuming a user has permissions in all workspaces (including fleet-local), which is not true. That's fine if we +// use it to evaluate InheritedFleetWorkspacePermissions.ResourceRules. However, it shouldn't be used in a more generic evaluation +// of permissions on the workspace backing namespace. func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { for _, name := range adminRoles { if gr.Name == name {