fix(app/registries): enforce user accesses on registries (#12088)

api/http/handler/endpoints/endpoint_delete.go

@@ -157,23 +157,20 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p
 		}
 	}
 
-	err = tx.Snapshot().Delete(endpointID)
+	if err := tx.Snapshot().Delete(endpointID); err != nil {
-	if err != nil {
+		log.Warn().Err(err).Msg("Unable to remove the snapshot from the database")
-		log.Warn().Err(err).Msgf("Unable to remove the snapshot from the database")
 	}
 
 	handler.ProxyManager.DeleteEndpointProxy(endpoint.ID)
 
 	if len(endpoint.UserAccessPolicies) > 0 || len(endpoint.TeamAccessPolicies) > 0 {
-		err = handler.AuthorizationService.UpdateUsersAuthorizationsTx(tx)
+		if err := handler.AuthorizationService.UpdateUsersAuthorizationsTx(tx); err != nil {
-		if err != nil {
+			log.Warn().Err(err).Msg("Unable to update user authorizations")
-			log.Warn().Err(err).Msgf("Unable to update user authorizations")
 		}
 	}
 
-	err = tx.EndpointRelation().DeleteEndpointRelation(endpoint.ID)
+	if err := tx.EndpointRelation().DeleteEndpointRelation(endpoint.ID); err != nil {
-	if err != nil {
+		log.Warn().Err(err).Msg("Unable to remove environment relation from the database")
-		log.Warn().Err(err).Msgf("Unable to remove environment relation from the database")
 	}
 
 	for _, tagID := range endpoint.TagIDs {
@@ -185,9 +182,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")
 		}
 	}
 
@@ -201,40 +198,39 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p
 			return e == endpoint.ID
 		})
 
-		err = tx.EdgeGroup().Update(edgeGroup.ID, &edgeGroup)
+		if err := tx.EdgeGroup().Update(edgeGroup.ID, &edgeGroup); err != nil {
-		if err != nil {
+			log.Warn().Err(err).Msg("Unable to update edge group")
-			log.Warn().Err(err).Msgf("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 {
+			if err := tx.EdgeStack().UpdateEdgeStack(edgeStack.ID, edgeStack); err != nil {
-				log.Warn().Err(err).Msgf("Unable to update edge stack")
+				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 := &registries[idx]
 		if _, ok := registry.RegistryAccesses[endpoint.ID]; ok {
 			delete(registry.RegistryAccesses, endpoint.ID)
-			err = tx.Registry().Update(registry.ID, registry)
+
-			if err != nil {
+			if err := tx.Registry().Update(registry.ID, registry); err != nil {
-				log.Warn().Err(err).Msgf("Unable to update registry accesses")
+				log.Warn().Err(err).Msg("Unable to update registry accesses")
 			}
 		}
 	}
@@ -242,7 +238,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 {
@@ -250,18 +246,16 @@ 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 := tx.EdgeJob().Update(edgeJob.ID, edgeJob); err != nil {
-				if err != nil {
+					log.Warn().Err(err).Msg("Unable to update edge job")
-					log.Warn().Err(err).Msgf("Unable to update edge job")
 				}
 			}
 		}
 	}
 
 	// delete the pending actions
-	err = tx.PendingActions().DeleteByEndpointID(endpoint.ID)
+	if err := tx.PendingActions().DeleteByEndpointID(endpoint.ID); err != nil {
-	if err != nil {
+		log.Warn().Err(err).Int("endpointId", int(endpoint.ID)).Msg("Unable to delete pending actions")
-		log.Warn().Err(err).Int("endpointId", int(endpoint.ID)).Msgf("Unable to delete pending actions")
 	}
 
 	err = tx.Endpoint().DeleteEndpoint(portainer.EndpointID(endpointID))

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
@@ -127,7 +128,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
 	}
 

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
-	if err != nil {
+	endpointIdStr, err := request.RetrieveNumericQueryParameter(r, "endpointId", false)
-		return false, false, err
-	}
-	endpoint, err := handler.DataStore.Endpoint().Endpoint(portainer.EndpointID(endpointID))
 	if err != nil {
 		return false, false, err
 	}
 
-	err = handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint)
+	endpointId := portainer.EndpointID(endpointIdStr)
+
+	endpoint, err := handler.DataStore.Endpoint().Endpoint(endpointId)
 	if err != nil {
 		return false, false, err
 	}
 
+	// 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
+	}
+
+	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
+}

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)
 }

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, &registry); 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, &registry); 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)

app/portainer/__module.js

@@ -20,6 +20,7 @@ angular
   .module('portainer.app', [
     'portainer.oauth',
     'portainer.rbac',
+    'portainer.registrymanagement',
     componentsModule,
     settingsModule,
     featureFlagModule,
@@ -352,47 +353,6 @@ angular
         },
       };
 
-      var registries = {
-        name: 'portainer.registries',
-        url: '/registries',
-        views: {
-          'content@': {
-            templateUrl: './views/registries/registries.html',
-            controller: 'RegistriesController',
-          },
-        },
-        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',
@@ -497,9 +457,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);

app/portainer/registry-management/index.js

@@ -0,0 +1,51 @@
+import { AccessHeaders } from '../authorization-guard';
+
+angular.module('portainer.registrymanagement', []).config(config);
+
+/* @ngInject */
+function config($stateRegistryProvider) {
+  const registries = {
+    name: 'portainer.registries',
+    url: '/registries',
+    views: {
+      'content@': {
+        templateUrl: './views/list/registries.html',
+        controller: 'RegistriesController',
+      },
+    },
+    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);
+}

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({