diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 2f572cdf54c..708c32257b2 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -19,9 +19,7 @@ import ( "k8s.io/kube-openapi/pkg/spec3" authlib "github.com/grafana/authlib/types" - "github.com/grafana/grafana-app-sdk/logging" - folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/apps/iam/pkg/reconcilers" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -203,11 +201,18 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API getter: folderStore, parents: b.parents, } - storage[resourceInfo.StoragePath("counts")] = &subCountREST{searcher: b.searcher} - storage[resourceInfo.StoragePath("access")] = &subAccessREST{folderStore, b.ac} + storage[resourceInfo.StoragePath("counts")] = &subCountREST{ + getter: folderStore, + searcher: b.searcher, + } + storage[resourceInfo.StoragePath("access")] = &subAccessREST{ + getter: folderStore, + ac: b.ac, + } // Adds a path to return children of a given folder storage[resourceInfo.StoragePath("children")] = &subChildrenREST{ + getter: folderStore, lister: storage[resourceInfo.StoragePath()].(rest.Lister), } diff --git a/pkg/registry/apis/folders/sub_access.go b/pkg/registry/apis/folders/sub_access.go index 2b6bb443252..5f3b11ad361 100644 --- a/pkg/registry/apis/folders/sub_access.go +++ b/pkg/registry/apis/folders/sub_access.go @@ -52,8 +52,7 @@ func (r *subAccessREST) Connect(ctx context.Context, name string, opts runtime.O } // Must be able to get the resource - _, err = r.getter.Get(ctx, name, &v1.GetOptions{}) - if err != nil { + if _, err = r.getter.Get(ctx, name, &v1.GetOptions{}); err != nil { return nil, err } diff --git a/pkg/registry/apis/folders/sub_children.go b/pkg/registry/apis/folders/sub_children.go index 13dccda8d73..4509cc9273a 100644 --- a/pkg/registry/apis/folders/sub_children.go +++ b/pkg/registry/apis/folders/sub_children.go @@ -6,6 +6,7 @@ import ( "net/http" "k8s.io/apimachinery/pkg/apis/meta/internalversion" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" @@ -15,6 +16,7 @@ import ( ) type subChildrenREST struct { + getter rest.Getter lister rest.Lister } @@ -45,6 +47,10 @@ func (r *subChildrenREST) NewConnectOptions() (runtime.Object, bool, string) { } func (r *subChildrenREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { + if _, err := r.getter.Get(ctx, name, &v1.GetOptions{}); err != nil { + return nil, err + } + obj, err := r.lister.List(ctx, &internalversion.ListOptions{ Limit: 500, // TODO, field selector diff --git a/pkg/registry/apis/folders/sub_count.go b/pkg/registry/apis/folders/sub_count.go index ebaeb590a50..9b68f3f063f 100644 --- a/pkg/registry/apis/folders/sub_count.go +++ b/pkg/registry/apis/folders/sub_count.go @@ -4,6 +4,7 @@ import ( "context" "net/http" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" @@ -13,6 +14,7 @@ import ( ) type subCountREST struct { + getter rest.Getter searcher resourcepb.ResourceIndexClient } @@ -45,6 +47,9 @@ func (r *subCountREST) NewConnectOptions() (runtime.Object, bool, string) { } func (r *subCountREST) Connect(ctx context.Context, name string, _ runtime.Object, responder rest.Responder) (http.Handler, error) { + if _, err := r.getter.Get(ctx, name, &v1.GetOptions{}); err != nil { + return nil, err + } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ns, err := request.NamespaceInfoFrom(ctx, true) if err != nil { diff --git a/pkg/registry/apis/folders/sub_parents.go b/pkg/registry/apis/folders/sub_parents.go index 220921570d7..b0ab29592a9 100644 --- a/pkg/registry/apis/folders/sub_parents.go +++ b/pkg/registry/apis/folders/sub_parents.go @@ -8,7 +8,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/apiserver/pkg/storage" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/pkg/services/folder" @@ -55,10 +54,6 @@ func (r *subParentsREST) Connect(ctx context.Context, name string, opts runtime. } obj, err := r.getter.Get(ctx, name, &metav1.GetOptions{}) - if storage.IsNotFound(err) { - responder.Object(http.StatusNotFound, nil) - return - } if err != nil { responder.Error(err) return diff --git a/pkg/tests/apis/folder/folder_tree_test.go b/pkg/tests/apis/folder/folder_tree_test.go index 8a22917327f..546a4d21aee 100644 --- a/pkg/tests/apis/folder/folder_tree_test.go +++ b/pkg/tests/apis/folder/folder_tree_test.go @@ -19,15 +19,13 @@ import ( "k8s.io/client-go/rest" dashboardV0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" - "github.com/grafana/grafana/pkg/api/dtos" - "github.com/grafana/grafana/pkg/apimachinery/identity" + folderV1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/pkg/apimachinery/utils" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/search/model" - "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tests/apis" "github.com/grafana/grafana/pkg/tests/testinfra" @@ -87,6 +85,10 @@ func TestIntegrationFolderTree(t *testing.T) { Children: []FolderDefinition{ {Name: "child", Creator: helper.Org1.Admin, + Permissions: []FolderPermission{{ + Permission: "View", + User: helper.Org1.None, + }}, }, }, }, @@ -95,18 +97,24 @@ func TestIntegrationFolderTree(t *testing.T) { }, }, Expected: []ExpectedTree{ - {Users: []apis.User{ - helper.Org1.Admin, - helper.Org1.Viewer, // By default, viewer can view all dashboards - }, Listing: ` - └── top - ....└── middle - ........└── child`}, - {Users: []apis.User{helper.Org1.None}, Listing: ``}, + {User: helper.Org1.Admin, Listing: ` + └── top (admin,edit,save,delete) + ....└── middle (admin,edit,save,delete) + ........└── child (admin,edit,save,delete)`}, + {User: helper.Org1.Viewer, Listing: ` + └── top (view) + ....└── middle (view) + ........└── child (view)`}, + {User: helper.Org1.None, Listing: ` + └── sharedwithme (???) + ....└── child (view)`, + E403: []string{"top", "middle"}, + }, }, }, } + var statusCode int for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { tt.Definition.RequireUniqueName(t, make(map[string]bool)) @@ -115,21 +123,56 @@ func TestIntegrationFolderTree(t *testing.T) { // CreateWithLegacyAPI for _, expect := range tt.Expected { - for _, user := range expect.Users { - t.Run(fmt.Sprintf("query as %s", user.Identity.GetLogin()), func(t *testing.T) { - legacy := getFoldersFromLegacyAPISearch(t, user) - legacy.requireEqual(t, expect.Listing, "legacy") + unstructured, client := getFolderClients(t, expect.User) + t.Run(fmt.Sprintf("query as %s", expect.User.Identity.GetLogin()), func(t *testing.T) { + legacy := getFoldersFromLegacyAPISearch(t, client) + legacy.requireEqual(t, expect.Listing, "legacy") - listed := getFoldersFromAPIServerList(t, user) - listed.requireEqual(t, expect.Listing, "listed") + listed := getFoldersFromAPIServerList(t, unstructured) + listed.requireEqual(t, expect.Listing, "listed") - search := getFoldersFromDashboardV0Search(t, user) - search.requireEqual(t, expect.Listing, "search") + search := getFoldersFromDashboardV0Search(t, client, expect.User.Identity.GetNamespace()) + search.requireEqual(t, expect.Listing, "search") - // ensure sure GET also works on each folder we can list - requireGettable(t, user, listed) + // ensure sure GET also works on each folder we can list + listed.forEach(func(fv *FolderView) { + if fv.Name == folder.SharedWithMeFolderUID { + return // skip it + } + found, err := unstructured.Get(context.Background(), fv.Name, v1.GetOptions{}) + require.NoErrorf(t, err, "getting folder: %s", fv.Name) + require.Equal(t, found.GetName(), fv.Name) }) - } + + // Forbidden things should really be hidden + for _, name := range expect.E403 { + _, err := unstructured.Get(context.Background(), name, v1.GetOptions{}) + require.Error(t, err) + require.Truef(t, apierrors.IsForbidden(err), "error: %w", err) // 404 vs 403 ???? + + result := client.Get().AbsPath("api", "folders", name). + Do(context.Background()). + StatusCode(&statusCode) + require.Equal(t, int(http.StatusForbidden), statusCode) + require.Error(t, result.Error()) + + // Verify sub-resources are hidden + for _, sub := range []string{"access", "parents", "children", "counts"} { + _, err := unstructured.Get(context.Background(), name, v1.GetOptions{}, sub) + require.Error(t, err, "expect error for subresource", sub) + require.Truef(t, apierrors.IsForbidden(err), "error: %w", err) // 404 vs 403 ???? + } + + // Verify legacy API access is also hidden + for _, sub := range []string{"permissions", "counts"} { + result := client.Get().AbsPath("api", "folders", name, sub). + Do(context.Background()). + StatusCode(&statusCode) + require.Equalf(t, int(http.StatusForbidden), statusCode, "legacy access to: %s", sub) + require.Error(t, result.Error()) + } + } + }) } }) } @@ -138,8 +181,9 @@ func TestIntegrationFolderTree(t *testing.T) { } type ExpectedTree struct { - Users []apis.User + User apis.User Listing string + E403 []string } type FolderDefinition struct { @@ -150,10 +194,10 @@ type FolderDefinition struct { } type FolderPermission struct { - User apis.User - Team team.Team - Role identity.RoleType - Access dashboardaccess.PermissionType + Permission string + User apis.User + // Team team.Team + // Role identity.RoleType } func (f *FolderDefinition) CreateWithLegacyAPI(t *testing.T, h *apis.K8sTestHelper, parent string) { @@ -183,39 +227,23 @@ func (f *FolderDefinition) CreateWithLegacyAPI(t *testing.T, h *apis.K8sTestHelp parent = f.Name if len(f.Permissions) > 0 { - cmd := dtos.UpdateDashboardACLCommand{} for _, def := range f.Permissions { - p := dtos.DashboardACLUpdateItem{ - TeamID: def.Team.ID, // likely zero - Role: &def.Role, - Permission: def.Access, - } + // http://localhost:3000/api/access-control/folders/feyx0ezuwqwowb/users/aeyx0jzgix9fkd + body = []byte(`{"permission": "` + def.Permission + `"}`) + require.NotEmpty(t, def.Permission, "invalid permission: %+v", def) if def.User.Identity != nil { - p.UserID, err = def.User.Identity.GetInternalID() - require.NoError(t, err) + result = client.Post().AbsPath( + "api", "access-control", "folders", f.Name, "users", def.User.Identity.GetIdentifier()). + Body(body). + SetHeader("Content-type", "application/json"). + Do(context.Background()). + StatusCode(&statusCode) + err = result.Error() + require.NoErrorf(t, err, "legacy access control: %s: %s", f.Name, body) + require.Equal(t, int(http.StatusOK), statusCode, f.Name) } - cmd.Items = append(cmd.Items, p) } - - body, err := json.Marshal(cmd) - require.NoError(t, err) - - var statusCode int // folders/{folder_uid}/permissions - result = client.Post().AbsPath("api", "folders", parent, "permissions"). - Body(body). - SetHeader("Content-type", "application/json"). - Do(context.Background()). - StatusCode(&statusCode) - require.NoError(t, result.Error(), f.Name) - require.Equal(t, int(http.StatusOK), statusCode, f.Name) } - - // Now check that we could get the folder - result = client.Get().AbsPath("api", "folders", f.Name). - Do(context.Background()). - StatusCode(&statusCode) - require.NoErrorf(t, result.Error(), "get folder after create: %s", f.Name) - require.Equal(t, int(http.StatusOK), statusCode, f.Name) } for _, child := range f.Children { @@ -272,6 +300,7 @@ type FolderView struct { Parent string Title string Children []*FolderView + Access *folderV1.FolderAccessInfo } func (n *FolderView) forEach(cb func(*FolderView)) { @@ -294,19 +323,37 @@ func (n *FolderView) requireEqual(t *testing.T, expect string, msg string) { require.Equal(t, expect, found, fmt.Sprintf("%s // EXPECT:\n%s\n\nFOUND:\n%s", msg, expect, found)) } +func accessDescription(access *folderV1.FolderAccessInfo) string { + if access == nil { + return "???" + } + perms := []string{} + if access.CanAdmin { + perms = append(perms, "admin") + } + if access.CanEdit { + perms = append(perms, "edit") + } + if access.CanSave { + perms = append(perms, "save") + } + if access.CanDelete { + perms = append(perms, "delete") + } + if len(perms) == 0 { + return "view" // because it was not nil! + } + return strings.Join(perms, ",") +} + func (n *FolderView) build(tree treeprint.Tree) treeprint.Tree { for _, child := range n.Children { - child.build(tree.AddBranch(child.Name)) + child.build(tree.AddBranch(fmt.Sprintf("%s (%s)", child.Name, accessDescription(child.Access)))) } return tree } -func getFoldersFromLegacyAPISearch(t *testing.T, who apis.User) *FolderView { - cfg := dynamic.ConfigFor(who.NewRestConfig()) - cfg.GroupVersion = &schema.GroupVersion{Group: "folder.grafana.app", Version: "v1beta1"} // group does not matter - client, err := rest.RESTClientFor(cfg) - require.NoError(t, err) - +func getFoldersFromLegacyAPISearch(t *testing.T, client *rest.RESTClient) *FolderView { var statusCode int result := client.Get().AbsPath("api", "search"). Param("type", "dash-folder"). @@ -324,41 +371,58 @@ func getFoldersFromLegacyAPISearch(t *testing.T, who apis.User) *FolderView { lookup := make(map[string]*FolderView, len(hits)) for _, hit := range hits { - lookup[hit.UID] = &FolderView{ + fv := &FolderView{ Name: hit.UID, Title: hit.Title, Parent: hit.FolderUID, } + + // Read the access info (note not the same model but the fields we care about do overlap) + result = client.Get().AbsPath("api", "folders", hit.UID). + Do(context.Background()). + StatusCode(&statusCode) + require.NoError(t, result.Error(), "getting folder access info (/api)") + require.Equal(t, int(http.StatusOK), statusCode) + + body, err := result.Raw() + require.NoError(t, err) + err = json.Unmarshal(body, &fv.Access) + require.NoError(t, err) + + lookup[hit.UID] = fv } - return makeRoot(t, lookup, "/api/search") + return makeRoot(lookup, "/api/search") } -func makeRoot(t *testing.T, lookup map[string]*FolderView, name string) *FolderView { +func makeRoot(lookup map[string]*FolderView, name string) *FolderView { + shared := &FolderView{} // when not found root := &FolderView{} for _, v := range lookup { if v.Parent == "" { root.Children = append(root.Children, v) } else { p, ok := lookup[v.Parent] - require.Truef(t, ok, "[%s] parent not found for: %s (parent:%s)", name, v.Name, v.Parent) - p.Children = append(p.Children, v) + if ok { + p.Children = append(p.Children, v) + } else { + shared.Children = append(shared.Children, v) + } } } + if len(shared.Children) > 0 { + shared.Name = folder.SharedWithMeFolderUID + root.Children = append([]*FolderView{shared}, root.Children...) + } return root } -func getFoldersFromDashboardV0Search(t *testing.T, who apis.User) *FolderView { - cfg := dynamic.ConfigFor(who.NewRestConfig()) - cfg.GroupVersion = &schema.GroupVersion{Group: "dashboard.grafana.app", Version: "v0alpha1"} // group does not matter - client, err := rest.RESTClientFor(cfg) - require.NoError(t, err) - +func getFoldersFromDashboardV0Search(t *testing.T, client *rest.RESTClient, ns string) *FolderView { var statusCode int - result := client.Get().AbsPath("apis", "dashboard.grafana.app", "v0alpha1", "namespaces", who.Identity.GetNamespace(), "search"). + result := client.Get().AbsPath("apis", "dashboard.grafana.app", "v0alpha1", "namespaces", ns, "search"). Param("limit", "1000"). Do(context.Background()). StatusCode(&statusCode) - err = result.Error() + err := result.Error() if err != nil { if apierrors.IsForbidden(err) { return &FolderView{} // empty list @@ -375,25 +439,45 @@ func getFoldersFromDashboardV0Search(t *testing.T, who apis.User) *FolderView { lookup := make(map[string]*FolderView, len(results.Hits)) for _, hit := range results.Hits { - lookup[hit.Name] = &FolderView{ + fv := &FolderView{ Name: hit.Name, Title: hit.Title, Parent: hit.Folder, } + + result = client.Get().AbsPath("apis", folderV1.APIGroup, + folderV1.APIVersion, "namespaces", ns, "folders", hit.Name, "access"). + Do(context.Background()). + StatusCode(&statusCode) + require.NoError(t, result.Error(), "getting folder access info (/access)") + require.Equal(t, int(http.StatusOK), statusCode) + + body, err := result.Raw() + require.NoError(t, err) + err = json.Unmarshal(body, &fv.Access) + require.NoError(t, err) + + lookup[hit.Name] = fv } - return makeRoot(t, lookup, "dashboards/search") + return makeRoot(lookup, "dashboards/search") } -func getFoldersFromAPIServerList(t *testing.T, who apis.User) *FolderView { - gvr := schema.GroupVersionResource{Group: "folder.grafana.app", Version: "v1beta1", Resource: "folders"} - +func getFolderClients(t *testing.T, who apis.User) (dynamic.ResourceInterface, *rest.RESTClient) { + gvr := schema.GroupVersionResource{Group: folderV1.APIGroup, Version: folderV1.APIVersion, Resource: "folders"} ns := who.Identity.GetNamespace() cfg := dynamic.ConfigFor(who.NewRestConfig()) dyn, err := dynamic.NewForConfig(cfg) require.NoError(t, err) - client := dyn.Resource(gvr).Namespace(ns) + dc := dyn.Resource(gvr).Namespace(ns) + cfg.GroupVersion = &schema.GroupVersion{Group: gvr.Group, Version: gvr.Version} + client, err := rest.RESTClientFor(cfg) + require.NoError(t, err) + return dc, client +} + +func getFoldersFromAPIServerList(t *testing.T, client dynamic.ResourceInterface) *FolderView { lookup := map[string]*FolderView{} continueToken := "" for { @@ -410,11 +494,20 @@ func getFoldersFromAPIServerList(t *testing.T, who apis.User) *FolderView { title, _, err := unstructured.NestedString(hit.Object, "spec", "title") require.NoError(t, err) - lookup[hit.GetName()] = &FolderView{ + fv := &FolderView{ Name: hit.GetName(), Title: title, Parent: obj.GetFolder(), } + + access, err := client.Get(context.Background(), fv.Name, v1.GetOptions{}, "access") + require.NoError(t, err) + jj, err := json.Marshal(access) + require.NoError(t, err) + err = json.Unmarshal(jj, &fv.Access) + require.NoError(t, err) + + lookup[fv.Name] = fv } continueToken = result.GetContinue() @@ -423,23 +516,7 @@ func getFoldersFromAPIServerList(t *testing.T, who apis.User) *FolderView { } } - return makeRoot(t, lookup, "folders/list") -} - -func requireGettable(t *testing.T, who apis.User, root *FolderView) { - gvr := schema.GroupVersionResource{Group: "folder.grafana.app", Version: "v1beta1", Resource: "folders"} - - ns := who.Identity.GetNamespace() - cfg := dynamic.ConfigFor(who.NewRestConfig()) - dyn, err := dynamic.NewForConfig(cfg) - require.NoError(t, err) - client := dyn.Resource(gvr).Namespace(ns) - - root.forEach(func(fv *FolderView) { - found, err := client.Get(context.Background(), fv.Name, v1.GetOptions{}) - require.NoErrorf(t, err, "getting folder: %s", fv.Name) - require.Equal(t, found.GetName(), fv.Name) - }) + return makeRoot(lookup, "folders/list") } func dotify(t treeprint.Tree) string {