From 191f8e17eea9ef2d255a7b6eb505d24581a63245 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Fri, 14 Oct 2022 19:42:31 -0300 Subject: [PATCH] fix(code): remove unused code EE-4431 (#7866) --- api/cmd/portainer/import.go | 28 ------ .../migrator/migrate_dbversion34_test.go | 94 ------------------- api/http/handler/backup/handler.go | 9 -- api/http/handler/customtemplates/handler.go | 18 ---- .../handler/hostmanagement/openamt/amtrpc.go | 17 ---- .../registries/registry_create_test.go | 88 ----------------- .../registries/registry_update_test.go | 88 ----------------- .../resourcecontrol_create.go | 5 +- api/http/handler/webhooks/handler.go | 45 +-------- api/http/proxy/factory/agent/transport.go | 13 +-- 10 files changed, 8 insertions(+), 397 deletions(-) delete mode 100644 api/cmd/portainer/import.go delete mode 100644 api/datastore/migrator/migrate_dbversion34_test.go delete mode 100644 api/http/handler/registries/registry_update_test.go diff --git a/api/cmd/portainer/import.go b/api/cmd/portainer/import.go deleted file mode 100644 index 7ce53f4a8..000000000 --- a/api/cmd/portainer/import.go +++ /dev/null @@ -1,28 +0,0 @@ -package main - -import ( - portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/datastore" - - "github.com/rs/zerolog/log" -) - -func importFromJson(fileService portainer.FileService, store *datastore.Store) { - // EXPERIMENTAL - if used with an incomplete json file, it will fail, as we don't have a way to default the model values - importFile := "/data/import.json" - if exists, _ := fileService.FileExists(importFile); exists { - if err := store.Import(importFile); err != nil { - log.Error().Str("filename", importFile).Err(err).Msg("import failed") - // TODO: should really rollback on failure, but then we have nothing. - } else { - log.Info().Str("filename", importFile).Msg("successfully imported the file to a new portainer database") - } - - // TODO: this is bad - its to ensure that any defaults that were broken in import, or migrations get set back to what we want - // I also suspect that everything from "Init to Init" is potentially a migration - err := store.Init() - if err != nil { - log.Fatal().Err(err).Msg("failed initializing data store") - } - } -} diff --git a/api/datastore/migrator/migrate_dbversion34_test.go b/api/datastore/migrator/migrate_dbversion34_test.go deleted file mode 100644 index 5938a4caa..000000000 --- a/api/datastore/migrator/migrate_dbversion34_test.go +++ /dev/null @@ -1,94 +0,0 @@ -package migrator - -const ( - db35TestFile = "portainer-mig-35.db" - username = "portainer" - password = "password" -) - -// TODO: this is exactly the kind of reaching into the internals of the store we should not do -// func setupDB35Test(t *testing.T) *Migrator { -// is := assert.New(t) -// dbConn, err := bolt.Open(path.Join(t.TempDir(), db35TestFile), 0600, &bolt.Options{Timeout: 1 * time.Second}) -// is.NoError(err, "failed to init testing DB connection") - -// // Create an old style dockerhub authenticated account -// dockerhubService, err := dockerhub.NewService(&database.DbConnection{DB: dbConn}) -// is.NoError(err, "failed to init testing registry service") -// err = dockerhubService.UpdateDockerHub(&portainer.DockerHub{true, username, password}) -// is.NoError(err, "failed to create dockerhub account") - -// registryService, err := registry.NewService(&database.DbConnection{DB: dbConn}) -// is.NoError(err, "failed to init testing registry service") - -// endpointService, err := endpoint.NewService(&database.DbConnection{DB: dbConn}) -// is.NoError(err, "failed to init endpoint service") - -// m := &Migrator{ -// db: dbConn, -// dockerhubService: dockerhubService, -// registryService: registryService, -// endpointService: endpointService, -// } - -// return m -// } - -// // TestUpdateDockerhubToDB32 tests a normal upgrade -// func TestUpdateDockerhubToDB32(t *testing.T) { -// is := assert.New(t) -// m := setupDB35Test(t) -// defer m.db.Close() -// defer os.Remove(db35TestFile) - -// if err := m.updateDockerhubToDB32(); err != nil { -// t.Errorf("failed to update settings: %v", err) -// } - -// // Verify we have a single registry were created -// registries, err := m.registryService.Registries() -// is.NoError(err, "failed to read registries from the RegistryService") -// is.Equal(len(registries), 1, "only one migrated registry expected") -// } - -// // TestUpdateDockerhubToDB32_with_duplicate_migrations tests an upgrade where in earlier versions a broken migration -// // created a large number of duplicate "dockerhub migrated" registry entries. -// func TestUpdateDockerhubToDB32_with_duplicate_migrations(t *testing.T) { -// is := assert.New(t) -// m := setupDB35Test(t) -// defer m.db.Close() -// defer os.Remove(db35TestFile) - -// // Create lots of duplicate entries... -// registry := &portainer.Registry{ -// Type: portainer.DockerHubRegistry, -// Name: "Dockerhub (authenticated - migrated)", -// URL: "docker.io", -// Authentication: true, -// Username: "portainer", -// Password: "password", -// RegistryAccesses: portainer.RegistryAccesses{}, -// } - -// for i := 1; i < 150; i++ { -// err := m.registryService.CreateRegistry(registry) -// assert.NoError(t, err, "create registry failed") -// } - -// // Verify they were created -// registries, err := m.registryService.Registries() -// is.NoError(err, "failed to read registries from the RegistryService") -// is.Condition(func() bool { -// return len(registries) > 1 -// }, "expected multiple duplicate registry entries") - -// // Now run the migrator -// if err := m.updateDockerhubToDB32(); err != nil { -// t.Errorf("failed to update settings: %v", err) -// } - -// // Verify we have a single registry were created -// registries, err = m.registryService.Registries() -// is.NoError(err, "failed to read registries from the RegistryService") -// is.Equal(len(registries), 1, "only one migrated registry expected") -// } diff --git a/api/http/handler/backup/handler.go b/api/http/handler/backup/handler.go index 500a3c374..cd95d71b8 100644 --- a/api/http/handler/backup/handler.go +++ b/api/http/handler/backup/handler.go @@ -6,7 +6,6 @@ import ( "github.com/gorilla/mux" httperror "github.com/portainer/libhttp/error" - portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/adminmonitor" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/demo" @@ -71,11 +70,3 @@ func adminAccess(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } - -func systemWasInitialized(dataStore dataservices.DataStore) (bool, error) { - users, err := dataStore.User().UsersByRole(portainer.AdministratorRole) - if err != nil { - return false, err - } - return len(users) > 0, nil -} diff --git a/api/http/handler/customtemplates/handler.go b/api/http/handler/customtemplates/handler.go index 7bc9b4b3d..54fad7d67 100644 --- a/api/http/handler/customtemplates/handler.go +++ b/api/http/handler/customtemplates/handler.go @@ -8,7 +8,6 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/http/security" - "github.com/portainer/portainer/api/internal/authorization" ) // Handler is the HTTP handler used to handle environment(endpoint) group operations. @@ -42,20 +41,3 @@ func NewHandler(bouncer *security.RequestBouncer) *Handler { func userCanEditTemplate(customTemplate *portainer.CustomTemplate, securityContext *security.RestrictedRequestContext) bool { return securityContext.IsAdmin || customTemplate.CreatedByUserID == securityContext.UserID } - -func userCanAccessTemplate(customTemplate portainer.CustomTemplate, securityContext *security.RestrictedRequestContext, resourceControl *portainer.ResourceControl) bool { - if securityContext.IsAdmin || customTemplate.CreatedByUserID == securityContext.UserID { - return true - } - - userTeamIDs := make([]portainer.TeamID, 0) - for _, membership := range securityContext.UserMemberships { - userTeamIDs = append(userTeamIDs, membership.TeamID) - } - - if resourceControl != nil && authorization.UserCanAccessResource(securityContext.UserID, userTeamIDs, resourceControl) { - return true - } - - return false -} diff --git a/api/http/handler/hostmanagement/openamt/amtrpc.go b/api/http/handler/hostmanagement/openamt/amtrpc.go index 643e5dd65..fe0c9e162 100644 --- a/api/http/handler/hostmanagement/openamt/amtrpc.go +++ b/api/http/handler/hostmanagement/openamt/amtrpc.go @@ -298,20 +298,3 @@ func (handler *Handler) activateDevice(endpoint *portainer.Endpoint, settings po return err } - -func (handler *Handler) deactivateDevice(endpoint *portainer.Endpoint, settings portainer.Settings) error { - ctx := context.TODO() - - config := settings.OpenAMTConfiguration - cmdLine := []string{ - "deactivate", - "-n", - "-v", - "-u", fmt.Sprintf("wss://%s/activate", config.MPSServer), - "-password", config.MPSPassword, - } - - _, err := handler.PullAndRunContainer(ctx, endpoint, rpcGoImageName, rpcGoContainerName, cmdLine) - - return err -} diff --git a/api/http/handler/registries/registry_create_test.go b/api/http/handler/registries/registry_create_test.go index 711a6ef36..515dc6c5e 100644 --- a/api/http/handler/registries/registry_create_test.go +++ b/api/http/handler/registries/registry_create_test.go @@ -1,15 +1,9 @@ package registries import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" "testing" portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/dataservices" - "github.com/portainer/portainer/api/http/security" "github.com/stretchr/testify/assert" ) @@ -49,85 +43,3 @@ func Test_registryCreatePayload_Validate(t *testing.T) { assert.NoError(t, err) }) } - -type testRegistryService struct { - dataservices.RegistryService - createRegistry func(r *portainer.Registry) error - updateRegistry func(ID portainer.RegistryID, r *portainer.Registry) error - getRegistry func(ID portainer.RegistryID) (*portainer.Registry, error) -} - -type testDataStore struct { - dataservices.DataStore - registry *testRegistryService -} - -func (t testDataStore) Registry() dataservices.RegistryService { - return t.registry -} - -func (t testRegistryService) CreateRegistry(r *portainer.Registry) error { - return t.createRegistry(r) -} - -func (t testRegistryService) UpdateRegistry(ID portainer.RegistryID, r *portainer.Registry) error { - return t.updateRegistry(ID, r) -} - -func (t testRegistryService) Registry(ID portainer.RegistryID) (*portainer.Registry, error) { - return t.getRegistry(ID) -} - -func (t testRegistryService) Registries() ([]portainer.Registry, error) { - return nil, nil -} - -func (t testRegistryService) Create(registry *portainer.Registry) error { - return nil -} - -// Not entirely sure what this is intended to test -func deleteTestHandler_registryCreate(t *testing.T) { - payload := registryCreatePayload{ - Name: "Test registry", - Type: portainer.ProGetRegistry, - URL: "http://example.com", - BaseURL: "http://example.com", - Authentication: false, - Username: "username", - Password: "password", - Gitlab: portainer.GitlabRegistryData{}, - } - payloadBytes, err := json.Marshal(payload) - assert.NoError(t, err) - r := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payloadBytes)) - w := httptest.NewRecorder() - - restrictedContext := &security.RestrictedRequestContext{ - IsAdmin: true, - UserID: portainer.UserID(1), - } - - ctx := security.StoreRestrictedRequestContext(r, restrictedContext) - r = r.WithContext(ctx) - - registry := portainer.Registry{} - handler := Handler{} - handler.DataStore = testDataStore{ - registry: &testRegistryService{ - createRegistry: func(r *portainer.Registry) error { - registry = *r - return nil - }, - }, - } - handlerError := handler.registryCreate(w, r) - assert.Nil(t, handlerError) - assert.Equal(t, payload.Name, registry.Name) - assert.Equal(t, payload.Type, registry.Type) - assert.Equal(t, payload.URL, registry.URL) - assert.Equal(t, payload.BaseURL, registry.BaseURL) - assert.Equal(t, payload.Authentication, registry.Authentication) - assert.Equal(t, payload.Username, registry.Username) - assert.Equal(t, payload.Password, registry.Password) -} diff --git a/api/http/handler/registries/registry_update_test.go b/api/http/handler/registries/registry_update_test.go deleted file mode 100644 index 59c28d695..000000000 --- a/api/http/handler/registries/registry_update_test.go +++ /dev/null @@ -1,88 +0,0 @@ -package registries - -import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/http/security" - "github.com/stretchr/testify/assert" -) - -func ps(s string) *string { - return &s -} - -func pb(b bool) *bool { - return &b -} - -type TestBouncer struct{} - -func (t TestBouncer) AdminAccess(h http.Handler) http.Handler { - return h -} - -func (t TestBouncer) AuthenticatedAccess(h http.Handler) http.Handler { - return h -} - -func (t TestBouncer) AuthorizedEndpointOperation(r *http.Request, endpoint *portainer.Endpoint) error { - return nil -} - -// TODO, no i don't know what this is actually intended to test either. -func delete_TestHandler_registryUpdate(t *testing.T) { - payload := registryUpdatePayload{ - Name: ps("Updated test registry"), - URL: ps("http://example.org/feed"), - BaseURL: ps("http://example.org"), - Authentication: pb(true), - Username: ps("username"), - Password: ps("password"), - } - payloadBytes, err := json.Marshal(payload) - assert.NoError(t, err) - registry := portainer.Registry{Type: portainer.ProGetRegistry, ID: 5} - r := httptest.NewRequest(http.MethodPut, "/registries/5", bytes.NewReader(payloadBytes)) - w := httptest.NewRecorder() - - restrictedContext := &security.RestrictedRequestContext{ - IsAdmin: true, - UserID: portainer.UserID(1), - } - - ctx := security.StoreRestrictedRequestContext(r, restrictedContext) - r = r.WithContext(ctx) - - updatedRegistry := portainer.Registry{} - handler := newHandler(nil) - handler.initRouter(TestBouncer{}) - handler.DataStore = testDataStore{ - registry: &testRegistryService{ - getRegistry: func(_ portainer.RegistryID) (*portainer.Registry, error) { - return ®istry, nil - }, - updateRegistry: func(ID portainer.RegistryID, r *portainer.Registry) error { - assert.Equal(t, ID, r.ID) - updatedRegistry = *r - return nil - }, - }, - } - - handler.ServeHTTP(w, r) - assert.Equal(t, http.StatusOK, w.Code) - // Registry type should remain intact - assert.Equal(t, registry.Type, updatedRegistry.Type) - - assert.Equal(t, *payload.Name, updatedRegistry.Name) - assert.Equal(t, *payload.URL, updatedRegistry.URL) - assert.Equal(t, *payload.BaseURL, updatedRegistry.BaseURL) - assert.Equal(t, *payload.Authentication, updatedRegistry.Authentication) - assert.Equal(t, *payload.Username, updatedRegistry.Username) - assert.Equal(t, *payload.Password, updatedRegistry.Password) -} diff --git a/api/http/handler/resourcecontrols/resourcecontrol_create.go b/api/http/handler/resourcecontrols/resourcecontrol_create.go index b3bd5741e..5eeb2e873 100644 --- a/api/http/handler/resourcecontrols/resourcecontrol_create.go +++ b/api/http/handler/resourcecontrols/resourcecontrol_create.go @@ -29,10 +29,7 @@ type resourceControlCreatePayload struct { SubResourceIDs []string `example:"617c5f22bb9b023d6daab7cba43a57576f83492867bc767d1c59416b065e5f08"` } -var ( - errResourceControlAlreadyExists = errors.New("A resource control is already applied on this resource") //http/resourceControl - errInvalidResourceControlType = errors.New("Unsupported resource control type") //http/resourceControl -) +var errResourceControlAlreadyExists = errors.New("A resource control is already applied on this resource") //http/resourceControl func (payload *resourceControlCreatePayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.ResourceID) { diff --git a/api/http/handler/webhooks/handler.go b/api/http/handler/webhooks/handler.go index 45cacfc73..eb0d8c030 100644 --- a/api/http/handler/webhooks/handler.go +++ b/api/http/handler/webhooks/handler.go @@ -1,12 +1,11 @@ package webhooks import ( - "github.com/portainer/portainer/api/dataservices" - "github.com/portainer/portainer/api/internal/authorization" "net/http" + "github.com/portainer/portainer/api/dataservices" + httperror "github.com/portainer/libhttp/error" - portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/docker" "github.com/portainer/portainer/api/http/security" @@ -39,43 +38,3 @@ func NewHandler(bouncer *security.RequestBouncer) *Handler { bouncer.PublicAccess(httperror.LoggerHandler(h.webhookExecute))).Methods(http.MethodPost) return h } - -func (handler *Handler) checkResourceAccess(r *http.Request, resourceID string, resourceControlType portainer.ResourceControlType) *httperror.HandlerError { - securityContext, err := security.RetrieveRestrictedRequestContext(r) - if err != nil { - return httperror.InternalServerError("Unable to retrieve user info from request context", err) - } - // non-admins - rc, err := handler.DataStore.ResourceControl().ResourceControlByResourceIDAndType(resourceID, resourceControlType) - if rc == nil || err != nil { - return httperror.InternalServerError("Unable to retrieve a resource control associated to the resource", err) - } - userTeamIDs := make([]portainer.TeamID, 0) - for _, membership := range securityContext.UserMemberships { - userTeamIDs = append(userTeamIDs, membership.TeamID) - } - canAccess := authorization.UserCanAccessResource(securityContext.UserID, userTeamIDs, rc) - if !canAccess { - return &httperror.HandlerError{StatusCode: http.StatusForbidden, Message: "This operation is disabled for non-admin users and unassigned access users"} - } - return nil -} - -func (handler *Handler) checkAuthorization(r *http.Request, endpoint *portainer.Endpoint, authorizations []portainer.Authorization) (bool, *httperror.HandlerError) { - err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint) - if err != nil { - return false, httperror.Forbidden("Permission denied to access environment", err) - } - - securityContext, err := security.RetrieveRestrictedRequestContext(r) - if err != nil { - return false, httperror.InternalServerError("Unable to retrieve user info from request context", err) - } - - authService := authorization.NewService(handler.DataStore) - isAdminOrAuthorized, err := authService.UserIsAdminOrAuthorized(securityContext.UserID, endpoint.ID, authorizations) - if err != nil { - return false, httperror.InternalServerError("Unable to get user authorizations", err) - } - return isAdminOrAuthorized, nil -} diff --git a/api/http/proxy/factory/agent/transport.go b/api/http/proxy/factory/agent/transport.go index 4250f861d..8abe497eb 100644 --- a/api/http/proxy/factory/agent/transport.go +++ b/api/http/proxy/factory/agent/transport.go @@ -6,14 +6,11 @@ import ( portainer "github.com/portainer/portainer/api" ) -type ( - // Transport is an http.Transport wrapper that adds custom http headers to communicate to an Agent - Transport struct { - httpTransport *http.Transport - signatureService portainer.DigitalSignatureService - endpointIdentifier portainer.EndpointID - } -) +// Transport is an http.Transport wrapper that adds custom http headers to communicate to an Agent +type Transport struct { + httpTransport *http.Transport + signatureService portainer.DigitalSignatureService +} // NewTransport returns a new transport that can be used to send signed requests to a Portainer agent func NewTransport(signatureService portainer.DigitalSignatureService, httpTransport *http.Transport) *Transport {