Org: Fix org deletion (#106193)

This commit is contained in:
Stephanie Hingtgen
2025-05-29 09:40:06 +03:00
committed by GitHub
parent ab99211b40
commit 5303a1cc7e
3 changed files with 108 additions and 4 deletions
@@ -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)
})
+25 -2
View File
@@ -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)
}