diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index 761b8f91a0..f119951b09 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -12,6 +12,8 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + + "xorm.io/builder" ) // Stopwatch represents a stopwatch for time tracking. @@ -232,3 +234,14 @@ func CancelStopwatch(ctx context.Context, user *user_model.User, issue *Issue) ( }) return ok, err } + +// RemoveStopwatchesByRepoID removes all stopwatches for a user in a specific repository +// this function should be called before removing all the issues of the repository +func RemoveStopwatchesByRepoID(ctx context.Context, userID, repoID int64) error { + _, err := db.GetEngine(ctx). + Where("`stopwatch`.user_id = ?", userID). + And(builder.In("`stopwatch`.issue_id", + builder.Select("id").From("issue").Where(builder.Eq{"repo_id": repoID}))). + Delete(new(Stopwatch)) + return err +} diff --git a/modules/eventsource/manager_run.go b/modules/eventsource/manager_run.go index f66dc78c7e..4a42224dda 100644 --- a/modules/eventsource/manager_run.go +++ b/modules/eventsource/manager_run.go @@ -9,6 +9,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" issues_model "code.gitea.io/gitea/models/issues" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -91,7 +92,13 @@ loop: } for _, userStopwatches := range usersStopwatches { - apiSWs, err := convert.ToStopWatches(ctx, userStopwatches.StopWatches) + u, err := user_model.GetUserByID(ctx, userStopwatches.UserID) + if err != nil { + log.Error("Unable to get user %d: %v", userStopwatches.UserID, err) + continue + } + + apiSWs, err := convert.ToStopWatches(ctx, u, userStopwatches.StopWatches) if err != nil { if !issues_model.IsErrIssueNotExist(err) { log.Error("Unable to APIFormat stopwatches: %v", err) diff --git a/routers/api/v1/repo/issue_stopwatch.go b/routers/api/v1/repo/issue_stopwatch.go index 0f28b9757d..f9fbff091d 100644 --- a/routers/api/v1/repo/issue_stopwatch.go +++ b/routers/api/v1/repo/issue_stopwatch.go @@ -224,7 +224,7 @@ func GetStopwatches(ctx *context.APIContext) { return } - apiSWs, err := convert.ToStopWatches(ctx, sws) + apiSWs, err := convert.ToStopWatches(ctx, ctx.Doer, sws) if err != nil { ctx.APIErrorInternal(err) return diff --git a/routers/web/user/stop_watch.go b/routers/web/user/stop_watch.go index 1d1cc61cc9..4bd8841573 100644 --- a/routers/web/user/stop_watch.go +++ b/routers/web/user/stop_watch.go @@ -29,7 +29,7 @@ func GetStopwatches(ctx *context.Context) { return } - apiSWs, err := convert.ToStopWatches(ctx, sws) + apiSWs, err := convert.ToStopWatches(ctx, ctx.Doer, sws) if err != nil { ctx.HTTPError(http.StatusInternalServerError, err.Error()) return diff --git a/services/convert/issue.go b/services/convert/issue.go index e26412bcca..b396dd0737 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -10,6 +10,7 @@ import ( "strings" issues_model "code.gitea.io/gitea/models/issues" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/label" @@ -163,11 +164,12 @@ func ToTrackedTime(ctx context.Context, doer *user_model.User, t *issues_model.T } // ToStopWatches convert Stopwatch list to api.StopWatches -func ToStopWatches(ctx context.Context, sws []*issues_model.Stopwatch) (api.StopWatches, error) { +func ToStopWatches(ctx context.Context, doer *user_model.User, sws []*issues_model.Stopwatch) (api.StopWatches, error) { result := api.StopWatches(make([]api.StopWatch, 0, len(sws))) issueCache := make(map[int64]*issues_model.Issue) repoCache := make(map[int64]*repo_model.Repository) + permCache := make(map[int64]access_model.Permission) var ( issue *issues_model.Issue repo *repo_model.Repository @@ -182,13 +184,30 @@ func ToStopWatches(ctx context.Context, sws []*issues_model.Stopwatch) (api.Stop if err != nil { return nil, err } + issueCache[sw.IssueID] = issue } repo, ok = repoCache[issue.RepoID] if !ok { repo, err = repo_model.GetRepositoryByID(ctx, issue.RepoID) if err != nil { - return nil, err + log.Error("GetRepositoryByID(%d): %v", issue.RepoID, err) + continue } + repoCache[issue.RepoID] = repo + } + + // ADD: Check user permissions + perm, ok := permCache[repo.ID] + if !ok { + perm, err = access_model.GetUserRepoPermission(ctx, repo, doer) + if err != nil { + continue + } + permCache[repo.ID] = perm + } + + if !perm.CanReadIssuesOrPulls(issue.IsPull) { + continue } result = append(result, api.StopWatch{ diff --git a/services/convert/issue_test.go b/services/convert/issue_test.go index 4d780f3f00..a12a69288a 100644 --- a/services/convert/issue_test.go +++ b/services/convert/issue_test.go @@ -8,9 +8,11 @@ import ( "testing" "time" + "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -55,3 +57,29 @@ func TestMilestone_APIFormat(t *testing.T) { Deadline: milestone.DeadlineUnix.AsTimePtr(), }, *ToAPIMilestone(milestone)) } + +func TestToStopWatchesRespectsPermissions(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + ctx := t.Context() + publicSW := unittest.AssertExistsAndLoadBean(t, &issues_model.Stopwatch{ID: 1}) + privateIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 3}) + privateSW := &issues_model.Stopwatch{IssueID: privateIssue.ID, UserID: 5} + assert.NoError(t, db.Insert(ctx, privateSW)) + assert.NotZero(t, privateSW.ID) + + regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + sws := []*issues_model.Stopwatch{publicSW, privateSW} + + visible, err := ToStopWatches(ctx, regularUser, sws) + assert.NoError(t, err) + assert.Len(t, visible, 1) + assert.Equal(t, "repo1", visible[0].RepoName) + + visibleAdmin, err := ToStopWatches(ctx, adminUser, sws) + assert.NoError(t, err) + assert.Len(t, visibleAdmin, 2) + assert.ElementsMatch(t, []string{"repo1", "repo3"}, []string{visibleAdmin[0].RepoName, visibleAdmin[1].RepoName}) +} diff --git a/services/org/team_test.go b/services/org/team_test.go index a5e01e7a54..5cb588b7dd 100644 --- a/services/org/team_test.go +++ b/services/org/team_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" @@ -62,6 +63,36 @@ func TestTeam_RemoveMember(t *testing.T) { assert.True(t, organization.IsErrLastOrgOwner(err)) } +func TestRemoveTeamMemberRemovesSubscriptionsAndStopwatches(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + ctx := t.Context() + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID}) + + assert.NoError(t, repo_model.WatchRepo(ctx, user, repo, true)) + assert.NoError(t, issues_model.CreateOrUpdateIssueWatch(ctx, user.ID, issue.ID, true)) + ok, err := issues_model.CreateIssueStopwatch(ctx, user, issue) + assert.NoError(t, err) + assert.True(t, ok) + + assert.NoError(t, RemoveTeamMember(ctx, team, user)) + + watch, err := repo_model.GetWatch(ctx, user.ID, repo.ID) + assert.NoError(t, err) + assert.False(t, repo_model.IsWatchMode(watch.Mode)) + + _, exists, err := issues_model.GetIssueWatch(ctx, user.ID, issue.ID) + assert.NoError(t, err) + assert.False(t, exists) + + hasStopwatch, _, _, err := issues_model.HasUserStopwatch(ctx, user.ID) + assert.NoError(t, err) + assert.False(t, hasStopwatch) +} + func TestNewTeam(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/repository/collaboration.go b/services/repository/collaboration.go index 53b3c2e203..cb56d90ae2 100644 --- a/services/repository/collaboration.go +++ b/services/repository/collaboration.go @@ -120,6 +120,11 @@ func ReconsiderWatches(ctx context.Context, repo *repo_model.Repository, user *u return err } + // Remove all stopwatches a user has running in the repository + if err := issues_model.RemoveStopwatchesByRepoID(ctx, user.ID, repo.ID); err != nil { + return err + } + // Remove all IssueWatches a user has subscribed to in the repository return issues_model.RemoveIssueWatchersByRepoID(ctx, user.ID, repo.ID) } diff --git a/services/repository/collaboration_test.go b/services/repository/collaboration_test.go index 5e33c50366..56d9d72e0a 100644 --- a/services/repository/collaboration_test.go +++ b/services/repository/collaboration_test.go @@ -6,7 +6,10 @@ package repository import ( "testing" + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" "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/unittest" user_model "code.gitea.io/gitea/models/user" @@ -32,8 +35,8 @@ func TestRepository_AddCollaborator(t *testing.T) { func TestRepository_DeleteCollaboration(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 22}) assert.NoError(t, repo.LoadOwner(t.Context())) assert.NoError(t, DeleteCollaboration(t.Context(), repo, user)) @@ -44,3 +47,50 @@ func TestRepository_DeleteCollaboration(t *testing.T) { unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repo.ID}) } + +func TestRepository_DeleteCollaborationRemovesSubscriptionsAndStopwatches(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + ctx := t.Context() + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 22}) + assert.NoError(t, repo.LoadOwner(ctx)) + assert.NoError(t, repo_model.WatchRepo(ctx, user, repo, true)) + + hasAccess, err := access_model.HasAnyUnitAccess(ctx, user.ID, repo) + assert.NoError(t, err) + assert.True(t, hasAccess) + + issueCount, err := db.GetEngine(ctx).Where("repo_id=?", repo.ID).Count(new(issues_model.Issue)) + assert.NoError(t, err) + tempIssue := &issues_model.Issue{ + RepoID: repo.ID, + Index: issueCount + 1, + PosterID: repo.OwnerID, + Title: "temp issue", + Content: "temp", + } + assert.NoError(t, db.Insert(ctx, tempIssue)) + assert.NoError(t, issues_model.CreateOrUpdateIssueWatch(ctx, user.ID, tempIssue.ID, true)) + ok, err := issues_model.CreateIssueStopwatch(ctx, user, tempIssue) + assert.NoError(t, err) + assert.True(t, ok) + + assert.NoError(t, DeleteCollaboration(ctx, repo, user)) + + hasAccess, err = access_model.HasAnyUnitAccess(ctx, user.ID, repo) + assert.NoError(t, err) + assert.False(t, hasAccess) + + watch, err := repo_model.GetWatch(ctx, user.ID, repo.ID) + assert.NoError(t, err) + assert.False(t, repo_model.IsWatchMode(watch.Mode)) + + _, exists, err := issues_model.GetIssueWatch(ctx, user.ID, tempIssue.ID) + assert.NoError(t, err) + assert.False(t, exists) + + hasStopwatch, _, _, err := issues_model.HasUserStopwatch(ctx, user.ID) + assert.NoError(t, err) + assert.False(t, hasStopwatch) +}