Skip to content
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

Enhance Ghost comment mitigation Settings #14392

Merged
5 changes: 2 additions & 3 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -688,9 +688,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
Expand Down
2 changes: 1 addition & 1 deletion docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

Expand Down
2 changes: 1 addition & 1 deletion models/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func removeStorageWithNotice(e Engine, bucket storage.ObjectStorage, title, path
if err := bucket.Delete(path); err != nil {
desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
log.Warn(title+" [%s]: %v", path, err)
if err = createNotice(x, NoticeRepository, desc); err != nil {
if err = createNotice(e, NoticeRepository, desc); err != nil {
6543 marked this conversation as resolved.
Show resolved Hide resolved
log.Error("CreateRepositoryNotice: %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion models/issue_assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func isUserAssignedToIssue(e Engine, issue *Issue, user *User) (isAssigned bool,
}

// ClearAssigneeByUserID deletes all assignments of an user
func clearAssigneeByUserID(sess *xorm.Session, userID int64) (err error) {
func clearAssigneeByUserID(sess Engine, userID int64) (err error) {
_, err = sess.Delete(&IssueAssignees{AssigneeID: userID})
return
}
Expand Down
20 changes: 14 additions & 6 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -1037,33 +1037,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
}

return sess.Commit()
return deleteReaction(e, &ReactionOptions{Comment: comment})
}

// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS
Expand Down
12 changes: 8 additions & 4 deletions models/issue_reaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,15 @@ func CreateCommentReaction(doer *User, issue *Issue, comment *Comment, content s
})
}

func deleteReaction(e *xorm.Session, opts *ReactionOptions) error {
func deleteReaction(e Engine, opts *ReactionOptions) error {
reaction := &Reaction{
Type: opts.Type,
UserID: opts.Doer.ID,
IssueID: opts.Issue.ID,
Type: opts.Type,
}
if opts.Doer != nil {
reaction.UserID = opts.Doer.ID
}
if opts.Issue != nil {
reaction.IssueID = opts.Issue.ID
}
if opts.Comment != nil {
reaction.CommentID = opts.Comment.ID
Expand Down
47 changes: 33 additions & 14 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"golang.org/x/crypto/scrypt"
"golang.org/x/crypto/ssh"
"xorm.io/builder"
"xorm.io/xorm"
)

// UserType defines the user type
Expand Down Expand Up @@ -1071,8 +1070,7 @@ func deleteBeans(e Engine, beans ...interface{}) (err error) {
return nil
}

// FIXME: need some kind of mechanism to record failure. HINT: system notice
func deleteUser(e *xorm.Session, u *User) error {
func deleteUser(e Engine, u *User) error {
// Note: A user owns any repository or belongs to any organization
// cannot perform delete operation.

Expand Down Expand Up @@ -1151,12 +1149,30 @@ func deleteUser(e *xorm.Session, 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
}
}

Expand Down Expand Up @@ -1195,18 +1211,21 @@ func deleteUser(e *xorm.Session, u *User) error {
return fmt.Errorf("Delete: %v", err)
}

// FIXME: system notice
// Note: There are something just cannot be roll back,
// so just keep error logs of those operations.
path := UserPath(u.Name)
if err := util.RemoveAll(path); err != nil {
return fmt.Errorf("Failed to RemoveAll %s: %v", path, err)
if err = util.RemoveAll(path); err != nil {
err = fmt.Errorf("Failed to RemoveAll %s: %v", path, err)
_ = createNotice(e, NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err))
return err
}

if len(u.Avatar) > 0 {
avatarPath := u.CustomAvatarRelativePath()
if err := storage.Avatars.Delete(avatarPath); err != nil {
return fmt.Errorf("Failed to remove %s: %v", avatarPath, err)
if err = storage.Avatars.Delete(avatarPath); err != nil {
err = fmt.Errorf("Failed to remove %s: %v", avatarPath, err)
_ = createNotice(e, NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err))
return err
}
}

Expand Down
5 changes: 3 additions & 2 deletions modules/setting/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package setting

import (
"regexp"
"time"

"code.gitea.io/gitea/modules/structs"
)
Expand Down Expand Up @@ -50,7 +51,7 @@ var Service struct {
AutoWatchNewRepos bool
AutoWatchOnChanges bool
DefaultOrgMemberVisible bool
UserDeleteWithCommentsMaxDays int
UserDeleteWithCommentsMaxTime time.Duration

// OpenID settings
EnableOpenIDSignIn bool
Expand Down Expand Up @@ -103,7 +104,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)
Expand Down
2 changes: 1 addition & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,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?
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,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
}
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -2125,7 +2125,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
}
Expand Down
6 changes: 3 additions & 3 deletions routers/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,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())
}
}
4 changes: 2 additions & 2 deletions services/comments/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ func UpdateComment(c *models.Comment, doer *models.User, oldContent string) erro
}

// DeleteComment deletes the comment
func DeleteComment(comment *models.Comment, doer *models.User) error {
if err := models.DeleteComment(comment, doer); err != nil {
func DeleteComment(doer *models.User, comment *models.Comment) error {
if err := models.DeleteComment(comment); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion templates/user/settings/account.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
<div class="ui red message">
<p class="text left">{{svg "octicon-alert"}} {{.i18n.Tr "settings.delete_prompt" | Str2html}}</p>
{{ if .UserDeleteWithComments }}
<p class="text left" style="font-weight: bold;">{{.i18n.Tr "settings.delete_with_all_comments" .UserDeleteWithCommentsMaxDays | Str2html}}</p>
<p class="text left" style="font-weight: bold;">{{.i18n.Tr "settings.delete_with_all_comments" .UserDeleteWithCommentsMaxTime | Str2html}}</p>
{{ end }}
</div>
<form class="ui form ignore-dirty" id="delete-form" action="{{AppSubUrl}}/user/settings/account/delete" method="post">
Expand Down