Refactor ActionsTaskID (#36503)

models/user/user_system.go

@@ -4,6 +4,7 @@
 package user
 
 import (
+	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/modules/structs"
@@ -23,10 +24,6 @@ func NewGhostUser() *User {
 	}
 }
 
-func IsGhostUserName(name string) bool {
-	return strings.EqualFold(name, GhostUserName)
-}
-
 // IsGhost check if user is fake user for a deleted account
 func (u *User) IsGhost() bool {
 	if u == nil {
@@ -41,10 +38,6 @@ const (
 	ActionsUserEmail       = "teabot@gitea.io"
 )
 
-func IsGiteaActionsUserName(name string) bool {
-	return strings.EqualFold(name, ActionsUserName)
-}
-
 // NewActionsUser creates and returns a fake user for running the actions.
 func NewActionsUser() *User {
 	return &User{
@@ -61,15 +54,36 @@ func NewActionsUser() *User {
 	}
 }
 
+func NewActionsUserWithTaskID(id int64) *User {
+	u := NewActionsUser()
+	// LoginName is for only internal usage in this case, so it can be moved to other fields in the future
+	u.LoginSource = -1
+	u.LoginName = "@" + ActionsUserName + "/" + strconv.FormatInt(id, 10)
+	return u
+}
+
+func GetActionsUserTaskID(u *User) (int64, bool) {
+	if u == nil || u.ID != ActionsUserID {
+		return 0, false
+	}
+	prefix, payload, _ := strings.Cut(u.LoginName, "/")
+	if prefix != "@"+ActionsUserName {
+		return 0, false
+	} else if taskID, err := strconv.ParseInt(payload, 10, 64); err == nil {
+		return taskID, true
+	}
+	return 0, false
+}
+
 func (u *User) IsGiteaActions() bool {
 	return u != nil && u.ID == ActionsUserID
 }
 
 func GetSystemUserByName(name string) *User {
-	if IsGhostUserName(name) {
+	if strings.EqualFold(name, GhostUserName) {
 		return NewGhostUser()
 	}
-	if IsGiteaActionsUserName(name) {
+	if strings.EqualFold(name, ActionsUserName) {
 		return NewActionsUser()
 	}
 	return nil

models/user/user_system_test.go

@@ -16,14 +16,20 @@ func TestSystemUser(t *testing.T) {
 	assert.Equal(t, "Ghost", u.Name)
 	assert.Equal(t, "ghost", u.LowerName)
 	assert.True(t, u.IsGhost())
-	assert.True(t, IsGhostUserName("gHost"))
+
+	u = GetSystemUserByName("gHost")
+	require.NotNil(t, u)
+	assert.Equal(t, "Ghost", u.Name)
 
 	u, err = GetPossibleUserByID(t.Context(), -2)
 	require.NoError(t, err)
 	assert.Equal(t, "gitea-actions", u.Name)
 	assert.Equal(t, "gitea-actions", u.LowerName)
 	assert.True(t, u.IsGiteaActions())
-	assert.True(t, IsGiteaActionsUserName("Gitea-actionS"))
+
+	u = GetSystemUserByName("Gitea-actionS")
+	require.NotNil(t, u)
+	assert.Equal(t, "Gitea Actions", u.FullName)
 
 	_, err = GetPossibleUserByID(t.Context(), -3)
 	require.Error(t, err)

routers/api/v1/api.go

@@ -188,8 +188,7 @@ func repoAssignment() func(ctx *context.APIContext) {
 		repo.Owner = owner
 		ctx.Repo.Repository = repo
 
-		if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID {
+		if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok {
-			taskID := ctx.Data["ActionsTaskID"].(int64)
 			ctx.Repo.Permission, err = access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID)
 			if err != nil {
 				ctx.APIErrorInternal(err)
@@ -349,11 +348,7 @@ func tokenRequiresScopes(requiredScopeCategories ...auth_model.AccessTokenScopeC
 // Contexter middleware already checks token for user sign in process.
 func reqToken() func(ctx *context.APIContext) {
 	return func(ctx *context.APIContext) {
-		// If actions token is present
+		// if a real user is signed in, or the user is from a Actions task, we are good
-		if true == ctx.Data["IsActionsToken"] {
-			return
-		}
-
 		if ctx.IsSigned {
 			return
 		}

routers/web/repo/githttp.go

@@ -22,6 +22,7 @@ import (
 	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/git"
 	"code.gitea.io/gitea/modules/git/gitcmd"
 	"code.gitea.io/gitea/modules/gitrepo"
@@ -166,7 +167,7 @@ func httpBase(ctx *context.Context, optGitService ...string) *serviceHandler {
 			return nil
 		}
 
-		if ctx.IsBasicAuth && ctx.Data["IsApiToken"] != true && ctx.Data["IsActionsToken"] != true {
+		if ctx.IsBasicAuth && ctx.Data["IsApiToken"] != true && !ctx.Doer.IsGiteaActions() {
 			_, err = auth_model.GetTwoFactorByUID(ctx, ctx.Doer.ID)
 			if err == nil {
 				// TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented
@@ -197,8 +198,7 @@ func httpBase(ctx *context.Context, optGitService ...string) *serviceHandler {
 				accessMode = perm.AccessModeRead
 			}
 
-			if ctx.Data["IsActionsToken"] == true {
+			if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok {
-				taskID := ctx.Data["ActionsTaskID"].(int64)
 				p, err := access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID)
 				if err != nil {
 					ctx.ServerError("GetActionsUserRepoPermission", err)

services/auth/basic.go

@@ -117,12 +117,8 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 	task, err := actions_model.GetRunningTaskByToken(req.Context(), authToken)
 	if err == nil && task != nil {
 		log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID)
-
 		store.GetData()["LoginMethod"] = ActionTokenMethodName
-		store.GetData()["IsActionsToken"] = true
+		return user_model.NewActionsUserWithTaskID(task.ID), nil
-		store.GetData()["ActionsTaskID"] = task.ID
-
-		return user_model.NewActionsUser(), nil
 	}
 
 	if !setting.Service.EnableBasicAuth {

services/auth/oauth2.go

@@ -6,6 +6,7 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"strings"
 	"time"
@@ -17,14 +18,12 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/actions"
 	"code.gitea.io/gitea/services/oauth2_provider"
 )
 
-// Ensure the struct implements the interface.
+var _ Method = &OAuth2{}
-var (
-	_ Method = &OAuth2{}
-)
 
 // GetOAuthAccessTokenScopeAndUserID returns access token scope and user id
 func GetOAuthAccessTokenScopeAndUserID(ctx context.Context, accessToken string) (auth_model.AccessTokenScope, int64) {
@@ -106,18 +105,16 @@ func parseToken(req *http.Request) (string, bool) {
 	return "", false
 }
 
-// userIDFromToken returns the user id corresponding to the OAuth token.
+// userFromToken returns the user corresponding to the OAuth token.
 // It will set 'IsApiToken' to true if the token is an API token and
-// set 'ApiTokenScope' to the scope of the access token
+// set 'ApiTokenScope' to the scope of the access token (TODO: this behavior should be fixed, don't set ctx.Data)
-func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) int64 {
+func (o *OAuth2) userFromToken(ctx context.Context, tokenSHA string, store DataStore) (*user_model.User, error) {
 	// Let's see if token is valid.
 	if strings.Contains(tokenSHA, ".") {
 		// First attempt to decode an actions JWT, returning the actions user
 		if taskID, err := actions.TokenToTaskID(tokenSHA); err == nil {
 			if CheckTaskIsRunning(ctx, taskID) {
-				store.GetData()["IsActionsToken"] = true
+				return user_model.NewActionsUserWithTaskID(taskID), nil
-				store.GetData()["ActionsTaskID"] = taskID
-				return user_model.ActionsUserID
 			}
 		}
 
@@ -127,33 +124,27 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat
 			store.GetData()["IsApiToken"] = true
 			store.GetData()["ApiTokenScope"] = accessTokenScope
 		}
-		return uid
+		return user_model.GetUserByID(ctx, uid)
 	}
 	t, err := auth_model.GetAccessTokenBySHA(ctx, tokenSHA)
 	if err != nil {
 		if auth_model.IsErrAccessTokenNotExist(err) {
 			// check task token
-			task, err := actions_model.GetRunningTaskByToken(ctx, tokenSHA)
+			if task, err := actions_model.GetRunningTaskByToken(ctx, tokenSHA); err == nil {
-			if err == nil && task != nil {
 				log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID)
-
+				return user_model.NewActionsUserWithTaskID(task.ID), nil
-				store.GetData()["IsActionsToken"] = true
-				store.GetData()["ActionsTaskID"] = task.ID
-
-				return user_model.ActionsUserID
 			}
-		} else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
-			log.Error("GetAccessTokenBySHA: %v", err)
 		}
-		return 0
+		return nil, err
 	}
+
 	t.UpdatedUnix = timeutil.TimeStampNow()
 	if err = auth_model.UpdateAccessToken(ctx, t); err != nil {
 		log.Error("UpdateAccessToken: %v", err)
 	}
 	store.GetData()["IsApiToken"] = true
 	store.GetData()["ApiTokenScope"] = t.Scope
-	return t.UID
+	return user_model.GetUserByID(ctx, t.UID)
 }
 
 // Verify extracts the user ID from the OAuth token in the query parameters
@@ -173,21 +164,9 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor
 		return nil, nil
 	}
 
-	id := o.userIDFromToken(req.Context(), token, store)
+	user, err := o.userFromToken(req.Context(), token, store)
-
+	if err != nil && !errors.Is(err, util.ErrNotExist) {
-	if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.
+		log.Error("userFromToken: %v", err) // the callers might ignore the error, so log it here
-		return nil, user_model.ErrUserNotExist{}
 	}
-	log.Trace("OAuth2 Authorization: Found token for user[%d]", id)
+	return user, err
-
-	user, err := user_model.GetPossibleUserByID(req.Context(), id)
-	if err != nil {
-		if !user_model.IsErrUserNotExist(err) {
-			log.Error("GetUserByName: %v", err)
-		}
-		return nil, err
-	}
-
-	log.Trace("OAuth2 Authorization: Logged in user %-v", user)
-	return user, nil
 }

services/auth/oauth2_test.go

@@ -12,23 +12,26 @@ import (
 	"code.gitea.io/gitea/services/actions"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserIDFromToken(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
 	t.Run("Actions JWT", func(t *testing.T) {
-		const RunningTaskID = 47
+		const RunningTaskID int64 = 47
 		token, err := actions.CreateAuthorizationToken(RunningTaskID, 1, 2)
 		assert.NoError(t, err)
 
 		ds := make(reqctx.ContextData)
 
 		o := OAuth2{}
-		uid := o.userIDFromToken(t.Context(), token, ds)
+		u, err := o.userFromToken(t.Context(), token, ds)
-		assert.Equal(t, user_model.ActionsUserID, uid)
+		require.NoError(t, err)
-		assert.Equal(t, true, ds["IsActionsToken"])
+		assert.Equal(t, user_model.ActionsUserID, u.ID)
-		assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID))
+		taskID, ok := user_model.GetActionsUserTaskID(u)
+		assert.True(t, ok)
+		assert.Equal(t, RunningTaskID, taskID)
 	})
 }
 

services/lfs/server.go

@@ -541,8 +541,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho
 		accessMode = perm_model.AccessModeWrite
 	}
 
-	if ctx.Data["IsActionsToken"] == true {
+	if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok {
-		taskID := ctx.Data["ActionsTaskID"].(int64)
 		perm, err := access_model.GetActionsUserRepoPermission(ctx, repository, ctx.Doer, taskID)
 		if err != nil {
 			log.Error("Unable to GetActionsUserRepoPermission for task[%d] Error: %v", taskID, err)

services/webhook/general.go

@@ -317,7 +317,7 @@ func getStatusPayloadInfo(p *api.CommitStatusPayload, linkFormatter linkFormatte
 	text = fmt.Sprintf("Commit Status changed: %s - %s", refLink, p.Description)
 	color = greenColor
 	if withSender {
-		if user_model.IsGiteaActionsUserName(p.Sender.UserName) {
+		if user_model.GetSystemUserByName(p.Sender.UserName) != nil {
 			text += " by " + p.Sender.FullName
 		} else {
 			text += " by " + linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName)