From aaec856282ffdced63fddcb4c9881ae2bbd6a214 Mon Sep 17 00:00:00 2001 From: LP B Date: Sat, 10 Aug 2024 11:53:16 +0200 Subject: [PATCH] fix(app/registries): enforce user accesses on registries (#12087) --- api/http/handler/endpoints/endpoint_delete.go | 37 +++-- .../endpoints/endpoint_registries_list.go | 7 +- api/http/handler/registries/handler.go | 73 ++++++++- .../handler/registries/registry_inspect.go | 16 +- api/internal/authorization/authorizations.go | 149 ++++++++++++++++++ app/portainer/__module.js | 44 +----- app/portainer/registry-management/index.js | 50 ++++++ .../views}/create/createRegistry.html | 0 .../views}/create/createRegistry.js | 0 .../views}/create/createRegistryController.js | 0 .../views}/edit/registry.html | 0 .../views}/edit/registry.js | 0 .../views}/edit/registryController.js | 0 .../HomeView/NodesDatatable/columns/name.tsx | 1 + 14 files changed, 299 insertions(+), 78 deletions(-) create mode 100644 app/portainer/registry-management/index.js rename app/portainer/{views/registries => registry-management/views}/create/createRegistry.html (100%) rename app/portainer/{views/registries => registry-management/views}/create/createRegistry.js (100%) rename app/portainer/{views/registries => registry-management/views}/create/createRegistryController.js (100%) rename app/portainer/{views/registries => registry-management/views}/edit/registry.html (100%) rename app/portainer/{views/registries => registry-management/views}/edit/registry.js (100%) rename app/portainer/{views/registries => registry-management/views}/edit/registryController.js (100%) diff --git a/api/http/handler/endpoints/endpoint_delete.go b/api/http/handler/endpoints/endpoint_delete.go index 72b28cbbf..0ae0eb920 100644 --- a/api/http/handler/endpoints/endpoint_delete.go +++ b/api/http/handler/endpoints/endpoint_delete.go @@ -144,19 +144,19 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p } if err := tx.Snapshot().Delete(endpointID); err != nil { - log.Warn().Err(err).Msgf("Unable to remove the snapshot from the database") + log.Warn().Err(err).Msg("Unable to remove the snapshot from the database") } handler.ProxyManager.DeleteEndpointProxy(endpoint.ID) if len(endpoint.UserAccessPolicies) > 0 || len(endpoint.TeamAccessPolicies) > 0 { if err := handler.AuthorizationService.UpdateUsersAuthorizationsTx(tx); err != nil { - log.Warn().Err(err).Msgf("Unable to update user authorizations") + log.Warn().Err(err).Msg("Unable to update user authorizations") } } if err := tx.EndpointRelation().DeleteEndpointRelation(endpoint.ID); err != nil { - log.Warn().Err(err).Msgf("Unable to remove environment relation from the database") + log.Warn().Err(err).Msg("Unable to remove environment relation from the database") } for _, tagID := range endpoint.TagIDs { @@ -168,9 +168,9 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p } if handler.DataStore.IsErrObjectNotFound(err) { - log.Warn().Err(err).Msgf("Unable to find tag inside the database") + log.Warn().Err(err).Msg("Unable to find tag inside the database") } else if err != nil { - log.Warn().Err(err).Msgf("Unable to delete tag relation from the database") + log.Warn().Err(err).Msg("Unable to delete tag relation from the database") } } @@ -185,38 +185,38 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p }) if err := tx.EdgeGroup().Update(edgeGroup.ID, &edgeGroup); err != nil { - log.Warn().Err(err).Msgf("Unable to update edge group") + log.Warn().Err(err).Msg("Unable to update edge group") } } edgeStacks, err := tx.EdgeStack().EdgeStacks() if err != nil { - log.Warn().Err(err).Msgf("Unable to retrieve edge stacks from the database") + log.Warn().Err(err).Msg("Unable to retrieve edge stacks from the database") } for idx := range edgeStacks { edgeStack := &edgeStacks[idx] if _, ok := edgeStack.Status[endpoint.ID]; ok { delete(edgeStack.Status, endpoint.ID) - err = tx.EdgeStack().UpdateEdgeStack(edgeStack.ID, edgeStack) - if err != nil { - log.Warn().Err(err).Msgf("Unable to update edge stack") + + if err := tx.EdgeStack().UpdateEdgeStack(edgeStack.ID, edgeStack); err != nil { + log.Warn().Err(err).Msg("Unable to update edge stack") } } } registries, err := tx.Registry().ReadAll() if err != nil { - log.Warn().Err(err).Msgf("Unable to retrieve registries from the database") + log.Warn().Err(err).Msg("Unable to retrieve registries from the database") } for idx := range registries { registry := ®istries[idx] if _, ok := registry.RegistryAccesses[endpoint.ID]; ok { delete(registry.RegistryAccesses, endpoint.ID) - err = tx.Registry().Update(registry.ID, registry) - if err != nil { - log.Warn().Err(err).Msgf("Unable to update registry accesses") + + if err := tx.Registry().Update(registry.ID, registry); err != nil { + log.Warn().Err(err).Msg("Unable to update registry accesses") } } } @@ -224,7 +224,7 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p if endpointutils.IsEdgeEndpoint(endpoint) { edgeJobs, err := handler.DataStore.EdgeJob().ReadAll() if err != nil { - log.Warn().Err(err).Msgf("Unable to retrieve edge jobs from the database") + log.Warn().Err(err).Msg("Unable to retrieve edge jobs from the database") } for idx := range edgeJobs { @@ -232,9 +232,8 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p if _, ok := edgeJob.Endpoints[endpoint.ID]; ok { delete(edgeJob.Endpoints, endpoint.ID) - err = tx.EdgeJob().Update(edgeJob.ID, edgeJob) - if err != nil { - log.Warn().Err(err).Msgf("Unable to update edge job") + if err := tx.EdgeJob().Update(edgeJob.ID, edgeJob); err != nil { + log.Warn().Err(err).Msg("Unable to update edge job") } } } @@ -242,7 +241,7 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p // delete the pending actions if err := tx.PendingActions().DeleteByEndpointID(endpoint.ID); err != nil { - log.Warn().Err(err).Int("endpointId", int(endpoint.ID)).Msgf("Unable to delete pending actions") + log.Warn().Err(err).Int("endpointId", int(endpoint.ID)).Msg("Unable to delete pending actions") } if err := tx.Endpoint().DeleteEndpoint(endpointID); err != nil { diff --git a/api/http/handler/endpoints/endpoint_registries_list.go b/api/http/handler/endpoints/endpoint_registries_list.go index 806f3e25c..812d3da38 100644 --- a/api/http/handler/endpoints/endpoint_registries_list.go +++ b/api/http/handler/endpoints/endpoint_registries_list.go @@ -3,15 +3,16 @@ package endpoints import ( "net/http" + "github.com/pkg/errors" + 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/endpointutils" + "github.com/portainer/portainer/api/kubernetes" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/portainer/portainer/pkg/libhttp/request" "github.com/portainer/portainer/pkg/libhttp/response" - - "github.com/pkg/errors" ) // @id endpointRegistriesList @@ -123,7 +124,7 @@ func (handler *Handler) isNamespaceAuthorized(endpoint *portainer.Endpoint, name return true, nil } - if namespace == "default" { + if !endpoint.Kubernetes.Configuration.RestrictDefaultNamespace && namespace == kubernetes.DefaultNamespace { return true, nil } diff --git a/api/http/handler/registries/handler.go b/api/http/handler/registries/handler.go index 67274b0a9..7e2d8243a 100644 --- a/api/http/handler/registries/handler.go +++ b/api/http/handler/registries/handler.go @@ -7,12 +7,16 @@ import ( "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/http/proxy" "github.com/portainer/portainer/api/http/security" + "github.com/portainer/portainer/api/internal/endpointutils" + "github.com/portainer/portainer/api/kubernetes" "github.com/portainer/portainer/api/kubernetes/cli" "github.com/portainer/portainer/api/pendingactions" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/portainer/portainer/pkg/libhttp/request" "github.com/gorilla/mux" + + "github.com/pkg/errors" ) func hideFields(registry *portainer.Registry, hideAccesses bool) { @@ -83,29 +87,88 @@ func (handler *Handler) registriesHaveSameURLAndCredentials(r1, r2 *portainer.Re return hasSameUrl && hasSameCredentials && r1.Gitlab.ProjectPath == r2.Gitlab.ProjectPath } -func (handler *Handler) userHasRegistryAccess(r *http.Request) (hasAccess bool, isAdmin bool, err error) { +// this function validates that +// +// 1. user has the appropriate authorizations to perform the request +// +// 2. user has a direct or indirect access to the registry +func (handler *Handler) userHasRegistryAccess(r *http.Request, registry *portainer.Registry) (hasAccess bool, isAdmin bool, err error) { securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { return false, false, err } + user, err := handler.DataStore.User().Read(securityContext.UserID) + if err != nil { + return false, false, err + } + + // Portainer admins always have access to everything if securityContext.IsAdmin { return true, true, nil } - endpointID, err := request.RetrieveNumericQueryParameter(r, "endpointId", false) + // mandatory query param that should become a path param + endpointIdStr, err := request.RetrieveNumericQueryParameter(r, "endpointId", false) if err != nil { return false, false, err } - endpoint, err := handler.DataStore.Endpoint().Endpoint(portainer.EndpointID(endpointID)) + endpointId := portainer.EndpointID(endpointIdStr) + + endpoint, err := handler.DataStore.Endpoint().Endpoint(endpointId) if err != nil { return false, false, err } - if err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint); err != nil { + // validate that the request is allowed for the user (READ/WRITE authorization on request path) + if err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint); errors.Is(err, security.ErrAuthorizationRequired) { + return false, false, nil + } else if err != nil { return false, false, err } - return true, false, nil + memberships, err := handler.DataStore.TeamMembership().TeamMembershipsByUserID(user.ID) + if err != nil { + return false, false, nil + } + + // validate access for kubernetes namespaces (leverage registry.RegistryAccesses[endpointId].Namespaces) + if endpointutils.IsKubernetesEndpoint(endpoint) { + kcl, err := handler.K8sClientFactory.GetKubeClient(endpoint) + if err != nil { + return false, false, errors.Wrap(err, "unable to retrieve kubernetes client to validate registry access") + } + accessPolicies, err := kcl.GetNamespaceAccessPolicies() + if err != nil { + return false, false, errors.Wrap(err, "unable to retrieve environment's namespaces policies to validate registry access") + } + + authorizedNamespaces := registry.RegistryAccesses[endpointId].Namespaces + + for _, namespace := range authorizedNamespaces { + // when the default namespace is authorized to use a registry, all users have the ability to use it + // unless the default namespace is restricted: in this case continue to search for other potential accesses authorizations + if namespace == kubernetes.DefaultNamespace && !endpoint.Kubernetes.Configuration.RestrictDefaultNamespace { + return true, false, nil + } + + namespacePolicy := accessPolicies[namespace] + if security.AuthorizedAccess(user.ID, memberships, namespacePolicy.UserAccessPolicies, namespacePolicy.TeamAccessPolicies) { + return true, false, nil + } + } + return false, false, nil + } + + // validate access for docker environments + // leverage registry.RegistryAccesses[endpointId].UserAccessPolicies (direct access) + // and registry.RegistryAccesses[endpointId].TeamAccessPolicies (indirect access via his teams) + if security.AuthorizedRegistryAccess(registry, user, memberships, endpoint.ID) { + return true, false, nil + } + + // when user has no access via their role, direct grant or indirect grant + // then they don't have access to the registry + return false, false, nil } diff --git a/api/http/handler/registries/registry_inspect.go b/api/http/handler/registries/registry_inspect.go index 21b4ee3d4..a1f0bd9c5 100644 --- a/api/http/handler/registries/registry_inspect.go +++ b/api/http/handler/registries/registry_inspect.go @@ -26,14 +26,6 @@ import ( // @failure 500 "Server error" // @router /registries/{id} [get] func (handler *Handler) registryInspect(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { - hasAccess, isAdmin, err := handler.userHasRegistryAccess(r) - if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) - } - if !hasAccess { - return httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) - } - registryID, err := request.RetrieveNumericRouteVariableValue(r, "id") if err != nil { return httperror.BadRequest("Invalid registry identifier route variable", err) @@ -46,6 +38,14 @@ func (handler *Handler) registryInspect(w http.ResponseWriter, r *http.Request) return httperror.InternalServerError("Unable to find a registry with the specified identifier inside the database", err) } + hasAccess, isAdmin, err := handler.userHasRegistryAccess(r, registry) + if err != nil { + return httperror.InternalServerError("Unable to retrieve info from request context", err) + } + if !hasAccess { + return httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) + } + hideFields(registry, !isAdmin) return response.JSON(w, registry) } diff --git a/api/internal/authorization/authorizations.go b/api/internal/authorization/authorizations.go index 2514dfedf..1e412a382 100644 --- a/api/internal/authorization/authorizations.go +++ b/api/internal/authorization/authorizations.go @@ -430,6 +430,155 @@ func DefaultPortainerAuthorizations() portainer.Authorizations { } } +// RemoveTeamAccessPolicies will remove all existing access policies associated to the specified team +func (service *Service) RemoveTeamAccessPolicies(tx dataservices.DataStoreTx, teamID portainer.TeamID) error { + endpoints, err := tx.Endpoint().Endpoints() + if err != nil { + return err + } + for _, endpoint := range endpoints { + for policyTeamID := range endpoint.TeamAccessPolicies { + if policyTeamID == teamID { + delete(endpoint.TeamAccessPolicies, policyTeamID) + + err := tx.Endpoint().UpdateEndpoint(endpoint.ID, &endpoint) + if err != nil { + return err + } + + break + } + } + } + + endpointGroups, err := tx.EndpointGroup().ReadAll() + if err != nil { + return err + } + + for _, endpointGroup := range endpointGroups { + for policyTeamID := range endpointGroup.TeamAccessPolicies { + if policyTeamID == teamID { + delete(endpointGroup.TeamAccessPolicies, policyTeamID) + + err := tx.EndpointGroup().Update(endpointGroup.ID, &endpointGroup) + if err != nil { + return err + } + + break + } + } + } + + registries, err := tx.Registry().ReadAll() + if err != nil { + return err + } + + // iterate over all environments for all registries + // and evict all direct accesses to the registries the team had + // we could have built a range of the teams's environments accesses while removing them above + // but ranging over all environments (registryAccessPolicy is indexed by environmentId) + // makes sure we cleanup all resources in case an access was not removed when a team was removed from an env + for _, registry := range registries { + updateRegistry := false + for _, registryAccessPolicy := range registry.RegistryAccesses { + if _, ok := registryAccessPolicy.TeamAccessPolicies[teamID]; ok { + delete(registryAccessPolicy.TeamAccessPolicies, teamID) + updateRegistry = true + } + } + if updateRegistry { + if err := tx.Registry().Update(registry.ID, ®istry); err != nil { + return err + } + } + } + + return service.UpdateUsersAuthorizationsTx(tx) +} + +// RemoveUserAccessPolicies will remove all existing access policies associated to the specified user +func (service *Service) RemoveUserAccessPolicies(tx dataservices.DataStoreTx, userID portainer.UserID) error { + endpoints, err := tx.Endpoint().Endpoints() + if err != nil { + return err + } + + for _, endpoint := range endpoints { + for policyUserID := range endpoint.UserAccessPolicies { + if policyUserID == userID { + delete(endpoint.UserAccessPolicies, policyUserID) + + err := tx.Endpoint().UpdateEndpoint(endpoint.ID, &endpoint) + if err != nil { + return err + } + + break + } + } + } + + endpointGroups, err := tx.EndpointGroup().ReadAll() + if err != nil { + return err + } + + for _, endpointGroup := range endpointGroups { + for policyUserID := range endpointGroup.UserAccessPolicies { + if policyUserID == userID { + delete(endpointGroup.UserAccessPolicies, policyUserID) + + err := tx.EndpointGroup().Update(endpointGroup.ID, &endpointGroup) + if err != nil { + return err + } + + break + } + } + } + + registries, err := tx.Registry().ReadAll() + if err != nil { + return err + } + + // iterate over all environments for all registries + // and evict all direct accesses to the registries the user had + // we could have built a range of the user's environments accesses while removing them above + // but ranging over all environments (registryAccessPolicy is indexed by environmentId) + // makes sure we cleanup all resources in case an access was not removed when a user was removed from an env + for _, registry := range registries { + updateRegistry := false + for _, registryAccessPolicy := range registry.RegistryAccesses { + if _, ok := registryAccessPolicy.UserAccessPolicies[userID]; ok { + delete(registryAccessPolicy.UserAccessPolicies, userID) + updateRegistry = true + } + } + if updateRegistry { + if err := tx.Registry().Update(registry.ID, ®istry); err != nil { + return err + } + } + } + + return nil +} + +// UpdateUserAuthorizations will update the authorizations for the provided userid +func (service *Service) UpdateUserAuthorizations(tx dataservices.DataStoreTx, userID portainer.UserID) error { + err := service.updateUserAuthorizations(tx, userID) + if err != nil { + return err + } + + return nil +} + // UpdateUsersAuthorizations will trigger an update of the authorizations for all the users. func (service *Service) UpdateUsersAuthorizations() error { return service.UpdateUsersAuthorizationsTx(service.dataStore) diff --git a/app/portainer/__module.js b/app/portainer/__module.js index f44af2c47..f4b8fbdaa 100644 --- a/app/portainer/__module.js +++ b/app/portainer/__module.js @@ -20,6 +20,7 @@ angular .module('portainer.app', [ 'portainer.oauth', 'portainer.rbac', + 'portainer.registrymanagement', componentsModule, settingsModule, featureFlagModule, @@ -319,46 +320,6 @@ angular }, }; - var registries = { - name: 'portainer.registries', - url: '/registries', - views: { - 'content@': { - component: 'registriesView', - }, - }, - data: { - docs: '/admin/registries', - access: AccessHeaders.Admin, - }, - }; - - var registry = { - name: 'portainer.registries.registry', - url: '/:id', - views: { - 'content@': { - component: 'editRegistry', - }, - }, - data: { - docs: '/admin/registries/edit', - }, - }; - - const registryCreation = { - name: 'portainer.registries.new', - url: '/new', - views: { - 'content@': { - component: 'createRegistry', - }, - }, - data: { - docs: '/admin/registries/add', - }, - }; - var settings = { name: 'portainer.settings', url: '/settings', @@ -460,9 +421,6 @@ angular $stateRegistryProvider.register(home); $stateRegistryProvider.register(init); $stateRegistryProvider.register(initAdmin); - $stateRegistryProvider.register(registries); - $stateRegistryProvider.register(registry); - $stateRegistryProvider.register(registryCreation); $stateRegistryProvider.register(settings); $stateRegistryProvider.register(settingsAuthentication); $stateRegistryProvider.register(settingsEdgeCompute); diff --git a/app/portainer/registry-management/index.js b/app/portainer/registry-management/index.js new file mode 100644 index 000000000..9afb35157 --- /dev/null +++ b/app/portainer/registry-management/index.js @@ -0,0 +1,50 @@ +import { AccessHeaders } from '../authorization-guard'; + +angular.module('portainer.registrymanagement', []).config(config); + +/* @ngInject */ +function config($stateRegistryProvider) { + const registries = { + name: 'portainer.registries', + url: '/registries', + views: { + 'content@': { + component: 'registriesView', + }, + }, + data: { + docs: '/admin/registries', + access: AccessHeaders.Admin, + }, + }; + + const registryCreation = { + name: 'portainer.registries.new', + url: '/new', + views: { + 'content@': { + component: 'createRegistry', + }, + }, + data: { + docs: '/admin/registries/add', + }, + }; + + const registry = { + name: 'portainer.registries.registry', + url: '/:id', + views: { + 'content@': { + component: 'editRegistry', + }, + }, + data: { + docs: '/admin/registries/edit', + }, + }; + + $stateRegistryProvider.register(registries); + $stateRegistryProvider.register(registry); + $stateRegistryProvider.register(registryCreation); +} diff --git a/app/portainer/views/registries/create/createRegistry.html b/app/portainer/registry-management/views/create/createRegistry.html similarity index 100% rename from app/portainer/views/registries/create/createRegistry.html rename to app/portainer/registry-management/views/create/createRegistry.html diff --git a/app/portainer/views/registries/create/createRegistry.js b/app/portainer/registry-management/views/create/createRegistry.js similarity index 100% rename from app/portainer/views/registries/create/createRegistry.js rename to app/portainer/registry-management/views/create/createRegistry.js diff --git a/app/portainer/views/registries/create/createRegistryController.js b/app/portainer/registry-management/views/create/createRegistryController.js similarity index 100% rename from app/portainer/views/registries/create/createRegistryController.js rename to app/portainer/registry-management/views/create/createRegistryController.js diff --git a/app/portainer/views/registries/edit/registry.html b/app/portainer/registry-management/views/edit/registry.html similarity index 100% rename from app/portainer/views/registries/edit/registry.html rename to app/portainer/registry-management/views/edit/registry.html diff --git a/app/portainer/views/registries/edit/registry.js b/app/portainer/registry-management/views/edit/registry.js similarity index 100% rename from app/portainer/views/registries/edit/registry.js rename to app/portainer/registry-management/views/edit/registry.js diff --git a/app/portainer/views/registries/edit/registryController.js b/app/portainer/registry-management/views/edit/registryController.js similarity index 100% rename from app/portainer/views/registries/edit/registryController.js rename to app/portainer/registry-management/views/edit/registryController.js diff --git a/app/react/kubernetes/cluster/HomeView/NodesDatatable/columns/name.tsx b/app/react/kubernetes/cluster/HomeView/NodesDatatable/columns/name.tsx index 4faa92143..1a0ddce53 100644 --- a/app/react/kubernetes/cluster/HomeView/NodesDatatable/columns/name.tsx +++ b/app/react/kubernetes/cluster/HomeView/NodesDatatable/columns/name.tsx @@ -12,6 +12,7 @@ import { columnHelper } from './helper'; export const name = columnHelper.accessor('Name', { header: 'Name', cell: NameCell, + id: 'name', }); function NameCell({