From 4c62873f8107ba8aba7b8fb966b9a37b89c07da6 Mon Sep 17 00:00:00 2001 From: Anton Khimich Date: Wed, 3 Feb 2021 15:43:50 -0500 Subject: [PATCH 1/3] Fix GPG key deletion when user is deleted Per #14531, deleting a user account will delete the user's GPG keys from the `gpg_key` table but not from `gpg_key_import`, which causes an error when creating an account with the same email and attempting to re-add the same key. This commit deletes all entries from `gpg_key_import` that match any GPG key IDs belonging to the user. --- models/user.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/models/user.go b/models/user.go index 8147c9f62610b..a9e9974cf7cda 100644 --- a/models/user.go +++ b/models/user.go @@ -1208,6 +1208,16 @@ func deleteUser(e Engine, u *User) error { // ***** END: PublicKey ***** // ***** START: GPGPublicKey ***** + keys, err := ListGPGKeys(u.ID, ListOptions{}) + if err != nil { + return fmt.Errorf("ListGPGKeys: %v", err) + } + // Delete GPGKeyImport(s). + for _, key := range keys { + if _, err = e.Delete(&GPGKeyImport{KeyID: key.KeyID}); err != nil { + return fmt.Errorf("deleteGPGKeyImports: %v", err) + } + } if _, err = e.Delete(&GPGKey{OwnerID: u.ID}); err != nil { return fmt.Errorf("deleteGPGKeys: %v", err) } From 11c2796c39c3df73543965ddb5edad2f73f6c972 Mon Sep 17 00:00:00 2001 From: Anton Khimich Date: Wed, 3 Feb 2021 16:06:09 -0500 Subject: [PATCH 2/3] Format added code in models/user.go --- models/user.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/models/user.go b/models/user.go index a9e9974cf7cda..642d9d56a887f 100644 --- a/models/user.go +++ b/models/user.go @@ -1208,16 +1208,16 @@ func deleteUser(e Engine, u *User) error { // ***** END: PublicKey ***** // ***** START: GPGPublicKey ***** - keys, err := ListGPGKeys(u.ID, ListOptions{}) - if err != nil { - return fmt.Errorf("ListGPGKeys: %v", err) - } - // Delete GPGKeyImport(s). - for _, key := range keys { - if _, err = e.Delete(&GPGKeyImport{KeyID: key.KeyID}); err != nil { - return fmt.Errorf("deleteGPGKeyImports: %v", err) - } - } + keys, err := ListGPGKeys(u.ID, ListOptions{}) + if err != nil { + return fmt.Errorf("ListGPGKeys: %v", err) + } + // Delete GPGKeyImport(s). + for _, key := range keys { + if _, err = e.Delete(&GPGKeyImport{KeyID: key.KeyID}); err != nil { + return fmt.Errorf("deleteGPGKeyImports: %v", err) + } + } if _, err = e.Delete(&GPGKey{OwnerID: u.ID}); err != nil { return fmt.Errorf("deleteGPGKeys: %v", err) } From 6c91cc227918a3fa2079b00cc4a28691a79cb0f9 Mon Sep 17 00:00:00 2001 From: Anton Khimich Date: Thu, 4 Feb 2021 00:41:52 -0500 Subject: [PATCH 3/3] Create a new function for listing GPG keys and apply it Create a new function `listGPGKeys` and replace a previous use of `ListGPGKeys`. Thanks to @6543 for the patch. --- models/gpg_key.go | 6 +++++- models/user.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/models/gpg_key.go b/models/gpg_key.go index b944fdcbffe45..3e8ddd9621c1e 100644 --- a/models/gpg_key.go +++ b/models/gpg_key.go @@ -65,7 +65,11 @@ func (key *GPGKey) AfterLoad(session *xorm.Session) { // ListGPGKeys returns a list of public keys belongs to given user. func ListGPGKeys(uid int64, listOptions ListOptions) ([]*GPGKey, error) { - sess := x.Where("owner_id=? AND primary_key_id=''", uid) + return listGPGKeys(x, uid, listOptions) +} + +func listGPGKeys(e Engine, uid int64, listOptions ListOptions) ([]*GPGKey, error) { + sess := e.Table(&GPGKey{}).Where("owner_id=? AND primary_key_id=''", uid) if listOptions.Page != 0 { sess = listOptions.setSessionPagination(sess) } diff --git a/models/user.go b/models/user.go index 642d9d56a887f..495fed1ff4d03 100644 --- a/models/user.go +++ b/models/user.go @@ -1208,7 +1208,7 @@ func deleteUser(e Engine, u *User) error { // ***** END: PublicKey ***** // ***** START: GPGPublicKey ***** - keys, err := ListGPGKeys(u.ID, ListOptions{}) + keys, err := listGPGKeys(e, u.ID, ListOptions{}) if err != nil { return fmt.Errorf("ListGPGKeys: %v", err) }