Refactor Actions Token Access (#35688)

* use a single function to do Action Tokens Permission checks
* allows easier customization
* add basic tests
* lfs file locks should work now

---------

Signed-off-by: ChristopherHX <christopher.homberger@web.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
ChristopherHX
2025-10-22 13:12:31 +02:00
committed by GitHub
parent a9f2ea720b
commit 08b9776970
15 changed files with 218 additions and 168 deletions

View File

@@ -8,7 +8,6 @@ import (
"fmt"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
@@ -42,30 +41,6 @@ func (err ErrLFSLockNotExist) Unwrap() error {
return util.ErrNotExist
}
// ErrLFSUnauthorizedAction represents a "LFSUnauthorizedAction" kind of error.
type ErrLFSUnauthorizedAction struct {
RepoID int64
UserName string
Mode perm.AccessMode
}
// IsErrLFSUnauthorizedAction checks if an error is a ErrLFSUnauthorizedAction.
func IsErrLFSUnauthorizedAction(err error) bool {
_, ok := err.(ErrLFSUnauthorizedAction)
return ok
}
func (err ErrLFSUnauthorizedAction) Error() string {
if err.Mode == perm.AccessModeWrite {
return fmt.Sprintf("User %s doesn't have write access for lfs lock [rid: %d]", err.UserName, err.RepoID)
}
return fmt.Sprintf("User %s doesn't have read access for lfs lock [rid: %d]", err.UserName, err.RepoID)
}
func (err ErrLFSUnauthorizedAction) Unwrap() error {
return util.ErrPermissionDenied
}
// ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error.
type ErrLFSLockAlreadyExist struct {
RepoID int64
@@ -93,12 +68,6 @@ type ErrLFSFileLocked struct {
UserName string
}
// IsErrLFSFileLocked checks if an error is a ErrLFSFileLocked.
func IsErrLFSFileLocked(err error) bool {
_, ok := err.(ErrLFSFileLocked)
return ok
}
func (err ErrLFSFileLocked) Error() string {
return fmt.Sprintf("File is lfs locked [repo: %d, locked by: %s, path: %s]", err.RepoID, err.UserName, err.Path)
}

View File

@@ -11,10 +11,7 @@ import (
"time"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
@@ -71,10 +68,6 @@ func (l *LFSLock) LoadOwner(ctx context.Context) error {
// CreateLFSLock creates a new lock.
func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) {
return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) {
if err := CheckLFSAccessForRepo(ctx, lock.OwnerID, repo, perm.AccessModeWrite); err != nil {
return nil, err
}
lock.Path = util.PathJoinRel(lock.Path)
lock.RepoID = repo.ID
@@ -165,10 +158,6 @@ func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repositor
return nil, err
}
if err := CheckLFSAccessForRepo(ctx, u.ID, repo, perm.AccessModeWrite); err != nil {
return nil, err
}
if !force && u.ID != lock.OwnerID {
return nil, errors.New("user doesn't own lock and force flag is not set")
}
@@ -180,22 +169,3 @@ func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repositor
return lock, nil
})
}
// CheckLFSAccessForRepo check needed access mode base on action
func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model.Repository, mode perm.AccessMode) error {
if ownerID == 0 {
return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode}
}
u, err := user_model.GetUserByID(ctx, ownerID)
if err != nil {
return err
}
perm, err := access_model.GetUserRepoPermission(ctx, repo, u)
if err != nil {
return err
}
if !perm.CanAccess(mode, unit.TypeCode) {
return ErrLFSUnauthorizedAction{repo.ID, u.DisplayName(), mode}
}
return nil
}

View File

@@ -5,9 +5,11 @@ package access
import (
"context"
"errors"
"fmt"
"slices"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
perm_model "code.gitea.io/gitea/models/perm"
@@ -253,6 +255,34 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) {
}
}
// GetActionsUserRepoPermission returns the actions user permissions to the repository
func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) {
if actionsUser.ID != user_model.ActionsUserID {
return perm, errors.New("api GetActionsUserRepoPermission can only be called by the actions user")
}
task, err := actions_model.GetTaskByID(ctx, taskID)
if err != nil {
return perm, err
}
if task.RepoID != repo.ID {
// FIXME allow public repo read access if tokenless pull is enabled
return perm, nil
}
var accessMode perm_model.AccessMode
if task.IsForkPullRequest {
accessMode = perm_model.AccessModeRead
} else {
accessMode = perm_model.AccessModeWrite
}
if err := repo.LoadUnits(ctx); err != nil {
return perm, err
}
perm.SetUnitsWithDefaultAccessMode(repo.Units, accessMode)
return perm, nil
}
// GetUserRepoPermission returns the user permissions to the repository
func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) {
defer func() {

View File

@@ -249,8 +249,13 @@ func (u *User) MaxCreationLimit() int {
}
// CanCreateRepoIn checks whether the doer(u) can create a repository in the owner
// NOTE: functions calling this assume a failure due to repository count limit; it ONLY checks the repo number LIMIT, if new checks are added, those functions should be revised
// NOTE: functions calling this assume a failure due to repository count limit, or the owner is not a real user.
// It ONLY checks the repo number LIMIT or whether owner user is real. If new checks are added, those functions should be revised.
// TODO: the callers can only return ErrReachLimitOfRepo, need to fine tune to support other error types in the future.
func (u *User) CanCreateRepoIn(owner *User) bool {
if u.ID <= 0 || owner.ID <= 0 {
return false // fake user like Ghost or Actions user
}
if u.IsAdmin {
return true
}

View File

@@ -48,17 +48,16 @@ func IsGiteaActionsUserName(name string) bool {
// NewActionsUser creates and returns a fake user for running the actions.
func NewActionsUser() *User {
return &User{
ID: ActionsUserID,
Name: ActionsUserName,
LowerName: ActionsUserName,
IsActive: true,
FullName: "Gitea Actions",
Email: ActionsUserEmail,
KeepEmailPrivate: true,
LoginName: ActionsUserName,
Type: UserTypeBot,
AllowCreateOrganization: true,
Visibility: structs.VisibleTypePublic,
ID: ActionsUserID,
Name: ActionsUserName,
LowerName: ActionsUserName,
IsActive: true,
FullName: "Gitea Actions",
Email: ActionsUserEmail,
KeepEmailPrivate: true,
LoginName: ActionsUserName,
Type: UserTypeBot,
Visibility: structs.VisibleTypePublic,
}
}

View File

@@ -648,33 +648,36 @@ func TestGetInactiveUsers(t *testing.T) {
func TestCanCreateRepo(t *testing.T) {
defer test.MockVariableValue(&setting.Repository.MaxCreationLimit)()
const noLimit = -1
doerNormal := &user_model.User{}
doerAdmin := &user_model.User{IsAdmin: true}
doerActions := user_model.NewActionsUser()
doerNormal := &user_model.User{ID: 2}
doerAdmin := &user_model.User{ID: 1, IsAdmin: true}
t.Run("NoGlobalLimit", func(t *testing.T) {
setting.Repository.MaxCreationLimit = noLimit
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit}))
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100}))
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit}))
assert.False(t, doerActions.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit}))
assert.False(t, doerAdmin.CanCreateRepoIn(doerActions))
})
t.Run("GlobalLimit50", func(t *testing.T) {
setting.Repository.MaxCreationLimit = 50
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit}))
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit})) // limited by global limit
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit}))
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: noLimit})) // limited by global limit
assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: noLimit}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100}))
assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: 100}))
})
}