Add more check for stopwatch read or list (go-gitea#36340)

Author: lunny

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
+}

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)

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

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

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{

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})
+}

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())
 

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)
 }

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)
+}