diff --git a/models/user/user_system.go b/models/user/user_system.go index 11008c77d4..cf4306df25 100644 --- a/models/user/user_system.go +++ b/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 diff --git a/models/user/user_system_test.go b/models/user/user_system_test.go index 5aa3fa463c..70a900378f 100644 --- a/models/user/user_system_test.go +++ b/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) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 6d37c67cc4..cba96aa08a 100644 --- a/routers/api/v1/api.go +++ b/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 { - taskID := ctx.Data["ActionsTaskID"].(int64) + if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok { 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 true == ctx.Data["IsActionsToken"] { - return - } - + // if a real user is signed in, or the user is from a Actions task, we are good if ctx.IsSigned { return } diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 8b3deb5a03..e922ed99fc 100644 --- a/routers/web/repo/githttp.go +++ b/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 { - taskID := ctx.Data["ActionsTaskID"].(int64) + if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok { p, err := access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID) if err != nil { ctx.ServerError("GetActionsUserRepoPermission", err) diff --git a/services/auth/basic.go b/services/auth/basic.go index 501924b4df..51613870c9 100644 --- a/services/auth/basic.go +++ b/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 - store.GetData()["ActionsTaskID"] = task.ID - - return user_model.NewActionsUser(), nil + return user_model.NewActionsUserWithTaskID(task.ID), nil } if !setting.Service.EnableBasicAuth { diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 7df6f4638e..13cbc77f7a 100644 --- a/services/auth/oauth2.go +++ b/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 -func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) int64 { +// set 'ApiTokenScope' to the scope of the access token (TODO: this behavior should be fixed, don't set ctx.Data) +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 - store.GetData()["ActionsTaskID"] = taskID - return user_model.ActionsUserID + return user_model.NewActionsUserWithTaskID(taskID), nil } } @@ -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 err == nil && task != nil { + if task, err := actions_model.GetRunningTaskByToken(ctx, tokenSHA); err == nil { log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID) - - store.GetData()["IsActionsToken"] = true - store.GetData()["ActionsTaskID"] = task.ID - - return user_model.ActionsUserID + return user_model.NewActionsUserWithTaskID(task.ID), nil } - } 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) - - if id <= 0 && id != -2 { // -2 means actions, so we need to allow it. - return nil, user_model.ErrUserNotExist{} + user, err := o.userFromToken(req.Context(), token, store) + if err != nil && !errors.Is(err, util.ErrNotExist) { + log.Error("userFromToken: %v", err) // the callers might ignore the error, so log it here } - log.Trace("OAuth2 Authorization: Found token for user[%d]", id) - - 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 + return user, err } diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go index f003742a94..308da846b8 100644 --- a/services/auth/oauth2_test.go +++ b/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) - assert.Equal(t, user_model.ActionsUserID, uid) - assert.Equal(t, true, ds["IsActionsToken"]) - assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID)) + u, err := o.userFromToken(t.Context(), token, ds) + require.NoError(t, err) + assert.Equal(t, user_model.ActionsUserID, u.ID) + taskID, ok := user_model.GetActionsUserTaskID(u) + assert.True(t, ok) + assert.Equal(t, RunningTaskID, taskID) }) } diff --git a/services/lfs/server.go b/services/lfs/server.go index 4819437bf1..10b4dba222 100644 --- a/services/lfs/server.go +++ b/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 { - taskID := ctx.Data["ActionsTaskID"].(int64) + if taskID, ok := user_model.GetActionsUserTaskID(ctx.Doer); ok { 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) diff --git a/services/webhook/general.go b/services/webhook/general.go index be457e46f5..3186f53d74 100644 --- a/services/webhook/general.go +++ b/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)