Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/perses/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions internal/api/core/middleware/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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))
}

Expand Down
75 changes: 75 additions & 0 deletions internal/api/core/middleware/verification.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
26 changes: 26 additions & 0 deletions internal/api/e2e/datasource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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{}
})
}
2 changes: 2 additions & 0 deletions internal/api/e2e/framework/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions internal/api/impl/v1/dashboard/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/api/shared/toolbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type Parameters struct {

func extractParameters(ctx echo.Context) Parameters {
return Parameters{
Project: getProjectParameter(ctx),
Project: GetProjectParameter(ctx),
Name: getNameParameter(ctx),
}
}
Expand Down
7 changes: 6 additions & 1 deletion internal/api/shared/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/api/shared/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/api/v1/datasource/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/api/v1/datasource/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)

Expand All @@ -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)
}