Merge branch 'main' into fix-render-external

wxiaoguang · web-flow · commit 1c0baee901a1 · 2025-10-23T07:12:40.000+08:00
diff --git a/models/git/lfs.go b/models/git/lfs.go
@@ -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)
 }
diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go
@@ -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
-}
diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go
@@ -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() {
diff --git a/models/user/user.go b/models/user/user.go
@@ -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
 	}
diff --git a/models/user/user_system.go b/models/user/user_system.go
@@ -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,
 	}
 }
 
diff --git a/models/user/user_test.go b/models/user/user_test.go
@@ -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, 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, 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{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}))
 	})
 }
diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
@@ -70,7 +70,6 @@ import (
 	"net/http"
 	"strings"
 
-	actions_model "code.gitea.io/gitea/models/actions"
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
@@ -190,27 +189,11 @@ func repoAssignment() func(ctx *context.APIContext) {
 
 		if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID {
 			taskID := ctx.Data["ActionsTaskID"].(int64)
-			task, err := actions_model.GetTaskByID(ctx, taskID)
+			ctx.Repo.Permission, err = access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID)
 			if err != nil {
 				ctx.APIErrorInternal(err)
 				return
 			}
-			if task.RepoID != repo.ID {
-				ctx.APIErrorNotFound()
-				return
-			}
-
-			if task.IsForkPullRequest {
-				ctx.Repo.Permission.AccessMode = perm.AccessModeRead
-			} else {
-				ctx.Repo.Permission.AccessMode = perm.AccessModeWrite
-			}
-
-			if err := ctx.Repo.Repository.LoadUnits(ctx); err != nil {
-				ctx.APIErrorInternal(err)
-				return
-			}
-			ctx.Repo.Permission.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.Repo.Permission.AccessMode)
 		} else {
 			needTwoFactor, err := doerNeedTwoFactorAuth(ctx, ctx.Doer)
 			if err != nil {
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
@@ -28,6 +28,7 @@ import (
 	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/validation"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/utils"
@@ -270,6 +271,8 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre
 			db.IsErrNamePatternNotAllowed(err) ||
 			label.IsErrTemplateLoad(err) {
 			ctx.APIError(http.StatusUnprocessableEntity, err)
+		} else if errors.Is(err, util.ErrPermissionDenied) {
+			ctx.APIError(http.StatusForbidden, err)
 		} else {
 			ctx.APIErrorInternal(err)
 		}
diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go
@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	actions_model "code.gitea.io/gitea/models/actions"
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/perm"
 	access_model "code.gitea.io/gitea/models/perm/access"
@@ -190,29 +189,17 @@ func httpBase(ctx *context.Context) *serviceHandler {
 
 			if ctx.Data["IsActionsToken"] == true {
 				taskID := ctx.Data["ActionsTaskID"].(int64)
-				task, err := actions_model.GetTaskByID(ctx, taskID)
+				p, err := access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID)
 				if err != nil {
-					ctx.ServerError("GetTaskByID", err)
-					return nil
-				}
-				if task.RepoID != repo.ID {
-					ctx.PlainText(http.StatusForbidden, "User permission denied")
+					ctx.ServerError("GetUserRepoPermission", err)
 					return nil
 				}
 
-				if task.IsForkPullRequest {
-					if accessMode > perm.AccessModeRead {
-						ctx.PlainText(http.StatusForbidden, "User permission denied")
-						return nil
-					}
-					environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeRead))
-				} else {
-					if accessMode > perm.AccessModeWrite {
-						ctx.PlainText(http.StatusForbidden, "User permission denied")
-						return nil
-					}
-					environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeWrite))
+				if !p.CanAccess(accessMode, unitType) {
+					ctx.PlainText(http.StatusNotFound, "Repository not found")
+					return nil
 				}
+				environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, p.UnitAccessMode(unitType)))
 			} else {
 				p, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
 				if err != nil {
diff --git a/services/convert/convert.go b/services/convert/convert.go
@@ -771,7 +771,7 @@ func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application {
 
 // ToLFSLock convert a LFSLock to api.LFSLock
 func ToLFSLock(ctx context.Context, l *git_model.LFSLock) *api.LFSLock {
-	u, err := user_model.GetUserByID(ctx, l.OwnerID)
+	u, err := user_model.GetPossibleUserByID(ctx, l.OwnerID)
 	if err != nil {
 		return nil
 	}
diff --git a/services/lfs/locks.go b/services/lfs/locks.go
@@ -187,13 +187,6 @@ func PostLockHandler(ctx *context.Context) {
 			})
 			return
 		}
-		if git_model.IsErrLFSUnauthorizedAction(err) {
-			ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`)
-			ctx.JSON(http.StatusUnauthorized, api.LFSLockError{
-				Message: "You must have push access to create locks : " + err.Error(),
-			})
-			return
-		}
 		log.Error("Unable to CreateLFSLock in repository %-v at %s for user %-v: Error: %v", repository, req.Path, ctx.Doer, err)
 		ctx.JSON(http.StatusInternalServerError, api.LFSLockError{
 			Message: "internal server error : Internal Server Error",
@@ -317,13 +310,6 @@ func UnLockHandler(ctx *context.Context) {
 
 	lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force)
 	if err != nil {
-		if git_model.IsErrLFSUnauthorizedAction(err) {
-			ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`)
-			ctx.JSON(http.StatusUnauthorized, api.LFSLockError{
-				Message: "You must have push access to delete locks : " + err.Error(),
-			})
-			return
-		}
 		log.Error("Unable to DeleteLFSLockByID[%d] by user %-v with force %t: Error: %v", ctx.PathParamInt64("lid"), ctx.Doer, req.Force, err)
 		ctx.JSON(http.StatusInternalServerError, api.LFSLockError{
 			Message: "unable to delete lock : Internal Server Error",
diff --git a/services/lfs/server.go b/services/lfs/server.go
diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go
diff --git a/tests/integration/api_repo_file_get_test.go b/tests/integration/api_repo_file_get_test.go
diff --git a/tests/integration/api_repo_lfs_test.go b/tests/integration/api_repo_lfs_test.go

Original file line number	Diff line number	Diff line change
`@@ -249,8 +249,13 @@ func (u *User) MaxCreationLimit() int {`
`249`	`249`	`}`
`250`	`250`
`251`	`251`	`// CanCreateRepoIn checks whether the doer(u) can create a repository in the owner`
`252`		`-// 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`
	`252`	`+// NOTE: functions calling this assume a failure due to repository count limit, or the owner is not a real user.`
	`253`	`+// It ONLY checks the repo number LIMIT or whether owner user is real. If new checks are added, those functions should be revised.`
	`254`	`+// TODO: the callers can only return ErrReachLimitOfRepo, need to fine tune to support other error types in the future.`
`253`	`255`	`func (u User) CanCreateRepoIn(owner User) bool {`
	`256`	`+ if u.ID <= 0 \|\| owner.ID <= 0 {`
	`257`	`+ return false // fake user like Ghost or Actions user`
	`258`	`+ }`
`254`	`259`	`if u.IsAdmin {`
`255`	`260`	`return true`
`256`	`261`	`}`
Original file line number	Diff line number	Diff line change
`@@ -771,7 +771,7 @@ func ToOAuth2Application(app auth.OAuth2Application) api.OAuth2Application {`
`771`	`771`
`772`	`772`	`// ToLFSLock convert a LFSLock to api.LFSLock`
`773`	`773`	`func ToLFSLock(ctx context.Context, l git_model.LFSLock) api.LFSLock {`
`774`		`- u, err := user_model.GetUserByID(ctx, l.OwnerID)`
	`774`	`+ u, err := user_model.GetPossibleUserByID(ctx, l.OwnerID)`
`775`	`775`	`if err != nil {`
`776`	`776`	`return nil`
`777`	`777`	`}`