Skip to content

Commit

Permalink
feat(unit-testing): add a mock for the RequestBouncer EE-5610 (portai…
Browse files Browse the repository at this point in the history
  • Loading branch information
andres-portainer authored Jun 16, 2023
1 parent 933e764 commit f7dd73b
Show file tree
Hide file tree
Showing 42 changed files with 147 additions and 126 deletions.
2 changes: 1 addition & 1 deletion api/http/handler/auth/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 18 additions & 3 deletions api/http/handler/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
7 changes: 4 additions & 3 deletions api/http/handler/backup/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@ 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"
"github.com/portainer/portainer/api/demo"
"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
Expand All @@ -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,
Expand Down
34 changes: 29 additions & 5 deletions api/http/handler/backup/restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/http/handler/customtemplates/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/docker/containers/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/docker/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ 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
containerService *docker.ContainerService
}

// 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,
Expand Down
2 changes: 1 addition & 1 deletion api/http/handler/edgegroups/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
2 changes: 1 addition & 1 deletion api/http/handler/edgejobs/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/edgestacks/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/edgetemplates/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/endpointedge/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion api/http/handler/endpointgroups/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
4 changes: 2 additions & 2 deletions api/http/handler/endpointproxy/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 4 additions & 26 deletions api/http/handler/endpoints/endpoint_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,39 +8,18 @@ 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) {
const endpointsCount = 100

_, 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)

Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand Down
16 changes: 3 additions & 13 deletions api/http/handler/endpoints/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand All @@ -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,
Expand Down
Loading

0 comments on commit f7dd73b

Please sign in to comment.