Enhance Ghost comment mitigation Settings (go-gitea#14392)

* refactor models.DeleteComment and delete related reactions too

* use deleteComment for UserDeleteWithCommentsMaxDays in DeleteUser

* nits

* Use time.Duration as other time settings have

* docs

* Resolve Fixme & fix potential deadlock

* Disabled by Default

* Update Config Value Description

* switch args

* Update models/issue_comment.go

Co-authored-by: zeripath <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>

Author: 2 people

custom/conf/app.example.ini

@@ -671,9 +671,8 @@ AUTO_WATCH_NEW_REPOS = true
 ; Default value for AutoWatchOnChanges
 ; Make the user watch a repository When they commit for the first time
 AUTO_WATCH_ON_CHANGES = false
-; Default value for the minimum age a user has to exist before deletion to keep issue comments.
-; If a user deletes his account before that amount of days, his comments will be deleted as well.
-USER_DELETE_WITH_COMMENTS_MAX_DAYS = 0
+; Minimum amount of time a user must exist before comments are kept when the user is deleted.
+USER_DELETE_WITH_COMMENTS_MAX_TIME = 0
 
 [webhook]
 ; Hook task queue length, increase if webhook shooting starts hanging

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -474,7 +474,7 @@ relation to port exhaustion.
 - `ALLOW_ONLY_EXTERNAL_REGISTRATION`: **false** Set to true to force registration only using third-party services.
 - `NO_REPLY_ADDRESS`: **DOMAIN** Default value for the domain part of the user's email address in the git log if he has set KeepEmailPrivate to true. 
   The user's email will be replaced with a concatenation of the user name in lower case, "@" and NO_REPLY_ADDRESS.
-- `USER_DELETE_WITH_COMMENTS_MAX_DAYS`: **0** If a user deletes his account before that amount of days, his comments will be deleted as well.
+- `USER_DELETE_WITH_COMMENTS_MAX_TIME`: **0** Minimum amount of time a user must exist before comments are kept when the user is deleted.
 
 ## SSH Minimum Key Sizes (`ssh.minimum_key_sizes`)
 

models/issue_comment.go

@@ -1059,37 +1059,41 @@ func UpdateComment(c *Comment, doer *User) error {
 }
 
 // DeleteComment deletes the comment
-func DeleteComment(comment *Comment, doer *User) error {
+func DeleteComment(comment *Comment) error {
 	sess := x.NewSession()
 	defer sess.Close()
 	if err := sess.Begin(); err != nil {
 		return err
 	}
 
-	if _, err := sess.Delete(&Comment{
+	if err := deleteComment(sess, comment); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}
+
+func deleteComment(e Engine, comment *Comment) error {
+	if _, err := e.Delete(&Comment{
 		ID: comment.ID,
 	}); err != nil {
 		return err
 	}
 
 	if comment.Type == CommentTypeComment {
-		if _, err := sess.Exec("UPDATE `issue` SET num_comments = num_comments - 1 WHERE id = ?", comment.IssueID); err != nil {
+		if _, err := e.Exec("UPDATE `issue` SET num_comments = num_comments - 1 WHERE id = ?", comment.IssueID); err != nil {
 			return err
 		}
 	}
-	if _, err := sess.Where("comment_id = ?", comment.ID).Cols("is_deleted").Update(&Action{IsDeleted: true}); err != nil {
+	if _, err := e.Where("comment_id = ?", comment.ID).Cols("is_deleted").Update(&Action{IsDeleted: true}); err != nil {
 		return err
 	}
 
-	if err := comment.neuterCrossReferences(sess); err != nil {
+	if err := comment.neuterCrossReferences(e); err != nil {
 		return err
 	}
 
-	if err := deleteReaction(sess, &ReactionOptions{Comment: comment}); err != nil {
-		return err
-	}
-
-	return sess.Commit()
+	return deleteReaction(e, &ReactionOptions{Comment: comment})
 }
 
 // CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS

models/user.go

@@ -1141,12 +1141,30 @@ func deleteUser(e Engine, u *User) error {
 		return fmt.Errorf("deleteBeans: %v", err)
 	}
 
-	if setting.Service.UserDeleteWithCommentsMaxDays != 0 &&
-		u.CreatedUnix.AsTime().Add(time.Duration(setting.Service.UserDeleteWithCommentsMaxDays)*24*time.Hour).After(time.Now()) {
-		if err = deleteBeans(e,
-			&Comment{PosterID: u.ID},
-		); err != nil {
-			return fmt.Errorf("deleteBeans: %v", err)
+	if setting.Service.UserDeleteWithCommentsMaxTime != 0 &&
+		u.CreatedUnix.AsTime().Add(setting.Service.UserDeleteWithCommentsMaxTime).After(time.Now()) {
+
+		// Delete Comments
+		const batchSize = 50
+		for start := 0; ; start += batchSize {
+			comments := make([]*Comment, 0, batchSize)
+			if err = e.Where("type=? AND poster_id=?", CommentTypeComment, u.ID).Limit(batchSize, start).Find(&comments); err != nil {
+				return err
+			}
+			if len(comments) == 0 {
+				break
+			}
+
+			for _, comment := range comments {
+				if err = deleteComment(e, comment); err != nil {
+					return err
+				}
+			}
+		}
+
+		// Delete Reactions
+		if err = deleteReaction(e, &ReactionOptions{Doer: u}); err != nil {
+			return err
 		}
 	}
 

modules/setting/service.go

@@ -6,6 +6,7 @@ package setting
 
 import (
 	"regexp"
+	"time"
 
 	"code.gitea.io/gitea/modules/structs"
 )
@@ -49,7 +50,7 @@ var Service struct {
 	AutoWatchNewRepos                       bool
 	AutoWatchOnChanges                      bool
 	DefaultOrgMemberVisible                 bool
-	UserDeleteWithCommentsMaxDays           int
+	UserDeleteWithCommentsMaxTime           time.Duration
 
 	// OpenID settings
 	EnableOpenIDSignIn bool
@@ -97,7 +98,7 @@ func newService() {
 	Service.DefaultOrgVisibility = sec.Key("DEFAULT_ORG_VISIBILITY").In("public", structs.ExtractKeysFromMapString(structs.VisibilityModes))
 	Service.DefaultOrgVisibilityMode = structs.VisibilityModes[Service.DefaultOrgVisibility]
 	Service.DefaultOrgMemberVisible = sec.Key("DEFAULT_ORG_MEMBER_VISIBLE").MustBool()
-	Service.UserDeleteWithCommentsMaxDays = sec.Key("USER_DELETE_WITH_COMMENTS_MAX_DAYS").MustInt(0)
+	Service.UserDeleteWithCommentsMaxTime = sec.Key("USER_DELETE_WITH_COMMENTS_MAX_TIME").MustDuration(0)
 
 	sec = Cfg.Section("openid")
 	Service.EnableOpenIDSignIn = sec.Key("ENABLE_OPENID_SIGNIN").MustBool(!InstallLock)

options/locale/locale_de-DE.ini

@@ -626,7 +626,7 @@ repos_none=Du besitzt keine Repositories
 
 delete_account=Konto löschen
 delete_prompt=Wenn du fortfährst, wird dein Account permanent gelöscht. Dies <strong>KANN NICHT</strong> rückgängig gemacht werden.
-delete_with_all_comments = Dein Account ist jünger als %d Tage. Um Geisterkommentare zu vermeiden, werden alle Issue/PR-Kommentare zusammen mit deinem Benutzeraccount gelöscht.
+delete_with_all_comments = Dein Account ist jünger als %s. Um Geisterkommentare zu vermeiden, werden alle Issue/PR-Kommentare zusammen mit deinem Benutzeraccount gelöscht.
 confirm_delete_account=Löschen bestätigen
 delete_account_title=Benutzerkonto löschen
 delete_account_desc=Bist du sicher, dass du diesen Account dauerhaft löschen möchtest?

options/locale/locale_en-US.ini

@@ -640,7 +640,7 @@ repos_none = You do not own any repositories
 
 delete_account = Delete Your Account
 delete_prompt = This operation will permanently delete your user account. It <strong>CAN NOT</strong> be undone.
-delete_with_all_comments = Your account is younger than %d days. To avoid ghost comments, all issue/PR comments will be deleted with it.
+delete_with_all_comments = Your account is younger than %s. To avoid ghost comments, all issue/PR comments will be deleted with it.
 confirm_delete_account = Confirm Deletion
 delete_account_title = Delete User Account
 delete_account_desc = Are you sure you want to permanently delete this user account?

routers/api/v1/repo/issue_comment.go

@@ -508,7 +508,7 @@ func deleteIssueComment(ctx *context.APIContext) {
 		return
 	}
 
-	if err = comment_service.DeleteComment(comment, ctx.User); err != nil {
+	if err = comment_service.DeleteComment(ctx.User, comment); err != nil {
 		ctx.Error(http.StatusInternalServerError, "DeleteCommentByID", err)
 		return
 	}

routers/repo/issue.go

@@ -2240,7 +2240,7 @@ func DeleteComment(ctx *context.Context) {
 		return
 	}
 
-	if err = comment_service.DeleteComment(comment, ctx.User); err != nil {
+	if err = comment_service.DeleteComment(ctx.User, comment); err != nil {
 		ctx.ServerError("DeleteCommentByID", err)
 		return
 	}

routers/user/setting/account.go

@@ -303,8 +303,8 @@ func loadAccountData(ctx *context.Context) {
 	ctx.Data["ActivationsPending"] = pendingActivation
 	ctx.Data["CanAddEmails"] = !pendingActivation || !setting.Service.RegisterEmailConfirm
 
-	if setting.Service.UserDeleteWithCommentsMaxDays != 0 {
-		ctx.Data["UserDeleteWithCommentsMaxDays"] = setting.Service.UserDeleteWithCommentsMaxDays
-		ctx.Data["UserDeleteWithComments"] = ctx.User.CreatedUnix.AsTime().Add(time.Duration(setting.Service.UserDeleteWithCommentsMaxDays) * 24 * time.Hour).After(time.Now())
+	if setting.Service.UserDeleteWithCommentsMaxTime != 0 {
+		ctx.Data["UserDeleteWithCommentsMaxTime"] = setting.Service.UserDeleteWithCommentsMaxTime.String()
+		ctx.Data["UserDeleteWithComments"] = ctx.User.CreatedUnix.AsTime().Add(setting.Service.UserDeleteWithCommentsMaxTime).After(time.Now())
 	}
 }