Add non-deletable service user (#1311)

* Add non-deletable flag for service users

* fix non deletable service user created as deletable

* Exclude non deletable service users in service users api response

* Fix broken tests

* Add test for non deletable service user

* Add handling for non-deletable service users in tests

* Remove non-deletable service users when fetching all users

* Ensure non-deletable users are filtered out when fetching all user data

management/server/account.go

@@ -211,6 +211,7 @@ type UserInfo struct {
 	Status               string               `json:"-"`
 	IsServiceUser        bool                 `json:"is_service_user"`
 	IsBlocked            bool                 `json:"is_blocked"`
+	NonDeletable         bool                 `json:"non_deletable"`
 	LastLogin            time.Time            `json:"last_login"`
 	Issued               string               `json:"issued"`
 	IntegrationReference IntegrationReference `json:"-"`

management/server/http/users_handler.go

@@ -197,10 +197,14 @@ func (h *UsersHandler) GetAllUsers(w http.ResponseWriter, r *http.Request) {
 
 	users := make([]*api.User, 0)
 	for _, r := range data {
+		if r.NonDeletable {
+			continue
+		}
 		if serviceUser == "" {
 			users = append(users, toUserResponse(r, claims.UserId))
 			continue
 		}
+
 		includeServiceUser, err := strconv.ParseBool(serviceUser)
 		log.Debugf("Should include service user: %v", includeServiceUser)
 		if err != nil {

management/server/http/users_handler_test.go

@@ -20,8 +20,9 @@ import (
 )
 
 const (
-	serviceUserID = "serviceUserID"
+	serviceUserID             = "serviceUserID"
-	regularUserID = "regularUserID"
+	nonDeletableServiceUserID = "nonDeletableServiceUserID"
+	regularUserID             = "regularUserID"
 )
 
 var usersTestAccount = &server.Account{
@@ -49,6 +50,13 @@ var usersTestAccount = &server.Account{
 			AutoGroups:    []string{"group_1"},
 			Issued:        server.UserIssuedAPI,
 		},
+		nonDeletableServiceUserID: {
+			Id:            serviceUserID,
+			Role:          "admin",
+			IsServiceUser: true,
+			NonDeletable:  true,
+			Issued:        server.UserIssuedIntegration,
+		},
 	},
 }
 
@@ -67,6 +75,7 @@ func initUsersTestData() *UsersHandler {
 						Name:          "",
 						Email:         "",
 						IsServiceUser: v.IsServiceUser,
+						NonDeletable:  v.NonDeletable,
 						Issued:        v.Issued,
 					})
 				}

management/server/user.go

@@ -69,6 +69,8 @@ type User struct {
 	AccountID     string `json:"-" gorm:"index"`
 	Role          UserRole
 	IsServiceUser bool
+	// NonDeletable indicates whether the service user can be deleted
+	NonDeletable bool
 	// ServiceUserName is only set if IsServiceUser is true
 	ServiceUserName string
 	// AutoGroups is a list of Group IDs to auto-assign to peers registered by this user
@@ -158,6 +160,7 @@ func (u *User) Copy() *User {
 		Role:                 u.Role,
 		AutoGroups:           autoGroups,
 		IsServiceUser:        u.IsServiceUser,
+		NonDeletable:         u.NonDeletable,
 		ServiceUserName:      u.ServiceUserName,
 		PATs:                 pats,
 		Blocked:              u.Blocked,
@@ -168,11 +171,12 @@ func (u *User) Copy() *User {
 }
 
 // NewUser creates a new user
-func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName string, autoGroups []string, issued string) *User {
+func NewUser(id string, role UserRole, isServiceUser bool, nonDeletable bool, serviceUserName string, autoGroups []string, issued string) *User {
 	return &User{
 		Id:              id,
 		Role:            role,
 		IsServiceUser:   isServiceUser,
+		NonDeletable:    nonDeletable,
 		ServiceUserName: serviceUserName,
 		AutoGroups:      autoGroups,
 		Issued:          issued,
@@ -181,16 +185,16 @@ func NewUser(id string, role UserRole, isServiceUser bool, serviceUserName strin
 
 // NewRegularUser creates a new user with role UserRoleUser
 func NewRegularUser(id string) *User {
-	return NewUser(id, UserRoleUser, false, "", []string{}, UserIssuedAPI)
+	return NewUser(id, UserRoleUser, false, false, "", []string{}, UserIssuedAPI)
 }
 
 // NewAdminUser creates a new user with role UserRoleAdmin
 func NewAdminUser(id string) *User {
-	return NewUser(id, UserRoleAdmin, false, "", []string{}, UserIssuedAPI)
+	return NewUser(id, UserRoleAdmin, false, false, "", []string{}, UserIssuedAPI)
 }
 
 // createServiceUser creates a new service user under the given account.
-func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUserID string, role UserRole, serviceUserName string, autoGroups []string) (*UserInfo, error) {
+func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUserID string, role UserRole, serviceUserName string, nonDeletable bool, autoGroups []string) (*UserInfo, error) {
 	unlock := am.Store.AcquireAccountLock(accountID)
 	defer unlock()
 
@@ -208,7 +212,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs
 	}
 
 	newUserID := uuid.New().String()
-	newUser := NewUser(newUserID, role, true, serviceUserName, autoGroups, UserIssuedAPI)
+	newUser := NewUser(newUserID, role, true, nonDeletable, serviceUserName, autoGroups, UserIssuedAPI)
 	log.Debugf("New User: %v", newUser)
 	account.Users[newUserID] = newUser
 
@@ -236,7 +240,7 @@ func (am *DefaultAccountManager) createServiceUser(accountID string, initiatorUs
 // CreateUser creates a new user under the given account. Effectively this is a user invite.
 func (am *DefaultAccountManager) CreateUser(accountID, userID string, user *UserInfo) (*UserInfo, error) {
 	if user.IsServiceUser {
-		return am.createServiceUser(accountID, userID, StrRoleToUserRole(user.Role), user.Name, user.AutoGroups)
+		return am.createServiceUser(accountID, userID, StrRoleToUserRole(user.Role), user.Name, user.NonDeletable, user.AutoGroups)
 	}
 	return am.inviteNewUser(accountID, userID, user)
 }
@@ -420,6 +424,10 @@ func (am *DefaultAccountManager) DeleteUser(accountID, initiatorUserID string, t
 
 	// handle service user first and exit, no need to fetch extra data from IDP, etc
 	if targetUser.IsServiceUser {
+		if targetUser.NonDeletable {
+			return status.Errorf(status.PermissionDenied, "service user is marked as non-deletable")
+		}
+
 		am.deleteServiceUser(account, initiatorUserID, targetUser)
 		return am.Store.SaveAccount(account)
 	}
@@ -955,6 +963,7 @@ func (am *DefaultAccountManager) GetUsersFromAccount(accountID, userID string) (
 				AutoGroups:    localUser.AutoGroups,
 				Status:        string(UserStatusActive),
 				IsServiceUser: localUser.IsServiceUser,
+				NonDeletable:  localUser.NonDeletable,
 			}
 		}
 		userInfos = append(userInfos, info)

management/server/user_test.go

@@ -332,7 +332,7 @@ func TestUser_CreateServiceUser(t *testing.T) {
 		eventStore: &activity.InMemoryEventStore{},
 	}
 
-	user, err := am.createServiceUser(mockAccountID, mockUserID, mockRole, mockServiceUserName, []string{"group1", "group2"})
+	user, err := am.createServiceUser(mockAccountID, mockUserID, mockRole, mockServiceUserName, false, []string{"group1", "group2"})
 	if err != nil {
 		t.Fatalf("Error when creating service user: %s", err)
 	}
@@ -413,31 +413,62 @@ func TestUser_CreateUser_RegularUser(t *testing.T) {
 }
 
 func TestUser_DeleteUser_ServiceUser(t *testing.T) {
-	store := newStore(t)
+	tests := []struct {
-	account := newAccountWithId(mockAccountID, mockUserID, "")
+		name             string
-	account.Users[mockServiceUserID] = &User{
+		serviceUser      *User
-		Id:              mockServiceUserID,
+		assertErrFunc    assert.ErrorAssertionFunc
-		IsServiceUser:   true,
+		assertErrMessage string
-		ServiceUserName: mockServiceUserName,
+	}{
+		{
+			name: "Can delete service user",
+			serviceUser: &User{
+				Id:              mockServiceUserID,
+				IsServiceUser:   true,
+				ServiceUserName: mockServiceUserName,
+			},
+			assertErrFunc: assert.NoError,
+		},
+		{
+			name: "Cannot delete non-deletable service user",
+			serviceUser: &User{
+				Id:              mockServiceUserID,
+				IsServiceUser:   true,
+				ServiceUserName: mockServiceUserName,
+				NonDeletable:    true,
+			},
+			assertErrFunc:    assert.Error,
+			assertErrMessage: "service user is marked as non-deletable",
+		},
 	}
 
-	err := store.SaveAccount(account)
+	for _, tt := range tests {
-	if err != nil {
+		t.Run(tt.name, func(t *testing.T) {
-		t.Fatalf("Error when saving account: %s", err)
+			store := newStore(t)
-	}
+			account := newAccountWithId(mockAccountID, mockUserID, "")
+			account.Users[mockServiceUserID] = tt.serviceUser
 
-	am := DefaultAccountManager{
+			err := store.SaveAccount(account)
-		Store:      store,
+			if err != nil {
-		eventStore: &activity.InMemoryEventStore{},
+				t.Fatalf("Error when saving account: %s", err)
-	}
+			}
 
-	err = am.DeleteUser(mockAccountID, mockUserID, mockServiceUserID)
+			am := DefaultAccountManager{
-	if err != nil {
+				Store:      store,
-		t.Fatalf("Error when deleting user: %s", err)
+				eventStore: &activity.InMemoryEventStore{},
-	}
+			}
 
-	assert.Equal(t, 1, len(store.Accounts[mockAccountID].Users))
+			err = am.DeleteUser(mockAccountID, mockUserID, mockServiceUserID)
-	assert.Nil(t, store.Accounts[mockAccountID].Users[mockServiceUserID])
+			tt.assertErrFunc(t, err, tt.assertErrMessage)
+
+			if err != nil {
+				assert.Equal(t, 2, len(store.Accounts[mockAccountID].Users))
+				assert.NotNil(t, store.Accounts[mockAccountID].Users[mockServiceUserID])
+			} else {
+				assert.Equal(t, 1, len(store.Accounts[mockAccountID].Users))
+				assert.Nil(t, store.Accounts[mockAccountID].Users[mockServiceUserID])
+			}
+		})
+	}
 }
 
 func TestUser_DeleteUser_SelfDelete(t *testing.T) {