From b84fd3a7aea83f894bcc2dc9246507fc84aa49fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 7 Feb 2018 17:54:21 +0100 Subject: [PATCH 1/5] fix: initial fix for #10822 --- pkg/services/sqlstore/dashboard.go | 1 + pkg/services/sqlstore/search_builder.go | 22 ++---------- pkg/services/sqlstore/sqlbuilder.go | 45 +++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 pkg/services/sqlstore/sqlbuilder.go diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index be8b11b1f5b..c187360dc33 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -279,6 +279,7 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear var res []DashboardSearchProjection sql, params := sb.ToSql() + sqlog.Info("sql", "sql", sql, "params", params) err := x.Sql(sql, params...).Find(&res) if err != nil { return nil, err diff --git a/pkg/services/sqlstore/search_builder.go b/pkg/services/sqlstore/search_builder.go index 627074d5453..91e2742e165 100644 --- a/pkg/services/sqlstore/search_builder.go +++ b/pkg/services/sqlstore/search_builder.go @@ -1,7 +1,6 @@ package sqlstore import ( - "bytes" "strings" m "github.com/grafana/grafana/pkg/models" @@ -9,6 +8,7 @@ import ( // SearchBuilder is a builder/object mother that builds a dashboard search query type SearchBuilder struct { + SqlBuilder tags []string isStarred bool limit int @@ -18,8 +18,6 @@ type SearchBuilder struct { whereTypeFolder bool whereTypeDash bool whereFolderIds []int64 - sql bytes.Buffer - params []interface{} } func NewSearchBuilder(signedInUser *m.SignedInUser, limit int) *SearchBuilder { @@ -176,23 +174,7 @@ func (sb *SearchBuilder) buildSearchWhereClause() { } } - if sb.signedInUser.OrgRole != m.ROLE_ADMIN { - allowedDashboardsSubQuery := ` AND (dashboard.has_acl = ` + dialect.BooleanStr(false) + ` OR 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 org_user ou on ou.role = da.role - WHERE - d.has_acl = ` + dialect.BooleanStr(true) + ` and - (da.user_id = ? or ugm.user_id = ? or ou.id is not null) - and d.org_id = ? - ) - )` - - sb.sql.WriteString(allowedDashboardsSubQuery) - sb.params = append(sb.params, sb.signedInUser.UserId, sb.signedInUser.UserId, sb.signedInUser.OrgId) - } + sb.writeDashboardPermissionFilter(sb.signedInUser, m.PERMISSION_VIEW) if len(sb.whereTitle) > 0 { sb.sql.WriteString(" AND dashboard.title " + dialect.LikeStr() + " ?") diff --git a/pkg/services/sqlstore/sqlbuilder.go b/pkg/services/sqlstore/sqlbuilder.go new file mode 100644 index 00000000000..9a0dd0d3989 --- /dev/null +++ b/pkg/services/sqlstore/sqlbuilder.go @@ -0,0 +1,45 @@ +package sqlstore + +import ( + "bytes" + "strings" + + m "github.com/grafana/grafana/pkg/models" +) + +type SqlBuilder struct { + sql bytes.Buffer + params []interface{} +} + +func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, minPermission m.PermissionType) { + + if user.OrgRole == m.ROLE_ADMIN { + return + } + + okRoles := []interface{}{user.OrgRole} + + if user.OrgRole == m.ROLE_EDITOR { + okRoles = append(okRoles, m.ROLE_VIEWER) + } + + sb.sql.WriteString(` AND + ( + dashboard.has_acl = ` + dialect.BooleanStr(false) + ` OR + 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 + 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) + `)) + ) + )`) + + sb.params = append(sb.params, user.OrgId, minPermission, user.UserId, user.UserId) + sb.params = append(sb.params, okRoles...) +} From 8e8f3c4332fa6effd3111be5bb83178e64e44def Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 8 Feb 2018 17:11:01 +0100 Subject: [PATCH 2/5] dashboard and folder search with permissions --- pkg/api/api.go | 2 - pkg/api/dashboard.go | 16 ------ pkg/api/search.go | 7 +++ pkg/models/dashboards.go | 12 ----- pkg/services/search/handlers.go | 1 + pkg/services/search/models.go | 3 +- pkg/services/sqlstore/dashboard.go | 53 ++----------------- .../sqlstore/dashboard_folder_test.go | 26 +++++---- pkg/services/sqlstore/search_builder.go | 6 ++- pkg/services/sqlstore/search_builder_test.go | 3 +- pkg/services/sqlstore/sqlbuilder.go | 4 +- .../dashboard/folder_picker/folder_picker.ts | 8 ++- 12 files changed, 43 insertions(+), 98 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 752af7602f5..793f5a2e830 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -261,8 +261,6 @@ func (hs *HttpServer) registerRoutes() { dashboardRoute.Get("/tags", GetDashboardTags) dashboardRoute.Post("/import", bind(dtos.ImportDashboardCommand{}), wrap(ImportDashboard)) - dashboardRoute.Get("/folders", wrap(GetFoldersForSignedInUser)) - dashboardRoute.Group("/id/:dashboardId", func(dashIdRoute RouteRegister) { dashIdRoute.Get("/versions", wrap(GetDashboardVersions)) dashIdRoute.Get("/versions/:id", wrap(GetDashboardVersion)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index d7676899eb2..a1f3560c0c6 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -490,19 +490,3 @@ func GetDashboardTags(c *middleware.Context) { c.JSON(200, query.Result) } - -func GetFoldersForSignedInUser(c *middleware.Context) Response { - title := c.Query("query") - query := m.GetFoldersForSignedInUserQuery{ - OrgId: c.OrgId, - SignedInUser: c.SignedInUser, - Title: title, - } - - err := bus.Dispatch(&query) - if err != nil { - return ApiError(500, "Failed to get folders from database", err) - } - - return Json(200, query.Result) -} diff --git a/pkg/api/search.go b/pkg/api/search.go index fee062a5599..f79385d83f8 100644 --- a/pkg/api/search.go +++ b/pkg/api/search.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/metrics" "github.com/grafana/grafana/pkg/middleware" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/search" ) @@ -15,11 +16,16 @@ func Search(c *middleware.Context) { starred := c.Query("starred") limit := c.QueryInt("limit") dashboardType := c.Query("type") + permission := models.PERMISSION_VIEW if limit == 0 { limit = 1000 } + if c.Query("permission") == "Edit" { + permission = models.PERMISSION_EDIT + } + dbids := make([]int64, 0) for _, id := range c.QueryStrings("dashboardIds") { dashboardId, err := strconv.ParseInt(id, 10, 64) @@ -46,6 +52,7 @@ func Search(c *middleware.Context) { DashboardIds: dbids, Type: dashboardType, FolderIds: folderIds, + Permission: permission, } err := bus.Dispatch(&searchQuery) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 12216718b44..a91d4c4ed62 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -270,18 +270,6 @@ type GetDashboardsBySlugQuery struct { Result []*Dashboard } -type GetFoldersForSignedInUserQuery struct { - OrgId int64 - SignedInUser *SignedInUser - Title string - Result []*DashboardFolder -} - -type DashboardFolder struct { - Id int64 `json:"id"` - Title string `json:"title"` -} - type DashboardPermissionForUser struct { DashboardId int64 `json:"dashboardId"` Permission PermissionType `json:"permission"` diff --git a/pkg/services/search/handlers.go b/pkg/services/search/handlers.go index 247585402ef..cf194c320bb 100644 --- a/pkg/services/search/handlers.go +++ b/pkg/services/search/handlers.go @@ -21,6 +21,7 @@ func searchHandler(query *Query) error { FolderIds: query.FolderIds, Tags: query.Tags, Limit: query.Limit, + Permission: query.Permission, } if err := bus.Dispatch(&dashQuery); err != nil { diff --git a/pkg/services/search/models.go b/pkg/services/search/models.go index 6dea975d9fe..2da09672f13 100644 --- a/pkg/services/search/models.go +++ b/pkg/services/search/models.go @@ -52,6 +52,7 @@ type Query struct { Type string DashboardIds []int64 FolderIds []int64 + Permission models.PermissionType Result HitList } @@ -66,7 +67,7 @@ type FindPersistedDashboardsQuery struct { FolderIds []int64 Tags []string Limit int - IsBrowse bool + Permission models.PermissionType Result HitList } diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index c187360dc33..dd739343d7b 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -1,6 +1,7 @@ package sqlstore import ( + "fmt" "strings" "time" @@ -21,7 +22,6 @@ func init() { bus.AddHandler("sql", GetDashboardSlugById) bus.AddHandler("sql", GetDashboardUIDById) bus.AddHandler("sql", GetDashboardsByPluginId) - bus.AddHandler("sql", GetFoldersForSignedInUser) bus.AddHandler("sql", GetDashboardPermissionsForUser) bus.AddHandler("sql", GetDashboardsBySlug) } @@ -256,7 +256,7 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear limit = 1000 } - sb := NewSearchBuilder(query.SignedInUser, limit). + sb := NewSearchBuilder(query.SignedInUser, limit, query.Permission). WithTags(query.Tags). WithDashboardIdsIn(query.DashboardIds) @@ -279,6 +279,7 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear var res []DashboardSearchProjection sql, params := sb.ToSql() + fmt.Printf("%s, %v", sql, params) sqlog.Info("sql", "sql", sql, "params", params) err := x.Sql(sql, params...).Find(&res) if err != nil { @@ -358,54 +359,6 @@ func GetDashboardTags(query *m.GetDashboardTagsQuery) error { return err } -func GetFoldersForSignedInUser(query *m.GetFoldersForSignedInUserQuery) error { - query.Result = make([]*m.DashboardFolder, 0) - var err error - - if query.SignedInUser.OrgRole == m.ROLE_ADMIN { - sql := `SELECT distinct d.id, d.title - FROM dashboard AS d WHERE d.is_folder = ? AND d.org_id = ? - ORDER BY d.title ASC` - - err = x.Sql(sql, dialect.BooleanStr(true), query.OrgId).Find(&query.Result) - } else { - params := make([]interface{}, 0) - sql := `SELECT distinct d.id, d.title - FROM dashboard AS d - LEFT JOIN dashboard_acl AS da ON d.id = da.dashboard_id - LEFT JOIN team_member AS ugm ON ugm.team_id = da.team_id - LEFT JOIN org_user ou ON ou.role = da.role AND ou.user_id = ? - LEFT JOIN org_user ouRole ON ouRole.role = 'Editor' AND ouRole.user_id = ? AND ouRole.org_id = ?` - params = append(params, query.SignedInUser.UserId) - params = append(params, query.SignedInUser.UserId) - params = append(params, query.OrgId) - - sql += ` WHERE - d.org_id = ? AND - d.is_folder = ? AND - ( - (d.has_acl = ? AND da.permission > 1 AND (da.user_id = ? OR ugm.user_id = ? OR ou.id IS NOT NULL)) - OR (d.has_acl = ? AND ouRole.id IS NOT NULL) - )` - params = append(params, query.OrgId) - params = append(params, dialect.BooleanStr(true)) - params = append(params, dialect.BooleanStr(true)) - params = append(params, query.SignedInUser.UserId) - params = append(params, query.SignedInUser.UserId) - params = append(params, dialect.BooleanStr(false)) - - if len(query.Title) > 0 { - sql += " AND d.title " + dialect.LikeStr() + " ?" - params = append(params, "%"+query.Title+"%") - } - - sql += ` ORDER BY d.title ASC` - err = x.Sql(sql, params...).Find(&query.Result) - } - - return err -} - func DeleteDashboard(cmd *m.DeleteDashboardCommand) error { return inTransaction(func(sess *DBSession) error { dashboard := m.Dashboard{Id: cmd.Id, OrgId: cmd.OrgId} diff --git a/pkg/services/sqlstore/dashboard_folder_test.go b/pkg/services/sqlstore/dashboard_folder_test.go index 4818deaae14..bd09e7490cb 100644 --- a/pkg/services/sqlstore/dashboard_folder_test.go +++ b/pkg/services/sqlstore/dashboard_folder_test.go @@ -227,12 +227,14 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("Admin users", func() { Convey("Should have write access to all dashboard folders in their org", func() { - query := m.GetFoldersForSignedInUserQuery{ + query := search.FindPersistedDashboardsQuery{ OrgId: 1, - SignedInUser: &m.SignedInUser{UserId: adminUser.Id, OrgRole: m.ROLE_ADMIN}, + SignedInUser: &m.SignedInUser{UserId: adminUser.Id, OrgRole: m.ROLE_ADMIN, OrgId: 1}, + Permission: m.PERMISSION_VIEW, + Type: "dash-folder", } - err := GetFoldersForSignedInUser(&query) + err := SearchDashboards(&query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -260,13 +262,14 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("Editor users", func() { - query := m.GetFoldersForSignedInUserQuery{ + query := search.FindPersistedDashboardsQuery{ OrgId: 1, - SignedInUser: &m.SignedInUser{UserId: editorUser.Id, OrgRole: m.ROLE_EDITOR}, + SignedInUser: &m.SignedInUser{UserId: editorUser.Id, OrgRole: m.ROLE_EDITOR, OrgId: 1}, + Permission: m.PERMISSION_EDIT, } Convey("Should have write access to all dashboard folders with default ACL", func() { - err := GetFoldersForSignedInUser(&query) + err := SearchDashboards(&query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -295,7 +298,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("Should have write access to one dashboard folder if default role changed to view for one folder", func() { updateTestDashboardWithAcl(folder1.Id, editorUser.Id, m.PERMISSION_VIEW) - err := GetFoldersForSignedInUser(&query) + err := SearchDashboards(&query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) @@ -305,13 +308,14 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("Viewer users", func() { - query := m.GetFoldersForSignedInUserQuery{ + query := search.FindPersistedDashboardsQuery{ OrgId: 1, - SignedInUser: &m.SignedInUser{UserId: viewerUser.Id, OrgRole: m.ROLE_VIEWER}, + SignedInUser: &m.SignedInUser{UserId: viewerUser.Id, OrgRole: m.ROLE_VIEWER, OrgId: 1}, + Permission: m.PERMISSION_EDIT, } Convey("Should have no write access to any dashboard folders with default ACL", func() { - err := GetFoldersForSignedInUser(&query) + err := SearchDashboards(&query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 0) @@ -338,7 +342,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("Should be able to get one dashboard folder if default role changed to edit for one folder", func() { updateTestDashboardWithAcl(folder1.Id, viewerUser.Id, m.PERMISSION_EDIT) - err := GetFoldersForSignedInUser(&query) + err := SearchDashboards(&query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) diff --git a/pkg/services/sqlstore/search_builder.go b/pkg/services/sqlstore/search_builder.go index 91e2742e165..0db0fca53b0 100644 --- a/pkg/services/sqlstore/search_builder.go +++ b/pkg/services/sqlstore/search_builder.go @@ -18,12 +18,14 @@ type SearchBuilder struct { whereTypeFolder bool whereTypeDash bool whereFolderIds []int64 + permission m.PermissionType } -func NewSearchBuilder(signedInUser *m.SignedInUser, limit int) *SearchBuilder { +func NewSearchBuilder(signedInUser *m.SignedInUser, limit int, permission m.PermissionType) *SearchBuilder { searchBuilder := &SearchBuilder{ signedInUser: signedInUser, limit: limit, + permission: permission, } return searchBuilder @@ -174,7 +176,7 @@ func (sb *SearchBuilder) buildSearchWhereClause() { } } - sb.writeDashboardPermissionFilter(sb.signedInUser, m.PERMISSION_VIEW) + sb.writeDashboardPermissionFilter(sb.signedInUser, sb.permission) if len(sb.whereTitle) > 0 { sb.sql.WriteString(" AND dashboard.title " + dialect.LikeStr() + " ?") diff --git a/pkg/services/sqlstore/search_builder_test.go b/pkg/services/sqlstore/search_builder_test.go index 32ccbc583f5..e8b02c445ec 100644 --- a/pkg/services/sqlstore/search_builder_test.go +++ b/pkg/services/sqlstore/search_builder_test.go @@ -16,7 +16,8 @@ func TestSearchBuilder(t *testing.T) { OrgId: 1, UserId: 1, } - sb := NewSearchBuilder(signedInUser, 1000) + + sb := NewSearchBuilder(signedInUser, 1000, m.PERMISSION_VIEW) Convey("When building a normal search", func() { sql, params := sb.IsStarred().WithTitle("test").ToSql() diff --git a/pkg/services/sqlstore/sqlbuilder.go b/pkg/services/sqlstore/sqlbuilder.go index 9a0dd0d3989..6274458818e 100644 --- a/pkg/services/sqlstore/sqlbuilder.go +++ b/pkg/services/sqlstore/sqlbuilder.go @@ -12,7 +12,7 @@ type SqlBuilder struct { params []interface{} } -func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, minPermission m.PermissionType) { +func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, permission m.PermissionType) { if user.OrgRole == m.ROLE_ADMIN { return @@ -40,6 +40,6 @@ func (sb *SqlBuilder) writeDashboardPermissionFilter(user *m.SignedInUser, minPe ) )`) - sb.params = append(sb.params, user.OrgId, minPermission, user.UserId, user.UserId) + sb.params = append(sb.params, user.OrgId, permission, user.UserId, user.UserId) sb.params = append(sb.params, okRoles...) } diff --git a/public/app/features/dashboard/folder_picker/folder_picker.ts b/public/app/features/dashboard/folder_picker/folder_picker.ts index 56284a877c5..0e5c22c4db2 100644 --- a/public/app/features/dashboard/folder_picker/folder_picker.ts +++ b/public/app/features/dashboard/folder_picker/folder_picker.ts @@ -30,7 +30,13 @@ export class FolderPickerCtrl { } getOptions(query) { - return this.backendSrv.get('api/dashboards/folders', { query: query }).then(result => { + const params = { + query: query, + type: 'dash-folder', + permission: 'Edit', + }; + + return this.backendSrv.get('api/search', params).then(result => { if ( query === '' || query.toLowerCase() === 'g' || From 4d5a24a6c4a9604174f4b0f2a2c948ae581b88c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 9 Feb 2018 17:24:34 +0100 Subject: [PATCH 3/5] permissions: might have a solution for search --- pkg/services/sqlstore/sqlbuilder.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/services/sqlstore/sqlbuilder.go b/pkg/services/sqlstore/sqlbuilder.go index 6274458818e..6f3b32be5e7 100644 --- a/pkg/services/sqlstore/sqlbuilder.go +++ b/pkg/services/sqlstore/sqlbuilder.go @@ -24,6 +24,26 @@ 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 + // sb.sql.WriteString(` AND ( dashboard.has_acl = ` + dialect.BooleanStr(false) + ` OR From e3c3f3ce4ca2f02424c393c632686b6c49188032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 13 Feb 2018 16:49:00 +0100 Subject: [PATCH 4/5] fix: sql search permissions filter fix --- .../sqlstore/dashboard_folder_test.go | 38 +++++++++------ pkg/services/sqlstore/dashboard_test.go | 11 +++-- pkg/services/sqlstore/search_builder.go | 5 +- pkg/services/sqlstore/sqlbuilder.go | 46 ++++++++----------- 4 files changed, 53 insertions(+), 47 deletions(-) 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) + `) + ) ) )`) From 162439a8d63756696071349a838feca048731bef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 13 Feb 2018 17:03:20 +0100 Subject: [PATCH 5/5] fix: removed logging --- pkg/services/sqlstore/dashboard.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 05cce3f001f..a681b7273d3 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -1,7 +1,6 @@ package sqlstore import ( - "fmt" "strings" "time" @@ -312,8 +311,6 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear var res []DashboardSearchProjection sql, params := sb.ToSql() - fmt.Printf("%s, %v", sql, params) - sqlog.Info("sql", "sql", sql, "params", params) err := x.Sql(sql, params...).Find(&res) if err != nil { return nil, err