diff --git a/cmd/gitops-server/cmd/cmd.go b/cmd/gitops-server/cmd/cmd.go index 8db3928100..562586b033 100644 --- a/cmd/gitops-server/cmd/cmd.go +++ b/cmd/gitops-server/cmd/cmd.go @@ -244,7 +244,7 @@ func runCmd(cmd *cobra.Command, args []string) error { fetcher := fetcher.NewSingleClusterFetcher(cl) - clustersManager := clustersmngr.NewClustersManager([]clustersmngr.ClusterFetcher{fetcher}, nsaccess.NewChecker(nsaccess.DefautltWegoAppRules), log) + clustersManager := clustersmngr.NewClustersManager([]clustersmngr.ClusterFetcher{fetcher}, nsaccess.NewSimplerChecker(), log) clustersManager.Start(ctx) healthChecker := health.NewHealthChecker() diff --git a/core/clustersmngr/factory_test.go b/core/clustersmngr/factory_test.go index 7362bcd0a2..3791c6f1de 100644 --- a/core/clustersmngr/factory_test.go +++ b/core/clustersmngr/factory_test.go @@ -268,7 +268,7 @@ func TestUpdateUserNamespacesFailsToConnect(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - nsChecker := nsaccess.NewChecker(nil) + nsChecker := nsaccess.NewSimplerChecker() clustersFetcher := new(clustersmngrfakes.FakeClusterFetcher) clustersManager := clustersmngr.NewClustersManager([]clustersmngr.ClusterFetcher{clustersFetcher}, nsChecker, logger) @@ -302,7 +302,7 @@ func TestGetClusters(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - nsChecker := nsaccess.NewChecker(nil) + nsChecker := nsaccess.NewSimplerChecker() clustersFetcher := new(clustersmngrfakes.FakeClusterFetcher) clustersManager := clustersmngr.NewClustersManager([]clustersmngr.ClusterFetcher{clustersFetcher}, nsChecker, logger) diff --git a/core/nsaccess/simpler_nsaccess.go b/core/nsaccess/simpler_nsaccess.go new file mode 100644 index 0000000000..98061a28f6 --- /dev/null +++ b/core/nsaccess/simpler_nsaccess.go @@ -0,0 +1,75 @@ +package nsaccess + +import ( + "context" + "fmt" + authv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + typedauth "k8s.io/client-go/kubernetes/typed/authorization/v1" +) + +type simplerChecker struct { +} + +func NewSimplerChecker() Checker { + return simplerChecker{} +} + +func (sc simplerChecker) FilterAccessibleNamespaces(ctx context.Context, auth typedauth.AuthorizationV1Interface, namespaces []corev1.Namespace) ([]corev1.Namespace, error) { + accessToAllNamespace, err := hasAccessToAllNamespaces(ctx, auth) + if err != nil { + return nil, err + } + + if accessToAllNamespace { + return namespaces, nil + } + + var result []corev1.Namespace + for _, ns := range namespaces { + ok, err := hasAccessToNamespace(ctx, auth, ns) + if err != nil { + return nil, fmt.Errorf("user namespace access: %v", err) + } + + if ok { + result = append(result, ns) + } + } + + return result, nil +} + +func hasAccessToNamespace(ctx context.Context, auth typedauth.AuthorizationV1Interface, ns corev1.Namespace) (bool, error) { + ssar := &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Verb: "get", + Resource: "configmaps", + Namespace: ns.Name, + }, + }, + } + res, err := auth.SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{}) + if err != nil { + return false, err + } + return res.Status.Allowed, nil +} + +func hasAccessToAllNamespaces(ctx context.Context, auth typedauth.AuthorizationV1Interface) (bool, error) { + ssar := &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Verb: "list", + Resource: "namespaces", + }, + }, + } + res, err := auth.SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{}) + if err != nil { + return false, err + } + return res.Status.Allowed, nil +} diff --git a/core/nsaccess/simpler_nsaccess_test.go b/core/nsaccess/simpler_nsaccess_test.go new file mode 100644 index 0000000000..0f07826c66 --- /dev/null +++ b/core/nsaccess/simpler_nsaccess_test.go @@ -0,0 +1,116 @@ +package nsaccess + +import ( + "context" + rbacv1 "k8s.io/api/rbac/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/kubectl/pkg/scheme" + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" +) + +func Test_simplerChecker_FilterAccessibleNamespaces(t *testing.T) { + g := NewGomegaWithT(t) + ctx := context.Background() + + testEnv := &envtest.Environment{} + + testCfg, err := testEnv.Start() + g.Expect(err).NotTo(HaveOccurred()) + + t.Cleanup(func() { + err := testEnv.Stop() + if err != nil { + t.Error(err) + } + }) + + adminClient, err := client.New(testCfg, client.Options{ + Scheme: scheme.Scheme, + }) + g.Expect(err).NotTo(HaveOccurred()) + + // The aggregated cluster role controller is not running in the simplified testEnv control plane. + // Prepare the default admin user-facing role for the following tests. + cr := &rbacv1.ClusterRole{} + g.Expect(adminClient.Get(ctx, client.ObjectKey{Name: "admin"}, cr)).To(Succeed()) + g.Expect(cr.Rules).To(BeEmpty()) + cr.Rules = append(cr.Rules, rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + Verbs: []string{"get", "list", "watch"}, + }) + g.Expect(adminClient.Update(ctx, cr)).To(Succeed()) + + t.Run("cluster-admin has access to all namespaces", func(t *testing.T) { + list := &corev1.NamespaceList{} + g.Expect(adminClient.List(ctx, list)).To(Succeed()) + g.Expect(list.Items).To(Not(BeEmpty())) + + cs, err := kubernetes.NewForConfig(testCfg) + g.Expect(err).NotTo(HaveOccurred()) + + res, err := NewSimplerChecker().FilterAccessibleNamespaces(ctx, cs.AuthorizationV1(), list.Items) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(ContainElements(list.Items)) + }) + + t.Run("standard user has access to no namespaces", func(t *testing.T) { + list := &corev1.NamespaceList{} + g.Expect(adminClient.List(ctx, list)).To(Succeed()) + g.Expect(list.Items).To(Not(BeEmpty())) + + user, err := testEnv.AddUser(envtest.User{Name: "no-access-user"}, nil) + cs, err := kubernetes.NewForConfig(user.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + res, err := NewSimplerChecker().FilterAccessibleNamespaces(ctx, cs.AuthorizationV1(), list.Items) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(BeEmpty()) + }) + + t.Run("namespace owner has access to namespace", func(t *testing.T) { + username := "test-user" + + ns := &corev1.Namespace{ + ObjectMeta: v1.ObjectMeta{ + GenerateName: "test-ns-", + }, + } + g.Expect(adminClient.Create(ctx, ns)).To(Succeed()) + + rb := &rbacv1.RoleBinding{ + ObjectMeta: v1.ObjectMeta{ + Name: "test-ns-admins", + Namespace: ns.Name, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "admin", + }, + Subjects: []rbacv1.Subject{{ + Kind: "User", + Name: username, + }}, + } + g.Expect(adminClient.Create(ctx, rb)).To(Succeed()) + + list := &corev1.NamespaceList{} + g.Expect(adminClient.List(ctx, list)).To(Succeed()) + g.Expect(list.Items).To(ContainElement(HaveField("Name", ns.Name))) + + user, err := testEnv.AddUser(envtest.User{Name: username}, nil) + cs, err := kubernetes.NewForConfig(user.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + res, err := NewSimplerChecker().FilterAccessibleNamespaces(ctx, cs.AuthorizationV1(), list.Items) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(ConsistOf(HaveField("Name", ns.Name))) + }) +} diff --git a/core/server/server.go b/core/server/server.go index 5da1ad70e1..d3804990e5 100644 --- a/core/server/server.go +++ b/core/server/server.go @@ -62,7 +62,7 @@ func NewCoreConfig(log logr.Logger, cfg *rest.Config, clusterName string, cluste log: log.WithName("core-server"), RestCfg: cfg, clusterName: clusterName, - NSAccess: nsaccess.NewChecker(nsaccess.DefautltWegoAppRules), + NSAccess: nsaccess.NewSimplerChecker(), ClustersManager: clustersManager, PrimaryKinds: kinds, HealthChecker: healthChecker, diff --git a/pkg/services/crd/suite_test.go b/pkg/services/crd/suite_test.go index 0513ef2d74..2cbfea4555 100644 --- a/pkg/services/crd/suite_test.go +++ b/pkg/services/crd/suite_test.go @@ -68,7 +68,7 @@ func createClient(k8sEnv *testutils.K8sTestEnv) (clustersmngr.Client, clustersmn clustersManager := clustersmngr.NewClustersManager( []clustersmngr.ClusterFetcher{fetcher}, - nsaccess.NewChecker(nsaccess.DefautltWegoAppRules), + nsaccess.NewSimplerChecker(), log, )