fix: API token generation api responses refactoring (#6788)

* fix: API token generation api responses refactoring

* fix: register custom validation against tag for api token name validations

* fix: register custom validation against tag for api token name validations

* Revert "fix: register custom validation against tag for api token name validations"

This reverts commit 7593c27.

* fix: remove `required` validation from Description and expiryAtInMs

* fix: adding resource conflict api response in WriteJsonResp utility

* fix: path params int validation updated to whole numbers only

* fix: handled resource not found response for update and delete api, token

Author: SATYAsasini

api/apiToken/ApiTokenRestHandler.go

@@ -18,11 +18,6 @@ package apiToken
 
 import (
 	"encoding/json"
-	"net/http"
-	"strconv"
-	"strings"
-	"time"
-
 	openapi "github.com/devtron-labs/devtron/api/openapi/openapiClient"
 	"github.com/devtron-labs/devtron/api/restHandler/common"
 	"github.com/devtron-labs/devtron/pkg/apiToken"
@@ -32,6 +27,8 @@ import (
 	"github.com/juju/errors"
 	"go.uber.org/zap"
 	"gopkg.in/go-playground/validator.v9"
+	"net/http"
+	"strconv"
 )
 
 type ApiTokenRestHandler interface {
@@ -105,65 +102,18 @@ func (impl ApiTokenRestHandlerImpl) CreateApiToken(w http.ResponseWriter, r *htt
 	err = decoder.Decode(&request)
 	if err != nil {
 		impl.logger.Errorw("err in decoding request in CreateApiToken", "err", err)
-		common.WriteJsonResp(w, errors.New("invalid JSON payload"), nil, http.StatusBadRequest)
+		common.WriteJsonResp(w, errors.New("invalid JSON payload "+err.Error()), nil, http.StatusBadRequest)
 		return
 	}
 
 	// validate request structure
 	err = impl.validator.Struct(request)
 	if err != nil {
-		impl.logger.Errorw("validation err in CreateApiToken", "err", err, "request", request)
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
-		return
-	}
-
-	// Comprehensive validation with specific error messages
-	if request.Name == nil || *request.Name == "" {
-		common.WriteJsonResp(w, errors.New("name field is required and cannot be empty"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Check name length (max 100 characters)
-	if len(*request.Name) > 100 {
-		common.WriteJsonResp(w, errors.New("name field cannot exceed 100 characters"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Check for invalid characters in name (spaces, commas)
-	if strings.Contains(*request.Name, " ") || strings.Contains(*request.Name, ",") {
-		common.WriteJsonResp(w, errors.New("name field cannot contain spaces or commas"), nil, http.StatusBadRequest)
+		impl.logger.Errorw("validation err in CreateApiToken ", "err", err, "request", request)
+		common.HandleValidationErrors(w, r, err)
 		return
 	}
 
-	// Check description length (max 350 characters as per UI)
-	if request.Description != nil && len(*request.Description) > 350 {
-		common.WriteJsonResp(w, errors.New("description field cannot exceed 350 characters"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Validate expireAtInMs field
-	if request.ExpireAtInMs != nil {
-		// Check if it's a valid positive timestamp
-		if *request.ExpireAtInMs <= 0 {
-			common.WriteJsonResp(w, errors.New("expireAtInMs must be a positive timestamp in milliseconds"), nil, http.StatusBadRequest)
-			return
-		}
-
-		// Check if it's not in the past (allow 1 minute buffer for clock skew)
-		currentTime := time.Now().UnixMilli()
-		if *request.ExpireAtInMs < (currentTime - 60000) {
-			common.WriteJsonResp(w, errors.New("expireAtInMs cannot be in the past"), nil, http.StatusBadRequest)
-			return
-		}
-
-		// Check if it's not too far in the future (max 10 years)
-		maxFutureTime := currentTime + (10 * 365 * 24 * 60 * 60 * 1000)
-		if *request.ExpireAtInMs > maxFutureTime {
-			common.WriteJsonResp(w, errors.New("expireAtInMs cannot be more than 10 years in the future"), nil, http.StatusBadRequest)
-			return
-		}
-	}
-
 	// service call
 	res, err := impl.apiTokenService.CreateApiToken(request, userId, impl.checkManagerAuth)
 	if err != nil {

api/openapi/openapiClient/model_create_api_token_request.go

@@ -17,9 +17,11 @@
 package common
 
 import (
+	"errors"
 	"fmt"
 	"github.com/devtron-labs/devtron/api/middleware"
 	"github.com/devtron-labs/devtron/internal/util"
+	"gopkg.in/go-playground/validator.v9"
 	"net/http"
 	"strconv"
 )
@@ -226,3 +228,41 @@ func HandleValidationError(w http.ResponseWriter, r *http.Request, fieldName, me
 	apiErr := util.NewValidationErrorForField(fieldName, message)
 	WriteJsonResp(w, apiErr, nil, apiErr.HttpStatusCode)
 }
+
+// HandleValidationErrors handles multiple validation errors
+func HandleValidationErrors(w http.ResponseWriter, r *http.Request, err error) {
+	// validator.ValidationErrors is a slice
+	var vErrs validator.ValidationErrors
+	if errors.As(err, &vErrs) {
+		for _, fe := range vErrs {
+			field := fe.Field()
+			message := validationMessage(fe)
+			HandleValidationError(w, r, field, message)
+			return
+		}
+	}
+
+	// fallback: generic
+	HandleValidationError(w, r, "request", "invalid request payload")
+}
+func validationMessage(fe validator.FieldError) string {
+	switch fe.Tag() {
+	// validation tag for api token name
+	case "validate-api-token-name":
+		return fmt.Sprintf(
+			"%s must start and end with a lowercase letter or digit; may only contain lowercase letters, digits, '_' or '-' (no spaces or commas)",
+			fe.Field(),
+		)
+
+	// if a certain validator tag is not included in switch case then,
+	// we will parse the error as generic validator error,
+	// and further divide them on basis of parametric and non-parametric validation tags
+	default:
+		if fe.Param() != "" {
+			// generic parametric fallback (e.g., min=3, max=50)
+			return fmt.Sprintf("%s failed validation rule '%s=%s'", fe.Field(), fe.Tag(), fe.Param())
+		}
+		// generic non-parametric fallback (e.g., required, email, uuid)
+		return fmt.Sprintf("%s failed validation rule '%s'", fe.Field(), fe.Tag())
+	}
+}

api/restHandler/common/EnhancedErrorResponse.go

@@ -71,7 +71,7 @@ func convertToIntWithContext(w http.ResponseWriter, paramValue, paramName, resou
 		return 0, apiErr
 	}
 
-	if paramIntValue <= 0 {
+	if paramIntValue < 0 {
 		apiErr := util.NewValidationErrorForField(paramName, "must be a positive integer")
 		WriteJsonResp(w, apiErr, nil, apiErr.HttpStatusCode)
 		return 0, apiErr

api/restHandler/common/ParamParserUtils.go

@@ -63,6 +63,20 @@ func WriteJsonResp(w http.ResponseWriter, err error, respBody interface{}, statu
 			apiErr := util.NewGenericResourceNotFoundError()
 			response.Errors = []*util.ApiError{apiErr}
 		}
+	} else if util.IsResourceConflictError(err) {
+		// Handles response for error due to resource with the same identifier already exists.
+		status = http.StatusConflict
+		// Try to extract resource context from respBody for better error messages
+		resourceType, resourceId := extractResourceContext(respBody)
+		if resourceType != "" && resourceId != "" {
+			// Create context-aware resource duplicate error
+			apiErr := util.NewDuplicateResourceError(resourceType, resourceId)
+			response.Errors = []*util.ApiError{apiErr}
+		} else {
+			// Fallback to generic resource duplicate error
+			apiErr := util.NewGenericDuplicateResourceError()
+			response.Errors = []*util.ApiError{apiErr}
+		}
 	} else if multiErr, ok := err.(*multierror.Error); ok {
 		var errorsResp []*util.ApiError
 		for _, e := range multiErr.Errors {

api/restHandler/common/apiError.go

@@ -27,6 +27,7 @@ import (
 	"google.golang.org/grpc/status"
 	"net/http"
 	"strconv"
+	"strings"
 )
 
 type ApiError struct {
@@ -89,7 +90,19 @@ func (e *ApiError) ErrorfUser(format string, a ...interface{}) error {
 func IsErrNoRows(err error) bool {
 	return pg.ErrNoRows == err
 }
-
+func IsResourceConflictError(err error) bool {
+	var resourceConflictPhrases = []string{
+		"already exists",
+		"already used",
+	}
+	msg := err.Error()
+	for _, phrase := range resourceConflictPhrases {
+		if strings.Contains(msg, phrase) {
+			return true
+		}
+	}
+	return false
+}
 func GetClientErrorDetailedMessage(err error) string {
 	if errStatus, ok := status.FromError(err); ok {
 		return errStatus.Message()

internal/util/ErrorUtil.go

@@ -89,3 +89,13 @@ func NewGenericResourceNotFoundError() *ApiError {
 	).WithCode(constants.ResourceNotFound).
 		WithUserDetailMessage("The requested resource does not exist or has been deleted.")
 }
+
+// NewGenericDuplicateResourceError creates a generic conflict error when a resource already exists
+func NewGenericDuplicateResourceError() *ApiError {
+	return NewApiError(
+		http.StatusConflict,
+		"Resource already exists",
+		"resource already exists",
+	).WithCode(constants.DuplicateResource).
+		WithUserDetailMessage("The resource you are trying to create already exists. Please use a different name or identifier.")
+}

internal/util/ResourceErrorFactory.go

@@ -107,6 +107,7 @@ func IntValidator() (*validator.Validate, error) {
 	if err != nil {
 		return v, err
 	}
+	err = v.RegisterValidation("validate-api-token-name", validateApiTokenName)
 	return v, err
 }
 
@@ -140,3 +141,9 @@ func validateGlobalEntityName(fl validator.FieldLevel) bool {
 	hostnameRegexRFC952 := regexp.MustCompile(hostnameRegexString)
 	return hostnameRegexRFC952.MatchString(fl.Field().String())
 }
+
+func validateApiTokenName(fl validator.FieldLevel) bool {
+	hostnameRegexString := `^[a-z0-9][a-z0-9_-]*[a-z0-9]$`
+	hostnameRegexRFC952 := regexp.MustCompile(hostnameRegexString)
+	return hostnameRegexRFC952.MatchString(fl.Field().String())
+}

internal/util/ValidateUtil.go

@@ -313,7 +313,7 @@ func (impl ApiTokenServiceImpl) UpdateApiToken(apiTokenId int, request *openapi.
 		return nil, err
 	}
 	if apiToken == nil || apiToken.Id == 0 {
-		return nil, errors.New(fmt.Sprintf("api-token corresponds to apiTokenId '%d' is not found", apiTokenId))
+		return nil, pg.ErrNoRows
 	}
 
 	previousTokenVersion := apiToken.Version
@@ -361,7 +361,7 @@ func (impl ApiTokenServiceImpl) DeleteApiToken(apiTokenId int, deletedBy int32)
 		return nil, err
 	}
 	if apiToken == nil || apiToken.Id == 0 {
-		return nil, errors.New(fmt.Sprintf("api-token corresponds to apiTokenId '%d' is not found", apiTokenId))
+		return nil, pg.ErrNoRows
 	}
 
 	apiToken.ExpireAtInMs = time.Now().UnixMilli()