Skip to content

Fix not removed watches on unallowed repositories #4201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 19, 2018
Merged
12 changes: 12 additions & 0 deletions models/issue_watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,15 @@ func getIssueWatchers(e Engine, issueID int64) (watches []*IssueWatch, err error
Find(&watches)
return
}

func removeIssueWatchersByRepoID(e Engine, userID int64, repoID int64) error {
iw := &IssueWatch{
IsWatching: false,
}
_, err := e.
Join("INNER", "issue", "`issue`.id = `issue_watch`.issue_id AND `issue`.repo_id = ?", repoID).
Cols("is_watching", "updated_unix").
Where("`issue_watch`.user_id = ?", userID).
Update(iw)
return err
}
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ var migrations = []Migration{
NewMigration("add u2f", addU2FReg),
// v66 -> v67
NewMigration("add login source id column for public_key table", addLoginSourceIDToPublicKeyTable),
// v67 -> v68
NewMigration("remove stale watches", removeStaleWatches),
}

// Migrate database to current version
Expand Down
158 changes: 158 additions & 0 deletions models/migrations/v67.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
// Copyright 2018 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"code.gitea.io/gitea/modules/setting"

"github.com/go-xorm/xorm"
)

func removeStaleWatches(x *xorm.Engine) error {
type Watch struct {
ID int64
UserID int64
RepoID int64
}

type IssueWatch struct {
ID int64
UserID int64
RepoID int64
IsWatching bool
}

type Repository struct {
ID int64
IsPrivate bool
OwnerID int64
}

type Access struct {
UserID int64
RepoID int64
Mode int
}

const (
// AccessModeNone no access
AccessModeNone int = iota // 0
// AccessModeRead read access
AccessModeRead // 1
)

accessLevel := func(userID int64, repo *Repository) (int, error) {
mode := AccessModeNone
if !repo.IsPrivate {
mode = AccessModeRead
}

if userID == 0 {
return mode, nil
}

if userID == repo.OwnerID {
return 4, nil
}

a := &Access{UserID: userID, RepoID: repo.ID}
if has, err := x.Get(a); !has || err != nil {
return mode, err
}
return a.Mode, nil
}

sess := x.NewSession()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need

defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. Totally forgot.

defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

repoCache := make(map[int64]*Repository)
err := x.BufferSize(setting.IterateBufferSize).Iterate(new(Watch),
func(idx int, bean interface{}) error {
watch := bean.(*Watch)

repo := repoCache[watch.RepoID]
if repo == nil {
repo = &Repository{
ID: watch.RepoID,
}
if _, err := x.Get(repo); err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should repoCache[watch.RepoID] = repo

repoCache[watch.RepoID] = repo
}

// Remove watches from now unaccessible repositories
mode, err := accessLevel(watch.UserID, repo)
if err != nil {
return err
}
has := AccessModeRead <= mode
if has {
return nil
}

if _, err = sess.Delete(&Watch{0, watch.UserID, repo.ID}); err != nil {
return err
}
_, err = sess.Exec("UPDATE `repository` SET num_watches = num_watches - 1 WHERE id = ?", repo.ID)

return err
})
if err != nil {
return err
}

repoCache = make(map[int64]*Repository)
err = x.BufferSize(setting.IterateBufferSize).
Distinct("issue_watch.user_id", "issue.repo_id").
Join("INNER", "issue", "issue_watch.issue_id = issue.id").
Where("issue_watch.is_watching = ?", true).
Iterate(new(IssueWatch),
func(idx int, bean interface{}) error {
watch := bean.(*IssueWatch)

repo := repoCache[watch.RepoID]
if repo == nil {
repo = &Repository{
ID: watch.RepoID,
}
if _, err := x.Get(repo); err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above

repoCache[watch.RepoID] = repo
}

// Remove issue watches from now unaccssible repositories
mode, err := accessLevel(watch.UserID, repo)
if err != nil {
return err
}
has := AccessModeRead <= mode
if has {
return nil
}

iw := &IssueWatch{
IsWatching: false,
}

_, err = sess.
Join("INNER", "issue", "`issue`.id = `issue_watch`.issue_id AND `issue`.repo_id = ?", watch.RepoID).
Cols("is_watching", "updated_unix").
Where("`issue_watch`.user_id = ?", watch.UserID).
Update(iw)

return err

})
if err != nil {
return err
}

return sess.Commit()
}
49 changes: 49 additions & 0 deletions models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e
if err = watchRepo(e, teamUser.UID, repo.ID, false); err != nil {
return err
}

// Remove all IssueWatches a user has subscribed to in the repositories
if err := removeIssueWatchersByRepoID(e, teamUser.UID, repo.ID); err != nil {
return err
}
}

return nil
Expand Down Expand Up @@ -374,11 +379,34 @@ func DeleteTeam(t *Team) error {
return err
}

if err := t.getMembers(sess); err != nil {
return err
}

// Delete all accesses.
for _, repo := range t.Repos {
if err := repo.recalculateTeamAccesses(sess, t.ID); err != nil {
return err
}

// Remove watches from all users and now unaccessible repos
for _, user := range t.Members {
has, err := hasAccess(sess, user.ID, repo, AccessModeRead)
if err != nil {
return err
} else if has {
continue
}

if err = watchRepo(sess, user.ID, repo.ID, false); err != nil {
return err
}

// Remove all IssueWatches a user has subscribed to in the repositories
if err = removeIssueWatchersByRepoID(sess, user.ID, repo.ID); err != nil {
return err
}
}
}

// Delete team-repo
Expand Down Expand Up @@ -518,6 +546,10 @@ func AddTeamMember(team *Team, userID int64) error {
if err := repo.recalculateTeamAccesses(sess, 0); err != nil {
return err
}

if err = watchRepo(sess, userID, repo.ID, true); err != nil {
return err
}
}

return sess.Commit()
Expand Down Expand Up @@ -558,6 +590,23 @@ func removeTeamMember(e *xorm.Session, team *Team, userID int64) error {
if err := repo.recalculateTeamAccesses(e, 0); err != nil {
return err
}

// Remove watches from now unaccessible
has, err := hasAccess(e, userID, repo, AccessModeRead)
if err != nil {
return err
} else if has {
continue
}

if err = watchRepo(e, userID, repo.ID, false); err != nil {
return err
}

// Remove all IssueWatches a user has subscribed to in the repositories
if err := removeIssueWatchersByRepoID(e, userID, repo.ID); err != nil {
return err
}
}

// Check if the user is a member of any team in the organization.
Expand Down
3 changes: 3 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -1851,6 +1851,9 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
if _, err = sess.In("issue_id", issueIDs).Delete(&Reaction{}); err != nil {
return err
}
if _, err = sess.In("issue_id", issueIDs).Delete(&IssueWatch{}); err != nil {
return err
}

attachments := make([]*Attachment, 0, 5)
if err = sess.
Expand Down
9 changes: 9 additions & 0 deletions models/repo_collaboration.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,5 +172,14 @@ func (repo *Repository) DeleteCollaboration(uid int64) (err error) {
return err
}

if err = watchRepo(sess, uid, repo.ID, false); err != nil {
return err
}

// Remove all IssueWatches a user has subscribed to in the repository
if err := removeIssueWatchersByRepoID(sess, uid, repo.ID); err != nil {
return err
}

return sess.Commit()
}