Public Dashboards: audit log paths and add traceId where user facing error is different (#56914)

Audit all paths for publicdashboards and implement traces where user facing error is different from the internal error.

pkg/services/publicdashboards/api/api.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"context"
 	"errors"
 	"net/http"
 	"strconv"
@@ -88,7 +89,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 	)
 
 	if err != nil {
-		return api.handleError(http.StatusInternalServerError, "failed to get public dashboard", err)
+		return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboard: failed to get public dashboard", err)
 	}
 
 	meta := dtos.DashboardMeta{
@@ -118,7 +119,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response {
 	resp, err := api.PublicDashboardService.ListPublicDashboards(c.Req.Context(), c.OrgID)
 	if err != nil {
-		return api.handleError(http.StatusInternalServerError, "failed to list public dashboards", err)
+		return api.handleError(c.Req.Context(), http.StatusInternalServerError, "ListPublicDashboards: failed to list public dashboards", err)
 	}
 	return response.JSON(http.StatusOK, resp)
 }
@@ -128,7 +129,7 @@ func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response {
 func (api *Api) GetPublicDashboardConfig(c *models.ReqContext) response.Response {
 	pdc, err := api.PublicDashboardService.GetPublicDashboardConfig(c.Req.Context(), c.OrgID, web.Params(c.Req)[":uid"])
 	if err != nil {
-		return api.handleError(http.StatusInternalServerError, "failed to get public dashboard config", err)
+		return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboardConfig: failed to get public dashboard config", err)
 	}
 	return response.JSON(http.StatusOK, pdc)
 }
@@ -139,12 +140,12 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons
 	// exit if we don't have a valid dashboardUid
 	dashboardUid := web.Params(c.Req)[":uid"]
 	if dashboardUid == "" || !util.IsValidShortUID(dashboardUid) {
-		api.handleError(http.StatusBadRequest, "no dashboardUid", dashboards.ErrDashboardIdentifierNotSet)
+		api.handleError(c.Req.Context(), http.StatusBadRequest, "SavePublicDashboardConfig: no dashboardUid", dashboards.ErrDashboardIdentifierNotSet)
 	}
 
 	pubdash := &PublicDashboard{}
 	if err := web.Bind(c.Req, pubdash); err != nil {
-		return response.Error(http.StatusBadRequest, "bad request data", err)
+		return response.Error(http.StatusBadRequest, "SavePublicDashboardConfig: bad request data", err)
 	}
 
 	// Always set the orgID and userID from the session
@@ -159,7 +160,7 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons
 	// Save the public dashboard
 	pubdash, err := api.PublicDashboardService.SavePublicDashboardConfig(c.Req.Context(), c.SignedInUser, &dto)
 	if err != nil {
-		return api.handleError(http.StatusInternalServerError, "failed to save public dashboard configuration", err)
+		return api.handleError(c.Req.Context(), http.StatusInternalServerError, "SavePublicDashboardConfig: failed to save public dashboard configuration", err)
 	}
 
 	return response.JSON(http.StatusOK, pubdash)
@@ -170,17 +171,17 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons
 func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response {
 	panelId, err := strconv.ParseInt(web.Params(c.Req)[":panelId"], 10, 64)
 	if err != nil {
-		return response.Error(http.StatusBadRequest, "invalid panel ID", err)
+		return response.Error(http.StatusBadRequest, "QueryPublicDashboard: invalid panel ID", err)
 	}
 
 	reqDTO := PublicDashboardQueryDTO{}
 	if err = web.Bind(c.Req, &reqDTO); err != nil {
-		return response.Error(http.StatusBadRequest, "bad request data", err)
+		return response.Error(http.StatusBadRequest, "QueryPublicDashboard: bad request data", err)
 	}
 
 	resp, err := api.PublicDashboardService.GetQueryDataResponse(c.Req.Context(), c.SkipCache, reqDTO, panelId, web.Params(c.Req)[":accessToken"])
 	if err != nil {
-		return api.handleError(http.StatusInternalServerError, "error running public dashboard panel queries", err)
+		return api.handleError(c.Req.Context(), http.StatusInternalServerError, "QueryPublicDashboard: error running public dashboard panel queries", err)
 	}
 
 	return toJsonStreamingResponse(api.Features, resp)
@@ -189,10 +190,10 @@ func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response {
 // util to help us unpack dashboard and publicdashboard errors or use default http code and message
 // we should look to do some future refactoring of these errors as publicdashboard err is the same as a dashboarderr, just defined in a
 // different package.
-func (api *Api) handleError(code int, message string, err error) response.Response {
+func (api *Api) handleError(ctx context.Context, code int, message string, err error) response.Response {
 	var publicDashboardErr PublicDashboardErr
-
-	api.Log.Error(message, "error", err.Error())
+	ctxLogger := api.Log.FromContext(ctx)
+	ctxLogger.Error(message, "error", err.Error())
 
 	// handle public dashboard error
 	if ok := errors.As(err, &publicDashboardErr); ok {

pkg/services/publicdashboards/database/database.go

@@ -100,7 +100,6 @@ func (d *PublicDashboardStoreImpl) GetPublicDashboard(ctx context.Context, acces
 
 	// find dashboard
 	dashRes, err := d.GetDashboard(ctx, pdRes.DashboardUid)
-
 	if err != nil {
 		return nil, nil, err
 	}
@@ -272,7 +271,6 @@ func (d *PublicDashboardStoreImpl) PublicDashboardEnabled(ctx context.Context, d
 		}
 
 		hasPublicDashboard = result > 0
-
 		return err
 	})
 
@@ -292,7 +290,6 @@ func (d *PublicDashboardStoreImpl) AccessTokenExists(ctx context.Context, access
 		}
 
 		hasPublicDashboard = result > 0
-
 		return err
 	})
 

pkg/services/publicdashboards/public_dashboard_service_mock.go

@@ -47,7 +47,7 @@ func (_m *FakePublicDashboardService) AccessTokenExists(ctx context.Context, acc
 }
 
 // BuildAnonymousUser provides a mock function with given fields: ctx, dashboard
-func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error) {
+func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser {
 	ret := _m.Called(ctx, dashboard)
 
 	var r0 *user.SignedInUser
@@ -59,14 +59,7 @@ func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, da
 		}
 	}
 
-	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, *models.Dashboard) error); ok {
-		r1 = rf(ctx, dashboard)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
+	return r0
 }
 
 // GetDashboard provides a mock function with given fields: ctx, dashboardUid

pkg/services/publicdashboards/public_dashboard_store_mock.go

@@ -1,4 +1,4 @@
-// Code generated by mockery v2.14.0. DO NOT EDIT.
+// Code generated by mockery v2.12.1. DO NOT EDIT.
 
 package publicdashboards
 
@@ -9,6 +9,8 @@ import (
 	mock "github.com/stretchr/testify/mock"
 
 	publicdashboardsmodels "github.com/grafana/grafana/pkg/services/publicdashboards/models"
+
+	testing "testing"
 )
 
 // FakePublicDashboardStore is an autogenerated mock type for the Store type
@@ -273,13 +275,8 @@ func (_m *FakePublicDashboardStore) UpdatePublicDashboardConfig(ctx context.Cont
 	return r0
 }
 
-type mockConstructorTestingTNewFakePublicDashboardStore interface {
-	mock.TestingT
-	Cleanup(func())
-}
-
-// NewFakePublicDashboardStore creates a new instance of FakePublicDashboardStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
-func NewFakePublicDashboardStore(t mockConstructorTestingTNewFakePublicDashboardStore) *FakePublicDashboardStore {
+// NewFakePublicDashboardStore creates a new instance of FakePublicDashboardStore. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations.
+func NewFakePublicDashboardStore(t testing.TB) *FakePublicDashboardStore {
 	mock := &FakePublicDashboardStore{}
 	mock.Mock.Test(t)
 

pkg/services/publicdashboards/publicdashboard.go

@@ -15,7 +15,7 @@ import (
 //go:generate mockery --name Service --structname FakePublicDashboardService --inpackage --filename public_dashboard_service_mock.go
 type Service interface {
 	AccessTokenExists(ctx context.Context, accessToken string) (bool, error)
-	BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error)
+	BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser
 	GetDashboard(ctx context.Context, dashboardUid string) (*models.Dashboard, error)
 	GetMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64, reqDTO PublicDashboardQueryDTO) (dtos.MetricRequest, error)
 	GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error)

pkg/services/publicdashboards/service/service.go

@@ -72,16 +72,24 @@ func (pd *PublicDashboardServiceImpl) GetDashboard(ctx context.Context, dashboar
 // Gets public dashboard via access token
 func (pd *PublicDashboardServiceImpl) GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) {
 	pubdash, dash, err := pd.store.GetPublicDashboard(ctx, accessToken)
+	ctxLogger := pd.log.FromContext(ctx)
 
 	if err != nil {
 		return nil, nil, err
 	}
 
-	if pubdash == nil || dash == nil {
+	if pubdash == nil {
+		ctxLogger.Error("GetPublicDashboard: Public dashboard not found", "accessToken", accessToken)
+		return nil, nil, ErrPublicDashboardNotFound
+	}
+
+	if dash == nil {
+		ctxLogger.Error("GetPublicDashboard: Dashboard not found", "accessToken", accessToken)
 		return nil, nil, ErrPublicDashboardNotFound
 	}
 
 	if !pubdash.IsEnabled {
+		ctxLogger.Error("GetPublicDashboard: Public dashboard is disabled", "accessToken", accessToken)
 		return nil, nil, ErrPublicDashboardNotFound
 	}
 
@@ -205,11 +213,7 @@ func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context,
 		return nil, err
 	}
 
-	anonymousUser, err := pd.BuildAnonymousUser(ctx, dashboard)
-	if err != nil {
-		return nil, err
-	}
-
+	anonymousUser := pd.BuildAnonymousUser(ctx, dashboard)
 	res, err := pd.QueryDataService.QueryData(ctx, anonymousUser, skipCache, metricReq)
 
 	reqDatasources := metricReq.GetUniqueDatasourceTypes()
@@ -225,8 +229,9 @@ func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context,
 }
 
 func (pd *PublicDashboardServiceImpl) GetMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64, queryDto PublicDashboardQueryDTO) (dtos.MetricRequest, error) {
-	if err := validation.ValidateQueryPublicDashboardRequest(queryDto); err != nil {
-		return dtos.MetricRequest{}, ErrPublicDashboardBadRequest
+	err := validation.ValidateQueryPublicDashboardRequest(queryDto)
+	if err != nil {
+		return dtos.MetricRequest{}, err
 	}
 
 	metricReqDTO, err := pd.buildMetricRequest(
@@ -270,7 +275,7 @@ func (pd *PublicDashboardServiceImpl) buildMetricRequest(ctx context.Context, da
 }
 
 // BuildAnonymousUser creates a user with permissions to read from all datasources used in the dashboard
-func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error) {
+func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser {
 	datasourceUids := queries.GetUniqueDashboardDatasourceUids(dashboard.Data)
 
 	// Create a temp user with read-only datasource permissions
@@ -286,7 +291,7 @@ func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, da
 	permissions[datasources.ActionRead] = readScopes
 	anonymousUser.Permissions[dashboard.OrgId] = permissions
 
-	return anonymousUser, nil
+	return anonymousUser
 }
 
 func (pd *PublicDashboardServiceImpl) PublicDashboardEnabled(ctx context.Context, dashboardUid string) (bool, error) {
@@ -337,7 +342,7 @@ func (pd *PublicDashboardServiceImpl) logIsEnabledChanged(existingPubdash *Publi
 		if newPubdash.IsEnabled {
 			verb = "enabled"
 		}
-		pd.log.Info(fmt.Sprintf("Public dashboard %v: dashboardUid: %v, user:%v", verb, newPubdash.Uid, u.Login))
+		pd.log.Info("Public dashboard "+verb, "publicDashboardUid", newPubdash.Uid, "dashboardUid", newPubdash.DashboardUid, "user", u.Login)
 	}
 }
 

pkg/services/publicdashboards/service/service_test.go

@@ -373,8 +373,7 @@ func TestBuildAnonymousUser(t *testing.T) {
 	}
 
 	t.Run("will add datasource read and query permissions to user for each datasource in dashboard", func(t *testing.T) {
-		user, err := service.BuildAnonymousUser(context.Background(), dashboard)
-		require.NoError(t, err)
+		user := service.BuildAnonymousUser(context.Background(), dashboard)
 		require.Equal(t, dashboard.OrgId, user.OrgID)
 		require.Equal(t, "datasources:uid:ds1", user.Permissions[user.OrgID]["datasources:query"][0])
 		require.Equal(t, "datasources:uid:ds3", user.Permissions[user.OrgID]["datasources:query"][1])