dashboard guardian refactoring starting to work

pkg/api/dashboard.go

@@ -35,6 +35,14 @@ func isDashboardStarredByUser(c *middleware.Context, dashId int64) (bool, error)
 	return query.Result, nil
 }
 
+func dashboardGuardianResponse(err error) Response {
+	if err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	} else {
+		return ApiError(403, "Access denied to this dashboard", nil)
+	}
+}
+
 func GetDashboard(c *middleware.Context) Response {
 	dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0)
 	if rsp != nil {
@@ -43,10 +51,8 @@ func GetDashboard(c *middleware.Context) Response {
 
 	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
 
-	if canView, err := guardian.CanView(); err != nil {
+	if canView, err := guardian.CanView(); err != nil || !canView {
-		return ApiError(500, "Error while checking dashboard permissions", err)
+		return dashboardGuardianResponse(err)
-	} else if !canView {
-		return ApiError(403, "Access denied to this dashboard", nil)
 	}
 
 	canEdit, _ := guardian.CanEdit()
@@ -130,12 +136,9 @@ func DeleteDashboard(c *middleware.Context) Response {
 		return rsp
 	}
 
-	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
-
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-	if canSave, err := guardian.CanSave(); err != nil {
+		return dashboardGuardianResponse(err)
-		return ApiError(500, "Error while checking dashboard permissions", err)
-	} else if !canSave {
-		return ApiError(403, "Does not have permission to delete this dashboard", nil)
 	}
 
 	cmd := m.DeleteDashboardCommand{OrgId: c.OrgId, Id: dash.Id}
@@ -160,12 +163,9 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 		}
 	}
 
-	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
-
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-	if canSave, err := guardian.CanSave(); err != nil {
+		return dashboardGuardianResponse(err)
-		return ApiError(500, "Error while checking dashboard permissions", err)
-	} else if !canSave {
-		return ApiError(403, "Does not have permission to save this dashboard", nil)
 	}
 
 	if dash.IsFolder && dash.ParentId > 0 {
@@ -306,27 +306,22 @@ func GetDashboardFromJsonFile(c *middleware.Context) {
 
 // GetDashboardVersions returns all dashboard versions as JSON
 func GetDashboardVersions(c *middleware.Context) Response {
-	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"))
+	dashId := c.ParamsInt64(":dashboardId")
-	if rsp != nil {
-		return rsp
-	}
 
-	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
-	if canSave, err := guardian.CanSave(); err != nil {
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-		return ApiError(500, "Error while checking dashboard permissions", err)
+		return dashboardGuardianResponse(err)
-	} else if !canSave {
-		return ApiError(403, "Dashboard access denied", nil)
 	}
 
 	query := m.GetDashboardVersionsQuery{
 		OrgId:       c.OrgId,
-		DashboardId: dash.Id,
+		DashboardId: dashId,
 		Limit:       c.QueryInt("limit"),
 		Start:       c.QueryInt("start"),
 	}
 
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(404, fmt.Sprintf("No versions found for dashboardId %d", dash.Id), err)
+		return ApiError(404, fmt.Sprintf("No versions found for dashboardId %d", dashId), err)
 	}
 
 	for _, version := range query.Result {
@@ -350,26 +345,21 @@ func GetDashboardVersions(c *middleware.Context) Response {
 
 // GetDashboardVersion returns the dashboard version with the given ID.
 func GetDashboardVersion(c *middleware.Context) Response {
-	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"))
+	dashId := c.ParamsInt64(":dashboardId")
-	if rsp != nil {
-		return rsp
-	}
 
-	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
-	if canSave, err := guardian.CanSave(); err != nil {
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-		return ApiError(500, "Error while checking dashboard permissions", err)
+		return dashboardGuardianResponse(err)
-	} else if !canSave {
-		return ApiError(403, "Dashboard access denied", nil)
 	}
 
 	query := m.GetDashboardVersionQuery{
 		OrgId:       c.OrgId,
-		DashboardId: dash.Id,
+		DashboardId: dashId,
 		Version:     c.ParamsInt(":id"),
 	}
 
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(500, fmt.Sprintf("Dashboard version %d not found for dashboardId %d", query.Version, dash.Id), err)
+		return ApiError(500, fmt.Sprintf("Dashboard version %d not found for dashboardId %d", query.Version, dashId), err)
 	}
 
 	creator := "Anonymous"
@@ -425,6 +415,11 @@ func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboard
 		return rsp
 	}
 
+	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
+		return dashboardGuardianResponse(err)
+	}
+
 	versionQuery := m.GetDashboardVersionQuery{DashboardId: dash.Id, Version: apiCmd.Version, OrgId: c.OrgId}
 	if err := bus.Dispatch(&versionQuery); err != nil {
 		return ApiError(404, "Dashboard version not found", nil)

pkg/api/dashboard_acl.go

@@ -10,20 +10,15 @@ import (
 )
 
 func GetDashboardAcl(c *middleware.Context) Response {
-	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":id"))
+	dashId := c.ParamsInt64(":id")
-	if rsp != nil {
+
-		return rsp
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
+
+	if canView, err := guardian.CanView(); err != nil || !canView {
+		return dashboardGuardianResponse(err)
 	}
 
-	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	query := m.GetDashboardPermissionsQuery{DashboardId: dashId}
-	canView, err := guardian.CanView()
-	if err != nil {
-		return ApiError(500, "Failed to get Dashboard ACL", err)
-	} else if !canView {
-		return ApiError(403, "Dashboard access denied", nil)
-	}
-
-	query := m.GetDashboardPermissionsQuery{DashboardId: dash.Id}
 	if err := bus.Dispatch(&query); err != nil {
 		return ApiError(500, "Failed to get Dashboard ACL", err)
 	}
@@ -32,8 +27,15 @@ func GetDashboardAcl(c *middleware.Context) Response {
 }
 
 func PostDashboardAcl(c *middleware.Context, cmd m.AddOrUpdateDashboardPermissionCommand) Response {
+	dashId := c.ParamsInt64(":id")
+
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
+		return dashboardGuardianResponse(err)
+	}
+
 	cmd.OrgId = c.OrgId
-	cmd.DashboardId = c.ParamsInt64(":id")
+	cmd.DashboardId = dashId
 
 	if err := bus.Dispatch(&cmd); err != nil {
 		if err == m.ErrDashboardPermissionUserOrUserGroupEmpty {
@@ -51,43 +53,37 @@ func PostDashboardAcl(c *middleware.Context, cmd m.AddOrUpdateDashboardPermissio
 }
 
 func DeleteDashboardAclByUser(c *middleware.Context) Response {
-	// dashboardId := c.ParamsInt64(":id")
+	dashId := c.ParamsInt64(":id")
-	// userId := c.ParamsInt64(":userId")
+	userId := c.ParamsInt64(":userId")
-	// cmd := m.RemoveDashboardPermissionCommand{DashboardId: dashboardId, UserId: userId, OrgId: c.OrgId}
+
-	//
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
-	// hasPermission, err := guardian.CanDeleteFromAcl(dashboardId, c.OrgRole, c.IsGrafanaAdmin, c.OrgId, c.UserId)
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-	// if err != nil {
+		return dashboardGuardianResponse(err)
-	// 	return ApiError(500, "Failed to delete from Dashboard ACL", err)
+	}
-	// }
+
-	//
+	cmd := m.RemoveDashboardPermissionCommand{DashboardId: dashId, UserId: userId, OrgId: c.OrgId}
-	// if !hasPermission {
+
-	// 	return Json(403, util.DynMap{"status": "Forbidden", "message": "Does not have access to this Dashboard ACL"})
+	if err := bus.Dispatch(&cmd); err != nil {
-	// }
+		return ApiError(500, "Failed to delete permission for user", err)
-	//
+	}
-	// if err := bus.Dispatch(&cmd); err != nil {
-	// 	return ApiError(500, "Failed to delete permission for user", err)
-	// }
 
 	return Json(200, "")
 }
 
 func DeleteDashboardAclByUserGroup(c *middleware.Context) Response {
-	// dashboardId := c.ParamsInt64(":id")
+	dashId := c.ParamsInt64(":id")
-	// userGroupId := c.ParamsInt64(":userGroupId")
+	userGroupId := c.ParamsInt64(":userGroupId")
-	// cmd := m.RemoveDashboardPermissionCommand{DashboardId: dashboardId, UserGroupId: userGroupId, OrgId: c.OrgId}
+
-	//
+	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
-	// hasPermission, err := guardian.CanDeleteFromAcl(dashboardId, c.OrgRole, c.IsGrafanaAdmin, c.OrgId, c.UserId)
+	if canSave, err := guardian.CanSave(); err != nil || !canSave {
-	// if err != nil {
+		return dashboardGuardianResponse(err)
-	// 	return ApiError(500, "Failed to delete from Dashboard ACL", err)
+	}
-	// }
+
-	//
+	cmd := m.RemoveDashboardPermissionCommand{DashboardId: dashId, UserGroupId: userGroupId, OrgId: c.OrgId}
-	// if !hasPermission {
+
-	// 	return Json(403, util.DynMap{"status": "Forbidden", "message": "Does not have access to this Dashboard ACL"})
+	if err := bus.Dispatch(&cmd); err != nil {
-	// }
+		return ApiError(500, "Failed to delete permission for user", err)
-	//
+	}
-	// if err := bus.Dispatch(&cmd); err != nil {
-	// 	return ApiError(500, "Failed to delete permission for user", err)
-	// }
 
 	return Json(200, "")
 }

pkg/services/guardian/guardian.go

@@ -2,6 +2,7 @@ package guardian
 
 import (
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/log"
 	m "github.com/grafana/grafana/pkg/models"
 )
 
@@ -11,6 +12,7 @@ type DashboardGuardian struct {
 	orgId  int64
 	acl    []*m.DashboardAcl
 	groups []*m.UserGroup
+	log    log.Logger
 }
 
 func NewDashboardGuardian(dashId int64, orgId int64, user *m.SignedInUser) *DashboardGuardian {
@@ -18,6 +20,7 @@ func NewDashboardGuardian(dashId int64, orgId int64, user *m.SignedInUser) *Dash
 		user:   user,
 		dashId: dashId,
 		orgId:  orgId,
+		log:    log.New("guardians.dashboard"),
 	}
 }
 
@@ -34,6 +37,10 @@ func (g *DashboardGuardian) CanView() (bool, error) {
 }
 
 func (g *DashboardGuardian) HasPermission(permission m.PermissionType, fallbackRole m.RoleType) (bool, error) {
+	if g.user.OrgRole == m.ROLE_ADMIN {
+		return true, nil
+	}
+
 	acl, err := g.getAcl()
 	if err != nil {
 		return false, err

public/app/features/dashboard/acl/acl.html

@@ -40,7 +40,7 @@
         <tbody>
           <tr ng-repeat="permission in ctrl.userPermissions" class="permissionlist__item">
             <td>{{permission.userLogin}}</td>
-            <td><select class="gf-form-input gf-size-auto" ng-model="permission.permissionType" ng-options="p.value as p.text for p in ctrl.permissionTypeOptions" ng-change="ctrl.updatePermission(permission)"></select></td>
+            <td><select class="gf-form-input gf-size-auto" ng-model="permission.permissions" ng-options="p.value as p.text for p in ctrl.permissionTypeOptions" ng-change="ctrl.updatePermission(permission)"></select></td>
             <td class="text-right">
               <a ng-click="ctrl.removeUserPermission(permission)" class="btn btn-danger btn-small">
                 <i class="fa fa-remove"></i>