From 29c05844545bf908717ddaf40b130ba34fbf0e37 Mon Sep 17 00:00:00 2001 From: Anthony Lapenna Date: Tue, 12 May 2020 16:08:01 +1200 Subject: [PATCH] fix(api): update restricted volume browsing operation logic (#3798) * fix(api): prevent a potential panic * fix(api): update restricted volume browsing operation logic --- api/http/proxy/factory/docker/access_control.go | 2 +- api/http/proxy/factory/docker/transport.go | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/api/http/proxy/factory/docker/access_control.go b/api/http/proxy/factory/docker/access_control.go index 83050f79e..2b8e183f9 100644 --- a/api/http/proxy/factory/docker/access_control.go +++ b/api/http/proxy/factory/docker/access_control.go @@ -158,7 +158,7 @@ func (transport *Transport) applyAccessControlOnResource(parameters *resourceOpe return responseutils.RewriteResponse(response, responseObject, http.StatusOK) } - if executor.operationContext.isAdmin || executor.operationContext.endpointResourceAccess || portainer.UserCanAccessResource(executor.operationContext.userID, executor.operationContext.userTeamIDs, resourceControl) { + if executor.operationContext.isAdmin || executor.operationContext.endpointResourceAccess || (resourceControl != nil && portainer.UserCanAccessResource(executor.operationContext.userID, executor.operationContext.userTeamIDs, resourceControl)) { responseObject = decorateObject(responseObject, resourceControl) return responseutils.RewriteResponse(response, responseObject, http.StatusOK) } diff --git a/api/http/proxy/factory/docker/transport.go b/api/http/proxy/factory/docker/transport.go index 219c7f38f..c511948fa 100644 --- a/api/http/proxy/factory/docker/transport.go +++ b/api/http/proxy/factory/docker/transport.go @@ -171,11 +171,13 @@ func (transport *Transport) proxyAgentRequest(r *http.Request) (*http.Response, switch { case strings.HasPrefix(requestPath, "/browse"): + // host file browser request volumeIDParameter, found := r.URL.Query()["volumeID"] if !found || len(volumeIDParameter) < 1 { return transport.administratorOperation(r) } + // volume browser request return transport.restrictedResourceOperation(r, volumeIDParameter[0], portainer.VolumeResourceControl, true) } @@ -443,10 +445,16 @@ func (transport *Transport) restrictedResourceOperation(request *http.Request, r return nil, err } - // Return access denied for all roles except endpoint-administrator - _, userCanBrowse := user.EndpointAuthorizations[transport.endpoint.ID][portainer.OperationDockerAgentBrowseList] - if rbacExtension != nil && !settings.AllowVolumeBrowserForRegularUsers && !userCanBrowse { - return responseutils.WriteAccessDeniedResponse() + if !settings.AllowVolumeBrowserForRegularUsers { + if rbacExtension == nil { + return responseutils.WriteAccessDeniedResponse() + } + + // Return access denied for all roles except endpoint-administrator + _, userCanBrowse := user.EndpointAuthorizations[transport.endpoint.ID][portainer.OperationDockerAgentBrowseList] + if !userCanBrowse { + return responseutils.WriteAccessDeniedResponse() + } } }