Redefine the meaning of column is_active to make Actions Registration Token generation easier (#27143) (#27304)

Backport #27143 by @lunny

Partially Fix #25041

This PR redefined the meaning of column `is_active` in table
`action_runner_token`.
Before this PR, `is_active` means whether it has been used by any
runner. If it's true, other runner cannot use it to register again.

In this PR, `is_active` means whether it's validated to be used to
register runner. And if it's true, then it can be used to register
runners until it become false. When creating a new `is_active` register
token, any previous tokens will be set `is_active` to false.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/actions/runner_token.go

@@ -22,7 +22,7 @@ type ActionRunnerToken struct {
 	Owner    *user_model.User       `xorm:"-"`
 	RepoID   int64                  `xorm:"index"` // repo level runner, if orgid also is zero, then it's a global
 	Repo     *repo_model.Repository `xorm:"-"`
-	IsActive bool
+	IsActive bool                   // true means it can be used
 
 	Created timeutil.TimeStamp `xorm:"created"`
 	Updated timeutil.TimeStamp `xorm:"updated"`
@@ -57,7 +57,7 @@ func UpdateRunnerToken(ctx context.Context, r *ActionRunnerToken, cols ...string
 	return err
 }
 
-// NewRunnerToken creates a new runner token
+// NewRunnerToken creates a new active runner token and invalidate all old tokens
 func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
 	token, err := util.CryptoRandomString(40)
 	if err != nil {
@@ -66,17 +66,27 @@ func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerTo
 	runnerToken := &ActionRunnerToken{
 		OwnerID:  ownerID,
 		RepoID:   repoID,
-		IsActive: false,
+		IsActive: true,
 		Token:    token,
 	}
-	_, err = db.GetEngine(ctx).Insert(runnerToken)
+
-	return runnerToken, err
+	return runnerToken, db.WithTx(ctx, func(ctx context.Context) error {
+		if _, err := db.GetEngine(ctx).Where("owner_id =? AND repo_id = ?", ownerID, repoID).Cols("is_active").Update(&ActionRunnerToken{
+			IsActive: false,
+		}); err != nil {
+			return err
+		}
+
+		_, err = db.GetEngine(ctx).Insert(runnerToken)
+		return err
+	})
 }
 
-// GetUnactivatedRunnerToken returns a unactivated runner token
+// GetLastestRunnerToken returns the latest runner token
-func GetUnactivatedRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
+func GetLastestRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
 	var runnerToken ActionRunnerToken
-	has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=? AND is_active=?", ownerID, repoID, false).OrderBy("id DESC").Get(&runnerToken)
+	has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=?", ownerID, repoID).
+		OrderBy("id DESC").Get(&runnerToken)
 	if err != nil {
 		return nil, err
 	} else if !has {

routers/api/actions/runner/runner.go

@@ -47,11 +47,11 @@ func (s *Service) Register(
 
 	runnerToken, err := actions_model.GetRunnerToken(ctx, req.Msg.Token)
 	if err != nil {
-		return nil, errors.New("runner token not found")
+		return nil, errors.New("runner registration token not found")
 	}
 
-	if runnerToken.IsActive {
+	if !runnerToken.IsActive {
-		return nil, errors.New("runner token has already been activated")
+		return nil, errors.New("runner registration token has been invalidated, please use the latest one")
 	}
 
 	labels := req.Msg.Labels

routers/private/actions.go

@@ -41,8 +41,8 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
 		})
 	}
 
-	token, err := actions_model.GetUnactivatedRunnerToken(ctx, owner, repo)
+	token, err := actions_model.GetLastestRunnerToken(ctx, owner, repo)
-	if errors.Is(err, util.ErrNotExist) {
+	if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
 		token, err = actions_model.NewRunnerToken(ctx, owner, repo)
 		if err != nil {
 			err := fmt.Sprintf("error while creating runner token: %v", err)

routers/web/shared/actions/runners.go

@@ -35,15 +35,15 @@ func RunnersList(ctx *context.Context, opts actions_model.FindRunnerOptions) {
 
 	// ownid=0,repo_id=0,means this token is used for global
 	var token *actions_model.ActionRunnerToken
-	token, err = actions_model.GetUnactivatedRunnerToken(ctx, opts.OwnerID, opts.RepoID)
+	token, err = actions_model.GetLastestRunnerToken(ctx, opts.OwnerID, opts.RepoID)
-	if errors.Is(err, util.ErrNotExist) {
+	if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
 		token, err = actions_model.NewRunnerToken(ctx, opts.OwnerID, opts.RepoID)
 		if err != nil {
 			ctx.ServerError("CreateRunnerToken", err)
 			return
 		}
 	} else if err != nil {
-		ctx.ServerError("GetUnactivatedRunnerToken", err)
+		ctx.ServerError("GetLastestRunnerToken", err)
 		return
 	}