From fcefd6fff9afd853dcd958cc0ecc1c896dac6884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Jim=C3=A9nez=20S=C3=A1nchez?= Date: Fri, 24 Apr 2026 09:01:54 +0200 Subject: [PATCH] LibraryPanels: Return 403 instead of 500 for insufficient permissions (#123407) * LibraryPanels: Return 403 instead of 500 for insufficient permissions Library element creation previously returned an untyped fmt.Errorf containing "insufficient permissions", which the library elements API mapped to 403 via a string match but which fell through to a generic 500 in the dashboard import handler. Make the error a typed errutil.Forbidden so callers such as /api/dashboards/import propagate the correct HTTP status. Fixes gz#225599 * fixup! LibraryPanels: Return 403 instead of 500 for insufficient permissions --- pkg/services/dashboardimport/api/api.go | 4 +++ pkg/services/dashboardimport/api/api_test.go | 31 ++++++++++++++++++++ pkg/services/libraryelements/api.go | 3 +- pkg/services/libraryelements/database.go | 6 ++-- pkg/services/libraryelements/model/model.go | 2 ++ 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/pkg/services/dashboardimport/api/api.go b/pkg/services/dashboardimport/api/api.go index 9ce7a72684a..7f866fc2c15 100644 --- a/pkg/services/dashboardimport/api/api.go +++ b/pkg/services/dashboardimport/api/api.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboardimport/utils" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" + libraryelementsmodel "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/web" @@ -125,6 +126,9 @@ func (api *ImportDashboardAPI) ImportDashboard(c *contextmodel.ReqContext) respo if errors.Is(err, utils.ErrDashboardInputMissing) { return response.Error(http.StatusBadRequest, err.Error(), err) } + if errors.Is(err, libraryelementsmodel.ErrLibraryElementInsufficientPermissions) { + return response.Error(http.StatusForbidden, err.Error(), err) + } return apierrors.ToDashboardErrorResponse(c.Req.Context(), api.pluginStore, err) } diff --git a/pkg/services/dashboardimport/api/api_test.go b/pkg/services/dashboardimport/api/api_test.go index 461e370352e..1618f20036e 100644 --- a/pkg/services/dashboardimport/api/api_test.go +++ b/pkg/services/dashboardimport/api/api_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "testing" @@ -16,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboardimport" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" + libraryelementsmodel "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/web/webtest" @@ -158,6 +160,35 @@ func TestImportDashboardAPI(t *testing.T) { require.Equal(t, http.StatusForbidden, resp.StatusCode) }) }) + + t.Run("Import service returns a library panel permission error", func(t *testing.T) { + service := &serviceMock{ + importDashboardFunc: func(ctx context.Context, req *dashboardimport.ImportDashboardRequest) (*dashboardimport.ImportDashboardResponse, error) { + return nil, fmt.Errorf("%w: folder UID 'abc'", libraryelementsmodel.ErrLibraryElementInsufficientPermissions) + }, + } + importDashboardAPI := New(service, quotaServiceFunc(quotaNotReached), nil, actest.FakeAccessControl{ExpectedEvaluate: true}, featuremgmt.WithFeatures()) + routeRegister := routing.NewRouteRegister() + importDashboardAPI.RegisterAPIEndpoints(routeRegister) + s := webtest.NewServer(t, routeRegister) + + cmd := &dashboardimport.ImportDashboardRequest{ + Dashboard: simplejson.New(), + } + jsonBytes, err := json.Marshal(cmd) + require.NoError(t, err) + req := s.NewPostRequest("/api/dashboards/import", bytes.NewReader(jsonBytes)) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{ + UserID: 1, + Permissions: map[int64]map[string][]string{ + 1: {dashboards.ActionDashboardsCreate: {}}, + }, + }) + resp, err := s.SendJSON(req) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + }) } func TestInterpolateDashboardFeatureFlag(t *testing.T) { diff --git a/pkg/services/libraryelements/api.go b/pkg/services/libraryelements/api.go index 3cde82428d7..4663804cc5f 100644 --- a/pkg/services/libraryelements/api.go +++ b/pkg/services/libraryelements/api.go @@ -6,7 +6,6 @@ import ( "fmt" "hash/fnv" "net/http" - "strings" k8serrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -421,7 +420,7 @@ func (l *LibraryElementService) toLibraryElementError(err error, message string) if errors.Is(err, model.ErrLibraryElementProvisionedFolder) { return response.Error(http.StatusConflict, model.ErrLibraryElementProvisionedFolder.Error(), err) } - if err != nil && strings.Contains(err.Error(), "insufficient permissions") { + if errors.Is(err, model.ErrLibraryElementInsufficientPermissions) { return response.Error(http.StatusForbidden, err.Error(), err) } diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index b602c7ea873..9eea2d0a241 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -181,12 +181,12 @@ func (l *LibraryElementService) CreateElement(c context.Context, signedInUser id err = l.SQLStore.WithTransactionalDbSession(c, func(session *db.Session) error { allowed, err := l.AccessControl.Evaluate(c, signedInUser, ac.EvalPermission(ActionLibraryPanelsCreate, folder.ScopeFoldersProvider.GetResourceScopeUID(folderUID))) - if !allowed { - return fmt.Errorf("insufficient permissions for creating library panel in folder with UID: '%s'", folderUID) - } if err != nil { return err } + if !allowed { + return fmt.Errorf("%w: folder UID '%s'", model.ErrLibraryElementInsufficientPermissions, folderUID) + } if _, err := session.Insert(&element); err != nil { if l.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { return model.ErrLibraryElementAlreadyExists diff --git a/pkg/services/libraryelements/model/model.go b/pkg/services/libraryelements/model/model.go index 7480997c4a1..4210b468e4c 100644 --- a/pkg/services/libraryelements/model/model.go +++ b/pkg/services/libraryelements/model/model.go @@ -134,6 +134,8 @@ var ( ErrLibraryElementUIDTooLong = errors.New("uid too long, max 40 characters") // ErrLibraryElementProvisionedFolder indicates that a library element cannot be created on a provisioned folder. ErrLibraryElementProvisionedFolder = errors.New("resource type not supported in repository-managed folders") + // ErrLibraryElementInsufficientPermissions is returned when the caller lacks permission to perform a library element operation in a folder. + ErrLibraryElementInsufficientPermissions = errors.New("insufficient permissions for library element operation") ) // Commands