Redefine the meaning of column is_active to make Actions Registration Token generatio...

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>

diff --git a/models/actions/runner_token.go b/models/actions/runner_token.go
index fabd6c644c8da3d191b77f4264ef49bb841463e3..cf4e90f3f11528ff29e90f3f7bdb5e651294eb9d 100644 (file)
--- a/models/actions/runner_token.go
+++ b/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
-func GetUnactivatedRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
+// GetLastestRunnerToken returns the latest runner token
+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 {

diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go
index cb206f56851731d838f52035cd24ba433c7129f0..8df6f297ceb96d59b50e4bbb02980a731c6f8bf3 100644 (file)
--- a/routers/api/actions/runner/runner.go
+++ b/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 {
-               return nil, errors.New("runner token has already been activated")
+       if !runnerToken.IsActive {
+               return nil, errors.New("runner registration token has been invalidated, please use the latest one")
        }
 
        labels := req.Msg.Labels

diff --git a/routers/private/actions.go b/routers/private/actions.go
index 2403b9c41a0b4c236acb12cd6308ab0561ee088a..74515256da9972bb96fa1a2192c238fa7f220c71 100644 (file)
--- a/routers/private/actions.go
+++ b/routers/private/actions.go
@@ -41,8 +41,8 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
                })
        }
 
-       token, err := actions_model.GetUnactivatedRunnerToken(ctx, owner, repo)
-       if errors.Is(err, util.ErrNotExist) {
+       token, err := actions_model.GetLastestRunnerToken(ctx, owner, repo)
+       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)

diff --git a/routers/web/shared/actions/runners.go b/routers/web/shared/actions/runners.go
index 7ff1f3e33f537018029be129ccc422f2bb7a6b09..cd60e9cbee348c5e4f5085cb4e7d39bc134be6d4 100644 (file)
--- a/routers/web/shared/actions/runners.go
+++ b/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)
-       if errors.Is(err, util.ErrNotExist) {
+       token, err = actions_model.GetLastestRunnerToken(ctx, opts.OwnerID, opts.RepoID)
+       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
        }