fix: deleted api token can be reused if created again with same name (#4978)

* introuddced api token versioning

* fix

* reverted wire_gen

* migration udpated and minor refactor

* refactor

* moved const from apiToken pkg to user

* reverted wire_gen

* refactor

* reverted wire_gen

* concurrency case handled

* fix

* commented wherever necessary

* refactor

* fix

* refactor

* refactor

* wip

* refactor

* added comments and minor refactor

* refactor

* refactoring

* fix

* added comments around cyclic import

* added few more comments

* sql script no updated

Author: ashishdevtron

pkg/apiToken/ApiTokenBean.go

@@ -0,0 +1,9 @@
+package apiToken
+
+import "github.com/golang-jwt/jwt/v4"
+
+type ApiTokenCustomClaims struct {
+	Email   string `json:"email"`
+	Version string `json:"version"`
+	jwt.RegisteredClaims
+}

pkg/apiToken/ApiTokenRepository.go

@@ -18,6 +18,7 @@
 package apiToken
 
 import (
+	"fmt"
 	"github.com/devtron-labs/devtron/pkg/auth/user/repository"
 	"github.com/devtron-labs/devtron/pkg/sql"
 	"github.com/go-pg/pg"
@@ -29,6 +30,7 @@ type ApiToken struct {
 	Id           int      `sql:"id,pk"`
 	UserId       int32    `sql:"user_id, notnull"`
 	Name         string   `sql:"name, notnull"`
+	Version      int      `sql:"version, notnull"`
 	Description  string   `sql:"description, notnull"`
 	ExpireAtInMs int64    `sql:"expire_at_in_ms"`
 	Token        string   `sql:"token, notnull"`
@@ -42,6 +44,7 @@ type ApiTokenRepository interface {
 	FindAllActive() ([]*ApiToken, error)
 	FindActiveById(id int) (*ApiToken, error)
 	FindByName(name string) (*ApiToken, error)
+	UpdateIf(apiToken *ApiToken, previousTokenVersion int) error
 }
 
 type ApiTokenRepositoryImpl struct {
@@ -60,6 +63,20 @@ func (impl ApiTokenRepositoryImpl) Update(apiToken *ApiToken) error {
 	return impl.dbConnection.Update(apiToken)
 }
 
+func (impl ApiTokenRepositoryImpl) UpdateIf(apiToken *ApiToken, previousTokenVersion int) error {
+	res, err := impl.dbConnection.Model(apiToken).
+		Where("id = ?", apiToken.Id).
+		Where("version = ?", previousTokenVersion).
+		Update()
+	if err != nil {
+		return err
+	}
+	if res.RowsAffected() == 0 {
+		return fmt.Errorf(TokenVersionMismatch)
+	}
+	return nil
+}
+
 func (impl ApiTokenRepositoryImpl) FindAllActive() ([]*ApiToken, error) {
 	var apiTokens []*ApiToken
 	err := impl.dbConnection.Model(&apiTokens).

pkg/apiToken/ApiTokenService.go

@@ -20,6 +20,7 @@ package apiToken
 import (
 	"errors"
 	"fmt"
+	userBean "github.com/devtron-labs/devtron/pkg/auth/user/bean"
 	"regexp"
 	"strconv"
 	"strings"
@@ -62,14 +63,13 @@ func NewApiTokenServiceImpl(logger *zap.SugaredLogger, apiTokenSecretService Api
 	}
 }
 
-const API_TOKEN_USER_EMAIL_PREFIX = "API-TOKEN:"
-
 var invalidCharsInApiTokenName = regexp.MustCompile("[,\\s]")
 
-type ApiTokenCustomClaims struct {
-	Email string `json:"email"`
-	jwt.RegisteredClaims
-}
+const (
+	ConcurrentTokenUpdateRequest  = "there is an ongoing request for the token with the same name, please try again after some time"
+	UniqueKeyViolationPgErrorCode = 23505
+	TokenVersionMismatch          = "token version mismatch"
+)
 
 func (impl ApiTokenServiceImpl) GetAllApiTokensForWebhook(projectName string, environmentName string, appName string, auth func(token string, projectObject string, envObject string) bool) ([]*openapi.ApiToken, error) {
 	impl.logger.Info("Getting active api tokens")
@@ -181,11 +181,21 @@ func (impl ApiTokenServiceImpl) CreateApiToken(request *openapi.CreateApiTokenRe
 
 	impl.logger.Info(fmt.Sprintf("apiTokenExists : %s", strconv.FormatBool(apiTokenExists)))
 
-	// step-2 - Build email
-	email := fmt.Sprintf("%s%s", API_TOKEN_USER_EMAIL_PREFIX, name)
+	// step-2 - Build email and version
+	email := fmt.Sprintf("%s%s", userBean.API_TOKEN_USER_EMAIL_PREFIX, name)
+	var (
+		tokenVersion         int
+		previousTokenVersion int
+	)
+	if apiTokenExists {
+		tokenVersion = apiToken.Version + 1
+		previousTokenVersion = apiToken.Version
+	} else {
+		tokenVersion = 1
+	}
 
 	// step-3 - Build token
-	token, err := impl.createApiJwtToken(email, *request.ExpireAtInMs)
+	token, err := impl.createApiJwtToken(email, tokenVersion, *request.ExpireAtInMs)
 	if err != nil {
 		return nil, err
 	}
@@ -214,21 +224,37 @@ func (impl ApiTokenServiceImpl) CreateApiToken(request *openapi.CreateApiTokenRe
 		Description:  *request.Description,
 		ExpireAtInMs: *request.ExpireAtInMs,
 		Token:        token,
+		Version:      tokenVersion,
 		AuditLog:     sql.AuditLog{UpdatedOn: time.Now()},
 	}
 	if apiTokenExists {
 		apiTokenSaveRequest.Id = apiToken.Id
 		apiTokenSaveRequest.CreatedBy = apiToken.CreatedBy
 		apiTokenSaveRequest.CreatedOn = apiToken.CreatedOn
 		apiTokenSaveRequest.UpdatedBy = createdBy
-		err = impl.apiTokenRepository.Update(apiTokenSaveRequest)
+		// update api-token only if `previousTokenVersion` is same as version stored in DB
+		// we are checking this to ensure that two users are not updating the same token at the same time
+		err = impl.apiTokenRepository.UpdateIf(apiTokenSaveRequest, previousTokenVersion)
 	} else {
 		apiTokenSaveRequest.CreatedBy = createdBy
 		apiTokenSaveRequest.CreatedOn = time.Now()
 		err = impl.apiTokenRepository.Save(apiTokenSaveRequest)
 	}
 	if err != nil {
 		impl.logger.Errorw("error while saving api-token into DB", "error", err)
+		// fetching error code from pg error for Unique key violation constraint
+		// in case of save
+		pgErr, ok := err.(pg.Error)
+		if ok {
+			errCode, conversionErr := strconv.Atoi(pgErr.Field('C'))
+			if conversionErr == nil && errCode == UniqueKeyViolationPgErrorCode {
+				return nil, fmt.Errorf(ConcurrentTokenUpdateRequest)
+			}
+		}
+		// in case of update
+		if errors.Is(err, fmt.Errorf(TokenVersionMismatch)) {
+			return nil, fmt.Errorf(ConcurrentTokenUpdateRequest)
+		}
 		return nil, err
 	}
 
@@ -254,22 +280,28 @@ func (impl ApiTokenServiceImpl) UpdateApiToken(apiTokenId int, request *openapi.
 		return nil, errors.New(fmt.Sprintf("api-token corresponds to apiTokenId '%d' is not found", apiTokenId))
 	}
 
+	previousTokenVersion := apiToken.Version
+	tokenVersion := apiToken.Version + 1
+
 	// step-2 - If expires_at is not same, then token needs to be generated again
 	if *request.ExpireAtInMs != apiToken.ExpireAtInMs {
 		// regenerate token
-		token, err := impl.createApiJwtToken(apiToken.User.EmailId, *request.ExpireAtInMs)
+		token, err := impl.createApiJwtToken(apiToken.User.EmailId, tokenVersion, *request.ExpireAtInMs)
 		if err != nil {
 			return nil, err
 		}
 		apiToken.Token = token
+		apiToken.Version = tokenVersion
 	}
 
 	// step-3 - update in DB
 	apiToken.Description = *request.Description
 	apiToken.ExpireAtInMs = *request.ExpireAtInMs
 	apiToken.UpdatedBy = updatedBy
 	apiToken.UpdatedOn = time.Now()
-	err = impl.apiTokenRepository.Update(apiToken)
+	// update api-token only if `previousTokenVersion` is same as version stored in DB
+	// we are checking this to ensure that two users are not updating the same token at the same time
+	err = impl.apiTokenRepository.UpdateIf(apiToken, previousTokenVersion)
 	if err != nil {
 		impl.logger.Errorw("error while updating api-token", "apiTokenId", apiTokenId, "error", err)
 		return nil, err
@@ -322,24 +354,41 @@ func (impl ApiTokenServiceImpl) DeleteApiToken(apiTokenId int, deletedBy int32)
 
 }
 
-func (impl ApiTokenServiceImpl) createApiJwtToken(email string, expireAtInMs int64) (string, error) {
+func (impl ApiTokenServiceImpl) createApiJwtToken(email string, tokenVersion int, expireAtInMs int64) (string, error) {
+	registeredClaims, secretByteArr, err := impl.setRegisteredClaims(expireAtInMs)
+	if err != nil {
+		return "", err
+	}
+	claims := &ApiTokenCustomClaims{
+		email,
+		strconv.Itoa(tokenVersion),
+		registeredClaims,
+	}
+	token, err := impl.generateToken(claims, secretByteArr)
+	if err != nil {
+		return "", err
+	}
+	return token, nil
+}
+
+func (impl ApiTokenServiceImpl) setRegisteredClaims(expireAtInMs int64) (jwt.RegisteredClaims, []byte, error) {
 	secretByteArr, err := impl.apiTokenSecretService.GetApiTokenSecretByteArr()
 	if err != nil {
 		impl.logger.Errorw("error while getting api token secret", "error", err)
-		return "", err
+		return jwt.RegisteredClaims{}, secretByteArr, err
 	}
 
 	registeredClaims := jwt.RegisteredClaims{
 		Issuer: middleware.ApiTokenClaimIssuer,
 	}
+
 	if expireAtInMs > 0 {
 		registeredClaims.ExpiresAt = jwt.NewNumericDate(time.Unix(expireAtInMs/1000, 0))
 	}
+	return registeredClaims, secretByteArr, nil
+}
 
-	claims := &ApiTokenCustomClaims{
-		email,
-		registeredClaims,
-	}
+func (impl ApiTokenServiceImpl) generateToken(claims *ApiTokenCustomClaims, secretByteArr []byte) (string, error) {
 	unsignedToken := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
 	token, err := unsignedToken.SignedString(secretByteArr)
 	if err != nil {

pkg/auth/user/UserAuthService.go

@@ -31,7 +31,7 @@ import (
 
 	"github.com/devtron-labs/authenticator/middleware"
 	casbin2 "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
-	bean2 "github.com/devtron-labs/devtron/pkg/auth/user/bean"
+	userBean "github.com/devtron-labs/devtron/pkg/auth/user/bean"
 	"github.com/devtron-labs/devtron/pkg/auth/user/repository"
 	"github.com/go-pg/pg"
 
@@ -473,7 +473,7 @@ func (impl UserAuthServiceImpl) AuthVerification(r *http.Request) (bool, error)
 		}
 		return false, err
 	}
-	emailId, err := impl.userService.GetEmailFromToken(token)
+	emailId, version, err := impl.userService.GetEmailAndVersionFromToken(token)
 	if err != nil {
 		impl.logger.Errorw("AuthVerification failed ", "error", err)
 		return false, err
@@ -488,22 +488,33 @@ func (impl UserAuthServiceImpl) AuthVerification(r *http.Request) (bool, error)
 		}
 		return false, err
 	}
+	// checking length of version, to ensure backward compatibility as earlier we did not
+	// have version for api-tokens
+	// therefore, for tokens without version we will skip the below part
+	if strings.HasPrefix(emailId, userBean.API_TOKEN_USER_EMAIL_PREFIX) && len(version) > 0 {
+		err := impl.userService.CheckIfTokenIsValid(emailId, version)
+		if err != nil {
+			impl.logger.Errorw("token is not valid", "error", err, "token", token)
+			return false, err
+		}
+	}
 
 	//TODO - extends for other purpose
 	return true, nil
 }
+
 func (impl UserAuthServiceImpl) DeleteRoles(entityType string, entityName string, tx *pg.Tx, envIdentifier string, workflowName string) (err error) {
 	var roleModels []*repository.RoleModel
 	switch entityType {
-	case bean2.PROJECT_TYPE:
+	case userBean.PROJECT_TYPE:
 		roleModels, err = impl.userAuthRepository.GetRolesForProject(entityName)
-	case bean2.ENV_TYPE:
+	case userBean.ENV_TYPE:
 		roleModels, err = impl.userAuthRepository.GetRolesForEnvironment(entityName, envIdentifier)
-	case bean2.APP_TYPE:
+	case userBean.APP_TYPE:
 		roleModels, err = impl.userAuthRepository.GetRolesForApp(entityName)
-	case bean2.CHART_GROUP_TYPE:
+	case userBean.CHART_GROUP_TYPE:
 		roleModels, err = impl.userAuthRepository.GetRolesForChartGroup(entityName)
-	case bean2.WorkflowType:
+	case userBean.WorkflowType:
 		roleModels, err = impl.userAuthRepository.GetRolesForWorkflow(workflowName, entityName)
 	}
 	if err != nil {