From 1267fd039cd7edce5e17e48edab3129ab26e819d Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Mon, 24 Oct 2022 11:28:33 +0200 Subject: [PATCH] add a middleware to verify the project exists during a request Signed-off-by: Augustin Husson --- cmd/perses/main.go | 3 +- internal/api/core/middleware/proxy.go | 3 +- internal/api/core/middleware/verification.go | 71 ++++++++++++++++++++ internal/api/e2e/datasource_test.go | 38 ++++++++++- internal/api/impl/v1/dashboard/service.go | 3 - internal/api/shared/toolbox.go | 2 +- internal/api/shared/utils.go | 7 +- internal/api/shared/validate/validate.go | 2 +- pkg/model/api/v1/datasource/http/http.go | 2 +- 9 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 internal/api/core/middleware/verification.go diff --git a/cmd/perses/main.go b/cmd/perses/main.go index 1e5c08f04a..21615514eb 100644 --- a/cmd/perses/main.go +++ b/cmd/perses/main.go @@ -75,7 +75,8 @@ func main() { runner.HTTPServerBuilder(). APIRegistration(persesAPI). APIRegistration(persesFrontend). - Middleware(middleware.Proxy(persistenceManager.GetDatasource(), persistenceManager.GetGlobalDatasource())) + Middleware(middleware.Proxy(persistenceManager.GetDatasource(), persistenceManager.GetGlobalDatasource())). + Middleware(middleware.CheckProject(persistenceManager.GetProject())) // start the application runner.Start() diff --git a/internal/api/core/middleware/proxy.go b/internal/api/core/middleware/proxy.go index 278f7c31c5..08f4f568a4 100644 --- a/internal/api/core/middleware/proxy.go +++ b/internal/api/core/middleware/proxy.go @@ -126,7 +126,7 @@ type proxy interface { } func newProxy(spec v1.DatasourceSpec, path string) (proxy, error) { - cfg, err := datasourceHTTP.CheckAndValidate(spec.Plugin.Spec) + cfg, err := datasourceHTTP.ValidateAndExtract(spec.Plugin.Spec) if err != nil { logrus.WithError(err).Error("unable to build or find the http config in the datasource") return nil, echo.NewHTTPError(http.StatusBadGateway, "unable to find the http config") @@ -137,7 +137,6 @@ func newProxy(spec v1.DatasourceSpec, path string) (proxy, error) { path: path, }, nil } - // TODO build the HTTP proxy return nil, echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("datasource type '%T' not managed", spec)) } diff --git a/internal/api/core/middleware/verification.go b/internal/api/core/middleware/verification.go new file mode 100644 index 0000000000..ae031bd9e9 --- /dev/null +++ b/internal/api/core/middleware/verification.go @@ -0,0 +1,71 @@ +// Copyright 2022 The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package middleware + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/labstack/echo/v4" + "github.com/perses/perses/internal/api/interface/v1/project" + "github.com/perses/perses/internal/api/shared" +) + +type partialMetadata struct { + Project string `json:"project"` +} + +type partialObject struct { + Metadata partialMetadata `json:"metadata"` +} + +// CheckProject is a middleware that will verify if the project used for the request exists. +func CheckProject(dao project.DAO) echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + projectName := shared.GetProjectParameter(c) + if len(projectName) == 0 { + // It's possible the HTTP Path doesn't contain the project because the user is calling the root endpoint + // to create a new dashboard for example. + // So we need to ensure the project name exists in the resource, which is why we will partially decode the body to get the project name. + // And just to avoid a non-necessary deserialization, we will ensure we are managing a resource that is part of a project by checking the HTTP Path. + if c.Request().Method == http.MethodPost { + for _, path := range shared.ProjectResourcePathList { + if strings.HasPrefix(c.Path(), fmt.Sprintf("%s/%s", shared.APIV1Prefix, path)) { + o := &partialObject{} + if err := c.Bind(o); err != nil { + return err + } + if len(o.Metadata.Project) == 0 { + return shared.HandleError(fmt.Errorf("%w: metadata.project cannot be empty", shared.BadRequestError)) + } + projectName = o.Metadata.Project + } + } + } + } + if len(projectName) > 0 { + if _, err := dao.Get(projectName); err != nil { + if errors.Is(err, shared.NotFoundError) { + return shared.HandleError(fmt.Errorf("%w, metadata.project %q doesn't exist", shared.BadRequestError, projectName)) + } + return shared.HandleError(err) + } + } + return next(c) + } + } +} diff --git a/internal/api/e2e/datasource_test.go b/internal/api/e2e/datasource_test.go index 5068a296d4..88ae6152bd 100644 --- a/internal/api/e2e/datasource_test.go +++ b/internal/api/e2e/datasource_test.go @@ -69,7 +69,7 @@ func TestCreateDatasourceWithConflict(t *testing.T) { } func TestCreateDatasourceBadRequest(t *testing.T) { - project := &v1.Datasource{Kind: v1.KindDatasource} + dts := &v1.Datasource{Kind: v1.KindDatasource} server, _ := utils.CreateServer(t) defer server.Close() @@ -80,7 +80,41 @@ func TestCreateDatasourceBadRequest(t *testing.T) { // metadata.name is not provided, it should return a bad request e.POST(fmt.Sprintf("%s/%s", shared.APIV1Prefix, shared.PathDatasource)). - WithJSON(project). + WithJSON(dts). + Expect(). + Status(http.StatusBadRequest) +} + +func TestCreateDatasourceWithEmptyProjectName(t *testing.T) { + dts := &v1.Datasource{Kind: v1.KindDatasource} + dts.Metadata.Project = "" + server, _ := utils.CreateServer(t) + defer server.Close() + e := httpexpect.WithConfig(httpexpect.Config{ + BaseURL: server.URL, + Reporter: httpexpect.NewAssertReporter(t), + }) + + // metadata.name is not provided, it should return a bad request + e.POST(fmt.Sprintf("%s/%s", shared.APIV1Prefix, shared.PathDatasource)). + WithJSON(dts). + Expect(). + Status(http.StatusBadRequest) +} + +func TestCreateDatasourceWithNonExistingProject(t *testing.T) { + dts := &v1.Datasource{Kind: v1.KindDatasource} + dts.Metadata.Project = "404NotFound" + server, _ := utils.CreateServer(t) + defer server.Close() + e := httpexpect.WithConfig(httpexpect.Config{ + BaseURL: server.URL, + Reporter: httpexpect.NewAssertReporter(t), + }) + + // metadata.name is not provided, it should return a bad request + e.POST(fmt.Sprintf("%s/%s", shared.APIV1Prefix, shared.PathDatasource)). + WithJSON(dts). Expect(). Status(http.StatusBadRequest) } diff --git a/internal/api/impl/v1/dashboard/service.go b/internal/api/impl/v1/dashboard/service.go index f240cb964e..4ffd9752df 100644 --- a/internal/api/impl/v1/dashboard/service.go +++ b/internal/api/impl/v1/dashboard/service.go @@ -47,9 +47,6 @@ func (s *service) Create(entity api.Entity) (interface{}, error) { } func (s *service) create(entity *v1.Dashboard) (*v1.Dashboard, error) { - // Note: you don't need to check that the project exists since once the permission middleware will be in place, - // it won't be possible to create a resources into a not known project - // verify this new dashboard passes the validation if err := validate.Dashboard(entity, s.sch); err != nil { return nil, fmt.Errorf("%w: %s", shared.BadRequestError, err) diff --git a/internal/api/shared/toolbox.go b/internal/api/shared/toolbox.go index 1bd9d866df..09c36e5e69 100644 --- a/internal/api/shared/toolbox.go +++ b/internal/api/shared/toolbox.go @@ -29,7 +29,7 @@ type Parameters struct { func extractParameters(ctx echo.Context) Parameters { return Parameters{ - Project: getProjectParameter(ctx), + Project: GetProjectParameter(ctx), Name: getNameParameter(ctx), } } diff --git a/internal/api/shared/utils.go b/internal/api/shared/utils.go index d4cc3ba95e..ed42bbd49b 100644 --- a/internal/api/shared/utils.go +++ b/internal/api/shared/utils.go @@ -35,11 +35,16 @@ const ( PathUser = "users" ) +// ProjectResourcePathList is containing the list of the resource path that are part of a project. +var ProjectResourcePathList = []string{ + PathDashboard, PathDatasource, PathFolder, +} + func getNameParameter(ctx echo.Context) string { return ctx.Param(ParamName) } -func getProjectParameter(ctx echo.Context) string { +func GetProjectParameter(ctx echo.Context) string { return ctx.Param(ParamProject) } diff --git a/internal/api/shared/validate/validate.go b/internal/api/shared/validate/validate.go index 51cccc6986..dba0cff5fd 100644 --- a/internal/api/shared/validate/validate.go +++ b/internal/api/shared/validate/validate.go @@ -40,7 +40,7 @@ func Dashboard(entity *modelV1.Dashboard, sch schemas.Schemas) error { func Datasource[T modelV1.DatasourceInterface](entity T, list []T, sch schemas.Schemas) error { plugin := entity.GetSpec().Plugin - if _, err := http.CheckAndValidate(plugin.Spec); err != nil { + if _, err := http.ValidateAndExtract(plugin.Spec); err != nil { return err } if list != nil { diff --git a/pkg/model/api/v1/datasource/http/http.go b/pkg/model/api/v1/datasource/http/http.go index 914a06e12b..ce8de33a5b 100644 --- a/pkg/model/api/v1/datasource/http/http.go +++ b/pkg/model/api/v1/datasource/http/http.go @@ -270,7 +270,7 @@ const ( httpProxySpec = "spec" ) -func CheckAndValidate(pluginSpec interface{}) (*Config, error) { +func ValidateAndExtract(pluginSpec interface{}) (*Config, error) { finder := &configFinder{} finder.find(reflect.ValueOf(pluginSpec)) return finder.config, finder.err