From 9dcd2231344fedf6b5f8c02a9730b9c89ca99b7a Mon Sep 17 00:00:00 2001 From: Anthony Lapenna Date: Wed, 13 May 2020 15:37:35 +1200 Subject: [PATCH] feat(stacks): prevent external stack removal by a non-administrator user (#3800) * fix(stacks): prevent external stacks removal by non admin * feat(stacks): add RBAC checks for external stack removals Co-authored-by: Maxime Bajeux --- api/http/handler/stacks/stack_delete.go | 52 ++++++++++++++----- .../stacksDatatableController.js | 11 +++- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/api/http/handler/stacks/stack_delete.go b/api/http/handler/stacks/stack_delete.go index 180279217..da7e82751 100644 --- a/api/http/handler/stacks/stack_delete.go +++ b/api/http/handler/stacks/stack_delete.go @@ -9,7 +9,7 @@ import ( httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" "github.com/portainer/libhttp/response" - "github.com/portainer/portainer/api" + portainer "github.com/portainer/portainer/api" ) // DELETE request on /api/stacks/:id?external=&endpointId= @@ -21,9 +21,14 @@ func (handler *Handler) stackDelete(w http.ResponseWriter, r *http.Request) *htt return &httperror.HandlerError{http.StatusBadRequest, "Invalid stack identifier route variable", err} } + securityContext, err := security.RetrieveRestrictedRequestContext(r) + if err != nil { + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve info from request context", err} + } + externalStack, _ := request.RetrieveBooleanQueryParameter(r, "external", true) if externalStack { - return handler.deleteExternalStack(r, w, stackID) + return handler.deleteExternalStack(r, w, stackID, securityContext) } id, err := strconv.Atoi(stackID) @@ -68,11 +73,6 @@ func (handler *Handler) stackDelete(w http.ResponseWriter, r *http.Request) *htt return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve a resource control associated to the stack", err} } - securityContext, err := security.RetrieveRestrictedRequestContext(r) - if err != nil { - return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve info from request context", err} - } - access, err := handler.userCanAccessStack(securityContext, endpoint.ID, resourceControl) if err != nil { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to verify user authorizations to validate stack access", err} @@ -106,7 +106,38 @@ func (handler *Handler) stackDelete(w http.ResponseWriter, r *http.Request) *htt return response.Empty(w) } -func (handler *Handler) deleteExternalStack(r *http.Request, w http.ResponseWriter, stackName string) *httperror.HandlerError { +func (handler *Handler) deleteExternalStack(r *http.Request, w http.ResponseWriter, stackName string, securityContext *security.RestrictedRequestContext) *httperror.HandlerError { + endpointID, err := request.RetrieveNumericQueryParameter(r, "endpointId", false) + if err != nil { + return &httperror.HandlerError{http.StatusBadRequest, "Invalid query parameter: endpointId", err} + } + + user, err := handler.UserService.User(securityContext.UserID) + if err != nil { + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to load user information from the database", err} + } + + rbacExtension, err := handler.ExtensionService.Extension(portainer.RBACExtension) + if err != nil && err != portainer.ErrObjectNotFound { + return &httperror.HandlerError{http.StatusInternalServerError, "Unable to verify if RBAC extension is loaded", err} + } + + endpointResourceAccess := false + _, ok := user.EndpointAuthorizations[portainer.EndpointID(endpointID)][portainer.EndpointResourcesAccess] + if ok { + endpointResourceAccess = true + } + + if rbacExtension != nil { + if !securityContext.IsAdmin && !endpointResourceAccess { + return &httperror.HandlerError{http.StatusUnauthorized, "Permission denied to delete the stack", portainer.ErrUnauthorized} + } + } else { + if !securityContext.IsAdmin { + return &httperror.HandlerError{http.StatusUnauthorized, "Permission denied to delete the stack", portainer.ErrUnauthorized} + } + } + stack, err := handler.StackService.StackByName(stackName) if err != nil && err != portainer.ErrObjectNotFound { return &httperror.HandlerError{http.StatusInternalServerError, "Unable to check for stack existence inside the database", err} @@ -115,11 +146,6 @@ func (handler *Handler) deleteExternalStack(r *http.Request, w http.ResponseWrit return &httperror.HandlerError{http.StatusBadRequest, "A stack with this name exists inside the database. Cannot use external delete method", portainer.ErrStackNotExternal} } - endpointID, err := request.RetrieveNumericQueryParameter(r, "endpointId", false) - if err != nil { - return &httperror.HandlerError{http.StatusBadRequest, "Invalid query parameter: endpointId", err} - } - endpoint, err := handler.EndpointService.Endpoint(portainer.EndpointID(endpointID)) if err == portainer.ErrObjectNotFound { return &httperror.HandlerError{http.StatusNotFound, "Unable to find the endpoint associated to the stack inside the database", err} diff --git a/app/portainer/components/datatables/stacks-datatable/stacksDatatableController.js b/app/portainer/components/datatables/stacks-datatable/stacksDatatableController.js index b2729255d..75e8e2400 100644 --- a/app/portainer/components/datatables/stacks-datatable/stacksDatatableController.js +++ b/app/portainer/components/datatables/stacks-datatable/stacksDatatableController.js @@ -2,17 +2,24 @@ angular.module('portainer.app').controller('StacksDatatableController', [ '$scope', '$controller', 'DatatableService', - function ($scope, $controller, DatatableService) { + 'Authentication', + function ($scope, $controller, DatatableService, Authentication) { angular.extend(this, $controller('GenericDatatableController', { $scope: $scope })); /** * Do not allow external items */ this.allowSelection = function (item) { - return !(item.External && item.Type === 2); + if (item.External && item.Type === 2) { + return false; + } + + return !(item.External && !this.isAdmin && !this.isEndpointAdmin); }; this.$onInit = function () { + this.isAdmin = Authentication.isAdmin(); + this.isEndpointAdmin = Authentication.hasAuthorizations(['EndpointResourcesAccess']); this.setDefaults(); this.prepareTableFromDataset();