add a middleware to verify the project exists during a request (#680)

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
perses · Oct 25, 2022 · 035af3a · 035af3a
1 parent 76d3944
commit 035af3a
Show file tree

Hide file tree

Showing 11 changed files with 118 additions and 13 deletions.
diff --git 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(serviceManager.GetProject()))
 
 	// start the application
 	runner.Start()

diff --git 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
@@ -0,0 +1,75 @@
+// 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(svc project.Service) echo.MiddlewareFunc {
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c echo.Context) error {
+			// we don't need to verify if a project exists in case we are in a PUT / GET / DELETE request since if the project doesn't exist, then the dashboard won't exist either.
+			// Also, we avoid an additional query to the DB like that.
+			if c.Request().Method != http.MethodPost {
+				return next(c)
+			}
+			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.
+				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
+						break
+					}
+				}
+			}
+			if len(projectName) > 0 {
+				if _, err := svc.Get(shared.Parameters{Name: 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
@@ -16,10 +16,14 @@
 package e2e
 
 import (
+	"fmt"
+	"net/http"
 	"testing"
 
+	"github.com/gavv/httpexpect/v2"
 	e2eframework "github.com/perses/perses/internal/api/e2e/framework"
 	"github.com/perses/perses/internal/api/shared"
+	"github.com/perses/perses/internal/api/shared/dependency"
 	"github.com/perses/perses/pkg/model/api"
 )
 
@@ -28,3 +32,25 @@ func TestMainScenarioDatasource(t *testing.T) {
 		return e2eframework.NewProject(projectName), e2eframework.NewDatasource(t, projectName, name)
 	})
 }
+
+func TestCreateDatasourceWithEmptyProjectName(t *testing.T) {
+	e2eframework.WithServer(t, func(expect *httpexpect.Expect, manager dependency.PersistenceManager) []api.Entity {
+		entity := e2eframework.NewDatasource(t, "", "myDTS")
+		expect.POST(fmt.Sprintf("%s/%s", shared.APIV1Prefix, shared.PathDatasource)).
+			WithJSON(entity).
+			Expect().
+			Status(http.StatusBadRequest)
+		return []api.Entity{}
+	})
+}
+
+func TestCreateDatasourceWithNonExistingProject(t *testing.T) {
+	e2eframework.WithServer(t, func(expect *httpexpect.Expect, manager dependency.PersistenceManager) []api.Entity {
+		entity := e2eframework.NewDatasource(t, "awesomeProjectThatDoesntExist", "myDTS")
+		expect.POST(fmt.Sprintf("%s/%s", shared.APIV1Prefix, shared.PathDatasource)).
+			WithJSON(entity).
+			Expect().
+			Status(http.StatusBadRequest)
+		return []api.Entity{}
+	})
+}
diff --git a/internal/api/e2e/framework/server.go b/internal/api/e2e/framework/server.go
@@ -26,6 +26,7 @@ import (
 	"github.com/labstack/echo/v4"
 	"github.com/perses/perses/internal/api/config"
 	"github.com/perses/perses/internal/api/core"
+	"github.com/perses/perses/internal/api/core/middleware"
 	"github.com/perses/perses/internal/api/shared/database"
 	"github.com/perses/perses/internal/api/shared/dependency"
 	"github.com/perses/perses/pkg/model/api"
@@ -76,6 +77,7 @@ func CreateServer(t *testing.T) (*httptest.Server, *httpexpect.Expect, dependenc
 			t.Fatal(err)
 		}
 	}
+	handler.Use(middleware.CheckProject(serviceManager.GetProject()))
 	persesAPI := core.NewPersesAPI(serviceManager, false)
 	persesAPI.RegisterRoute(handler)
 	server := httptest.NewServer(handler)

diff --git 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
@@ -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
@@ -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
@@ -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.GetDTSSpec().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
@@ -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

diff --git a/pkg/model/api/v1/datasource/http/http_test.go b/pkg/model/api/v1/datasource/http/http_test.go
@@ -419,7 +419,7 @@ method: POST
 	}
 }
 
-func TestCheckAndValidate(t *testing.T) {
+func TestValidateAndExtract(t *testing.T) {
 	// Check and Validate HTTPProxy contained in a proper struct
 	type aStruct struct {
 		A struct {
@@ -444,7 +444,7 @@ func TestCheckAndValidate(t *testing.T) {
 		}{Kind: "HTTPProxy", Spec: &Config{URL: u}}},
 	}
 
-	c, err := CheckAndValidate(b)
+	c, err := ValidateAndExtract(b)
 	assert.NoError(t, err)
 	assert.Equal(t, &Config{URL: u}, c)
 
@@ -458,7 +458,7 @@ func TestCheckAndValidate(t *testing.T) {
 			},
 		},
 	}
-	c, err = CheckAndValidate(uglyStruct)
+	c, err = ValidateAndExtract(uglyStruct)
 	assert.NoError(t, err)
 	assert.Equal(t, &Config{URL: u}, c)
 }