diff --git a/pkg/services/sqlstore/dashboard_folder_test.go b/pkg/services/sqlstore/dashboard_folder_test.go index bd09e7490cb..b32a4dfed1d 100644 --- a/pkg/services/sqlstore/dashboard_folder_test.go +++ b/pkg/services/sqlstore/dashboard_folder_test.go @@ -26,7 +26,11 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("and no acls are set", func() { Convey("should return all dashboards", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, + OrgId: 1, + DashboardIds: []int64{folder.Id, dashInRoot.Id}, + } err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -40,7 +44,10 @@ func TestDashboardFolderDataAccess(t *testing.T) { updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) Convey("should not return folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, + OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}, + } err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) @@ -51,7 +58,11 @@ func TestDashboardFolderDataAccess(t *testing.T) { updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) Convey("should be able to access folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, + OrgId: 1, + DashboardIds: []int64{folder.Id, dashInRoot.Id}, + } err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -87,7 +98,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT) Convey("should not return folder or child", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) @@ -98,7 +109,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { updateTestDashboardWithAcl(childDash.Id, currentUser.Id, m.PERMISSION_EDIT) Convey("should be able to search for child dashboard but not folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -141,7 +152,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("and one folder is expanded, the other collapsed", func() { Convey("should return dashboards in root and expanded folder", func() { - query := &search.FindPersistedDashboardsQuery{FolderIds: []int64{rootFolderId, folder1.Id}, SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1} + query := &search.FindPersistedDashboardsQuery{FolderIds: []int64{rootFolderId, folder1.Id}, SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1} err := SearchDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 4) @@ -162,7 +173,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("should not return folder with acl or its children", func() { query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder1.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, } @@ -172,14 +183,14 @@ func TestDashboardFolderDataAccess(t *testing.T) { So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) }) }) - Convey("and a dashboard is moved from folder with acl to the folder without an acl", func() { + movedDash := moveDashboard(1, childDash1.Data, folder2.Id) So(movedDash.HasAcl, ShouldBeFalse) Convey("should return folder without acl and its children", func() { query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, } @@ -200,16 +211,17 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("should return folder without acl but not the dashboard with acl", func() { query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, } err := SearchDashboards(query) So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 3) + So(len(query.Result), ShouldEqual, 4) So(query.Result[0].Id, ShouldEqual, folder2.Id) - So(query.Result[1].Id, ShouldEqual, childDash2.Id) - So(query.Result[2].Id, ShouldEqual, dashInRoot.Id) + So(query.Result[1].Id, ShouldEqual, childDash1.Id) + So(query.Result[2].Id, ShouldEqual, childDash2.Id) + So(query.Result[3].Id, ShouldEqual, dashInRoot.Id) }) }) }) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index bd769d307eb..de7cdf19927 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -512,7 +512,7 @@ func TestDashboardDataAccess(t *testing.T) { query := search.FindPersistedDashboardsQuery{ Title: "1 test dash folder", OrgId: 1, - SignedInUser: &m.SignedInUser{OrgId: 1}, + SignedInUser: &m.SignedInUser{OrgId: 1, OrgRole: m.ROLE_EDITOR}, } err := SearchDashboards(&query) @@ -529,7 +529,7 @@ func TestDashboardDataAccess(t *testing.T) { query := search.FindPersistedDashboardsQuery{ OrgId: 1, FolderIds: []int64{savedFolder.Id}, - SignedInUser: &m.SignedInUser{OrgId: 1}, + SignedInUser: &m.SignedInUser{OrgId: 1, OrgRole: m.ROLE_EDITOR}, } err := SearchDashboards(&query) @@ -549,7 +549,7 @@ func TestDashboardDataAccess(t *testing.T) { Convey("should be able to find two dashboards by id", func() { query := search.FindPersistedDashboardsQuery{ DashboardIds: []int64{2, 3}, - SignedInUser: &m.SignedInUser{OrgId: 1}, + SignedInUser: &m.SignedInUser{OrgId: 1, OrgRole: m.ROLE_EDITOR}, } err := SearchDashboards(&query) @@ -578,7 +578,10 @@ func TestDashboardDataAccess(t *testing.T) { }) Convey("Should be able to search for starred dashboards", func() { - query := search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: 10, OrgId: 1}, IsStarred: true} + query := search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: 10, OrgId: 1, OrgRole: m.ROLE_EDITOR}, + IsStarred: true, + } err := SearchDashboards(&query) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/search_builder.go b/pkg/services/sqlstore/search_builder.go index 0db0fca53b0..ddfbfbfc551 100644 --- a/pkg/services/sqlstore/search_builder.go +++ b/pkg/services/sqlstore/search_builder.go @@ -153,10 +153,7 @@ func (sb *SearchBuilder) buildMainQuery() { sb.sql.WriteString(` WHERE `) sb.buildSearchWhereClause() - sb.sql.WriteString(` - LIMIT ?) as ids - INNER JOIN dashboard on ids.id = dashboard.id - `) + sb.sql.WriteString(` LIMIT ?) as ids INNER JOIN dashboard on ids.id = dashboard.id `) sb.params = append(sb.params, sb.limit) } diff --git a/pkg/services/sqlstore/sqlbuilder.go b/pkg/services/sqlstore/sqlbuilder.go index 6f3b32be5e7..b38bd693e66 100644 --- a/pkg/services/sqlstore/sqlbuilder.go +++ b/pkg/services/sqlstore/sqlbuilder.go @@ -24,39 +24,33 @@ func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, permi okRoles = append(okRoles, m.ROLE_VIEWER) } - // SELECT dash.id, dash.title, dash.folder_id - // FROM dashboard AS dash - // LEFT JOIN dashboard folder on folder.id = dash.folder_id - // LEFT JOIN dashboard_acl AS da ON - // da.dashboard_id = dash.id OR - // da.dashboard_id = dash.folder_id OR - // ( - // -- include default permissions --> - // da.org_id = -1 AND (folder.has_acl = 0 OR (dash.has_acl = 0 AND dash.folder_id = 0)) - // ) - // LEFT JOIN team_member as ugm on ugm.team_id = da.team_id - // WHERE - // dash.org_id = 5 AND - // ( - // da.user_id = 8 or - // ugm.user_id = 8 or - // da.role in ('Viewer', 'Editor') - // ) AND - // da.permission > 1 - // + falseStr := dialect.BooleanStr(false) + sb.sql.WriteString(` AND ( - dashboard.has_acl = ` + dialect.BooleanStr(false) + ` OR - dashboard.id in ( + dashboard.id IN ( SELECT distinct d.id AS DashboardId FROM dashboard AS d - LEFT JOIN dashboard_acl as da on d.folder_id = da.dashboard_id or d.id = da.dashboard_id - LEFT JOIN team_member as ugm on ugm.team_id = da.team_id + LEFT JOIN dashboard folder on folder.id = d.folder_id + LEFT JOIN dashboard_acl AS da ON + da.dashboard_id = d.id OR + da.dashboard_id = d.folder_id OR + ( + -- include default permissions --> + da.org_id = -1 AND ( + (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR + (folder.id IS NULL AND d.has_acl = ` + falseStr + `) + ) + ) + LEFT JOIN team_member as ugm on ugm.team_id = da.team_id WHERE - d.has_acl = ` + dialect.BooleanStr(true) + ` AND d.org_id = ? AND da.permission >= ? AND - (da.user_id = ? or ugm.user_id = ? or da.role IN (?` + strings.Repeat(",?", len(okRoles)-1) + `)) + ( + da.user_id = ? OR + ugm.user_id = ? OR + da.role IN (?` + strings.Repeat(",?", len(okRoles)-1) + `) + ) ) )`)