fix(rbac): RoleBindings and ClusterRoleBindings cleanup [BE-13116] (#3084)

This commit is contained in:
Oscar Zhou
2026-07-14 18:00:42 +12:00
committed by GitHub
parent 8b7f177384
commit d8774e2b60
14 changed files with 1387 additions and 29 deletions
@@ -159,6 +159,9 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) *
endpoint.Kubernetes = *payload.Kubernetes
}
oldUserAccessPolicies := endpoint.UserAccessPolicies
oldTeamAccessPolicies := endpoint.TeamAccessPolicies
if payload.UserAccessPolicies != nil && !reflect.DeepEqual(payload.UserAccessPolicies, endpoint.UserAccessPolicies) {
updateAuthorizations = true
endpoint.UserAccessPolicies = payload.UserAccessPolicies
@@ -274,6 +277,8 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) *
log.Warn().Err(err).Msg("unable to schedule pending action to clean NAP with override policies")
}
}
handler.reconcileK8sServiceAccounts(endpoint, oldUserAccessPolicies, oldTeamAccessPolicies)
}
if err := handler.DataStore.Endpoint().UpdateEndpoint(endpoint.ID, endpoint); err != nil {
@@ -295,6 +300,65 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) *
return response.JSON(w, endpoint)
}
func (handler *Handler) reconcileK8sServiceAccounts(
endpoint *portainer.Endpoint,
oldUserPolicies portainer.UserAccessPolicies,
oldTeamPolicies portainer.TeamAccessPolicies,
) {
if handler.K8sClientFactory == nil {
return
}
kubecli, err := handler.K8sClientFactory.GetPrivilegedKubeClient(endpoint)
if err != nil {
log.Error().Err(err).Int("environment_id", int(endpoint.ID)).Msg("failed getting kube client for environment during SA reconcile")
return
}
// Collect all users who had access before (direct or via team).
affected := make(map[portainer.UserID]struct{})
for userID := range oldUserPolicies {
affected[userID] = struct{}{}
}
for teamID := range oldTeamPolicies {
memberships, err := handler.DataStore.TeamMembership().TeamMembershipsByTeamID(teamID)
if err != nil {
log.Error().Err(err).Int("environment_id", int(endpoint.ID)).Int("team_id", int(teamID)).Msg("failed fetching memberships for team")
continue
}
for _, m := range memberships {
affected[m.UserID] = struct{}{}
}
}
for userID := range affected {
// Determine which of the user's teams still have access to this endpoint.
memberships, err := handler.DataStore.TeamMembership().TeamMembershipsByUserID(userID)
if err != nil {
log.Error().Err(err).Int("user_id", int(userID)).Msg("failed fetching memberships for user")
continue
}
remainingTeamIDs := make([]int, 0)
for _, m := range memberships {
if _, ok := endpoint.TeamAccessPolicies[m.TeamID]; ok {
remainingTeamIDs = append(remainingTeamIDs, int(m.TeamID))
}
}
_, hasDirectAccess := endpoint.UserAccessPolicies[userID]
if !hasDirectAccess && len(remainingTeamIDs) == 0 {
if err := kubecli.RemoveUserServiceAccountBindings(int(userID)); err != nil {
log.Error().Err(err).Int("user_id", int(userID)).Int("environment_id", int(endpoint.ID)).Msg("failed removing SA bindings for user")
}
continue
}
if err := kubecli.SetupUserServiceAccount(int(userID), remainingTeamIDs, endpoint.Kubernetes.Configuration.RestrictDefaultNamespace); err != nil {
log.Error().Err(err).Int("user_id", int(userID)).Int("environment_id", int(endpoint.ID)).Msg("failed updating SA for user")
}
}
}
func shouldReloadTLSConfiguration(endpoint *portainer.Endpoint, payload *endpointUpdatePayload) bool {
// If we change anything in the tls config then we need to reload the proxy
if payload.TLS != nil && endpoint.TLSConfig.TLS != *payload.TLS {
@@ -8,15 +8,252 @@ import (
"net/http/httptest"
"testing"
"github.com/portainer/portainer/api"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/datastore"
"github.com/portainer/portainer/api/http/security"
"github.com/portainer/portainer/api/internal/testhelpers"
kcli "github.com/portainer/portainer/api/kubernetes/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kfake "k8s.io/client-go/kubernetes/fake"
)
func TestReconcileK8sServiceAccounts(t *testing.T) {
t.Parallel()
const (
userID = portainer.UserID(1)
crbName = "portainer-crb-user"
rbName = "portainer-rb-test-default"
)
saName := kcli.UserServiceAccountName(int(userID), "test")
saSubject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
existingBindingsWithUser := func() *kfake.Clientset {
return kfake.NewSimpleClientset(
&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: "portainer"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{saSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: "default"},
Subjects: []rbacv1.Subject{saSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
)
}
saInCRB := func(t *testing.T, fakeK8s *kfake.Clientset) bool {
t.Helper()
crb, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
for _, s := range crb.Subjects {
if s.Name == saName {
return true
}
}
return false
}
saInRB := func(t *testing.T, fakeK8s *kfake.Clientset) bool {
t.Helper()
rb, err := fakeK8s.RbacV1().RoleBindings("default").Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
for _, s := range rb.Subjects {
if s.Name == saName {
return true
}
}
return false
}
t.Run("removes bindings when user's only direct access is removed", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
fakeK8s := existingBindingsWithUser()
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{},
TeamAccessPolicies: portainer.TeamAccessPolicies{},
}
h := &Handler{
DataStore: store,
K8sClientFactory: kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s)),
}
h.reconcileK8sServiceAccounts(endpoint,
portainer.UserAccessPolicies{userID: {RoleID: 1}},
portainer.TeamAccessPolicies{},
)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "SA must be removed from CRB when user's only direct access is revoked")
gotRB, err := fakeK8s.RbacV1().RoleBindings("default").Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "SA must be removed from namespace RoleBinding when user's only direct access is revoked")
})
t.Run("removes bindings when both direct and team access are removed", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
const teamID = portainer.TeamID(1)
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: userID, TeamID: teamID, Role: portainer.TeamMember,
}))
fakeK8s := existingBindingsWithUser()
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{},
TeamAccessPolicies: portainer.TeamAccessPolicies{},
}
h := &Handler{
DataStore: store,
K8sClientFactory: kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s)),
}
h.reconcileK8sServiceAccounts(endpoint,
portainer.UserAccessPolicies{userID: {RoleID: 1}},
portainer.TeamAccessPolicies{teamID: {RoleID: 1}},
)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "SA must be removed from CRB when all access is revoked")
gotRB, err := fakeK8s.RbacV1().RoleBindings("default").Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "SA must be removed from namespace RoleBinding when all access is revoked")
})
t.Run("removes bindings when team access is removed", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
const teamID = portainer.TeamID(1)
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: userID, TeamID: teamID, Role: portainer.TeamMember,
}))
fakeK8s := existingBindingsWithUser()
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{},
TeamAccessPolicies: portainer.TeamAccessPolicies{},
}
h := &Handler{
DataStore: store,
K8sClientFactory: kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s)),
}
h.reconcileK8sServiceAccounts(endpoint,
portainer.UserAccessPolicies{},
portainer.TeamAccessPolicies{teamID: {RoleID: 1}},
)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "SA must be removed from CRB when team loses access")
gotRB, err := fakeK8s.RbacV1().RoleBindings("default").Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "SA must be removed from namespace RoleBinding when team loses access")
})
t.Run("keeps SA when team loses access but user is in another team with access", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
const (
removedTeamID = portainer.TeamID(1)
retainedTeamID = portainer.TeamID(2)
)
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: userID, TeamID: removedTeamID, Role: portainer.TeamMember,
}))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 2, UserID: userID, TeamID: retainedTeamID, Role: portainer.TeamMember,
}))
fakeK8s := kfake.NewSimpleClientset(
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
)
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{},
TeamAccessPolicies: portainer.TeamAccessPolicies{retainedTeamID: {RoleID: 1}},
}
h := &Handler{
DataStore: store,
K8sClientFactory: kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s)),
}
h.reconcileK8sServiceAccounts(endpoint,
portainer.UserAccessPolicies{},
portainer.TeamAccessPolicies{removedTeamID: {RoleID: 1}},
)
assert.True(t, saInCRB(t, fakeK8s), "SA must remain in CRB when user still has access via another team")
assert.True(t, saInRB(t, fakeK8s), "SA must remain in namespace RoleBinding when user still has access via another team")
})
t.Run("keeps SA when direct access is removed but team access remains", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
const teamID = portainer.TeamID(1)
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: userID, TeamID: teamID, Role: portainer.TeamMember,
}))
fakeK8s := kfake.NewSimpleClientset(
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
)
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{},
TeamAccessPolicies: portainer.TeamAccessPolicies{teamID: {RoleID: 1}},
}
h := &Handler{
DataStore: store,
K8sClientFactory: kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s)),
}
h.reconcileK8sServiceAccounts(endpoint,
portainer.UserAccessPolicies{userID: {RoleID: 1}},
portainer.TeamAccessPolicies{},
)
assert.True(t, saInCRB(t, fakeK8s), "SA must remain in CRB when team still grants access")
assert.True(t, saInRB(t, fakeK8s), "SA must remain in namespace RoleBinding when team still grants access")
})
}
func Test_endpointPut_TLSRejectedForEdgeEndpoint(t *testing.T) {
t.Parallel()
+42 -12
View File
@@ -43,20 +43,50 @@ func (handler *Handler) updateUserServiceAccounts(membership *portainer.TeamMemb
log.Error().Err(err).Msgf("failed fetching environments for team %d", membership.TeamID)
return
}
// Query remaining team memberships after the deletion has already been committed.
remainingMemberships, err := handler.DataStore.TeamMembership().TeamMembershipsByUserID(membership.UserID)
if err != nil {
log.Error().Err(err).Msgf("failed fetching team memberships for user %d", membership.UserID)
return
}
remainingTeamIDs := make([]int, 0, len(remainingMemberships))
for _, m := range remainingMemberships {
remainingTeamIDs = append(remainingTeamIDs, int(m.TeamID))
}
for _, endpoint := range endpoints {
restrictDefaultNamespace := endpoint.Kubernetes.Configuration.RestrictDefaultNamespace
// update kubernenets service accounts if the team is associated with a kubernetes environment
if endpointutils.IsKubernetesEndpoint(&endpoint) {
kubecli, err := handler.K8sClientFactory.GetPrivilegedKubeClient(&endpoint)
if err != nil {
log.Error().Err(err).Msgf("failed getting kube client for environment %d", endpoint.ID)
continue
}
teamIDs := []int{int(membership.TeamID)}
err = kubecli.SetupUserServiceAccount(int(membership.UserID), teamIDs, restrictDefaultNamespace)
if err != nil {
log.Error().Err(err).Msgf("failed setting-up service account for user %d", membership.UserID)
if !endpointutils.IsKubernetesEndpoint(&endpoint) {
continue
}
kubecli, err := handler.K8sClientFactory.GetPrivilegedKubeClient(&endpoint)
if err != nil {
log.Error().Err(err).Int("environment_id", int(endpoint.ID)).Msg("failed getting kube client for environment")
continue
}
if !userHasEndpointAccess(int(membership.UserID), remainingTeamIDs, &endpoint) {
if err := kubecli.RemoveUserServiceAccountBindings(int(membership.UserID)); err != nil {
log.Error().Err(err).Int("environment_id", int(endpoint.ID)).Int("user_id", int(membership.UserID)).Msg("failed removing SA bindings for user")
}
continue
}
if err := kubecli.SetupUserServiceAccount(int(membership.UserID), remainingTeamIDs, endpoint.Kubernetes.Configuration.RestrictDefaultNamespace); err != nil {
log.Error().Err(err).Int("environment_id", int(endpoint.ID)).Int("user_id", int(membership.UserID)).Msg("failed setting-up service account for user")
}
}
}
func userHasEndpointAccess(userID int, teamIDs []int, endpoint *portainer.Endpoint) bool {
if _, ok := endpoint.UserAccessPolicies[portainer.UserID(userID)]; ok {
return true
}
for _, teamID := range teamIDs {
if _, ok := endpoint.TeamAccessPolicies[portainer.TeamID(teamID)]; ok {
return true
}
}
return false
}
@@ -0,0 +1,185 @@
package teammemberships
import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/datastore"
"github.com/portainer/portainer/api/http/security"
"github.com/portainer/portainer/api/internal/testhelpers"
cli "github.com/portainer/portainer/api/kubernetes/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kfake "k8s.io/client-go/kubernetes/fake"
)
func TestDeleteTeamMembership_removesUserSABindings(t *testing.T) {
t.Parallel()
const (
crbName = "portainer-crb-user"
rbName = "portainer-rb-test-default"
ns = "default"
teamID = portainer.TeamID(1)
otherSAName = "portainer-sa-user-test-99"
)
setupTestStore := func(t *testing.T) (*portainer.User, *portainer.Endpoint, *portainer.TeamMembership, *datastore.Store) {
t.Helper()
_, store := datastore.MustNewTestStore(t, true, true)
user := &portainer.User{Username: "standard", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
endpoint := &portainer.Endpoint{
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{
teamID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
membership := &portainer.TeamMembership{UserID: user.ID, TeamID: teamID, Role: portainer.TeamMember}
require.NoError(t, store.TeamMembershipService.Create(membership))
return user, endpoint, membership, store
}
deleteTeamMembership := func(t *testing.T, store *datastore.Store, endpointID portainer.EndpointID, fakeK8s *kfake.Clientset, membershipID portainer.TeamMembershipID) {
t.Helper()
h := NewHandler(testhelpers.NewTestRequestBouncer())
h.DataStore = store
h.K8sClientFactory = cli.NewTestClientFactory(endpointID, cli.NewTestKubeClient(fakeK8s))
rr := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/team_memberships/%d", membershipID), nil)
r = r.WithContext(security.StoreRestrictedRequestContext(r, &security.RestrictedRequestContext{IsAdmin: true}))
h.ServeHTTP(rr, r)
require.Equal(t, http.StatusNoContent, rr.Code)
}
t.Run("removes SA from RoleBinding and CRB subjects when SA is the only subject", func(t *testing.T) {
t.Parallel()
user, endpoint, membership, store := setupTestStore(t)
saName := cli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: ns},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "cluster-admin"},
},
)
deleteTeamMembership(t, store, endpoint.ID, fakeK8s, membership.ID)
// CE updates bindings in-place; the binding remains but with the SA stripped from subjects.
gotRB, err := fakeK8s.RbacV1().RoleBindings(ns).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err, "RoleBinding must still exist")
assert.Empty(t, gotRB.Subjects, "user SA must be removed from RoleBinding subjects")
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err, "CRB must still exist")
assert.Empty(t, gotCRB.Subjects, "user SA must be removed from CRB subjects")
})
t.Run("removes SA from subjects but preserves RoleBinding and CRB when other subjects remain", func(t *testing.T) {
t.Parallel()
user, endpoint, membership, store := setupTestStore(t)
saName := cli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
otherSubject := rbacv1.Subject{Kind: "ServiceAccount", Name: otherSAName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: ns},
Subjects: []rbacv1.Subject{subject, otherSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject, otherSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "cluster-admin"},
},
)
deleteTeamMembership(t, store, endpoint.ID, fakeK8s, membership.ID)
gotRB, err := fakeK8s.RbacV1().RoleBindings(ns).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err, "RoleBinding must be preserved when other subjects remain")
require.Len(t, gotRB.Subjects, 1)
assert.Equal(t, otherSAName, gotRB.Subjects[0].Name, "only the other subject must remain in RoleBinding")
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err, "CRB must be preserved when other subjects remain")
require.Len(t, gotCRB.Subjects, 1)
assert.Equal(t, otherSAName, gotCRB.Subjects[0].Name, "only the other subject must remain in CRB")
})
t.Run("keeps SA when user retains direct endpoint access after leaving team", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
user := &portainer.User{Username: "standard", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
endpoint := &portainer.Endpoint{
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{
teamID: {RoleID: 1},
},
UserAccessPolicies: portainer.UserAccessPolicies{
user.ID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
membership := &portainer.TeamMembership{UserID: user.ID, TeamID: teamID, Role: portainer.TeamMember}
require.NoError(t, store.TeamMembershipService.Create(membership))
saName := cli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: "portainer"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
)
deleteTeamMembership(t, store, endpoint.ID, fakeK8s, membership.ID)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
saInCRB := false
for _, s := range gotCRB.Subjects {
if s.Name == saName {
saInCRB = true
break
}
}
assert.True(t, saInCRB, "SA must remain in CRB when user still has direct endpoint access")
})
}
+3 -1
View File
@@ -5,6 +5,7 @@ import (
"github.com/portainer/portainer/api/dataservices"
"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"
@@ -13,7 +14,8 @@ import (
// Handler is the HTTP handler used to handle team operations.
type Handler struct {
*mux.Router
DataStore dataservices.DataStore
DataStore dataservices.DataStore
K8sClientFactory *cli.ClientFactory
}
// NewHandler creates a handler to manage team operations.
+150 -5
View File
@@ -4,11 +4,14 @@ import (
"net/http"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/dataservices"
"github.com/portainer/portainer/api/internal/endpointutils"
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"
"github.com/rs/zerolog/log"
)
// @id TeamDelete
@@ -38,25 +41,167 @@ func (handler *Handler) teamDelete(w http.ResponseWriter, r *http.Request) *http
return httperror.InternalServerError("Unable to find a team with the specified identifier inside the database", err)
}
err = handler.DataStore.Team().Delete(portainer.TeamID(teamID))
memberships, err := handler.DataStore.TeamMembership().TeamMembershipsByTeamID(portainer.TeamID(teamID))
if err != nil {
return httperror.InternalServerError("Unable to fetch team memberships", err)
}
handler.cleanupTeamK8sServiceAccounts(portainer.TeamID(teamID), memberships)
if err := handler.DataStore.Team().Delete(portainer.TeamID(teamID)); err != nil {
return httperror.InternalServerError("Unable to delete the team from the database", err)
}
err = handler.DataStore.TeamMembership().DeleteTeamMembershipByTeamID(portainer.TeamID(teamID))
if err != nil {
if err := handler.DataStore.TeamMembership().DeleteTeamMembershipByTeamID(portainer.TeamID(teamID)); err != nil {
return httperror.InternalServerError("Unable to delete associated team memberships from the database", err)
}
if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error {
return handler.removeTeamAccessPolicies(tx, portainer.TeamID(teamID), memberships)
}); err != nil {
return httperror.InternalServerError("Unable to clean-up team access policies", err)
}
// update default team if deleted team was default
err = handler.updateDefaultTeamIfDeleted(portainer.TeamID(teamID))
if err != nil {
if err := handler.updateDefaultTeamIfDeleted(portainer.TeamID(teamID)); err != nil {
return httperror.InternalServerError("Unable to reset default team", err)
}
return response.Empty(w)
}
func (handler *Handler) removeTeamAccessPolicies(tx dataservices.DataStoreTx, teamID portainer.TeamID, memberships []portainer.TeamMembership) error {
endpoints, err := tx.Endpoint().Endpoints()
if err != nil {
return err
}
for i := range endpoints {
ep := &endpoints[i]
if _, hasTeam := ep.TeamAccessPolicies[teamID]; !hasTeam {
continue
}
delete(ep.TeamAccessPolicies, teamID)
for _, m := range memberships {
otherMemberships, err := tx.TeamMembership().TeamMembershipsByUserID(m.UserID)
if err != nil {
return err
}
hasOtherTeamAccess := false
for _, om := range otherMemberships {
if _, ok := ep.TeamAccessPolicies[om.TeamID]; ok {
hasOtherTeamAccess = true
break
}
}
if !hasOtherTeamAccess {
delete(ep.UserAccessPolicies, m.UserID)
}
}
if err := tx.Endpoint().UpdateEndpoint(ep.ID, ep); err != nil {
return err
}
}
groups, err := tx.EndpointGroup().ReadAll()
if err != nil {
return err
}
for i := range groups {
g := &groups[i]
if _, hasTeam := g.TeamAccessPolicies[teamID]; !hasTeam {
continue
}
delete(g.TeamAccessPolicies, teamID)
if err := tx.EndpointGroup().Update(g.ID, g); err != nil {
return err
}
}
registries, err := tx.Registry().ReadAll()
if err != nil {
return err
}
for i := range registries {
changed := false
for _, rap := range registries[i].RegistryAccesses {
if _, ok := rap.TeamAccessPolicies[teamID]; ok {
delete(rap.TeamAccessPolicies, teamID)
changed = true
}
}
if changed {
if err := tx.Registry().Update(registries[i].ID, &registries[i]); err != nil {
return err
}
}
}
return nil
}
// cleanupTeamK8sServiceAccounts removes SA bindings for team members who lose all access to a
// K8s endpoint when the team is deleted. Must be called before team memberships are removed from DB.
func (handler *Handler) cleanupTeamK8sServiceAccounts(teamID portainer.TeamID, memberships []portainer.TeamMembership) {
if handler.K8sClientFactory == nil {
return
}
var endpoints []portainer.Endpoint
if err := handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error {
var txErr error
endpoints, txErr = tx.Endpoint().Endpoints()
return txErr
}); err != nil {
log.Error().Err(err).Msg("failed fetching endpoints for K8s SA cleanup")
return
}
for _, endpoint := range endpoints {
if _, hasTeamAccess := endpoint.TeamAccessPolicies[teamID]; !hasTeamAccess {
continue
}
if !endpointutils.IsKubernetesEndpoint(&endpoint) {
continue
}
kubecli, err := handler.K8sClientFactory.GetPrivilegedKubeClient(&endpoint)
if err != nil {
log.Error().Err(err).Int("endpoint_id", int(endpoint.ID)).Msg("failed getting kube client for team SA cleanup")
continue
}
for _, m := range memberships {
if _, hasDirect := endpoint.UserAccessPolicies[m.UserID]; hasDirect {
continue
}
userMemberships, err := handler.DataStore.TeamMembership().TeamMembershipsByUserID(m.UserID)
if err != nil {
log.Error().Err(err).Int("user_id", int(m.UserID)).Msg("failed fetching user memberships for K8s SA cleanup")
continue
}
hasOtherTeamAccess := false
for _, um := range userMemberships {
if um.TeamID == teamID {
continue
}
if _, ok := endpoint.TeamAccessPolicies[um.TeamID]; ok {
hasOtherTeamAccess = true
break
}
}
if !hasOtherTeamAccess {
if err := kubecli.RemoveUserServiceAccountBindings(int(m.UserID)); err != nil {
log.Error().Err(err).Int("user_id", int(m.UserID)).Int("endpoint_id", int(endpoint.ID)).Msg("failed removing SA bindings for team member")
}
}
}
}
}
// updateDefaultTeamIfDeleted resets the default team to nil if default team was the deleted team
func (handler *Handler) updateDefaultTeamIfDeleted(teamID portainer.TeamID) error {
settings, err := handler.DataStore.Settings().Settings()
+319
View File
@@ -0,0 +1,319 @@
package teams
import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/datastore"
"github.com/portainer/portainer/api/internal/testhelpers"
kcli "github.com/portainer/portainer/api/kubernetes/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kfake "k8s.io/client-go/kubernetes/fake"
)
func newTestTeamHandler(t *testing.T, store *datastore.Store) *Handler {
t.Helper()
h := NewHandler(testhelpers.NewTestRequestBouncer())
h.DataStore = store
return h
}
func TestTeamDelete_removesTeamFromEndpointAccessPolicies(t *testing.T) {
t.Parallel()
t.Run("team with no members", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
team := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(team))
endpoint := &portainer.Endpoint{
ID: 1,
Name: "k8s",
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{
team.ID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h := newTestTeamHandler(t, store)
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", team.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
updated, err := store.Endpoint().Endpoint(endpoint.ID)
require.NoError(t, err)
_, stillPresent := updated.TeamAccessPolicies[team.ID]
assert.False(t, stillPresent, "deleted team must be removed from endpoint TeamAccessPolicies")
})
t.Run("team with member has both team and user policies removed", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
team := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(team))
user := &portainer.User{Username: "alice", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: user.ID, TeamID: team.ID, Role: portainer.TeamMember,
}))
endpoint := &portainer.Endpoint{
ID: 1,
Name: "k8s",
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{
team.ID: {RoleID: 1},
},
UserAccessPolicies: portainer.UserAccessPolicies{
user.ID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h := newTestTeamHandler(t, store)
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", team.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
updated, err := store.Endpoint().Endpoint(endpoint.ID)
require.NoError(t, err)
_, teamStillPresent := updated.TeamAccessPolicies[team.ID]
assert.False(t, teamStillPresent, "deleted team must be removed from endpoint TeamAccessPolicies")
_, userStillPresent := updated.UserAccessPolicies[user.ID]
assert.False(t, userStillPresent, "team member must be removed from endpoint UserAccessPolicies")
})
t.Run("team member in another team retains user access policy", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
deletedTeam := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(deletedTeam))
otherTeam := &portainer.Team{Name: "ops"}
require.NoError(t, store.Team().Create(otherTeam))
user := &portainer.User{Username: "alice", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: user.ID, TeamID: deletedTeam.ID, Role: portainer.TeamMember,
}))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 2, UserID: user.ID, TeamID: otherTeam.ID, Role: portainer.TeamMember,
}))
endpoint := &portainer.Endpoint{
ID: 1,
Name: "k8s",
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{
deletedTeam.ID: {RoleID: 1},
otherTeam.ID: {RoleID: 1},
},
UserAccessPolicies: portainer.UserAccessPolicies{
user.ID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h := newTestTeamHandler(t, store)
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", deletedTeam.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
updated, err := store.Endpoint().Endpoint(endpoint.ID)
require.NoError(t, err)
_, deletedTeamStillPresent := updated.TeamAccessPolicies[deletedTeam.ID]
assert.False(t, deletedTeamStillPresent, "deleted team must be removed from endpoint TeamAccessPolicies")
_, otherTeamStillPresent := updated.TeamAccessPolicies[otherTeam.ID]
assert.True(t, otherTeamStillPresent, "other team must remain in endpoint TeamAccessPolicies")
_, userStillPresent := updated.UserAccessPolicies[user.ID]
assert.True(t, userStillPresent, "team member must retain user access policy when still in another team with access")
})
}
func TestTeamDelete_cleansUpK8sServiceAccountBindings(t *testing.T) {
t.Parallel()
const (
crbName = "portainer-crb-user"
rbName = "portainer-rb-test-default"
)
t.Run("removes SA from CRB and RoleBinding when member loses all access", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
team := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(team))
user := &portainer.User{Username: "alice", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: user.ID, TeamID: team.ID, Role: portainer.TeamMember,
}))
saName := kcli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: "portainer"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: "default"},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
)
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{team.ID: {RoleID: 1}},
UserAccessPolicies: portainer.UserAccessPolicies{},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h := newTestTeamHandler(t, store)
h.K8sClientFactory = kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s))
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", team.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "team member SA must be removed from shared CRB when team is deleted")
gotRB, err := fakeK8s.RbacV1().RoleBindings("default").Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "team member SA must be removed from namespace RoleBinding when team is deleted")
})
t.Run("preserves SA bindings when member has direct endpoint access", func(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
team := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(team))
user := &portainer.User{Username: "alice", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
require.NoError(t, store.TeamMembership().Create(&portainer.TeamMembership{
ID: 1, UserID: user.ID, TeamID: team.ID, Role: portainer.TeamMember,
}))
saName := kcli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: "portainer"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: "default"},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "portainer-cr-user"},
},
)
endpoint := &portainer.Endpoint{
ID: 1,
Type: portainer.AgentOnKubernetesEnvironment,
TeamAccessPolicies: portainer.TeamAccessPolicies{team.ID: {RoleID: 1}},
UserAccessPolicies: portainer.UserAccessPolicies{user.ID: {RoleID: 1}},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h := newTestTeamHandler(t, store)
h.K8sClientFactory = kcli.NewTestClientFactory(endpoint.ID, kcli.NewTestKubeClient(fakeK8s))
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", team.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
saInCRB := false
for _, s := range gotCRB.Subjects {
if s.Name == saName {
saInCRB = true
break
}
}
assert.True(t, saInCRB, "SA must remain in CRB when member still has direct endpoint access")
})
}
func TestTeamDelete_removesTeamFromEndpointGroupAccessPolicies(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
team := &portainer.Team{Name: "dev"}
require.NoError(t, store.Team().Create(team))
group := &portainer.EndpointGroup{
Name: "prod",
TeamAccessPolicies: portainer.TeamAccessPolicies{
team.ID: {RoleID: 1},
},
}
require.NoError(t, store.EndpointGroup().Create(group))
h := newTestTeamHandler(t, store)
req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/teams/%d", team.ID), nil)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
require.Equal(t, http.StatusNoContent, rr.Code)
updated, err := store.EndpointGroup().Read(group.ID)
require.NoError(t, err)
_, stillPresent := updated.TeamAccessPolicies[team.ID]
assert.False(t, stillPresent, "deleted team must be removed from endpoint group TeamAccessPolicies")
}
+4
View File
@@ -8,6 +8,8 @@ import (
"github.com/portainer/portainer/api/apikey"
"github.com/portainer/portainer/api/dataservices"
"github.com/portainer/portainer/api/http/security"
"github.com/portainer/portainer/api/internal/authorization"
"github.com/portainer/portainer/api/kubernetes/cli"
httperror "github.com/portainer/portainer/pkg/libhttp/error"
"github.com/gorilla/mux"
@@ -36,6 +38,8 @@ type Handler struct {
AdminCreationDone chan<- struct{}
FileService portainer.FileService
SetupToken string
AuthorizationService *authorization.Service
K8sClientFactory *cli.ClientFactory
}
// NewHandler creates a handler to manage user operations.
+51 -9
View File
@@ -5,10 +5,13 @@ import (
"net/http"
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"
httperror "github.com/portainer/portainer/pkg/libhttp/error"
"github.com/portainer/portainer/pkg/libhttp/request"
"github.com/portainer/portainer/pkg/libhttp/response"
"github.com/rs/zerolog/log"
)
// @id UserDelete
@@ -84,14 +87,24 @@ func (handler *Handler) deleteAdminUser(w http.ResponseWriter, user *portainer.U
}
func (handler *Handler) deleteUser(w http.ResponseWriter, user *portainer.User) *httperror.HandlerError {
err := handler.DataStore.User().Delete(user.ID)
if err != nil {
return httperror.InternalServerError("Unable to remove user from the database", err)
}
handler.cleanupUserK8sServiceAccounts(user.ID)
err = handler.DataStore.TeamMembership().DeleteTeamMembershipByUserID(user.ID)
if err != nil {
return httperror.InternalServerError("Unable to remove user memberships from the database", err)
if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error {
if err := handler.AuthorizationService.RemoveUserAccessPolicies(tx, user.ID); err != nil {
return err
}
if err := tx.User().Delete(user.ID); err != nil {
return err
}
if err := tx.TeamMembership().DeleteTeamMembershipByUserID(user.ID); err != nil {
return err
}
return nil
}); err != nil {
return httperror.InternalServerError("Unable to remove user from the database", err)
}
// Remove all of the users persisted API keys
@@ -100,11 +113,40 @@ func (handler *Handler) deleteUser(w http.ResponseWriter, user *portainer.User)
return httperror.InternalServerError("Unable to retrieve user API keys from the database", err)
}
for _, k := range apiKeys {
err = handler.apiKeyService.DeleteAPIKey(k.ID)
if err != nil {
if err := handler.apiKeyService.DeleteAPIKey(k.ID); err != nil {
return httperror.InternalServerError("Unable to remove user API key from the database", err)
}
}
return response.Empty(w)
}
func (handler *Handler) cleanupUserK8sServiceAccounts(userID portainer.UserID) {
if handler.K8sClientFactory == nil {
return
}
var endpoints []portainer.Endpoint
if err := handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error {
var txErr error
endpoints, txErr = tx.Endpoint().ReadAll(func(e portainer.Endpoint) bool {
return endpointutils.IsKubernetesEndpoint(&e)
})
return txErr
}); err != nil {
log.Error().Err(err).Int("user_id", int(userID)).Msg("failed fetching K8s environments for user cleanup")
return
}
for i := range endpoints {
kubecli, err := handler.K8sClientFactory.GetPrivilegedKubeClient(&endpoints[i])
if err != nil {
log.Error().Err(err).Int("environment_id", int(endpoints[i].ID)).Msg("failed getting kube client for environment")
continue
}
if err := kubecli.RemoveUserServiceAccount(int(userID)); err != nil {
log.Error().Err(err).Int("environment_id", int(endpoints[i].ID)).Msg("failed removing service account for user")
}
}
}
+92 -1
View File
@@ -7,11 +7,102 @@ import (
portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/datastore"
cli "github.com/portainer/portainer/api/kubernetes/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kfake "k8s.io/client-go/kubernetes/fake"
)
func Test_deleteUserRemovesUserFromEndpointAccessPolicies(t *testing.T) {
t.Parallel()
_, store := datastore.MustNewTestStore(t, true, true)
user := &portainer.User{ID: 2, Username: "standard", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
endpoint := &portainer.Endpoint{
ID: 1,
Name: "test-k8s",
Type: portainer.AgentOnKubernetesEnvironment,
UserAccessPolicies: portainer.UserAccessPolicies{
user.ID: {RoleID: 1},
},
}
require.NoError(t, store.Endpoint().Create(endpoint))
h, _, _ := newTestHandler(t, store)
rr := httptest.NewRecorder()
handleErr := h.deleteUser(rr, user)
require.Nil(t, handleErr)
assert.Equal(t, http.StatusNoContent, rr.Code)
updated, err := store.Endpoint().Endpoint(endpoint.ID)
require.NoError(t, err)
_, userStillPresent := updated.UserAccessPolicies[user.ID]
assert.False(t, userStillPresent, "deleted user should be removed from endpoint access policies")
}
func Test_deleteUserCleansUpK8sServiceAccount(t *testing.T) {
t.Parallel()
const (
crbName = "portainer-crb-user"
rbName = "portainer-rb-test-default"
ns = "default"
)
_, store := datastore.MustNewTestStore(t, true, true)
user := &portainer.User{Username: "standard", Role: portainer.StandardUserRole}
require.NoError(t, store.User().Create(user))
endpoint := &portainer.Endpoint{Type: portainer.AgentOnKubernetesEnvironment}
require.NoError(t, store.Endpoint().Create(endpoint))
saName := cli.UserServiceAccountName(int(user.ID), "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: "portainer"}
fakeK8s := kfake.NewSimpleClientset(
&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: "portainer"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}},
&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: crbName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "cluster-admin"},
},
&rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: ns},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
},
)
h, _, _ := newTestHandler(t, store)
h.K8sClientFactory = cli.NewTestClientFactory(endpoint.ID, cli.NewTestKubeClient(fakeK8s))
rr := httptest.NewRecorder()
handleErr := h.deleteUser(rr, user)
require.Nil(t, handleErr)
assert.Equal(t, http.StatusNoContent, rr.Code)
_, err := fakeK8s.CoreV1().ServiceAccounts("portainer").Get(t.Context(), saName, metav1.GetOptions{})
assert.True(t, k8serrors.IsNotFound(err), "SA object must be deleted when user is deleted")
gotCRB, err := fakeK8s.RbacV1().ClusterRoleBindings().Get(t.Context(), crbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "user SA must be removed from shared CRB")
gotRB, err := fakeK8s.RbacV1().RoleBindings(ns).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "user SA must be removed from namespace RoleBinding")
}
func Test_deleteUserRemovesAccessTokens(t *testing.T) {
t.Parallel()
is := assert.New(t)
+3
View File
@@ -261,6 +261,7 @@ func (server *Server) Start(ctx context.Context) error {
var teamHandler = teams.NewHandler(requestBouncer)
teamHandler.DataStore = server.DataStore
teamHandler.K8sClientFactory = server.KubernetesClientFactory
var teamMembershipHandler = teammemberships.NewHandler(requestBouncer)
teamMembershipHandler.DataStore = server.DataStore
@@ -286,6 +287,8 @@ func (server *Server) Start(ctx context.Context) error {
userHandler.AdminCreationDone = server.AdminCreationDone
userHandler.FileService = server.FileService
userHandler.SetupToken = server.SetupToken
userHandler.AuthorizationService = server.AuthorizationService
userHandler.K8sClientFactory = server.KubernetesClientFactory
var websocketHandler = websocket.NewHandler(server.KubernetesTokenCacheManager, requestBouncer)
websocketHandler.DataStore = server.DataStore
+51
View File
@@ -261,6 +261,57 @@ func (kcl *KubeClient) removeNamespaceAccessForServiceAccount(serviceAccountName
return err
}
func (kcl *KubeClient) RemoveUserServiceAccountBindings(userID int) error {
serviceAccountName := UserServiceAccountName(userID, kcl.instanceID)
namespaces, err := kcl.cli.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
if err != nil {
return err
}
for _, ns := range namespaces.Items {
if err := kcl.removeNamespaceAccessForServiceAccount(serviceAccountName, ns.Name); err != nil {
return err
}
}
return kcl.removeServiceAccountFromUserClusterRoleBinding(serviceAccountName)
}
func (kcl *KubeClient) RemoveUserServiceAccount(userID int) error {
if err := kcl.RemoveUserServiceAccountBindings(userID); err != nil {
return err
}
serviceAccountName := UserServiceAccountName(userID, kcl.instanceID)
if err := kcl.cli.CoreV1().ServiceAccounts(portainerNamespace).Delete(context.TODO(), serviceAccountName, metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) {
return err
}
return nil
}
func (kcl *KubeClient) removeServiceAccountFromUserClusterRoleBinding(serviceAccountName string) error {
crb, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(context.TODO(), portainerUserCRBName, metav1.GetOptions{})
if k8serrors.IsNotFound(err) {
return nil
}
if err != nil {
return err
}
updated := crb.Subjects[:0]
for _, s := range crb.Subjects {
if s.Name != serviceAccountName {
updated = append(updated, s)
}
}
crb.Subjects = updated
_, err = kcl.cli.RbacV1().ClusterRoleBindings().Update(context.TODO(), crb, metav1.UpdateOptions{})
return err
}
func (kcl *KubeClient) AddImagePullSecretToServiceAccount(namespace, serviceAccountName, secretName string) error {
sa, err := kcl.cli.CoreV1().ServiceAccounts(namespace).Get(context.TODO(), serviceAccountName, metav1.GetOptions{})
if err != nil {
+183
View File
@@ -7,6 +7,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kfake "k8s.io/client-go/kubernetes/fake"
)
@@ -420,3 +422,184 @@ func TestUpdateServiceAccountImagePullSecrets(t *testing.T) {
assert.Equal(t, "s2", sa.ImagePullSecrets[1].Name)
})
}
func TestRemoveServiceAccountFromUserClusterRoleBinding(t *testing.T) {
t.Parallel()
saName := UserServiceAccountName(1, "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: portainerNamespace}
t.Run("removes SA from CRB when it is the only subject", func(t *testing.T) {
t.Parallel()
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: portainerUserCRBName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: portainerUserCRName},
}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(crb), instanceID: "test"}
require.NoError(t, kcl.removeServiceAccountFromUserClusterRoleBinding(saName))
got, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(t.Context(), portainerUserCRBName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, got.Subjects)
})
t.Run("preserves other subjects when removing target SA", func(t *testing.T) {
t.Parallel()
otherSAName := UserServiceAccountName(2, "test")
otherSubject := rbacv1.Subject{Kind: "ServiceAccount", Name: otherSAName, Namespace: portainerNamespace}
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: portainerUserCRBName},
Subjects: []rbacv1.Subject{subject, otherSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: portainerUserCRName},
}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(crb), instanceID: "test"}
require.NoError(t, kcl.removeServiceAccountFromUserClusterRoleBinding(saName))
got, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(t.Context(), portainerUserCRBName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, got.Subjects, 1)
assert.Equal(t, otherSAName, got.Subjects[0].Name)
})
t.Run("is a no-op when CRB does not exist", func(t *testing.T) {
t.Parallel()
kcl := &KubeClient{cli: kfake.NewSimpleClientset(), instanceID: "test"}
require.NoError(t, kcl.removeServiceAccountFromUserClusterRoleBinding(saName))
})
}
func TestRemoveUserServiceAccountBindings(t *testing.T) {
t.Parallel()
const (
userID = 1
namespace = "default"
)
saName := UserServiceAccountName(userID, "test")
rbName := namespaceClusterRoleBindingName(namespace, "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: portainerNamespace}
t.Run("removes SA from namespace RoleBinding and shared CRB but keeps SA object", func(t *testing.T) {
t.Parallel()
sa := &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: portainerNamespace}}
ns := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: portainerUserCRBName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: portainerUserCRName},
}
rb := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: namespace},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(sa, ns, crb, rb), instanceID: "test"}
require.NoError(t, kcl.RemoveUserServiceAccountBindings(userID))
_, err := kcl.cli.CoreV1().ServiceAccounts(portainerNamespace).Get(t.Context(), saName, metav1.GetOptions{})
require.NoError(t, err, "SA object must not be deleted on access revocation")
gotCRB, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(t.Context(), portainerUserCRBName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "SA must be removed from shared CRB")
gotRB, err := kcl.cli.RbacV1().RoleBindings(namespace).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "SA must be removed from namespace RoleBinding")
})
t.Run("preserves other users subjects when removing one user", func(t *testing.T) {
t.Parallel()
otherSAName := UserServiceAccountName(2, "test")
otherSubject := rbacv1.Subject{Kind: "ServiceAccount", Name: otherSAName, Namespace: portainerNamespace}
ns := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: portainerUserCRBName},
Subjects: []rbacv1.Subject{subject, otherSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: portainerUserCRName},
}
rb := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: namespace},
Subjects: []rbacv1.Subject{subject, otherSubject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(ns, crb, rb), instanceID: "test"}
require.NoError(t, kcl.RemoveUserServiceAccountBindings(userID))
gotCRB, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(t.Context(), portainerUserCRBName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, gotCRB.Subjects, 1)
assert.Equal(t, otherSAName, gotCRB.Subjects[0].Name)
gotRB, err := kcl.cli.RbacV1().RoleBindings(namespace).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
require.Len(t, gotRB.Subjects, 1)
assert.Equal(t, otherSAName, gotRB.Subjects[0].Name)
})
}
func TestRemoveUserServiceAccount(t *testing.T) {
t.Parallel()
const (
userID = 1
namespace = "default"
)
saName := UserServiceAccountName(userID, "test")
rbName := namespaceClusterRoleBindingName(namespace, "test")
subject := rbacv1.Subject{Kind: "ServiceAccount", Name: saName, Namespace: portainerNamespace}
t.Run("deletes SA object and removes it from all bindings", func(t *testing.T) {
t.Parallel()
sa := &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName, Namespace: portainerNamespace}}
ns := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
crb := &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: portainerUserCRBName},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: portainerUserCRName},
}
rb := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: rbName, Namespace: namespace},
Subjects: []rbacv1.Subject{subject},
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: "edit"},
}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(sa, ns, crb, rb), instanceID: "test"}
require.NoError(t, kcl.RemoveUserServiceAccount(userID))
_, err := kcl.cli.CoreV1().ServiceAccounts(portainerNamespace).Get(t.Context(), saName, metav1.GetOptions{})
assert.True(t, k8serrors.IsNotFound(err), "SA object must be deleted on user deletion")
gotCRB, err := kcl.cli.RbacV1().ClusterRoleBindings().Get(t.Context(), portainerUserCRBName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotCRB.Subjects, "SA must be removed from shared CRB")
gotRB, err := kcl.cli.RbacV1().RoleBindings(namespace).Get(t.Context(), rbName, metav1.GetOptions{})
require.NoError(t, err)
assert.Empty(t, gotRB.Subjects, "SA must be removed from namespace RoleBinding")
})
t.Run("is idempotent when SA does not exist", func(t *testing.T) {
t.Parallel()
ns := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
kcl := &KubeClient{cli: kfake.NewSimpleClientset(ns), instanceID: "test"}
require.NoError(t, kcl.RemoveUserServiceAccount(userID))
})
}
+2
View File
@@ -1984,6 +1984,8 @@ type (
RemoveImagePullSecretFromServiceAccount(namespace, serviceAccountName, secretName string) error
UpdateServiceAccountImagePullSecrets(namespace, name string, secretNames []string) error
SetupUserServiceAccount(int, []int, bool) error
RemoveUserServiceAccountBindings(userID int) error
RemoveUserServiceAccount(userID int) error
GetPortainerUserServiceAccount(tokendata *TokenData) (*corev1.ServiceAccount, error)
GetServiceAccountBearerToken(userID int) (string, error)