From f7dd73b0f7e68c4352e198c63a8ed64b79cb644f Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Fri, 16 Jun 2023 10:44:22 -0300 Subject: [PATCH] feat(unit-testing): add a mock for the RequestBouncer EE-5610 (#9089) --- api/http/handler/auth/handler.go | 2 +- api/http/handler/backup/backup_test.go | 21 ++++++++++-- api/http/handler/backup/handler.go | 7 ++-- api/http/handler/backup/restore_test.go | 34 ++++++++++++++++--- api/http/handler/customtemplates/handler.go | 2 +- api/http/handler/docker/containers/handler.go | 4 +-- api/http/handler/docker/handler.go | 4 +-- api/http/handler/edgegroups/handler.go | 2 +- api/http/handler/edgejobs/handler.go | 2 +- api/http/handler/edgestacks/handler.go | 4 +-- api/http/handler/edgetemplates/handler.go | 4 +-- api/http/handler/endpointedge/handler.go | 4 +-- api/http/handler/endpointgroups/handler.go | 2 +- api/http/handler/endpointproxy/handler.go | 4 +-- .../handler/endpoints/endpoint_delete_test.go | 30 +++------------- api/http/handler/endpoints/handler.go | 16 ++------- api/http/handler/gitops/handler.go | 2 +- api/http/handler/helm/handler.go | 10 ++---- .../handler/hostmanagement/fdo/handler.go | 2 +- .../handler/hostmanagement/openamt/handler.go | 2 +- api/http/handler/kubernetes/handler.go | 2 +- api/http/handler/ldap/handler.go | 2 +- api/http/handler/motd/handler.go | 2 +- api/http/handler/registries/handler.go | 6 ++-- api/http/handler/resourcecontrols/handler.go | 2 +- api/http/handler/roles/handler.go | 2 +- api/http/handler/settings/handler.go | 2 +- api/http/handler/ssl/handler.go | 2 +- api/http/handler/stacks/handler.go | 4 +-- .../handler/stacks/webhook_invoke_test.go | 7 ++-- api/http/handler/system/handler.go | 2 +- api/http/handler/tags/handler.go | 2 +- api/http/handler/tags/tag_delete_test.go | 20 ++--------- api/http/handler/teammemberships/handler.go | 2 +- api/http/handler/teams/handler.go | 2 +- api/http/handler/templates/handler.go | 2 +- api/http/handler/upload/handler.go | 2 +- api/http/handler/users/handler.go | 4 +-- api/http/handler/webhooks/handler.go | 7 ++-- api/http/handler/websocket/handler.go | 4 +-- api/http/security/bouncer.go | 17 +++++++++- api/internal/testhelpers/request_bouncer.go | 20 +++++++++-- 42 files changed, 147 insertions(+), 126 deletions(-) diff --git a/api/http/handler/auth/handler.go b/api/http/handler/auth/handler.go index 259941298..60c3fc259 100644 --- a/api/http/handler/auth/handler.go +++ b/api/http/handler/auth/handler.go @@ -27,7 +27,7 @@ type Handler struct { } // NewHandler creates a handler to manage authentication operations. -func NewHandler(bouncer *security.RequestBouncer, rateLimiter *security.RateLimiter, passwordStrengthChecker security.PasswordStrengthChecker) *Handler { +func NewHandler(bouncer security.BouncerService, rateLimiter *security.RateLimiter, passwordStrengthChecker security.PasswordStrengthChecker) *Handler { h := &Handler{ Router: mux.NewRouter(), passwordStrengthChecker: passwordStrengthChecker, diff --git a/api/http/handler/backup/backup_test.go b/api/http/handler/backup/backup_test.go index 23954b9c9..b5ff5df88 100644 --- a/api/http/handler/backup/backup_test.go +++ b/api/http/handler/backup/backup_test.go @@ -18,7 +18,8 @@ import ( "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/demo" "github.com/portainer/portainer/api/http/offlinegate" - i "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/stretchr/testify/assert" ) @@ -48,7 +49,14 @@ func Test_backupHandlerWithoutPassword_shouldCreateATarballArchive(t *testing.T) gate := offlinegate.NewOfflineGate() adminMonitor := adminmonitor.New(time.Hour, nil, context.Background()) - handlerErr := NewHandler(nil, i.NewDatastore(), gate, "./test_assets/handler_test", func() {}, adminMonitor, &demo.Service{}).backup(w, r) + handlerErr := NewHandler( + testhelpers.NewTestRequestBouncer(), + testhelpers.NewDatastore(), + gate, + "./test_assets/handler_test", + func() {}, + adminMonitor, + &demo.Service{}).backup(w, r) assert.Nil(t, handlerErr, "Handler should not fail") response := w.Result() @@ -85,7 +93,14 @@ func Test_backupHandlerWithPassword_shouldCreateEncryptedATarballArchive(t *test gate := offlinegate.NewOfflineGate() adminMonitor := adminmonitor.New(time.Hour, nil, nil) - handlerErr := NewHandler(nil, i.NewDatastore(), gate, "./test_assets/handler_test", func() {}, adminMonitor, &demo.Service{}).backup(w, r) + handlerErr := NewHandler( + testhelpers.NewTestRequestBouncer(), + testhelpers.NewDatastore(), + gate, + "./test_assets/handler_test", + func() {}, + adminMonitor, + &demo.Service{}).backup(w, r) assert.Nil(t, handlerErr, "Handler should not fail") response := w.Result() diff --git a/api/http/handler/backup/handler.go b/api/http/handler/backup/handler.go index a8782fa64..102427342 100644 --- a/api/http/handler/backup/handler.go +++ b/api/http/handler/backup/handler.go @@ -4,7 +4,6 @@ import ( "context" "net/http" - "github.com/gorilla/mux" httperror "github.com/portainer/libhttp/error" "github.com/portainer/portainer/api/adminmonitor" "github.com/portainer/portainer/api/dataservices" @@ -12,12 +11,14 @@ import ( "github.com/portainer/portainer/api/http/middlewares" "github.com/portainer/portainer/api/http/offlinegate" "github.com/portainer/portainer/api/http/security" + + "github.com/gorilla/mux" ) // Handler is an http handler responsible for backup and restore portainer state type Handler struct { *mux.Router - bouncer *security.RequestBouncer + bouncer security.BouncerService dataStore dataservices.DataStore gate *offlinegate.OfflineGate filestorePath string @@ -27,7 +28,7 @@ type Handler struct { // NewHandler creates an new instance of backup handler func NewHandler( - bouncer *security.RequestBouncer, + bouncer security.BouncerService, dataStore dataservices.DataStore, gate *offlinegate.OfflineGate, filestorePath string, diff --git a/api/http/handler/backup/restore_test.go b/api/http/handler/backup/restore_test.go index 087179fa3..aacbe029b 100644 --- a/api/http/handler/backup/restore_test.go +++ b/api/http/handler/backup/restore_test.go @@ -16,7 +16,8 @@ import ( "github.com/portainer/portainer/api/adminmonitor" "github.com/portainer/portainer/api/demo" "github.com/portainer/portainer/api/http/offlinegate" - i "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/stretchr/testify/assert" ) @@ -49,10 +50,21 @@ func Test_restoreArchive_usingCombinationOfPasswords(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - datastore := i.NewDatastore(i.WithUsers([]portainer.User{}), i.WithEdgeJobs([]portainer.EdgeJob{})) + datastore := testhelpers.NewDatastore( + testhelpers.WithUsers([]portainer.User{}), + testhelpers.WithEdgeJobs([]portainer.EdgeJob{}), + ) adminMonitor := adminmonitor.New(time.Hour, datastore, context.Background()) - h := NewHandler(nil, datastore, offlinegate.NewOfflineGate(), "./test_assets/handler_test", func() {}, adminMonitor, &demo.Service{}) + h := NewHandler( + testhelpers.NewTestRequestBouncer(), + datastore, + offlinegate.NewOfflineGate(), + "./test_assets/handler_test", + func() {}, + adminMonitor, + &demo.Service{}, + ) //backup archive := backup(t, h, test.backupPassword) @@ -72,10 +84,20 @@ func Test_restoreArchive_shouldFailIfSystemWasAlreadyInitialized(t *testing.T) { admin := portainer.User{ Role: portainer.AdministratorRole, } - datastore := i.NewDatastore(i.WithUsers([]portainer.User{admin}), i.WithEdgeJobs([]portainer.EdgeJob{})) + datastore := testhelpers.NewDatastore( + testhelpers.WithUsers([]portainer.User{admin}), + testhelpers.WithEdgeJobs([]portainer.EdgeJob{}), + ) adminMonitor := adminmonitor.New(time.Hour, datastore, context.Background()) - h := NewHandler(nil, datastore, offlinegate.NewOfflineGate(), "./test_assets/handler_test", func() {}, adminMonitor, &demo.Service{}) + h := NewHandler(testhelpers.NewTestRequestBouncer(), + datastore, + offlinegate.NewOfflineGate(), + "./test_assets/handler_test", + func() {}, + adminMonitor, + &demo.Service{}, + ) //backup archive := backup(t, h, "password") @@ -106,11 +128,13 @@ func backup(t *testing.T, h *Handler, password string) []byte { func prepareMultipartRequest(password string, file []byte) (*http.Request, error) { var body bytes.Buffer + w := multipart.NewWriter(&body) err := w.WriteField("password", password) if err != nil { return nil, err } + fw, err := w.CreateFormFile("file", "filename") if err != nil { return nil, err diff --git a/api/http/handler/customtemplates/handler.go b/api/http/handler/customtemplates/handler.go index 17589abf1..875f4110d 100644 --- a/api/http/handler/customtemplates/handler.go +++ b/api/http/handler/customtemplates/handler.go @@ -21,7 +21,7 @@ type Handler struct { } // NewHandler creates a handler to manage environment(endpoint) group operations. -func NewHandler(bouncer *security.RequestBouncer, dataStore dataservices.DataStore, fileService portainer.FileService, gitService portainer.GitService) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, fileService portainer.FileService, gitService portainer.GitService) *Handler { h := &Handler{ Router: mux.NewRouter(), DataStore: dataStore, diff --git a/api/http/handler/docker/containers/handler.go b/api/http/handler/docker/containers/handler.go index 26f03b108..21019e3bb 100644 --- a/api/http/handler/docker/containers/handler.go +++ b/api/http/handler/docker/containers/handler.go @@ -16,11 +16,11 @@ type Handler struct { dockerClientFactory *dockerclient.ClientFactory dataStore dataservices.DataStore containerService *docker.ContainerService - bouncer *security.RequestBouncer + bouncer security.BouncerService } // NewHandler creates a handler to process non-proxied requests to docker APIs directly. -func NewHandler(routePrefix string, bouncer *security.RequestBouncer, dataStore dataservices.DataStore, dockerClientFactory *dockerclient.ClientFactory, containerService *docker.ContainerService) *Handler { +func NewHandler(routePrefix string, bouncer security.BouncerService, dataStore dataservices.DataStore, dockerClientFactory *dockerclient.ClientFactory, containerService *docker.ContainerService) *Handler { h := &Handler{ Router: mux.NewRouter(), dataStore: dataStore, diff --git a/api/http/handler/docker/handler.go b/api/http/handler/docker/handler.go index 28eae13ee..b0b520bb2 100644 --- a/api/http/handler/docker/handler.go +++ b/api/http/handler/docker/handler.go @@ -20,7 +20,7 @@ import ( // Handler is the HTTP handler which will natively deal with to external environments(endpoints). type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService dataStore dataservices.DataStore dockerClientFactory *dockerclient.ClientFactory authorizationService *authorization.Service @@ -28,7 +28,7 @@ type Handler struct { } // NewHandler creates a handler to process non-proxied requests to docker APIs directly. -func NewHandler(bouncer *security.RequestBouncer, authorizationService *authorization.Service, dataStore dataservices.DataStore, dockerClientFactory *dockerclient.ClientFactory, containerService *docker.ContainerService) *Handler { +func NewHandler(bouncer security.BouncerService, authorizationService *authorization.Service, dataStore dataservices.DataStore, dockerClientFactory *dockerclient.ClientFactory, containerService *docker.ContainerService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/edgegroups/handler.go b/api/http/handler/edgegroups/handler.go index 8d3870249..806efb81a 100644 --- a/api/http/handler/edgegroups/handler.go +++ b/api/http/handler/edgegroups/handler.go @@ -21,7 +21,7 @@ type Handler struct { } // NewHandler creates a handler to manage environment(endpoint) group operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/edgejobs/handler.go b/api/http/handler/edgejobs/handler.go index b4f925bc0..9e45c6755 100644 --- a/api/http/handler/edgejobs/handler.go +++ b/api/http/handler/edgejobs/handler.go @@ -22,7 +22,7 @@ type Handler struct { } // NewHandler creates a handler to manage Edge job operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/edgestacks/handler.go b/api/http/handler/edgestacks/handler.go index b504794bc..8d7810072 100644 --- a/api/http/handler/edgestacks/handler.go +++ b/api/http/handler/edgestacks/handler.go @@ -17,7 +17,7 @@ import ( // Handler is the HTTP handler used to handle environment(endpoint) group operations. type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService DataStore dataservices.DataStore FileService portainer.FileService GitService portainer.GitService @@ -28,7 +28,7 @@ type Handler struct { const contextKey = "edgeStack_item" // NewHandler creates a handler to manage environment(endpoint) group operations. -func NewHandler(bouncer *security.RequestBouncer, dataStore dataservices.DataStore, edgeStacksService *edgestackservice.Service) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, edgeStacksService *edgestackservice.Service) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/edgetemplates/handler.go b/api/http/handler/edgetemplates/handler.go index a5074625a..fe37124ba 100644 --- a/api/http/handler/edgetemplates/handler.go +++ b/api/http/handler/edgetemplates/handler.go @@ -13,12 +13,12 @@ import ( // Handler is the HTTP handler used to handle edge environment(endpoint) operations. type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService DataStore dataservices.DataStore } // NewHandler creates a handler to manage environment(endpoint) operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/endpointedge/handler.go b/api/http/handler/endpointedge/handler.go index 35e5d5cd9..8676ccda1 100644 --- a/api/http/handler/endpointedge/handler.go +++ b/api/http/handler/endpointedge/handler.go @@ -15,14 +15,14 @@ import ( // Handler is the HTTP handler used to handle edge environment(endpoint) operations. type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService DataStore dataservices.DataStore FileService portainer.FileService ReverseTunnelService portainer.ReverseTunnelService } // NewHandler creates a handler to manage environment(endpoint) operations. -func NewHandler(bouncer *security.RequestBouncer, dataStore dataservices.DataStore, fileService portainer.FileService, reverseTunnelService portainer.ReverseTunnelService) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, fileService portainer.FileService, reverseTunnelService portainer.ReverseTunnelService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/endpointgroups/handler.go b/api/http/handler/endpointgroups/handler.go index e8545eaaf..90f7a3278 100644 --- a/api/http/handler/endpointgroups/handler.go +++ b/api/http/handler/endpointgroups/handler.go @@ -19,7 +19,7 @@ type Handler struct { } // NewHandler creates a handler to manage environment(endpoint) group operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/endpointproxy/handler.go b/api/http/handler/endpointproxy/handler.go index f9015a06a..90ec22b55 100644 --- a/api/http/handler/endpointproxy/handler.go +++ b/api/http/handler/endpointproxy/handler.go @@ -13,13 +13,13 @@ import ( type Handler struct { *mux.Router DataStore dataservices.DataStore - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService ProxyManager *proxy.Manager ReverseTunnelService portainer.ReverseTunnelService } // NewHandler creates a handler to proxy requests to external APIs. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/endpoints/endpoint_delete_test.go b/api/http/handler/endpoints/endpoint_delete_test.go index 93052c371..96631d1a4 100644 --- a/api/http/handler/endpoints/endpoint_delete_test.go +++ b/api/http/handler/endpoints/endpoint_delete_test.go @@ -8,12 +8,10 @@ import ( "testing" portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/apikey" "github.com/portainer/portainer/api/datastore" "github.com/portainer/portainer/api/demo" "github.com/portainer/portainer/api/http/proxy" - "github.com/portainer/portainer/api/http/security" - "github.com/portainer/portainer/api/jwt" + "github.com/portainer/portainer/api/internal/testhelpers" ) func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { @@ -21,26 +19,7 @@ func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { _, store := datastore.MustNewTestStore(t, true, false) - user := &portainer.User{ID: 2, Username: "admin", Role: portainer.AdministratorRole} - err := store.User().Create(user) - if err != nil { - t.Fatal("could not create admin user:", err) - } - - jwtService, err := jwt.NewService("1h", store) - if err != nil { - t.Fatal("could not initialize the JWT service:", err) - } - - apiKeyService := apikey.NewAPIKeyService(store.APIKeyRepository(), store.User()) - rawAPIKey, _, err := apiKeyService.GenerateApiKey(*user, "test") - if err != nil { - t.Fatal("could not generate API key:", err) - } - - bouncer := security.NewRequestBouncer(store, jwtService, apiKeyService) - - handler := NewHandler(bouncer, demo.NewService()) + handler := NewHandler(testhelpers.NewTestRequestBouncer(), demo.NewService()) handler.DataStore = store handler.ProxyManager = proxy.NewManager(nil, nil, nil, nil, nil, nil, nil) @@ -51,7 +30,7 @@ func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { for i := 0; i < endpointsCount; i++ { endpointID := portainer.EndpointID(i) + 1 - err = store.Endpoint().Create(&portainer.Endpoint{ + err := store.Endpoint().Create(&portainer.Endpoint{ ID: endpointID, Name: "env-" + strconv.Itoa(int(endpointID)), Type: portainer.EdgeAgentOnDockerEnvironment, @@ -63,7 +42,7 @@ func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { endpointIDs = append(endpointIDs, endpointID) } - err = store.EdgeGroup().Create(&portainer.EdgeGroup{ + err := store.EdgeGroup().Create(&portainer.EdgeGroup{ ID: 1, Name: "edgegroup-1", Endpoints: endpointIDs, @@ -86,7 +65,6 @@ func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { t.Fail() return } - req.Header.Add("X-Api-Key", rawAPIKey) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) diff --git a/api/http/handler/endpoints/handler.go b/api/http/handler/endpoints/handler.go index 00c6cbaa7..5f7ead6c0 100644 --- a/api/http/handler/endpoints/handler.go +++ b/api/http/handler/endpoints/handler.go @@ -6,6 +6,7 @@ import ( "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/demo" "github.com/portainer/portainer/api/http/proxy" + "github.com/portainer/portainer/api/http/security" "github.com/portainer/portainer/api/internal/authorization" "github.com/portainer/portainer/api/kubernetes/cli" @@ -21,21 +22,10 @@ func hideFields(endpoint *portainer.Endpoint) { } } -// This requestBouncer exists because security.RequestBounder is a type and not an interface. -// Therefore we can not swit it out with a dummy bouncer for go tests. This interface works around it -type requestBouncer interface { - AuthenticatedAccess(h http.Handler) http.Handler - AdminAccess(h http.Handler) http.Handler - RestrictedAccess(h http.Handler) http.Handler - PublicAccess(h http.Handler) http.Handler - AuthorizedEndpointOperation(r *http.Request, endpoint *portainer.Endpoint) error - AuthorizedEdgeEndpointOperation(r *http.Request, endpoint *portainer.Endpoint) error -} - // Handler is the HTTP handler used to handle environment(endpoint) operations. type Handler struct { *mux.Router - requestBouncer requestBouncer + requestBouncer security.BouncerService demoService *demo.Service DataStore dataservices.DataStore FileService portainer.FileService @@ -50,7 +40,7 @@ type Handler struct { } // NewHandler creates a handler to manage environment(endpoint) operations. -func NewHandler(bouncer requestBouncer, demoService *demo.Service) *Handler { +func NewHandler(bouncer security.BouncerService, demoService *demo.Service) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/gitops/handler.go b/api/http/handler/gitops/handler.go index 7b6693201..cfe96e7a7 100644 --- a/api/http/handler/gitops/handler.go +++ b/api/http/handler/gitops/handler.go @@ -18,7 +18,7 @@ type Handler struct { fileService portainer.FileService } -func NewHandler(bouncer *security.RequestBouncer, dataStore dataservices.DataStore, gitService portainer.GitService, fileService portainer.FileService) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, gitService portainer.GitService, fileService portainer.FileService) *Handler { h := &Handler{ Router: mux.NewRouter(), dataStore: dataStore, diff --git a/api/http/handler/helm/handler.go b/api/http/handler/helm/handler.go index a55345cc2..de71861e0 100644 --- a/api/http/handler/helm/handler.go +++ b/api/http/handler/helm/handler.go @@ -14,14 +14,10 @@ import ( "github.com/portainer/portainer/pkg/libhelm/options" ) -type requestBouncer interface { - AuthenticatedAccess(h http.Handler) http.Handler -} - // Handler is the HTTP handler used to handle environment(endpoint) group operations. type Handler struct { *mux.Router - requestBouncer requestBouncer + requestBouncer security.BouncerService dataStore dataservices.DataStore jwtService dataservices.JWTService kubeClusterAccessService kubernetes.KubeClusterAccessService @@ -30,7 +26,7 @@ type Handler struct { } // NewHandler creates a handler to manage endpoint group operations. -func NewHandler(bouncer requestBouncer, dataStore dataservices.DataStore, jwtService dataservices.JWTService, kubernetesDeployer portainer.KubernetesDeployer, helmPackageManager libhelm.HelmPackageManager, kubeClusterAccessService kubernetes.KubeClusterAccessService) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, jwtService dataservices.JWTService, kubernetesDeployer portainer.KubernetesDeployer, helmPackageManager libhelm.HelmPackageManager, kubeClusterAccessService kubernetes.KubeClusterAccessService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, @@ -64,7 +60,7 @@ func NewHandler(bouncer requestBouncer, dataStore dataservices.DataStore, jwtSer } // NewTemplateHandler creates a template handler to manage environment(endpoint) group operations. -func NewTemplateHandler(bouncer requestBouncer, helmPackageManager libhelm.HelmPackageManager) *Handler { +func NewTemplateHandler(bouncer security.BouncerService, helmPackageManager libhelm.HelmPackageManager) *Handler { h := &Handler{ Router: mux.NewRouter(), helmPackageManager: helmPackageManager, diff --git a/api/http/handler/hostmanagement/fdo/handler.go b/api/http/handler/hostmanagement/fdo/handler.go index 3c0afb378..bc8287601 100644 --- a/api/http/handler/hostmanagement/fdo/handler.go +++ b/api/http/handler/hostmanagement/fdo/handler.go @@ -17,7 +17,7 @@ type Handler struct { FileService portainer.FileService } -func NewHandler(bouncer *security.RequestBouncer, dataStore dataservices.DataStore, fileService portainer.FileService) *Handler { +func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStore, fileService portainer.FileService) *Handler { h := &Handler{ Router: mux.NewRouter(), DataStore: dataStore, diff --git a/api/http/handler/hostmanagement/openamt/handler.go b/api/http/handler/hostmanagement/openamt/handler.go index aee3385a7..50597d26c 100644 --- a/api/http/handler/hostmanagement/openamt/handler.go +++ b/api/http/handler/hostmanagement/openamt/handler.go @@ -21,7 +21,7 @@ type Handler struct { } // NewHandler returns a new Handler -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/kubernetes/handler.go b/api/http/handler/kubernetes/handler.go index d634da743..b81add188 100644 --- a/api/http/handler/kubernetes/handler.go +++ b/api/http/handler/kubernetes/handler.go @@ -31,7 +31,7 @@ type Handler struct { } // NewHandler creates a handler to process pre-proxied requests to external APIs. -func NewHandler(bouncer *security.RequestBouncer, authorizationService *authorization.Service, dataStore dataservices.DataStore, jwtService dataservices.JWTService, kubeClusterAccessService kubernetes.KubeClusterAccessService, kubernetesClientFactory *cli.ClientFactory, kubernetesClient portainer.KubeClient) *Handler { +func NewHandler(bouncer security.BouncerService, authorizationService *authorization.Service, dataStore dataservices.DataStore, jwtService dataservices.JWTService, kubeClusterAccessService kubernetes.KubeClusterAccessService, kubernetesClientFactory *cli.ClientFactory, kubernetesClient portainer.KubeClient) *Handler { h := &Handler{ Router: mux.NewRouter(), authorizationService: authorizationService, diff --git a/api/http/handler/ldap/handler.go b/api/http/handler/ldap/handler.go index 8dea082cf..7b1aadd31 100644 --- a/api/http/handler/ldap/handler.go +++ b/api/http/handler/ldap/handler.go @@ -20,7 +20,7 @@ type Handler struct { } // NewHandler returns a new Handler -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/motd/handler.go b/api/http/handler/motd/handler.go index f7aa79e84..1b5616cba 100644 --- a/api/http/handler/motd/handler.go +++ b/api/http/handler/motd/handler.go @@ -13,7 +13,7 @@ type Handler struct { } // NewHandler returns a new Handler -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/registries/handler.go b/api/http/handler/registries/handler.go index 10760c34a..37a9cfff9 100644 --- a/api/http/handler/registries/handler.go +++ b/api/http/handler/registries/handler.go @@ -24,7 +24,7 @@ func hideFields(registry *portainer.Registry, hideAccesses bool) { // Handler is the HTTP handler used to handle registry operations. type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService DataStore dataservices.DataStore FileService portainer.FileService ProxyManager *proxy.Manager @@ -32,14 +32,14 @@ type Handler struct { } // NewHandler creates a handler to manage registry operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := newHandler(bouncer) h.initRouter(bouncer) return h } -func newHandler(bouncer *security.RequestBouncer) *Handler { +func newHandler(bouncer security.BouncerService) *Handler { return &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/resourcecontrols/handler.go b/api/http/handler/resourcecontrols/handler.go index 768d9bc8b..215c7b687 100644 --- a/api/http/handler/resourcecontrols/handler.go +++ b/api/http/handler/resourcecontrols/handler.go @@ -16,7 +16,7 @@ type Handler struct { } // NewHandler creates a handler to manage resource control operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/roles/handler.go b/api/http/handler/roles/handler.go index 251cad39e..a8a48fff1 100644 --- a/api/http/handler/roles/handler.go +++ b/api/http/handler/roles/handler.go @@ -16,7 +16,7 @@ type Handler struct { } // NewHandler creates a handler to manage role operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/settings/handler.go b/api/http/handler/settings/handler.go index 36067327b..2e75e4f03 100644 --- a/api/http/handler/settings/handler.go +++ b/api/http/handler/settings/handler.go @@ -29,7 +29,7 @@ type Handler struct { } // NewHandler creates a handler to manage settings operations. -func NewHandler(bouncer *security.RequestBouncer, demoService *demo.Service) *Handler { +func NewHandler(bouncer security.BouncerService, demoService *demo.Service) *Handler { h := &Handler{ Router: mux.NewRouter(), demoService: demoService, diff --git a/api/http/handler/ssl/handler.go b/api/http/handler/ssl/handler.go index 8a82f4995..0226b1963 100644 --- a/api/http/handler/ssl/handler.go +++ b/api/http/handler/ssl/handler.go @@ -16,7 +16,7 @@ type Handler struct { } // NewHandler returns a new Handler -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/stacks/handler.go b/api/http/handler/stacks/handler.go index cffb05b84..b0afb010d 100644 --- a/api/http/handler/stacks/handler.go +++ b/api/http/handler/stacks/handler.go @@ -27,7 +27,7 @@ import ( type Handler struct { stackCreationMutex *sync.Mutex stackDeletionMutex *sync.Mutex - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService *mux.Router DataStore dataservices.DataStore DockerClientFactory *dockerclient.ClientFactory @@ -48,7 +48,7 @@ func stackExistsError(name string) *httperror.HandlerError { } // NewHandler creates a handler to manage stack operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), stackCreationMutex: &sync.Mutex{}, diff --git a/api/http/handler/stacks/webhook_invoke_test.go b/api/http/handler/stacks/webhook_invoke_test.go index 7f50a2195..1f1404da6 100644 --- a/api/http/handler/stacks/webhook_invoke_test.go +++ b/api/http/handler/stacks/webhook_invoke_test.go @@ -5,10 +5,11 @@ import ( "net/http/httptest" "testing" + portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/datastore" + "github.com/portainer/portainer/api/internal/testhelpers" "github.com/gofrs/uuid" - portainer "github.com/portainer/portainer/api" "github.com/stretchr/testify/assert" ) @@ -23,7 +24,7 @@ func TestHandler_webhookInvoke(t *testing.T) { }, }) - h := NewHandler(nil) + h := NewHandler(testhelpers.NewTestRequestBouncer()) h.DataStore = store t.Run("invalid uuid results in http.StatusBadRequest", func(t *testing.T) { @@ -32,12 +33,14 @@ func TestHandler_webhookInvoke(t *testing.T) { h.Router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) }) + t.Run("registered webhook ID in http.StatusNoContent", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest(webhookID) h.Router.ServeHTTP(w, req) assert.Equal(t, http.StatusNoContent, w.Code) }) + t.Run("unregistered webhook ID in http.StatusNotFound", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest(newGuidString(t)) diff --git a/api/http/handler/system/handler.go b/api/http/handler/system/handler.go index 29cbb15ee..5b1d9decf 100644 --- a/api/http/handler/system/handler.go +++ b/api/http/handler/system/handler.go @@ -22,7 +22,7 @@ type Handler struct { } // NewHandler creates a handler to manage status operations. -func NewHandler(bouncer *security.RequestBouncer, +func NewHandler(bouncer security.BouncerService, status *portainer.Status, demoService *demo.Service, dataStore dataservices.DataStore, diff --git a/api/http/handler/tags/handler.go b/api/http/handler/tags/handler.go index d72b7b0d3..e1b3071dc 100644 --- a/api/http/handler/tags/handler.go +++ b/api/http/handler/tags/handler.go @@ -19,7 +19,7 @@ type Handler struct { } // NewHandler creates a handler to manage tag operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/tags/tag_delete_test.go b/api/http/handler/tags/tag_delete_test.go index 0c2c45738..7dea1a87a 100644 --- a/api/http/handler/tags/tag_delete_test.go +++ b/api/http/handler/tags/tag_delete_test.go @@ -8,10 +8,8 @@ import ( "testing" portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/apikey" "github.com/portainer/portainer/api/datastore" - "github.com/portainer/portainer/api/http/security" - "github.com/portainer/portainer/api/jwt" + "github.com/portainer/portainer/api/internal/testhelpers" ) func TestTagDeleteEdgeGroupsConcurrently(t *testing.T) { @@ -25,20 +23,7 @@ func TestTagDeleteEdgeGroupsConcurrently(t *testing.T) { t.Fatal("could not create admin user:", err) } - jwtService, err := jwt.NewService("1h", store) - if err != nil { - t.Fatal("could not initialize the JWT service:", err) - } - - apiKeyService := apikey.NewAPIKeyService(store.APIKeyRepository(), store.User()) - rawAPIKey, _, err := apiKeyService.GenerateApiKey(*user, "test") - if err != nil { - t.Fatal("could not generate API key:", err) - } - - bouncer := security.NewRequestBouncer(store, jwtService, apiKeyService) - - handler := NewHandler(bouncer) + handler := NewHandler(testhelpers.NewTestRequestBouncer()) handler.DataStore = store // Create all the tags and add them to the same edge group @@ -82,7 +67,6 @@ func TestTagDeleteEdgeGroupsConcurrently(t *testing.T) { t.Fail() return } - req.Header.Add("X-Api-Key", rawAPIKey) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) diff --git a/api/http/handler/teammemberships/handler.go b/api/http/handler/teammemberships/handler.go index b8d8c8c59..38d5cbf61 100644 --- a/api/http/handler/teammemberships/handler.go +++ b/api/http/handler/teammemberships/handler.go @@ -17,7 +17,7 @@ type Handler struct { } // NewHandler creates a handler to manage team membership operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/teams/handler.go b/api/http/handler/teams/handler.go index 93d012d0e..45cf156b9 100644 --- a/api/http/handler/teams/handler.go +++ b/api/http/handler/teams/handler.go @@ -16,7 +16,7 @@ type Handler struct { } // NewHandler creates a handler to manage team operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/templates/handler.go b/api/http/handler/templates/handler.go index e688e8ab0..7f5dcd5b1 100644 --- a/api/http/handler/templates/handler.go +++ b/api/http/handler/templates/handler.go @@ -19,7 +19,7 @@ type Handler struct { } // NewHandler returns a new instance of Handler. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/upload/handler.go b/api/http/handler/upload/handler.go index 9c8cca2a4..935a09345 100644 --- a/api/http/handler/upload/handler.go +++ b/api/http/handler/upload/handler.go @@ -17,7 +17,7 @@ type Handler struct { } // NewHandler creates a handler to manage upload operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), } diff --git a/api/http/handler/users/handler.go b/api/http/handler/users/handler.go index d6287ad10..4491fee72 100644 --- a/api/http/handler/users/handler.go +++ b/api/http/handler/users/handler.go @@ -31,7 +31,7 @@ func hideFields(user *portainer.User) { // Handler is the HTTP handler used to handle user operations. type Handler struct { *mux.Router - bouncer *security.RequestBouncer + bouncer security.BouncerService apiKeyService apikey.APIKeyService demoService *demo.Service DataStore dataservices.DataStore @@ -41,7 +41,7 @@ type Handler struct { } // NewHandler creates a handler to manage user operations. -func NewHandler(bouncer *security.RequestBouncer, rateLimiter *security.RateLimiter, apiKeyService apikey.APIKeyService, demoService *demo.Service, passwordStrengthChecker security.PasswordStrengthChecker) *Handler { +func NewHandler(bouncer security.BouncerService, rateLimiter *security.RateLimiter, apiKeyService apikey.APIKeyService, demoService *demo.Service, passwordStrengthChecker security.PasswordStrengthChecker) *Handler { h := &Handler{ Router: mux.NewRouter(), bouncer: bouncer, diff --git a/api/http/handler/webhooks/handler.go b/api/http/handler/webhooks/handler.go index 6df3560bf..0453c08f1 100644 --- a/api/http/handler/webhooks/handler.go +++ b/api/http/handler/webhooks/handler.go @@ -3,10 +3,9 @@ package webhooks import ( "net/http" + httperror "github.com/portainer/libhttp/error" "github.com/portainer/portainer/api/dataservices" dockerclient "github.com/portainer/portainer/api/docker/client" - - httperror "github.com/portainer/libhttp/error" "github.com/portainer/portainer/api/http/security" "github.com/gorilla/mux" @@ -15,13 +14,13 @@ import ( // Handler is the HTTP handler used to handle webhook operations. type Handler struct { *mux.Router - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService DataStore dataservices.DataStore DockerClientFactory *dockerclient.ClientFactory } // NewHandler creates a handler to manage webhooks operations. -func NewHandler(bouncer *security.RequestBouncer) *Handler { +func NewHandler(bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), requestBouncer: bouncer, diff --git a/api/http/handler/websocket/handler.go b/api/http/handler/websocket/handler.go index cfa6b2aaf..cf0262c21 100644 --- a/api/http/handler/websocket/handler.go +++ b/api/http/handler/websocket/handler.go @@ -18,13 +18,13 @@ type Handler struct { SignatureService portainer.DigitalSignatureService ReverseTunnelService portainer.ReverseTunnelService KubernetesClientFactory *cli.ClientFactory - requestBouncer *security.RequestBouncer + requestBouncer security.BouncerService connectionUpgrader websocket.Upgrader kubernetesTokenCacheManager *kubernetes.TokenCacheManager } // NewHandler creates a handler to manage websocket operations. -func NewHandler(kubernetesTokenCacheManager *kubernetes.TokenCacheManager, bouncer *security.RequestBouncer) *Handler { +func NewHandler(kubernetesTokenCacheManager *kubernetes.TokenCacheManager, bouncer security.BouncerService) *Handler { h := &Handler{ Router: mux.NewRouter(), connectionUpgrader: websocket.Upgrader{}, diff --git a/api/http/security/bouncer.go b/api/http/security/bouncer.go index 4a10f16f1..8c61f48f7 100644 --- a/api/http/security/bouncer.go +++ b/api/http/security/bouncer.go @@ -5,15 +5,30 @@ import ( "strings" "time" - "github.com/pkg/errors" httperror "github.com/portainer/libhttp/error" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/apikey" "github.com/portainer/portainer/api/dataservices" httperrors "github.com/portainer/portainer/api/http/errors" + + "github.com/pkg/errors" ) type ( + BouncerService interface { + PublicAccess(http.Handler) http.Handler + AdminAccess(http.Handler) http.Handler + RestrictedAccess(http.Handler) http.Handler + TeamLeaderAccess(http.Handler) http.Handler + AuthenticatedAccess(http.Handler) http.Handler + EdgeComputeOperation(http.Handler) http.Handler + + AuthorizedEndpointOperation(*http.Request, *portainer.Endpoint) error + AuthorizedEdgeEndpointOperation(*http.Request, *portainer.Endpoint) error + TrustedEdgeEnvironmentAccess(dataservices.DataStoreTx, *portainer.Endpoint) error + JWTAuthLookup(*http.Request) *portainer.TokenData + } + // RequestBouncer represents an entity that manages API request accesses RequestBouncer struct { dataStore dataservices.DataStore diff --git a/api/internal/testhelpers/request_bouncer.go b/api/internal/testhelpers/request_bouncer.go index a3926fe80..ea936de07 100644 --- a/api/internal/testhelpers/request_bouncer.go +++ b/api/internal/testhelpers/request_bouncer.go @@ -4,10 +4,10 @@ import ( "net/http" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/dataservices" ) -type testRequestBouncer struct { -} +type testRequestBouncer struct{} // NewTestRequestBouncer creates new mock for requestBouncer func NewTestRequestBouncer() *testRequestBouncer { @@ -30,6 +30,14 @@ func (testRequestBouncer) PublicAccess(h http.Handler) http.Handler { return h } +func (testRequestBouncer) TeamLeaderAccess(h http.Handler) http.Handler { + return h +} + +func (testRequestBouncer) EdgeComputeOperation(h http.Handler) http.Handler { + return h +} + func (testRequestBouncer) AuthorizedEndpointOperation(r *http.Request, endpoint *portainer.Endpoint) error { return nil } @@ -37,3 +45,11 @@ func (testRequestBouncer) AuthorizedEndpointOperation(r *http.Request, endpoint func (testRequestBouncer) AuthorizedEdgeEndpointOperation(r *http.Request, endpoint *portainer.Endpoint) error { return nil } + +func (testRequestBouncer) TrustedEdgeEnvironmentAccess(tx dataservices.DataStoreTx, endpoint *portainer.Endpoint) error { + return nil +} + +func (testRequestBouncer) JWTAuthLookup(r *http.Request) *portainer.TokenData { + return nil +}