Skip to content

Commit ed57c70

Browse files
lunnyCopilotwxiaoguang
authored
Fix track time list permission check (#36662)
Signed-off-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent 75efc51 commit ed57c70

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

services/convert/issue.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
access_model "code.gitea.io/gitea/models/perm/access"
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
user_model "code.gitea.io/gitea/models/user"
16+
"code.gitea.io/gitea/modules/cache"
1617
"code.gitea.io/gitea/modules/label"
1718
"code.gitea.io/gitea/modules/log"
1819
"code.gitea.io/gitea/modules/setting"
@@ -226,7 +227,21 @@ func ToStopWatches(ctx context.Context, doer *user_model.User, sws []*issues_mod
226227
// ToTrackedTimeList converts TrackedTimeList to API format
227228
func ToTrackedTimeList(ctx context.Context, doer *user_model.User, tl issues_model.TrackedTimeList) api.TrackedTimeList {
228229
result := make([]*api.TrackedTime, 0, len(tl))
230+
permCache := cache.NewEphemeralCache()
229231
for _, t := range tl {
232+
// If the issue is not loaded, conservatively skip this entry to avoid bypassing permission checks.
233+
if t.Issue == nil || t.Issue.Repo == nil {
234+
continue
235+
}
236+
perm, err := cache.GetWithEphemeralCache(ctx, permCache, "repo-perm", t.Issue.RepoID, func(ctx context.Context, repoID int64) (access_model.Permission, error) {
237+
return access_model.GetUserRepoPermission(ctx, t.Issue.Repo, doer)
238+
})
239+
if err != nil {
240+
continue
241+
}
242+
if !perm.CanReadIssuesOrPulls(t.Issue.IsPull) {
243+
continue
244+
}
230245
result = append(result, ToTrackedTime(ctx, doer, t))
231246
}
232247
return result

services/convert/issue_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"code.gitea.io/gitea/modules/timeutil"
1919

2020
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
2122
)
2223

2324
func TestLabel_ToLabel(t *testing.T) {
@@ -83,3 +84,43 @@ func TestToStopWatchesRespectsPermissions(t *testing.T) {
8384
assert.Len(t, visibleAdmin, 2)
8485
assert.ElementsMatch(t, []string{"repo1", "repo3"}, []string{visibleAdmin[0].RepoName, visibleAdmin[1].RepoName})
8586
}
87+
88+
func TestToTrackedTime(t *testing.T) {
89+
require.NoError(t, unittest.PrepareTestDatabase())
90+
91+
ctx := t.Context()
92+
publicIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 1})
93+
privateIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 3})
94+
regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
95+
adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
96+
97+
publicTrackedTime := &issues_model.TrackedTime{IssueID: publicIssue.ID, UserID: regularUser.ID, Time: 3600}
98+
privateTrackedTime := &issues_model.TrackedTime{IssueID: privateIssue.ID, UserID: regularUser.ID, Time: 1800}
99+
require.NoError(t, db.Insert(ctx, publicTrackedTime))
100+
require.NoError(t, db.Insert(ctx, privateTrackedTime))
101+
102+
t.Run("NilIssues", func(t *testing.T) {
103+
list := ToTrackedTimeList(ctx, regularUser, issues_model.TrackedTimeList{publicTrackedTime, privateTrackedTime})
104+
assert.Empty(t, list)
105+
})
106+
107+
t.Run("NilRepo", func(t *testing.T) {
108+
badTrackedTime := &issues_model.TrackedTime{Issue: &issues_model.Issue{RepoID: 999999}}
109+
visible := ToTrackedTimeList(ctx, regularUser, issues_model.TrackedTimeList{badTrackedTime})
110+
assert.Empty(t, visible)
111+
})
112+
113+
trackedTimes := issues_model.TrackedTimeList{publicTrackedTime, privateTrackedTime}
114+
require.NoError(t, trackedTimes.LoadAttributes(ctx))
115+
116+
t.Run("ToRegularUser", func(t *testing.T) {
117+
list := ToTrackedTimeList(ctx, regularUser, trackedTimes)
118+
require.Len(t, list, 1)
119+
assert.Equal(t, "repo1", list[0].Issue.Repo.Name)
120+
})
121+
t.Run("ToAdminUser", func(t *testing.T) {
122+
list := ToTrackedTimeList(ctx, adminUser, trackedTimes)
123+
require.Len(t, list, 2)
124+
assert.ElementsMatch(t, []string{"repo1", "repo3"}, []string{list[0].Issue.Repo.Name, list[1].Issue.Repo.Name})
125+
})
126+
}

0 commit comments

Comments
 (0)