fix: cleanup team memberships, assignments and subscriptions when users lose access to a project

This commit is contained in:
kolaente
2025-10-09 13:19:58 +02:00
parent 7442fbb9c2
commit 9358954c90
7 changed files with 278 additions and 2 deletions

View File

@@ -257,6 +257,18 @@ func (t *TeamMemberAddedEvent) Name() string {
return "team.member.added"
}
// TeamMemberRemovedEvent defines an event where a user is removed from a team
type TeamMemberRemovedEvent struct {
Team *Team `json:"team"`
Member *user.User `json:"member"`
Doer *user.User `json:"doer"`
}
// Name defines the name for TeamMemberRemovedEvent
func (t *TeamMemberRemovedEvent) Name() string {
return "team.member.removed"
}
// TeamCreatedEvent represents a TeamCreatedEvent event
type TeamCreatedEvent struct {
Team *Team `json:"team"`

View File

@@ -53,6 +53,7 @@ func RegisterListeners() {
events.RegisterListener((&TaskDeletedEvent{}).Name(), &SendTaskDeletedNotification{})
events.RegisterListener((&ProjectCreatedEvent{}).Name(), &SendProjectCreatedNotification{})
events.RegisterListener((&TeamMemberAddedEvent{}).Name(), &SendTeamMemberAddedNotification{})
events.RegisterListener((&TeamMemberRemovedEvent{}).Name(), &CleanupTaskAssignmentsAfterTeamRemoval{})
events.RegisterListener((&TaskCommentUpdatedEvent{}).Name(), &HandleTaskCommentEditMentions{})
events.RegisterListener((&TaskCreatedEvent{}).Name(), &HandleTaskCreateMentions{})
events.RegisterListener((&TaskUpdatedEvent{}).Name(), &HandleTaskUpdatedMentions{})
@@ -1072,6 +1073,43 @@ func (s *DecreaseTeamCounter) Handle(_ *message.Message) (err error) {
return keyvalue.DecrBy(metrics.TeamCountKey, 1)
}
// CleanupTaskAssignmentsAfterTeamRemoval represents a listener
type CleanupTaskAssignmentsAfterTeamRemoval struct{}
// Name defines the name of the listener
func (l *CleanupTaskAssignmentsAfterTeamRemoval) Name() string {
return "task.assignees.cleanup.team_removal"
}
// Handle cleans up task assignments and subscriptions for members removed from teams
func (l *CleanupTaskAssignmentsAfterTeamRemoval) Handle(msg *message.Message) (err error) {
event := &TeamMemberRemovedEvent{}
err = json.Unmarshal(msg.Payload, event)
if err != nil {
return err
}
s := db.NewSession()
defer s.Close()
if event == nil || event.Team == nil || event.Member == nil {
return nil
}
err = s.Begin()
if err != nil {
return err
}
err = cleanupTaskMembersAfterTeamRemoval(s, event.Team.ID, event.Member.ID)
if err != nil {
_ = s.Rollback()
return err
}
return s.Commit()
}
// SendTeamMemberAddedNotification represents a listener
type SendTeamMemberAddedNotification struct {
}

View File

@@ -88,7 +88,7 @@ func (tm *TeamMember) Create(s *xorm.Session, a web.Auth) (err error) {
// @Success 200 {object} models.Message "The user was successfully removed from the team."
// @Failure 500 {object} models.Message "Internal error"
// @Router /teams/{id}/members/{username} [delete]
func (tm *TeamMember) Delete(s *xorm.Session, _ web.Auth) (err error) {
func (tm *TeamMember) Delete(s *xorm.Session, a web.Auth) (err error) {
t, err := GetTeamByID(s, tm.TeamID)
if err != nil {
@@ -115,7 +115,21 @@ func (tm *TeamMember) Delete(s *xorm.Session, _ web.Auth) (err error) {
tm.UserID = user.ID
_, err = s.Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID).Delete(&TeamMember{})
return
if err != nil {
return err
}
err = s.Commit()
if err != nil {
return err
}
doer, _ := user2.GetFromAuth(a)
return events.Dispatch(&TeamMemberRemovedEvent{
Team: t,
Member: user,
Doer: doer,
})
}
func (tm *TeamMember) MembershipExists(s *xorm.Session) (exists bool, err error) {

View File

@@ -165,3 +165,90 @@ func TestTeamMember_Update(t *testing.T) {
}, false)
})
}
func TestCleanupTaskMembersAfterTeamRemoval(t *testing.T) {
t.Run("removes data when member loses team access", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
task := &Task{
Title: "team cleanup",
ProjectID: 19,
CreatedByID: 7,
Index: 2,
}
_, err := s.Insert(task)
require.NoError(t, err)
_, err = s.Insert(&TaskAssginee{TaskID: task.ID, UserID: 2})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityTask, EntityID: task.ID, UserID: 2})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityProject, EntityID: 19, UserID: 2})
require.NoError(t, err)
_, err = s.Where("team_id = ? AND user_id = ?", 9, 2).Delete(&TeamMember{})
require.NoError(t, err)
_, err = s.Where("project_id = ? AND user_id = ?", 19, 2).Delete(&ProjectUser{})
require.NoError(t, err)
require.NoError(t, s.Commit())
err = cleanupTaskMembersAfterTeamRemoval(s, 9, 2)
require.NoError(t, err)
db.AssertMissing(t, "task_assignees", map[string]interface{}{"task_id": task.ID, "user_id": 2})
db.AssertMissing(t, "subscriptions", map[string]interface{}{"entity_type": SubscriptionEntityTask, "entity_id": task.ID, "user_id": 2})
db.AssertMissing(t, "subscriptions", map[string]interface{}{"entity_type": SubscriptionEntityProject, "entity_id": 19, "user_id": 2})
})
t.Run("removes orphaned data for deleted project", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
const orphanProjectID int64 = 54321
s := db.NewSession()
defer s.Close()
_, err := s.Insert(&TeamProject{TeamID: 9, ProjectID: orphanProjectID, Permission: PermissionRead})
require.NoError(t, err)
task := &Task{
Title: "orphan cleanup",
ProjectID: orphanProjectID,
CreatedByID: 7,
Index: 5,
}
_, err = s.Insert(task)
require.NoError(t, err)
_, err = s.Insert(&TaskAssginee{TaskID: task.ID, UserID: 2})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityTask, EntityID: task.ID, UserID: 2})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityProject, EntityID: orphanProjectID, UserID: 2})
require.NoError(t, err)
_, err = s.Where("team_id = ? AND user_id = ?", 9, 2).Delete(&TeamMember{})
require.NoError(t, err)
_, err = s.Where("project_id = ? AND user_id = ?", orphanProjectID, 2).Delete(&ProjectUser{})
require.NoError(t, err)
require.NoError(t, s.Commit())
err = cleanupTaskMembersAfterTeamRemoval(s, 9, 2)
require.NoError(t, err)
db.AssertMissing(t, "task_assignees", map[string]interface{}{"task_id": task.ID, "user_id": 2})
db.AssertMissing(t, "subscriptions", map[string]interface{}{"entity_type": SubscriptionEntityTask, "entity_id": task.ID, "user_id": 2})
db.AssertMissing(t, "subscriptions", map[string]interface{}{"entity_type": SubscriptionEntityProject, "entity_id": orphanProjectID, "user_id": 2})
})
}

View File

@@ -396,3 +396,73 @@ func (t *Team) Update(s *xorm.Session, _ web.Auth) (err error) {
return
}
func cleanupTaskMembersAfterTeamRemoval(s *xorm.Session, teamID int64, memberID int64) (err error) {
teamProjectIDs := []int64{}
err = s.Table("team_projects").
Select("project_id").
Where("team_id = ?", teamID).
Find(&teamProjectIDs)
if err != nil {
return err
}
if len(teamProjectIDs) == 0 {
return nil
}
projectsToCleanup := make([]int64, 0, len(teamProjectIDs))
for _, projectID := range teamProjectIDs {
project, projErr := GetProjectSimpleByID(s, projectID)
if projErr != nil {
if IsErrProjectDoesNotExist(projErr) {
projectsToCleanup = append(projectsToCleanup, projectID)
continue
}
return projErr
}
canRead, _, permErr := project.CanRead(s, &user.User{ID: memberID})
if permErr != nil {
return permErr
}
if !canRead {
projectsToCleanup = append(projectsToCleanup, projectID)
}
}
if len(projectsToCleanup) == 0 {
return nil
}
taskIDs := []int64{}
err = s.Table("tasks").
Select("id").
In("project_id", projectsToCleanup).
Find(&taskIDs)
if err != nil {
return err
}
if len(taskIDs) > 0 {
_, err = s.In("task_id", taskIDs).
And("user_id = ?", memberID).
Delete(&TaskAssginee{})
if err != nil {
return err
}
_, err = s.In("entity_id", taskIDs).
Where("entity_type = ? AND user_id = ?", SubscriptionEntityTask, memberID).
Delete(&Subscription{})
if err != nil {
return err
}
}
_, err = s.In("entity_id", projectsToCleanup).
Where("entity_type = ? AND user_id = ?", SubscriptionEntityProject, memberID).
Delete(&Subscription{})
return err
}

View File

@@ -149,6 +149,27 @@ func DeleteUser(s *xorm.Session, u *user.User) (err error) {
}
}
// Delete all related entities
relatedEntities := []struct {
column string
model any
}{
{"user_id", &TaskAssginee{}},
{"user_id", &Subscription{}},
{"user_id", &TeamMember{}},
{"owner_id", &SavedFilter{}},
{"user_id", &Reaction{}},
{"user_id", &Favorite{}},
{"owner_id", &APIToken{}},
}
for _, entity := range relatedEntities {
_, err = s.Where(entity.column+" = ?", u.ID).Delete(entity.model)
if err != nil {
return err
}
}
_, err = s.Where("id = ?", u.ID).Delete(&user.User{})
if err != nil {
return err

View File

@@ -71,4 +71,38 @@ func TestDeleteUser(t *testing.T) {
db.AssertMissing(t, "users", map[string]interface{}{"id": u.ID})
db.AssertMissing(t, "projects", map[string]interface{}{"id": 37}) // only user16 had access to this project, and it was their default
})
t.Run("cleans up task assignments and subscriptions", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
notifications.Fake()
task := &Task{
Title: "user cleanup",
ProjectID: 19,
CreatedByID: 7,
Index: 4,
}
_, err := s.Insert(task)
require.NoError(t, err)
_, err = s.Insert(&TaskAssginee{TaskID: task.ID, UserID: 4})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityTask, EntityID: task.ID, UserID: 4})
require.NoError(t, err)
_, err = s.Insert(&Subscription{EntityType: SubscriptionEntityProject, EntityID: 19, UserID: 4})
require.NoError(t, err)
_, err = s.Insert(&TeamMember{TeamID: 9, UserID: 4})
require.NoError(t, err)
err = DeleteUser(s, &user.User{ID: 4})
require.NoError(t, err)
db.AssertMissing(t, "task_assignees", map[string]interface{}{"user_id": 4})
db.AssertMissing(t, "subscriptions", map[string]interface{}{"user_id": 4})
db.AssertMissing(t, "team_members", map[string]interface{}{"user_id": 4})
})
}