From 3785894b40c10fa6bbabe573e0311ccb688aa11e Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Thu, 27 Apr 2017 22:36:27 +0200 Subject: [PATCH] WIP: guardian service for search Removes restricted dashboards from search result. --- pkg/models/dashboards.go | 8 +++ pkg/models/user_group_member.go | 2 +- pkg/services/guardian/guardian.go | 36 ++++++++++ pkg/services/guardian/guardian_test.go | 70 +++++++++++++++++++ pkg/services/search/handlers.go | 29 ++++++++ pkg/services/search/handlers_test.go | 5 ++ pkg/services/sqlstore/guardian.go | 38 ++++++++++ pkg/services/sqlstore/guardian_test.go | 32 +++++++++ .../sqlstore/migrations/dashboard_mig.go | 4 ++ 9 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 pkg/services/guardian/guardian.go create mode 100644 pkg/services/guardian/guardian_test.go create mode 100644 pkg/services/sqlstore/guardian.go create mode 100644 pkg/services/sqlstore/guardian_test.go diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 9a861483c0d..2b704a69584 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -198,3 +198,11 @@ type GetDashboardSlugByIdQuery struct { Id int64 Result string } + +type GetAllowedDashboardsQuery struct { + UserId int64 + OrgId int64 + DashList []int64 + + Result []int64 +} diff --git a/pkg/models/user_group_member.go b/pkg/models/user_group_member.go index 1c0324a1072..23ddd84601b 100644 --- a/pkg/models/user_group_member.go +++ b/pkg/models/user_group_member.go @@ -48,7 +48,7 @@ type GetUserGroupMembersQuery struct { type UserGroupMemberDTO struct { OrgId int64 `json:"orgId"` - UserGroupId int64 `json:"orgId"` + UserGroupId int64 `json:"userGroupId"` UserId int64 `json:"userId"` Email string `json:"email"` Login string `json:"login"` diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go new file mode 100644 index 00000000000..76a7fefa880 --- /dev/null +++ b/pkg/services/guardian/guardian.go @@ -0,0 +1,36 @@ +package guardian + +import ( + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" +) + +// RemoveRestrictedDashboards filters out dashboards from the list that the user does have access to +func RemoveRestrictedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) { + user, err := getUser(userId) + if err != nil { + return nil, err + } + + if user.IsGrafanaAdmin || user.OrgRole == m.ROLE_ADMIN { + return dashList, nil + } + + filteredList, err := getAllowedDashboards(dashList, orgId, userId) + + return filteredList, err +} + +func getUser(userId int64) (*m.SignedInUser, error) { + query := m.GetSignedInUserQuery{UserId: userId} + err := bus.Dispatch(&query) + + return query.Result, err +} + +func getAllowedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) { + query := m.GetAllowedDashboardsQuery{UserId: userId, OrgId: orgId, DashList: dashList} + err := bus.Dispatch(&query) + + return query.Result, err +} diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go new file mode 100644 index 00000000000..bee33a07924 --- /dev/null +++ b/pkg/services/guardian/guardian_test.go @@ -0,0 +1,70 @@ +package guardian + +import ( + "testing" + + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +func TestGuardian(t *testing.T) { + + Convey("Given a user with list of dashboards that they have access to", t, func() { + hitList := []int64{1, 2} + + var orgId int64 = 1 + var userId int64 = 1 + + Convey("And the user is a Grafana admin", func() { + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{IsGrafanaAdmin: true} + return nil + }) + + filteredHitlist, err := RemoveRestrictedDashboards(hitList, orgId, userId) + So(err, ShouldBeNil) + + Convey("should return all dashboards", func() { + So(len(filteredHitlist), ShouldEqual, 2) + So(filteredHitlist[0], ShouldEqual, 1) + So(filteredHitlist[1], ShouldEqual, 2) + }) + }) + + Convey("And the user is an org admin", func() { + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_ADMIN} + return nil + }) + + filteredHitlist, err := RemoveRestrictedDashboards(hitList, orgId, userId) + So(err, ShouldBeNil) + + Convey("should return all dashboards", func() { + So(len(filteredHitlist), ShouldEqual, 2) + So(filteredHitlist[0], ShouldEqual, 1) + So(filteredHitlist[1], ShouldEqual, 2) + }) + }) + + Convey("And the user is an editor", func() { + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_EDITOR} + return nil + }) + bus.AddHandler("test2", func(query *m.GetAllowedDashboardsQuery) error { + query.Result = []int64{1} + return nil + }) + + filteredHitlist, err := RemoveRestrictedDashboards(hitList, orgId, userId) + So(err, ShouldBeNil) + + Convey("should return dashboard that editor has access to", func() { + So(len(filteredHitlist), ShouldEqual, 1) + So(filteredHitlist[0], ShouldEqual, 1) + }) + }) + }) +} diff --git a/pkg/services/search/handlers.go b/pkg/services/search/handlers.go index 2e9fd2ff689..87b34cabd90 100644 --- a/pkg/services/search/handlers.go +++ b/pkg/services/search/handlers.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" ) @@ -73,6 +74,11 @@ func searchHandler(query *Query) error { hits = filtered } + hits, err := removeRestrictedDashboardsFromList(hits, query) + if err != nil { + return err + } + // sort main result array sort.Sort(hits) @@ -94,6 +100,29 @@ func searchHandler(query *Query) error { return nil } +func removeRestrictedDashboardsFromList(hits HitList, query *Query) (HitList, error) { + var dashboardIds = []int64{} + for _, hit := range hits { + dashboardIds = append(dashboardIds, hit.Id) + } + + filteredHits, err := guardian.RemoveRestrictedDashboards(dashboardIds, query.OrgId, query.UserId) + if err != nil { + return nil, err + } + + filtered := HitList{} + for _, hit := range hits { + for _, dashId := range filteredHits { + if hit.Id == dashId { + filtered = append(filtered, hit) + } + } + } + + return filtered, nil +} + func stringInSlice(a string, list []string) bool { for _, b := range list { if b == a { diff --git a/pkg/services/search/handlers_test.go b/pkg/services/search/handlers_test.go index 8535cc48e82..fc49f9a5972 100644 --- a/pkg/services/search/handlers_test.go +++ b/pkg/services/search/handlers_test.go @@ -32,6 +32,11 @@ func TestSearch(t *testing.T) { return nil }) + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{IsGrafanaAdmin: true} + return nil + }) + Convey("That is empty", func() { err := searchHandler(&query) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/guardian.go b/pkg/services/sqlstore/guardian.go new file mode 100644 index 00000000000..70a4462d0a0 --- /dev/null +++ b/pkg/services/sqlstore/guardian.go @@ -0,0 +1,38 @@ +package sqlstore + +import ( + "strconv" + + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" +) + +func init() { + bus.AddHandler("sql", GetAllowedDashboards) +} + +func GetAllowedDashboards(query *m.GetAllowedDashboardsQuery) error { + + rawSQL := `select distinct d.id as DashboardId +from dashboard as d + left join dashboard as df on d.parent_id = df.id + left join dashboard_acl as dfa on d.parent_id = dfa.dashboard_id or d.id = dfa.dashboard_id + left join user_group_member as ugm on ugm.user_group_id = dfa.user_group_id +where ( + (d.has_acl = 1 and (dfa.user_id = ? or ugm.user_id = ? or df.created_by = ? or (d.is_folder = 1 and d.created_by = ?))) + or d.has_acl = 0) + and d.org_id = ?` + + res, err := x.Query(rawSQL, query.UserId, query.UserId, query.UserId, query.UserId, query.OrgId) + if err != nil { + return err + } + + query.Result = make([]int64, 0) + for _, dash := range res { + id, _ := strconv.ParseInt(string(dash["DashboardId"]), 10, 64) + query.Result = append(query.Result, id) + } + + return nil +} diff --git a/pkg/services/sqlstore/guardian_test.go b/pkg/services/sqlstore/guardian_test.go new file mode 100644 index 00000000000..72d34eb759c --- /dev/null +++ b/pkg/services/sqlstore/guardian_test.go @@ -0,0 +1,32 @@ +package sqlstore + +import ( + "testing" + + m "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +func TestGuardianAccess(t *testing.T) { + + Convey("Testing DB", t, func() { + InitTestDB(t) + + Convey("Given one dashboard folder with two dashboard and one dashboard in the root folder", func() { + folder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") + // dashInFolder1 := insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") + // dashInFolder2 := insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") + dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod", "webapp") + + Convey("and no acls are set", func() { + Convey("should return all dashboards", func() { + query := &m.GetAllowedDashboardsQuery{UserId: 1, OrgId: 1, DashList: []int64{folder.Id, dashInRoot.Id}} + err := GetAllowedDashboards(query) + So(err, ShouldBeNil) + So(query.Result[0], ShouldEqual, folder.Id) + So(query.Result[1], ShouldEqual, dashInRoot.Id) + }) + }) + }) + }) +} diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index 5ab17a0b6cb..dfc24f137bd 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -172,4 +172,8 @@ func addDashboardMigration(mg *Migrator) { mg.AddMigration("add unique index dashboard_acl_dashboard_id_user_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[1])) mg.AddMigration("add unique index dashboard_acl_dashboard_id_group_id", NewAddIndexMigration(dashboardAclV1, dashboardAclV1.Indices[2])) + // add column to flag if dashboard has an ACL + mg.AddMigration("Add column has_acl in dashboard", NewAddColumnMigration(dashboardV2, &Column{ + Name: "has_acl", Type: DB_Bool, Nullable: false, Default: "0", + })) }