feat(cli): allow updates to runners' secrets

This commit allows the `forgejo-cli actions register` command to change
an existing runner's secret, as discussed in #4610.

It refactors `RegisterRunner` to extract the code that hashes the token,
moving this code to a method called `UpdateSecret` on `ActionRunner`.
A test for the method has been added.

The `RegisterRunner` function is updated so that:
- it relies on `ActionRunner.UpdateSecret` when creating new runners,
- it checks whether an existing runner's secret still matches the one
  passed on the command line,
- it updates the runner's secret if it wasn't created and it no longer
  matches.

A test has been added for the new behaviour.
This commit is contained in:
Emmanuel BENOÎT 2024-07-22 11:55:43 +02:00
parent fdb1874ada
commit 320ab7ed7f
No known key found for this signature in database
4 changed files with 88 additions and 12 deletions

View File

@ -4,7 +4,7 @@ package actions
import (
"context"
"encoding/hex"
"crypto/subtle"
"fmt"
auth_model "code.gitea.io/gitea/models/auth"
@ -26,25 +26,30 @@ func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, la
has, err := db.GetEngine(ctx).Where("uuid=?", uuidString).Get(&runner)
if err != nil {
return nil, fmt.Errorf("GetRunner %v", err)
} else if !has {
}
var mustUpdateSecret bool
if has {
//
// The runner exists, check if the rest of the token has changed.
//
mustUpdateSecret = subtle.ConstantTimeCompare(
[]byte(runner.TokenHash),
[]byte(auth_model.HashToken(token, runner.TokenSalt)),
) != 1
} else {
//
// The runner does not exist yet, create it
//
saltBytes, err := util.CryptoRandomBytes(16)
if err != nil {
return nil, fmt.Errorf("CryptoRandomBytes %v", err)
}
salt := hex.EncodeToString(saltBytes)
hash := auth_model.HashToken(token, salt)
runner = ActionRunner{
UUID: uuidString,
TokenHash: hash,
TokenSalt: salt,
AgentLabels: []string{},
}
if err := runner.UpdateSecret(token); err != nil {
return &runner, fmt.Errorf("can't set new runner's secret: %w", err)
}
if err := CreateRunner(ctx, &runner); err != nil {
return &runner, fmt.Errorf("can't create new runner %w", err)
}
@ -64,6 +69,12 @@ func RegisterRunner(ctx context.Context, ownerID, repoID int64, token string, la
runner.AgentLabels = *labels
cols = append(cols, "agent_labels")
}
if mustUpdateSecret {
if err := runner.UpdateSecret(token); err != nil {
return &runner, fmt.Errorf("can't change runner's secret: %w", err)
}
cols = append(cols, "token_hash", "token_salt")
}
if err := UpdateRunner(ctx, &runner, cols...); err != nil {
return &runner, fmt.Errorf("can't update the runner %+v %w", runner, err)

View File

@ -11,6 +11,7 @@ import (
"code.gitea.io/gitea/models/unittest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestActions_RegisterRunner_Token(t *testing.T) {
@ -28,6 +29,36 @@ func TestActions_RegisterRunner_Token(t *testing.T) {
assert.EqualValues(t, 1, subtle.ConstantTimeCompare([]byte(runner.TokenHash), []byte(auth_model.HashToken(token, runner.TokenSalt))), "the token cannot be verified with the same method as routers/api/actions/runner/interceptor.go as of 8228751c55d6a4263f0fec2932ca16181c09c97d")
}
// TestActions_RegisterRunner_TokenUpdate tests that a token's secret is updated
// when a runner already exists and RegisterRunner is called with a token
// parameter whose first 16 bytes match that record but where the last 24 bytes
// do not match.
func TestActions_RegisterRunner_TokenUpdate(t *testing.T) {
const recordID = 12345678
oldToken := "7e577e577e577e57feedfacefeedfacefeedface"
newToken := "7e577e577e577e57deadbeefdeadbeefdeadbeef"
assert.NoError(t, unittest.PrepareTestDatabase())
before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
require.Equal(t,
before.TokenHash, auth_model.HashToken(oldToken, before.TokenSalt),
"the initial token should match the runner's secret",
)
RegisterRunner(db.DefaultContext, before.OwnerID, before.RepoID, newToken, nil, before.Name, before.Version)
after := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
assert.Equal(t, before.UUID, after.UUID)
assert.NotEqual(t,
after.TokenHash, auth_model.HashToken(oldToken, after.TokenSalt),
"the old token can still be verified",
)
assert.Equal(t,
after.TokenHash, auth_model.HashToken(newToken, after.TokenSalt),
"the new token cannot be verified",
)
}
func TestActions_RegisterRunner_CreateWithLabels(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
ownerID := int64(0)

View File

@ -6,10 +6,12 @@ package actions
import (
"context"
"encoding/binary"
"encoding/hex"
"fmt"
"strings"
"time"
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/shared/types"
@ -151,6 +153,22 @@ func (r *ActionRunner) GenerateToken() (err error) {
return err
}
// UpdateSecret updates the hash based on the specified token. It does not
// ensure that the runner's UUID matches the first 16 bytes of the token.
func (r *ActionRunner) UpdateSecret(token string) error {
saltBytes, err := util.CryptoRandomBytes(16)
if err != nil {
return fmt.Errorf("CryptoRandomBytes %v", err)
}
salt := hex.EncodeToString(saltBytes)
r.Token = token
r.TokenSalt = salt
r.TokenHash = auth_model.HashToken(token, salt)
return nil
}
func init() {
db.RegisterModel(&ActionRunner{})
}

View File

@ -7,12 +7,28 @@ import (
"fmt"
"testing"
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestUpdateSecret checks that ActionRunner.UpdateSecret() sets the Token,
// TokenSalt and TokenHash fields based on the specified token.
func TestUpdateSecret(t *testing.T) {
runner := ActionRunner{}
token := "0123456789012345678901234567890123456789"
err := runner.UpdateSecret(token)
require.NoError(t, err)
assert.Equal(t, token, runner.Token)
assert.Regexp(t, "^[0-9a-f]{32}$", runner.TokenSalt)
assert.Equal(t, runner.TokenHash, auth_model.HashToken(token, runner.TokenSalt))
}
func TestDeleteRunner(t *testing.T) {
const recordID = 12345678
assert.NoError(t, unittest.PrepareTestDatabase())