Selaa lähdekoodia

fix: sql search permissions filter fix

Torkel Ödegaard 7 vuotta sitten
vanhempi
commit
e3c3f3ce4c

+ 25 - 13
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)
 					})
 				})
 			})

+ 7 - 4
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)

+ 1 - 4
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)
 }
 

+ 20 - 26
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) + `)
+				)
 		)
 	)`)