Skip to content

fix forgot removed records when deleting user #5429

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 4 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ ignored = ["google.golang.org/appengine*"]
name = "github.com/go-xorm/xorm"
revision = "401f4ee8ff8cbc40a4754cb12192fbe4f02f3979"

[[override]]
name = "github.com/go-xorm/builder"
version = "0.3.3"

[[override]]
name = "github.com/go-sql-driver/mysql"
revision = "d523deb1b23d913de5bdada721a6071e71283618"
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ var migrations = []Migration{
NewMigration("add must_change_password column for users table", addMustChangePassword),
// v74 -> v75
NewMigration("add approval whitelists to protected branches", addApprovalWhitelistsToProtectedBranches),
// v75 -> v76
NewMigration("clear nonused data which not deleted when user was deleted", clearNonusedData),
}

// Migrate database to current version
Expand Down
33 changes: 33 additions & 0 deletions models/migrations/v75.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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 (
"github.com/go-xorm/builder"
"github.com/go-xorm/xorm"
)

func clearNonusedData(x *xorm.Engine) error {
condDelete := func(colName string) builder.Cond {
return builder.NotIn(colName, builder.Select("id").From("user"))
}

if _, err := x.Exec(builder.Delete(condDelete("uid")).From("team_user")); err != nil {
return err
}

if _, err := x.Exec(builder.Delete(condDelete("user_id")).From("collaboration")); err != nil {
return err
}

if _, err := x.Exec(builder.Delete(condDelete("user_id")).From("stop_watch")); err != nil {
return err
}

if _, err := x.Exec(builder.Delete(condDelete("owner_id")).From("gpg_key")); err != nil {
return err
}
return nil
}
7 changes: 6 additions & 1 deletion models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ func DeletePublicKey(doer *User, id int64) (err error) {
if err = sess.Commit(); err != nil {
return err
}
sess.Close()

return RewriteAllPublicKeys()
}
Expand All @@ -557,6 +558,10 @@ func DeletePublicKey(doer *User, id int64) (err error) {
// Note: x.Iterate does not get latest data after insert/delete, so we have to call this function
// outside any session scope independently.
func RewriteAllPublicKeys() error {
return rewriteAllPublicKeys(x)
}

func rewriteAllPublicKeys(e Engine) error {
//Don't rewrite key if internal server
if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
return nil
Expand All @@ -583,7 +588,7 @@ func RewriteAllPublicKeys() error {
}
}

err = x.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) {
err = e.Iterate(new(PublicKey), func(idx int, bean interface{}) (err error) {
_, err = t.WriteString((bean.(*PublicKey)).AuthorizedString())
return err
})
Expand Down
27 changes: 12 additions & 15 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -1015,25 +1015,26 @@ func deleteUser(e *xorm.Session, u *User) error {
&EmailAddress{UID: u.ID},
&UserOpenID{UID: u.ID},
&Reaction{UserID: u.ID},
&TeamUser{UID: u.ID},
&Collaboration{UserID: u.ID},
&Stopwatch{UserID: u.ID},
); err != nil {
return fmt.Errorf("deleteBeans: %v", err)
}

// ***** START: PublicKey *****
keys := make([]*PublicKey, 0, 10)
if err = e.Find(&keys, &PublicKey{OwnerID: u.ID}); err != nil {
return fmt.Errorf("get all public keys: %v", err)
}

keyIDs := make([]int64, len(keys))
for i := range keys {
keyIDs[i] = keys[i].ID
}
if err = deletePublicKeys(e, keyIDs...); err != nil {
if _, err = e.Delete(&PublicKey{OwnerID: u.ID}); err != nil {
return fmt.Errorf("deletePublicKeys: %v", err)
}
rewriteAllPublicKeys(e)
// ***** END: PublicKey *****
Copy link
Contributor

Choose a reason for hiding this comment

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

RewriteAllPublicKeys is called at the end of DeleteUser too, does it still need to be called there?


// ***** START: GPGPublicKey *****
if _, err = e.Delete(&GPGKey{OwnerID: u.ID}); err != nil {
return fmt.Errorf("deleteGPGKeys: %v", err)
}
// ***** END: GPGPublicKey *****

// Clear assignee.
if err = clearAssigneeByUserID(e, u.ID); err != nil {
return fmt.Errorf("clear assignee: %v", err)
Expand Down Expand Up @@ -1084,11 +1085,7 @@ func DeleteUser(u *User) (err error) {
return err
}

if err = sess.Commit(); err != nil {
return err
}

return RewriteAllPublicKeys()
return sess.Commit()
}

// DeleteInactivateUsers deletes all inactivate users and email addresses.
Expand Down
Loading