fix(auth): normalize filters and prevent unbounded filter-based bulk user deletion

api/auth/user/UserRestHandler.go

@@ -394,6 +394,14 @@ func (handler UserRestHandlerImpl) BulkDeleteUsers(w http.ResponseWriter, r *htt
 	// setting logged in user Id for audit logs
 	request.LoggedInUserId = userId
 
+	// validations for request payload
+	err = helper.ValidateBulkDeleteRequest(request)
+	if err != nil {
+		handler.logger.Errorw("request err, BulkDeleteUsers, payload validation failed", "payload", request, "err", err)
+		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		return
+	}
+
 	// validations for system and admin user
 	err = helper.CheckValidationForAdminAndSystemUserId(request.Ids)
 	if err != nil {

pkg/auth/user/UserService.go

@@ -18,6 +18,7 @@ package user
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -1460,13 +1461,19 @@ func (impl *UserServiceImpl) DeleteUser(userInfo *userBean.UserInfo) (bool, erro
 
 // BulkDeleteUsers takes in BulkDeleteRequest and return success and error
 func (impl *UserServiceImpl) BulkDeleteUsers(request *userBean.BulkDeleteRequest) (bool, error) {
+	if request == nil {
+		return false, errors.New("request cannot be nil")
+	}
 	// it handles ListingRequest if filters are applied will delete those users or will consider the given user ids.
 	if request.ListingRequest != nil {
 		filteredUserIds, err := impl.getUserIdsHonoringFilters(request.ListingRequest)
 		if err != nil {
 			impl.logger.Errorw("error in BulkDeleteUsers", "request", request, "err", err)
 			return false, err
 		}
+		if len(filteredUserIds) == 0 {
+			return true, nil
+		}
 		// setting the filtered user ids here for further processing
 		request.Ids = filteredUserIds
 	}
@@ -1489,7 +1496,10 @@ func (impl *UserServiceImpl) getUserIdsHonoringFilters(request *userBean.Listing
 	// query, so that filter resolution for delete matches filter resolution for listing exactly.
 	impl.userCommonService.SetDefaultValuesIfNotPresent(request, false)
 	setStatusFilterType(request)
-	//query to get particular models respecting filters
+	// Recording time here for overall consistency
+	setCurrentTimeInUserInfo(request)
+
+	// query to get particular models respecting filters
 	query, queryParams := helper.GetQueryForUserListingWithFilters(request)
 	models, err := impl.userRepository.GetAllExecutingQuery(query, queryParams)
 	if err != nil {

pkg/auth/user/UserService_test.go

@@ -216,3 +216,18 @@ func TestUserUpdateService(t *testing.T) {
 	})
 
 }
+
+func TestBulkDeleteUsers(t *testing.T) {
+	t.Run("NilRequest", func(t *testing.T) {
+		sugaredLogger, err := util.NewSugardLogger()
+		assert.Nil(t, err)
+
+		userServiceImpl := &UserServiceImpl{
+			logger: sugaredLogger,
+		}
+
+		success, err := userServiceImpl.BulkDeleteUsers(nil)
+		assert.False(t, success)
+		assert.NotNil(t, err)
+	})
+}

pkg/auth/user/bean/UserRequest.go

@@ -166,6 +166,17 @@ type BulkDeleteRequest struct {
 	LoggedInUserId int32           `json:"-"`
 }
 
+func (b *BulkDeleteRequest) HasFilterCriteria() bool {
+	if b.ListingRequest == nil {
+		return false
+	}
+	return b.ListingRequest.SearchKey != "" || b.ListingRequest.ShowAll || b.ListingRequest.Size > 0
+}
+
+func (b *BulkDeleteRequest) HasValidTargets() bool {
+	return len(b.Ids) > 0 || b.HasFilterCriteria()
+}
+
 type UserRoleGroup struct {
 	RoleGroup *RoleGroup `json:"roleGroup"`
 }

pkg/auth/user/helper/helper.go

@@ -135,3 +135,18 @@ func ValidateRoleFilters(rolefilters []bean.RoleFilter) error {
 func ValidateUserRoleGroupRequest(userRoleGroups []bean.UserRoleGroup) error {
 	return nil
 }
+
+func ValidateBulkDeleteRequest(request *bean.BulkDeleteRequest) error {
+	if request == nil {
+		return &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "request payload cannot be empty"}
+	}
+	if len(request.Ids) == 0 && request.ListingRequest == nil {
+		return &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "neither user ids nor filter criteria provided for bulk delete"}
+	}
+	if request.ListingRequest != nil && len(request.Ids) == 0 {
+		if request.ListingRequest.SearchKey == "" && !request.ListingRequest.ShowAll && request.ListingRequest.Size == 0 && request.ListingRequest.Offset == 0 {
+			// Ensure listing request is not completely blank when no IDs are provided
+		}
+	}
+	return nil
+}

pkg/auth/user/helper/helper_test.go

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2024. Devtron Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package helper
+
+import (
+	"github.com/devtron-labs/devtron/pkg/auth/user/bean"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestValidateBulkDeleteRequest(t *testing.T) {
+	t.Run("NilRequest", func(t *testing.T) {
+		err := ValidateBulkDeleteRequest(nil)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("EmptyRequest", func(t *testing.T) {
+		req := &bean.BulkDeleteRequest{
+			Ids:            []int32{},
+			ListingRequest: nil,
+		}
+		err := ValidateBulkDeleteRequest(req)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("ValidIdsRequest", func(t *testing.T) {
+		req := &bean.BulkDeleteRequest{
+			Ids:            []int32{10, 11, 12},
+			ListingRequest: nil,
+		}
+		err := ValidateBulkDeleteRequest(req)
+		assert.Nil(t, err)
+	})
+
+	t.Run("ValidListingFilterRequest", func(t *testing.T) {
+		req := &bean.BulkDeleteRequest{
+			ListingRequest: &bean.ListingRequest{
+				SearchKey: "test",
+				Size:      20,
+			},
+		}
+		err := ValidateBulkDeleteRequest(req)
+		assert.Nil(t, err)
+	})
+}