[release-12.0.2] Org: Fix org deletion (#106461)
Org: Fix org deletion (#106193)
(cherry picked from commit 5303a1cc7e)
Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>
This commit is contained in:
co-authored by
Stephanie Hingtgen
parent
b2712d5086
commit
c3ffd59702
@@ -14,8 +14,10 @@ import (
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards"
|
||||
"github.com/grafana/grafana/pkg/services/folder"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
@@ -997,16 +999,28 @@ func TestIntegrationGetFolders(t *testing.T) {
|
||||
func CreateOrg(t *testing.T, db db.DB, cfg *setting.Cfg) int64 {
|
||||
t.Helper()
|
||||
|
||||
requester := &identity.StaticRequester{
|
||||
OrgID: 1,
|
||||
Permissions: map[int64]map[string][]string{
|
||||
1: map[string][]string{
|
||||
accesscontrol.ActionOrgsDelete: {"*"},
|
||||
},
|
||||
2: map[string][]string{
|
||||
accesscontrol.ActionOrgsDelete: {"*"},
|
||||
},
|
||||
},
|
||||
}
|
||||
orgService, err := orgimpl.ProvideService(db, cfg, quotatest.New(false, nil))
|
||||
require.NoError(t, err)
|
||||
dashSvc := &dashboards.FakeDashboardService{}
|
||||
dashSvc.On("DeleteAllDashboards", mock.Anything, mock.Anything).Return(nil)
|
||||
deleteOrgService, err := orgimpl.ProvideDeletionService(db, cfg, dashSvc)
|
||||
deleteOrgService, err := orgimpl.ProvideDeletionService(db, cfg, dashSvc, acimpl.ProvideAccessControlTest())
|
||||
require.NoError(t, err)
|
||||
orgID, err := orgService.GetOrCreate(context.Background(), "test-org")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() {
|
||||
err = deleteOrgService.Delete(context.Background(), &org.DeleteOrgCommand{ID: orgID})
|
||||
ctx := identity.WithRequester(context.Background(), requester)
|
||||
err = deleteOrgService.Delete(ctx, &org.DeleteOrgCommand{ID: orgID})
|
||||
require.NoError(t, err)
|
||||
})
|
||||
|
||||
|
||||
@@ -2,9 +2,12 @@ package orgimpl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
@@ -15,9 +18,10 @@ type DeletionService struct {
|
||||
cfg *setting.Cfg
|
||||
log log.Logger
|
||||
dashSvc dashboards.DashboardService
|
||||
ac accesscontrol.AccessControl
|
||||
}
|
||||
|
||||
func ProvideDeletionService(db db.DB, cfg *setting.Cfg, dashboardService dashboards.DashboardService) (org.DeletionService, error) {
|
||||
func ProvideDeletionService(db db.DB, cfg *setting.Cfg, dashboardService dashboards.DashboardService, ac accesscontrol.AccessControl) (org.DeletionService, error) {
|
||||
log := log.New("org deletion service")
|
||||
s := &DeletionService{
|
||||
store: &sqlStore{
|
||||
@@ -28,13 +32,32 @@ func ProvideDeletionService(db db.DB, cfg *setting.Cfg, dashboardService dashboa
|
||||
cfg: cfg,
|
||||
dashSvc: dashboardService,
|
||||
log: log,
|
||||
ac: ac,
|
||||
}
|
||||
|
||||
return s, nil
|
||||
}
|
||||
|
||||
func (s *DeletionService) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error {
|
||||
err := s.dashSvc.DeleteAllDashboards(ctx, cmd.ID)
|
||||
// we need to use a service identity to delete dashboards from the dashboard service (because the currently signed in user
|
||||
// has to be signed into a different org to delete another org, and so this will fail the namespace check). While we already
|
||||
// do auth checks on the /api layer, since this is available on the service, adding a check here as well to be safe, in case any additional
|
||||
// usage is added internally.
|
||||
requester, err := identity.GetRequester(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
hasAccess, err := s.ac.Evaluate(ctx, requester, accesscontrol.EvalPermission(accesscontrol.ActionOrgsDelete))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !hasAccess {
|
||||
return errors.New("access denied to delete org")
|
||||
}
|
||||
|
||||
ctx, _ = identity.WithServiceIdentity(ctx, cmd.ID)
|
||||
err = s.dashSvc.DeleteAllDashboards(ctx, cmd.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
package orgimpl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
|
||||
"github.com/grafana/grafana/pkg/services/dashboards"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
func TestDeletionService_Delete(t *testing.T) {
|
||||
store := &FakeOrgStore{}
|
||||
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures())
|
||||
dashSvc := dashboards.NewFakeDashboardService(t)
|
||||
svc := &DeletionService{
|
||||
store: store,
|
||||
cfg: setting.NewCfg(),
|
||||
dashSvc: dashSvc,
|
||||
ac: ac,
|
||||
}
|
||||
|
||||
// if a user has access to delete orgs, then the dashboards should be deleted with a service identity
|
||||
requester := &identity.StaticRequester{
|
||||
OrgID: 1,
|
||||
Permissions: map[int64]map[string][]string{
|
||||
1: map[string][]string{
|
||||
accesscontrol.ActionOrgsDelete: {"*"},
|
||||
},
|
||||
2: map[string][]string{
|
||||
accesscontrol.ActionOrgsDelete: {"*"},
|
||||
},
|
||||
},
|
||||
}
|
||||
dashSvc.On("DeleteAllDashboards", mock.MatchedBy(func(ctx context.Context) bool {
|
||||
return identity.IsServiceIdentity(ctx)
|
||||
}), int64(2)).Return(nil).Once()
|
||||
ctx := context.Background()
|
||||
ctx = identity.WithRequester(ctx, requester)
|
||||
err := svc.Delete(ctx, &org.DeleteOrgCommand{ID: 2})
|
||||
require.NoError(t, err)
|
||||
dashSvc.AssertExpectations(t)
|
||||
|
||||
// if a user does not have access to delete orgs, then the dashboards should not be deleted
|
||||
requester = &identity.StaticRequester{
|
||||
OrgID: 1,
|
||||
Permissions: map[int64]map[string][]string{
|
||||
1: map[string][]string{
|
||||
accesscontrol.ActionOrgsRead: {"*"},
|
||||
},
|
||||
2: map[string][]string{
|
||||
accesscontrol.ActionOrgsRead: {"*"},
|
||||
},
|
||||
},
|
||||
}
|
||||
ctx = context.Background()
|
||||
ctx = identity.WithRequester(ctx, requester)
|
||||
err = svc.Delete(ctx, &org.DeleteOrgCommand{ID: 2})
|
||||
require.Error(t, err)
|
||||
}
|
||||
Reference in New Issue
Block a user