From bf520f5184327eea9590ac5eb52d98f16af43c12 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 20 Nov 2024 20:55:32 -0800 Subject: [PATCH 1/4] Fix GetInactiveUsers (#32540) Fix #31480 (cherry picked from commit 9bf821ae6c108379d22ae11d8d5784a4ed7ad647) Conflicts: models/user/user_test.go trivial context conflict --- models/fixtures/user.yml | 1 + models/user/user.go | 18 ++++++++++++------ models/user/user_test.go | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 8e216fbc7d..73b9a97e1b 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -332,6 +332,7 @@ repo_admin_change_team_access: false theme: "" keep_activity_private: false + created_unix: 1730468968 - id: 10 diff --git a/models/user/user.go b/models/user/user.go index 528b4dbfd9..3f12f8e83f 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -50,19 +50,19 @@ const ( UserTypeIndividual UserType = iota // Historic reason to make it starts at 0. // UserTypeOrganization defines an organization - UserTypeOrganization + UserTypeOrganization // 1 // UserTypeUserReserved reserves a (non-existing) user, i.e. to prevent a spam user from re-registering after being deleted, or to reserve the name until the user is actually created later on - UserTypeUserReserved + UserTypeUserReserved // 2 // UserTypeOrganizationReserved reserves a (non-existing) organization, to be used in combination with UserTypeUserReserved - UserTypeOrganizationReserved + UserTypeOrganizationReserved // 3 // UserTypeBot defines a bot user - UserTypeBot + UserTypeBot // 4 // UserTypeRemoteUser defines a remote user for federated users - UserTypeRemoteUser + UserTypeRemoteUser // 5 ) const ( @@ -917,7 +917,13 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error { // GetInactiveUsers gets all inactive users func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) { - var cond builder.Cond = builder.Eq{"is_active": false} + cond := builder.And( + builder.Eq{"is_active": false}, + builder.Or( // only plain user + builder.Eq{"`type`": UserTypeIndividual}, + builder.Eq{"`type`": UserTypeUserReserved}, + ), + ) if olderThan > 0 { cond = cond.And(builder.Lt{"created_unix": time.Now().Add(-olderThan).Unix()}) diff --git a/models/user/user_test.go b/models/user/user_test.go index 4979cc1329..f0b7e16824 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -765,3 +765,17 @@ func TestVerifyUserAuthorizationToken(t *testing.T) { assert.Nil(t, authToken) }) } + +func TestGetInactiveUsers(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + // all inactive users + // user1's createdunix is 1730468968 + users, err := user_model.GetInactiveUsers(db.DefaultContext, 0) + require.NoError(t, err) + assert.Len(t, users, 1) + interval := time.Now().Unix() - 1730468968 + 3600*24 + users, err = user_model.GetInactiveUsers(db.DefaultContext, time.Duration(interval*int64(time.Second))) + require.NoError(t, err) + require.Empty(t, users) +} From 1c04f8f10a7ebc02f41af3d2db6a2a4e85127441 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 19 Nov 2024 11:44:21 -0800 Subject: [PATCH 2/4] Fix submodule parsing (cherry picked from commit 33850a83fe4ebd23a762a7aac81614c42e303bfa) This really is just the cherry pick of 407b6e6dfc7ee9ebb8a16c7f1a786e4c24d0516e which is the first commit of the pull request, the one with the change. The rest of the changes is a refactor that is unrelated to the bug fix. Conflicts: modules/git/commit_test.go trivial context conflict --- modules/git/commit.go | 45 ++++++++++++++++++-------------------- modules/git/commit_test.go | 30 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index b5ae2e0e52..0f3b749227 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -17,6 +17,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" + + "github.com/go-git/go-git/v5/config" ) // Commit represents a git commit. @@ -365,37 +367,32 @@ func (c *Commit) GetSubModules() (*ObjectCache, error) { return nil, err } - rd, err := entry.Blob().DataAsync() + content, err := entry.Blob().GetBlobContent(10 * 1024) if err != nil { return nil, err } - defer rd.Close() - scanner := bufio.NewScanner(rd) - c.submoduleCache = newObjectCache() - var ismodule bool - var path string - for scanner.Scan() { - if strings.HasPrefix(scanner.Text(), "[submodule") { - ismodule = true - continue - } - if ismodule { - fields := strings.Split(scanner.Text(), "=") - k := strings.TrimSpace(fields[0]) - if k == "path" { - path = strings.TrimSpace(fields[1]) - } else if k == "url" { - c.submoduleCache.Set(path, &SubModule{path, strings.TrimSpace(fields[1])}) - ismodule = false - } - } + c.submoduleCache, err = parseSubmoduleContent([]byte(content)) + if err != nil { + return nil, err } - if err = scanner.Err(); err != nil { - return nil, fmt.Errorf("GetSubModules scan: %w", err) + return c.submoduleCache, nil +} + +func parseSubmoduleContent(bs []byte) (*ObjectCache, error) { + cfg := config.NewModules() + if err := cfg.Unmarshal(bs); err != nil { + return nil, err + } + submoduleCache := newObjectCache() + if len(cfg.Submodules) == 0 { + return nil, fmt.Errorf("no submodules found") + } + for _, subModule := range cfg.Submodules { + submoduleCache.Set(subModule.Path, subModule.URL) } - return c.submoduleCache, nil + return submoduleCache, nil } // GetSubModule get the sub module according entryname diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index af85bfe093..6bb7d776f5 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -369,3 +369,33 @@ func TestParseCommitRenames(t *testing.T) { assert.Equal(t, testcase.renames, renames) } } + +func Test_parseSubmoduleContent(t *testing.T) { + submoduleFiles := []struct { + fileContent string + expectedPath string + expectedURL string + }{ + { + fileContent: `[submodule "jakarta-servlet"] +url = ../../ALP-pool/jakarta-servlet +path = jakarta-servlet`, + expectedPath: "jakarta-servlet", + expectedURL: "../../ALP-pool/jakarta-servlet", + }, + { + fileContent: `[submodule "jakarta-servlet"] +path = jakarta-servlet +url = ../../ALP-pool/jakarta-servlet`, + expectedPath: "jakarta-servlet", + expectedURL: "../../ALP-pool/jakarta-servlet", + }, + } + for _, kase := range submoduleFiles { + submodule, err := parseSubmoduleContent([]byte(kase.fileContent)) + require.NoError(t, err) + v, ok := submodule.Get(kase.expectedPath) + assert.True(t, ok) + assert.Equal(t, kase.expectedURL, v) + } +} From 48872d11ca920849ec174f76c0d667ca2b289aef Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 20 Nov 2024 09:24:09 -0600 Subject: [PATCH 3/4] allow the actions user to login via the jwt token (#32527) We have some actions that leverage the Gitea API that began receiving 401 errors, with a message that the user was not found. These actions use the `ACTIONS_RUNTIME_TOKEN` env var in the actions job to authenticate with the Gitea API. The format of this env var in actions jobs changed with go-gitea/gitea/pull/28885 to be a JWT (with a corresponding update to `act_runner`) Since it was a JWT, the OAuth parsing logic attempted to parse it as an OAuth token, and would return user not found, instead of falling back to look up the running task and assigning it to the actions user. Make ACTIONS_RUNTIME_TOKEN in action runners could be used, attempting to parse Oauth JWTs. The code to parse potential old `ACTION_RUNTIME_TOKEN` was kept in case someone is running an older version of act_runner that doesn't support the Actions JWT. (cherry picked from commit 407b6e6dfc7ee9ebb8a16c7f1a786e4c24d0516e) Conflicts: services/auth/oauth2.go trivial context conflicts because OAuth2 scopes are in Forgejo and not yet in Gitea --- models/fixtures/action_task.yml | 19 ++++++++++++ services/actions/auth.go | 11 +++++-- services/auth/oauth2.go | 24 +++++++++++++- services/auth/oauth2_test.go | 55 +++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 services/auth/oauth2_test.go diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index 443effe08c..d88a8ed8a9 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -1,3 +1,22 @@ +- + id: 46 + attempt: 3 + runner_id: 1 + status: 3 # 3 is the status code for "cancelled" + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: 6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2aaaaa + token_salt: eeeeeeee + token_last_eight: eeeeeeee + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 - id: 47 job_id: 192 diff --git a/services/actions/auth.go b/services/actions/auth.go index 8e934d89a8..1ef21f6e0e 100644 --- a/services/actions/auth.go +++ b/services/actions/auth.go @@ -83,7 +83,12 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) { return 0, fmt.Errorf("split token failed") } - token, err := jwt.ParseWithClaims(parts[1], &actionsClaims{}, func(t *jwt.Token) (any, error) { + return TokenToTaskID(parts[1]) +} + +// TokenToTaskID returns the TaskID associated with the provided JWT token +func TokenToTaskID(token string) (int64, error) { + parsedToken, err := jwt.ParseWithClaims(token, &actionsClaims{}, func(t *jwt.Token) (any, error) { if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) } @@ -93,8 +98,8 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) { return 0, err } - c, ok := token.Claims.(*actionsClaims) - if !token.Valid || !ok { + c, ok := parsedToken.Claims.(*actionsClaims) + if !parsedToken.Valid || !ok { return 0, fmt.Errorf("invalid token claim") } diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 8b625a193e..b983e57ecd 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/actions" "code.gitea.io/gitea/services/auth/source/oauth2" ) @@ -94,6 +95,18 @@ func CheckOAuthAccessToken(ctx context.Context, accessToken string) (int64, stri return grant.UserID, grantScopes } +// CheckTaskIsRunning verifies that the TaskID corresponds to a running task +func CheckTaskIsRunning(ctx context.Context, taskID int64) bool { + // Verify the task exists + task, err := actions_model.GetTaskByID(ctx, taskID) + if err != nil { + return false + } + + // Verify that it's running + return task.Status == actions_model.StatusRunning +} + // OAuth2 implements the Auth interface and authenticates requests // (API requests only) by looking for an OAuth token in query parameters or the // "Authorization" header. @@ -137,8 +150,17 @@ func parseToken(req *http.Request) (string, bool) { func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) int64 { // Let's see if token is valid. if strings.Contains(tokenSHA, ".") { - uid, grantScopes := CheckOAuthAccessToken(ctx, 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 + } + } + // Otherwise, check if this is an OAuth access token + uid, grantScopes := CheckOAuthAccessToken(ctx, tokenSHA) if uid != 0 { store.GetData()["IsApiToken"] = true if grantScopes != "" { diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go new file mode 100644 index 0000000000..c9b4ed06cc --- /dev/null +++ b/services/auth/oauth2_test.go @@ -0,0 +1,55 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "context" + "testing" + + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/actions" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUserIDFromToken(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Actions JWT", func(t *testing.T) { + const RunningTaskID = 47 + token, err := actions.CreateAuthorizationToken(RunningTaskID, 1, 2) + require.NoError(t, err) + + ds := make(middleware.ContextData) + + o := OAuth2{} + uid := o.userIDFromToken(context.Background(), token, ds) + assert.Equal(t, int64(user_model.ActionsUserID), uid) + assert.Equal(t, true, ds["IsActionsToken"]) + assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID)) + }) +} + +func TestCheckTaskIsRunning(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + cases := map[string]struct { + TaskID int64 + Expected bool + }{ + "Running": {TaskID: 47, Expected: true}, + "Missing": {TaskID: 1, Expected: false}, + "Cancelled": {TaskID: 46, Expected: false}, + } + + for name := range cases { + c := cases[name] + t.Run(name, func(t *testing.T) { + actual := CheckTaskIsRunning(context.Background(), c.TaskID) + assert.Equal(t, c.Expected, actual) + }) + } +} From 1f9a1537a54f84ac9e28e549553c198dc5837094 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 24 Nov 2024 16:53:22 +0000 Subject: [PATCH 4/4] chore(release-notes): notes for the week 2024-48-v9.0 weekly cherry pick --- release-notes/6064.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 release-notes/6064.md diff --git a/release-notes/6064.md b/release-notes/6064.md new file mode 100644 index 0000000000..c146c8db9e --- /dev/null +++ b/release-notes/6064.md @@ -0,0 +1,3 @@ +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/bf520f5184327eea9590ac5eb52d98f16af43c12) Fix GetInactiveUsers +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/1c04f8f10a7ebc02f41af3d2db6a2a4e85127441) Fix submodule parsing +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/48872d11ca920849ec174f76c0d667ca2b289aef) allow the actions user to login via the jwt token