From b7e906701adffa6ed1f9b002cddf6b5dcc409919 Mon Sep 17 00:00:00 2001 From: Cara Ryan Date: Fri, 11 Jul 2025 14:55:48 +1200 Subject: [PATCH] fix(kubernetes): Namespace access permission changes role bindings not created [R8S-366] (#826) --- api/http/handler/auth/authenticate.go | 6 +++ api/http/handler/auth/handler.go | 5 ++- api/http/handler/auth/logout.go | 2 + .../endpoints/endpoint_registries_list.go | 18 ++++----- api/http/handler/kubernetes/client.go | 10 ++++- api/http/handler/kubernetes/handler.go | 6 +-- .../proxy/factory/kubernetes/transport.go | 1 + api/http/server.go | 2 +- api/kubernetes/cli/client.go | 40 ++++++++++++++++++- api/kubernetes/cli/client_test.go | 22 ++++++++++ 10 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 api/kubernetes/cli/client_test.go diff --git a/api/http/handler/auth/authenticate.go b/api/http/handler/auth/authenticate.go index 989949daa..4df31c92c 100644 --- a/api/http/handler/auth/authenticate.go +++ b/api/http/handler/auth/authenticate.go @@ -2,6 +2,7 @@ package auth import ( "net/http" + "strconv" "strings" portainer "github.com/portainer/portainer/api" @@ -82,6 +83,11 @@ func (handler *Handler) authenticate(rw http.ResponseWriter, r *http.Request) *h } } + // Clear any existing user caches + if user != nil { + handler.KubernetesClientFactory.ClearUserClientCache(strconv.Itoa(int(user.ID))) + } + if user != nil && isUserInitialAdmin(user) || settings.AuthenticationMethod == portainer.AuthenticationInternal { return handler.authenticateInternal(rw, user, payload.Password) } diff --git a/api/http/handler/auth/handler.go b/api/http/handler/auth/handler.go index 3b7210fbf..035ceabf8 100644 --- a/api/http/handler/auth/handler.go +++ b/api/http/handler/auth/handler.go @@ -8,6 +8,7 @@ import ( "github.com/portainer/portainer/api/http/proxy" "github.com/portainer/portainer/api/http/proxy/factory/kubernetes" "github.com/portainer/portainer/api/http/security" + "github.com/portainer/portainer/api/kubernetes/cli" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/gorilla/mux" @@ -23,16 +24,18 @@ type Handler struct { OAuthService portainer.OAuthService ProxyManager *proxy.Manager KubernetesTokenCacheManager *kubernetes.TokenCacheManager + KubernetesClientFactory *cli.ClientFactory passwordStrengthChecker security.PasswordStrengthChecker bouncer security.BouncerService } // NewHandler creates a handler to manage authentication operations. -func NewHandler(bouncer security.BouncerService, rateLimiter *security.RateLimiter, passwordStrengthChecker security.PasswordStrengthChecker) *Handler { +func NewHandler(bouncer security.BouncerService, rateLimiter *security.RateLimiter, passwordStrengthChecker security.PasswordStrengthChecker, kubernetesClientFactory *cli.ClientFactory) *Handler { h := &Handler{ Router: mux.NewRouter(), passwordStrengthChecker: passwordStrengthChecker, bouncer: bouncer, + KubernetesClientFactory: kubernetesClientFactory, } h.Handle("/auth/oauth/validate", diff --git a/api/http/handler/auth/logout.go b/api/http/handler/auth/logout.go index 977fafa69..73288565d 100644 --- a/api/http/handler/auth/logout.go +++ b/api/http/handler/auth/logout.go @@ -2,6 +2,7 @@ package auth import ( "net/http" + "strconv" "github.com/portainer/portainer/api/http/security" "github.com/portainer/portainer/api/logoutcontext" @@ -23,6 +24,7 @@ func (handler *Handler) logout(w http.ResponseWriter, r *http.Request) *httperro if tokenData != nil { handler.KubernetesTokenCacheManager.RemoveUserFromCache(tokenData.ID) + handler.KubernetesClientFactory.ClearUserClientCache(strconv.Itoa(int(tokenData.ID))) logoutcontext.Cancel(tokenData.Token) } diff --git a/api/http/handler/endpoints/endpoint_registries_list.go b/api/http/handler/endpoints/endpoint_registries_list.go index e81bc34a9..5bc4a930d 100644 --- a/api/http/handler/endpoints/endpoint_registries_list.go +++ b/api/http/handler/endpoints/endpoint_registries_list.go @@ -75,7 +75,7 @@ func (handler *Handler) listRegistries(tx dataservices.DataStoreTx, r *http.Requ return nil, httperror.InternalServerError("Unable to retrieve registries from the database", err) } - registries, handleError := handler.filterRegistriesByAccess(r, registries, endpoint, user, securityContext.UserMemberships) + registries, handleError := handler.filterRegistriesByAccess(tx, r, registries, endpoint, user, securityContext.UserMemberships) if handleError != nil { return nil, handleError } @@ -87,15 +87,15 @@ func (handler *Handler) listRegistries(tx dataservices.DataStoreTx, r *http.Requ return registries, err } -func (handler *Handler) filterRegistriesByAccess(r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User, memberships []portainer.TeamMembership) ([]portainer.Registry, *httperror.HandlerError) { +func (handler *Handler) filterRegistriesByAccess(tx dataservices.DataStoreTx, r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User, memberships []portainer.TeamMembership) ([]portainer.Registry, *httperror.HandlerError) { if !endpointutils.IsKubernetesEndpoint(endpoint) { return security.FilterRegistries(registries, user, memberships, endpoint.ID), nil } - return handler.filterKubernetesEndpointRegistries(r, registries, endpoint, user, memberships) + return handler.filterKubernetesEndpointRegistries(tx, r, registries, endpoint, user, memberships) } -func (handler *Handler) filterKubernetesEndpointRegistries(r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User, memberships []portainer.TeamMembership) ([]portainer.Registry, *httperror.HandlerError) { +func (handler *Handler) filterKubernetesEndpointRegistries(tx dataservices.DataStoreTx, r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User, memberships []portainer.TeamMembership) ([]portainer.Registry, *httperror.HandlerError) { namespaceParam, _ := request.RetrieveQueryParameter(r, "namespace", true) isAdmin, err := security.IsAdmin(r) if err != nil { @@ -116,7 +116,7 @@ func (handler *Handler) filterKubernetesEndpointRegistries(r *http.Request, regi return registries, nil } - return handler.filterKubernetesRegistriesByUserRole(r, registries, endpoint, user) + return handler.filterKubernetesRegistriesByUserRole(tx, r, registries, endpoint, user) } func (handler *Handler) isNamespaceAuthorized(endpoint *portainer.Endpoint, namespace string, userId portainer.UserID, memberships []portainer.TeamMembership, isAdmin bool) (bool, error) { @@ -169,7 +169,7 @@ func registryAccessPoliciesContainsNamespace(registryAccess portainer.RegistryAc return false } -func (handler *Handler) filterKubernetesRegistriesByUserRole(r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User) ([]portainer.Registry, *httperror.HandlerError) { +func (handler *Handler) filterKubernetesRegistriesByUserRole(tx dataservices.DataStoreTx, r *http.Request, registries []portainer.Registry, endpoint *portainer.Endpoint, user *portainer.User) ([]portainer.Registry, *httperror.HandlerError) { err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint) if errors.Is(err, security.ErrAuthorizationRequired) { return nil, httperror.Forbidden("User is not authorized", err) @@ -178,7 +178,7 @@ func (handler *Handler) filterKubernetesRegistriesByUserRole(r *http.Request, re return nil, httperror.InternalServerError("Unable to retrieve info from request context", err) } - userNamespaces, err := handler.userNamespaces(endpoint, user) + userNamespaces, err := handler.userNamespaces(tx, endpoint, user) if err != nil { return nil, httperror.InternalServerError("unable to retrieve user namespaces", err) } @@ -186,7 +186,7 @@ func (handler *Handler) filterKubernetesRegistriesByUserRole(r *http.Request, re return filterRegistriesByNamespaces(registries, endpoint.ID, userNamespaces), nil } -func (handler *Handler) userNamespaces(endpoint *portainer.Endpoint, user *portainer.User) ([]string, error) { +func (handler *Handler) userNamespaces(tx dataservices.DataStoreTx, endpoint *portainer.Endpoint, user *portainer.User) ([]string, error) { kcl, err := handler.K8sClientFactory.GetPrivilegedKubeClient(endpoint) if err != nil { return nil, err @@ -197,7 +197,7 @@ func (handler *Handler) userNamespaces(endpoint *portainer.Endpoint, user *porta return nil, err } - userMemberships, err := handler.DataStore.TeamMembership().TeamMembershipsByUserID(user.ID) + userMemberships, err := tx.TeamMembership().TeamMembershipsByUserID(user.ID) if err != nil { return nil, err } diff --git a/api/http/handler/kubernetes/client.go b/api/http/handler/kubernetes/client.go index 6612ab7f4..a7f2485e3 100644 --- a/api/http/handler/kubernetes/client.go +++ b/api/http/handler/kubernetes/client.go @@ -2,8 +2,10 @@ package kubernetes import ( "net/http" + "strconv" "github.com/portainer/portainer/api/http/middlewares" + "github.com/portainer/portainer/api/http/security" "github.com/portainer/portainer/api/kubernetes/cli" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/rs/zerolog/log" @@ -25,7 +27,13 @@ func (handler *Handler) prepareKubeClient(r *http.Request) (*cli.KubeClient, *ht return nil, httperror.NotFound("Unable to find the Kubernetes endpoint associated to the request.", err) } - pcli, err := handler.KubernetesClientFactory.GetPrivilegedKubeClient(endpoint) + tokenData, err := security.RetrieveTokenData(r) + if err != nil { + log.Error().Err(err).Str("context", "prepareKubeClient").Msg("Unable to retrieve token data associated to the request.") + return nil, httperror.InternalServerError("Unable to retrieve token data associated to the request.", err) + } + + pcli, err := handler.KubernetesClientFactory.GetPrivilegedUserKubeClient(endpoint, strconv.Itoa(int(tokenData.ID))) if err != nil { log.Error().Err(err).Str("context", "prepareKubeClient").Msg("Unable to get a privileged Kubernetes client for the user.") return nil, httperror.InternalServerError("Unable to get a privileged Kubernetes client for the user.", err) diff --git a/api/http/handler/kubernetes/handler.go b/api/http/handler/kubernetes/handler.go index 07f6bdf3f..a8a5898c8 100644 --- a/api/http/handler/kubernetes/handler.go +++ b/api/http/handler/kubernetes/handler.go @@ -146,7 +146,7 @@ func (h *Handler) getProxyKubeClient(r *http.Request) (portainer.KubeClient, *ht return nil, httperror.Forbidden(fmt.Sprintf("an error occurred during the getProxyKubeClient operation, permission denied to access the environment /api/kubernetes/%d. Error: ", endpointID), err) } - cli, ok := h.KubernetesClientFactory.GetProxyKubeClient(strconv.Itoa(endpointID), tokenData.Token) + cli, ok := h.KubernetesClientFactory.GetProxyKubeClient(strconv.Itoa(endpointID), strconv.Itoa(int(tokenData.ID))) if !ok { return nil, httperror.InternalServerError("an error occurred during the getProxyKubeClient operation,failed to get proxy KubeClient", nil) } @@ -179,7 +179,7 @@ func (handler *Handler) kubeClientMiddleware(next http.Handler) http.Handler { } // Check if we have a kubeclient against this auth token already, otherwise generate a new one - _, ok := handler.KubernetesClientFactory.GetProxyKubeClient(strconv.Itoa(endpointID), tokenData.Token) + _, ok := handler.KubernetesClientFactory.GetProxyKubeClient(strconv.Itoa(endpointID), strconv.Itoa(int(tokenData.ID))) if ok { next.ServeHTTP(w, r) return @@ -269,7 +269,7 @@ func (handler *Handler) kubeClientMiddleware(next http.Handler) http.Handler { return } - handler.KubernetesClientFactory.SetProxyKubeClient(strconv.Itoa(int(endpoint.ID)), tokenData.Token, kubeCli) + handler.KubernetesClientFactory.SetProxyKubeClient(strconv.Itoa(int(endpoint.ID)), strconv.Itoa(int(tokenData.ID)), kubeCli) next.ServeHTTP(w, r) }) } diff --git a/api/http/proxy/factory/kubernetes/transport.go b/api/http/proxy/factory/kubernetes/transport.go index 76e9daa68..ddab7a096 100644 --- a/api/http/proxy/factory/kubernetes/transport.go +++ b/api/http/proxy/factory/kubernetes/transport.go @@ -58,6 +58,7 @@ func (transport *baseTransport) proxyKubernetesRequest(request *http.Request) (* switch { case strings.EqualFold(requestPath, "/namespaces/portainer/configmaps/portainer-config") && (request.Method == "PUT" || request.Method == "POST"): + transport.k8sClientFactory.ClearClientCache() defer transport.tokenManager.UpdateUserServiceAccountsForEndpoint(portainer.EndpointID(endpointID)) return transport.executeKubernetesRequest(request) case strings.EqualFold(requestPath, "/namespaces"): diff --git a/api/http/server.go b/api/http/server.go index 5bfa36379..8f073ce58 100644 --- a/api/http/server.go +++ b/api/http/server.go @@ -131,7 +131,7 @@ func (server *Server) Start() error { passwordStrengthChecker := security.NewPasswordStrengthChecker(server.DataStore.Settings()) - var authHandler = auth.NewHandler(requestBouncer, rateLimiter, passwordStrengthChecker) + var authHandler = auth.NewHandler(requestBouncer, rateLimiter, passwordStrengthChecker, server.KubernetesClientFactory) authHandler.DataStore = server.DataStore authHandler.CryptoService = server.CryptoService authHandler.JWTService = server.JWTService diff --git a/api/kubernetes/cli/client.go b/api/kubernetes/cli/client.go index a40a865f1..550ade1d3 100644 --- a/api/kubernetes/cli/client.go +++ b/api/kubernetes/cli/client.go @@ -77,9 +77,26 @@ func (factory *ClientFactory) ClearClientCache() { factory.endpointProxyClients.Flush() } +// ClearClientCache removes all cached kube clients for a userId +func (factory *ClientFactory) ClearUserClientCache(userID string) { + for key := range factory.endpointProxyClients.Items() { + if strings.HasSuffix(key, "."+userID) { + factory.endpointProxyClients.Delete(key) + } + } +} + // Remove the cached kube client so a new one can be created func (factory *ClientFactory) RemoveKubeClient(endpointID portainer.EndpointID) { factory.endpointProxyClients.Delete(strconv.Itoa(int(endpointID))) + + endpointPrefix := strconv.Itoa(int(endpointID)) + "." + + for key := range factory.endpointProxyClients.Items() { + if strings.HasPrefix(key, endpointPrefix) { + factory.endpointProxyClients.Delete(key) + } + } } func (factory *ClientFactory) GetAddrHTTPS() string { @@ -104,6 +121,24 @@ func (factory *ClientFactory) GetPrivilegedKubeClient(endpoint *portainer.Endpoi return kcl, nil } +// GetPrivilegedUserKubeClient checks if an existing admin client is already registered for the environment(endpoint) and user and returns it if one is found. +// If no client is registered, it will create a new client, register it, and returns it. +func (factory *ClientFactory) GetPrivilegedUserKubeClient(endpoint *portainer.Endpoint, userID string) (*KubeClient, error) { + key := strconv.Itoa(int(endpoint.ID)) + ".admin." + userID + pcl, ok := factory.endpointProxyClients.Get(key) + if ok { + return pcl.(*KubeClient), nil + } + + kcl, err := factory.createCachedPrivilegedKubeClient(endpoint) + if err != nil { + return nil, err + } + + factory.endpointProxyClients.Set(key, kcl, cache.DefaultExpiration) + return kcl, nil +} + // GetProxyKubeClient retrieves a KubeClient from the cache. You should be // calling SetProxyKubeClient before first. It is normally, called the // kubernetes middleware. @@ -156,8 +191,9 @@ func (factory *ClientFactory) createCachedPrivilegedKubeClient(endpoint *portain } return &KubeClient{ - cli: cli, - instanceID: factory.instanceID, + cli: cli, + instanceID: factory.instanceID, + IsKubeAdmin: true, }, nil } diff --git a/api/kubernetes/cli/client_test.go b/api/kubernetes/cli/client_test.go new file mode 100644 index 000000000..993a966e3 --- /dev/null +++ b/api/kubernetes/cli/client_test.go @@ -0,0 +1,22 @@ +package cli + +import ( + "testing" +) + +func TestClearUserClientCache(t *testing.T) { + factory, _ := NewClientFactory(nil, nil, nil, "", "", "") + kcl := &KubeClient{} + factory.endpointProxyClients.Set("12.1", kcl, 0) + factory.endpointProxyClients.Set("12.12", kcl, 0) + factory.endpointProxyClients.Set("12", kcl, 0) + + factory.ClearUserClientCache("12") + + if len(factory.endpointProxyClients.Items()) != 2 { + t.Errorf("Incorrect clients cached after clearUserClientCache;\ngot=\n%d\nwant=\n%d", len(factory.endpointProxyClients.Items()), 2) + } + if _, ok := factory.GetProxyKubeClient("12", "12"); ok { + t.Errorf("Expected not to find client cache for user after clear") + } +}