Browse Source

refactoring dashboard folder security checks

Torkel Ödegaard 8 years ago
parent
commit
74840178cf
4 changed files with 43 additions and 25 deletions
  1. 0 1
      pkg/api/dashboard.go
  2. 19 10
      pkg/api/dashboard_acl_test.go
  3. 23 13
      pkg/api/dashboard_test.go
  4. 1 1
      pkg/services/guardian/guardian.go

+ 0 - 1
pkg/api/dashboard.go

@@ -50,7 +50,6 @@ func GetDashboard(c *middleware.Context) Response {
 	}
 
 	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
-
 	if canView, err := guardian.CanView(); err != nil || !canView {
 		return dashboardGuardianResponse(err)
 	}

+ 19 - 10
pkg/api/dashboard_acl_test.go

@@ -12,19 +12,32 @@ import (
 
 func TestDashboardAclApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard acl", t, func() {
-		mockResult := []*models.DashboardAclInfoDTO{
+		mockResult := []*models.DashboardAcl{
 			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permissions: models.PERMISSION_EDIT},
 			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permissions: models.PERMISSION_VIEW},
 			{Id: 3, OrgId: 1, DashboardId: 1, UserGroupId: 1, Permissions: models.PERMISSION_EDIT},
 			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 2, Permissions: models.PERMISSION_READ_ONLY_EDIT},
 		}
+		dtoRes := []*models.DashboardAclInfoDTO{
+			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permissions: models.PERMISSION_EDIT},
+			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permissions: models.PERMISSION_VIEW},
+			{Id: 3, OrgId: 1, DashboardId: 1, UserGroupId: 1, Permissions: models.PERMISSION_EDIT},
+			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 2, Permissions: models.PERMISSION_READ_ONLY_EDIT},
+		}
+
 		bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
+			query.Result = dtoRes
+			return nil
+		})
+
+		bus.AddHandler("test", func(query *models.GetInheritedDashboardAclQuery) error {
 			query.Result = mockResult
 			return nil
 		})
 
+		userGroupResp := []*models.UserGroup{}
 		bus.AddHandler("test", func(query *models.GetUserGroupsByUserQuery) error {
-			query.Result = []*models.UserGroup{}
+			query.Result = userGroupResp
 			return nil
 		})
 
@@ -46,7 +59,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 
 		Convey("When user is editor and in the ACL", func() {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/1/acl", "/api/dashboards/:id/acl", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
+				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
 
 				Convey("Should be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAcl
@@ -57,7 +70,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 			})
 
 			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/1/acl/user/1", "/api/dashboards/:id/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
+				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
 
 				bus.AddHandler("test3", func(cmd *models.RemoveDashboardPermissionCommand) error {
 					return nil
@@ -73,11 +86,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 
 			Convey("When user is a member of a user group in the ACL with edit permission", func() {
 				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/1/acl/user/1", "/api/dashboards/:id/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-
-					bus.AddHandler("test3", func(query *models.GetUserGroupsByUserQuery) error {
-						query.Result = []*models.UserGroup{{Id: 1, OrgId: 1, Name: "UG1"}}
-						return nil
-					})
+					userGroupResp = append(userGroupResp, &models.UserGroup{Id: 1, OrgId: 1, Name: "UG1"})
 
 					bus.AddHandler("test3", func(cmd *models.RemoveDashboardPermissionCommand) error {
 						return nil
@@ -105,7 +114,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 			})
 
 			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/1/acl/user/1", "/api/dashboards/:id/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_VIEW})
+				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_VIEW})
 				bus.AddHandler("test3", func(cmd *models.RemoveDashboardPermissionCommand) error {
 					return nil
 				})

+ 23 - 13
pkg/api/dashboard_test.go

@@ -30,6 +30,12 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			return nil
 		})
 
+		aclMockResp := []*models.DashboardAcl{}
+		bus.AddHandler("test", func(query *models.GetInheritedDashboardAclQuery) error {
+			query.Result = aclMockResp
+			return nil
+		})
+
 		cmd := models.SaveDashboardCommand{
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 				"parentId": fakeDash.ParentId,
@@ -165,6 +171,19 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		fakeDash.ParentId = 1
 		fakeDash.HasAcl = true
 
+		aclMockResp := []*models.DashboardAcl{
+			{
+				DashboardId: 1,
+				Permissions: models.PERMISSION_EDIT,
+				UserId:      200,
+			},
+		}
+
+		bus.AddHandler("test", func(query *models.GetInheritedDashboardAclQuery) error {
+			query.Result = aclMockResp
+			return nil
+		})
+
 		bus.AddHandler("test", func(query *models.GetDashboardQuery) error {
 			query.Result = fakeDash
 			return nil
@@ -186,10 +205,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 
 		Convey("When user is an Org Viewer and has no permissions for this dashboard", func() {
 			role := models.ROLE_VIEWER
-			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
-				query.Result = []*models.DashboardAclInfoDTO{}
-				return nil
-			})
 
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
@@ -224,11 +239,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		Convey("When user is an Org Editor and has no permissions for this dashboard", func() {
 			role := models.ROLE_EDITOR
 
-			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
-				query.Result = []*models.DashboardAclInfoDTO{}
-				return nil
-			})
-
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
 				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -262,11 +272,11 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		Convey("When user is an Org Viewer but has an edit permission", func() {
 			role := models.ROLE_VIEWER
 
-			mockResult := []*models.DashboardAclInfoDTO{
+			mockResult := []*models.DashboardAcl{
 				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permissions: models.PERMISSION_EDIT},
 			}
 
-			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
+			bus.AddHandler("test", func(query *models.GetInheritedDashboardAclQuery) error {
 				query.Result = mockResult
 				return nil
 			})
@@ -304,11 +314,11 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		Convey("When user is an Org Editor but has a view permission", func() {
 			role := models.ROLE_EDITOR
 
-			mockResult := []*models.DashboardAclInfoDTO{
+			mockResult := []*models.DashboardAcl{
 				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permissions: models.PERMISSION_VIEW},
 			}
 
-			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
+			bus.AddHandler("test", func(query *models.GetInheritedDashboardAclQuery) error {
 				query.Result = mockResult
 				return nil
 			})

+ 1 - 1
pkg/services/guardian/guardian.go

@@ -87,7 +87,7 @@ func (g *DashboardGuardian) getAcl() ([]*m.DashboardAcl, error) {
 }
 
 func (g *DashboardGuardian) getUserGroups() ([]*m.UserGroup, error) {
-	if g.groups == nil {
+	if g.groups != nil {
 		return g.groups, nil
 	}