review comments

devtron-labs · Feb 12, 2024 · 0e50351 · 0e50351
1 parent 0a8a53e
commit 0e50351
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 15 deletions.
diff --git a/api/auth/user/UserRestHandler.go b/api/auth/user/UserRestHandler.go
@@ -466,6 +466,14 @@ func (handler UserRestHandlerImpl) BulkDeleteUsers(w http.ResponseWriter, r *htt
 	// setting logged in user Id for audit logs
 	request.LoggedInUserId = userId
 
+	// validations for system and admin user
+	err = helper.CheckValidationForAdminAndSystemUserId(request.Ids)
+	if err != nil {
+		handler.logger.Errorw("request err, BulkDeleteUsers, validation failed", "payload", request, "err", err)
+		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		return
+	}
+
 	// RBAC enforcer applying
 	token := r.Header.Get("token")
 	if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok {
@@ -479,13 +487,6 @@ func (handler UserRestHandlerImpl) BulkDeleteUsers(w http.ResponseWriter, r *htt
 		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
 		return
 	}
-	// validations for system and admin user
-	err = helper.CheckValidationForAdminAndSystemUserId(request.Ids)
-	if err != nil {
-		handler.logger.Errorw("request err, BulkDeleteUsers, validation failed", "payload", request, "err", err)
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
-		return
-	}
 	// service call
 	res, err := handler.userService.BulkDeleteUsers(request)
 	if err != nil {
@@ -840,12 +841,6 @@ func (handler UserRestHandlerImpl) BulkDeleteRoleGroups(w http.ResponseWriter, r
 	// setting logged in user Id for audit logs
 	request.LoggedInUserId = userId
 
-	// RBAC enforcer applying
-	token := r.Header.Get("token")
-	if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok {
-		common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden)
-		return
-	}
 	// struct validation
 	err = handler.validator.Struct(request)
 	if err != nil {
@@ -854,6 +849,13 @@ func (handler UserRestHandlerImpl) BulkDeleteRoleGroups(w http.ResponseWriter, r
 		return
 	}
 
+	// RBAC enforcer applying
+	token := r.Header.Get("token")
+	if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok {
+		common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden)
+		return
+	}
+
 	// service call
 	res, err := handler.roleGroupService.BulkDeleteRoleGroups(request)
 	if err != nil {

diff --git a/pkg/auth/user/RoleGroupService.go b/pkg/auth/user/RoleGroupService.go
@@ -826,7 +826,7 @@ func (impl RoleGroupServiceImpl) deleteRoleGroupsByIds(request *bean.BulkDeleteR
 	tx, err := impl.roleGroupRepository.StartATransaction()
 	if err != nil {
 		impl.logger.Errorw("error in starting a transaction", "err", err)
-		return err
+		return &util.ApiError{Code: "500", HttpStatusCode: 500, UserMessage: "error starting a transaction in db", InternalMessage: "error starting a transaction in db"}
 	}
 	// Rollback tx on error.
 	defer tx.Rollback()
@@ -860,7 +860,7 @@ func (impl RoleGroupServiceImpl) deleteRoleGroupsByIds(request *bean.BulkDeleteR
 	err = impl.roleGroupRepository.CommitATransaction(tx)
 	if err != nil {
 		impl.logger.Errorw("error in committing a transaction in deleteRoleGroupsByIds", "err", err)
-		return err
+		return &util.ApiError{Code: "500", HttpStatusCode: 500, UserMessage: "error committing a transaction in db", InternalMessage: "error committing a transaction in db"}
 	}
 	return nil