From c87ca11a520c29dfeaf31b330d06c614504dd0df Mon Sep 17 00:00:00 2001 From: Rob Loranger Date: Thu, 31 Oct 2019 15:16:10 -0700 Subject: [PATCH 1/6] add account deletion CLI only but backend supports calls from app.db.DeleteAccount already takes --delete-account user_id_number with optional --posts to also delete posts. if --posts is omitted all user posts will be updated to anonymous posts --- account.go | 4 ++ app.go | 45 +++++++++++- cmd/writefreely/main.go | 14 +++- database.go | 149 +++++++++++++++++++++++++++------------- 4 files changed, 161 insertions(+), 51 deletions(-) diff --git a/account.go b/account.go index 2a66ecf..c8e946a 100644 --- a/account.go +++ b/account.go @@ -1068,3 +1068,7 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s // Return value return s } + +func deleteAccount(app *App, userID int64, posts bool) error { + return app.db.DeleteAccount(userID, posts) +} diff --git a/app.go b/app.go index 514c7c8..8e8471f 100644 --- a/app.go +++ b/app.go @@ -30,7 +30,7 @@ import ( "github.com/gorilla/schema" "github.com/gorilla/sessions" "github.com/manifoldco/promptui" - "github.com/writeas/go-strip-markdown" + stripmd "github.com/writeas/go-strip-markdown" "github.com/writeas/impart" "github.com/writeas/web-core/auth" "github.com/writeas/web-core/converter" @@ -681,6 +681,49 @@ func ResetPassword(apper Apper, username string) error { return nil } +// DoDeleteAccount runs the confirmation and account delete process. +func DoDeleteAccount(apper Apper, userID int64, posts bool) error { + // Connect to the database + apper.LoadConfig() + connectToDatabase(apper.App()) + defer shutdown(apper.App()) + + // do not delete the root admin account + // TODO: check for other admins and skip? + if userID == 1 { + log.Error("Can not delete admin account") + os.Exit(1) + } + // check user exists + if _, err := apper.App().db.GetUserByID(userID); err != nil { + log.Error("%s", err) + os.Exit(1) + } + + // confirm deletion, w/ w/out posts + prompt := promptui.Prompt{ + Templates: &promptui.PromptTemplates{ + Success: "{{ . | bold | faint }}: ", + }, + Label: fmt.Sprintf("Delete user with ID: %d", userID), + IsConfirm: true, + } + _, err := prompt.Run() + if err != nil { + log.Info("Aborted...") + os.Exit(0) + } + + log.Info("Deleting...") + err = deleteAccount(apper.App(), userID, posts) + if err != nil { + log.Error("%s", err) + os.Exit(1) + } + log.Info("Success.") + return nil +} + func connectToDatabase(app *App) { log.Info("Connecting to %s database...", app.cfg.Database.Type) diff --git a/cmd/writefreely/main.go b/cmd/writefreely/main.go index 48993c7..10d8141 100644 --- a/cmd/writefreely/main.go +++ b/cmd/writefreely/main.go @@ -13,11 +13,12 @@ package main import ( "flag" "fmt" + "os" + "strings" + "github.com/gorilla/mux" "github.com/writeas/web-core/log" "github.com/writeas/writefreely" - "os" - "strings" ) func main() { @@ -38,6 +39,8 @@ func main() { // Admin actions createAdmin := flag.String("create-admin", "", "Create an admin with the given username:password") createUser := flag.String("create-user", "", "Create a regular user with the given username:password") + deleteUserID := flag.Int64("delete-user", 0, "Delete a user with the given id, does not delete posts. Use `--delete-user id --posts`") + deletePosts := flag.Bool("posts", false, "Optionally delete the user's posts during account deletion") resetPassUser := flag.String("reset-pass", "", "Reset the given user's password") outputVersion := flag.Bool("v", false, "Output the current version") flag.Parse() @@ -102,6 +105,13 @@ func main() { os.Exit(1) } os.Exit(0) + } else if *deleteUserID != 0 { + err := writefreely.DoDeleteAccount(app, *deleteUserID, *deletePosts) + if err != nil { + log.Error(err.Error()) + os.Exit(1) + } + os.Exit(0) } else if *migrate { err := writefreely.Migrate(app) if err != nil { diff --git a/database.go b/database.go index a3235b6..cecd169 100644 --- a/database.go +++ b/database.go @@ -61,7 +61,7 @@ type writestore interface { GetAccessToken(userID int64) (string, error) GetTemporaryAccessToken(userID int64, validSecs int) (string, error) GetTemporaryOneTimeAccessToken(userID int64, validSecs int, oneTime bool) (string, error) - DeleteAccount(userID int64) (l *string, err error) + DeleteAccount(userID int64, posts bool) error ChangeSettings(app *App, u *User, s *userSettings) error ChangePassphrase(userID int64, sudo bool, curPass string, hashedPass []byte) error @@ -2079,22 +2079,14 @@ func (db *datastore) CollectionHasAttribute(id int64, attr string) bool { return true } -func (db *datastore) DeleteAccount(userID int64) (l *string, err error) { - debug := "" - l = &debug - - t, err := db.Begin() - if err != nil { - stringLogln(l, "Unable to begin: %v", err) - return - } - +// DeleteAccount will delete the entire account for userID, and if posts +// is true, also all posts associated with the userID +func (db *datastore) DeleteAccount(userID int64, posts bool) error { // Get all collections rows, err := db.Query("SELECT id, alias FROM collections WHERE owner_id = ?", userID) if err != nil { - t.Rollback() - stringLogln(l, "Unable to get collections: %v", err) - return + log.Error("Unable to get collections: %v", err) + return err } defer rows.Close() colls := []Collection{} @@ -2102,13 +2094,20 @@ func (db *datastore) DeleteAccount(userID int64) (l *string, err error) { for rows.Next() { err = rows.Scan(&c.ID, &c.Alias) if err != nil { - t.Rollback() - stringLogln(l, "Unable to scan collection cols: %v", err) - return + log.Error("Unable to scan collection cols: %v", err) + return err } colls = append(colls, c) } + // Start transaction + t, err := db.Begin() + if err != nil { + log.Error("Unable to begin: %v", err) + return err + } + + // Clean up all collection related information var res sql.Result for _, c := range colls { // TODO: user deleteCollection() func @@ -2116,89 +2115,143 @@ func (db *datastore) DeleteAccount(userID int64) (l *string, err error) { res, err = t.Exec("DELETE FROM collectionattributes WHERE collection_id = ?", c.ID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete attributes on %s: %v", c.Alias, err) - return + log.Error("Unable to delete attributes on %s: %v", c.Alias, err) + return err } rs, _ := res.RowsAffected() - stringLogln(l, "Deleted %d for %s from collectionattributes", rs, c.Alias) + log.Info("Deleted %d for %s from collectionattributes", rs, c.Alias) // Remove any optional collection password res, err = t.Exec("DELETE FROM collectionpasswords WHERE collection_id = ?", c.ID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete passwords on %s: %v", c.Alias, err) - return + log.Error("Unable to delete passwords on %s: %v", c.Alias, err) + return err } rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d for %s from collectionpasswords", rs, c.Alias) + log.Info("Deleted %d for %s from collectionpasswords", rs, c.Alias) // Remove redirects to this collection res, err = t.Exec("DELETE FROM collectionredirects WHERE new_alias = ?", c.Alias) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete redirects on %s: %v", c.Alias, err) - return + log.Error("Unable to delete redirects on %s: %v", c.Alias, err) + return err } rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d for %s from collectionredirects", rs, c.Alias) + log.Info("Deleted %d for %s from collectionredirects", rs, c.Alias) + + // Remove any collection keys + res, err = t.Exec("DELETE FROM collectionkeys WHERE collection_id = ?", c.ID) + if err != nil { + t.Rollback() + log.Error("Unable to delete keys on %s: %v", c.Alias, err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Deleted %d for %s from collectionkeys", rs, c.Alias) + + // only remove collection in posts if not deleting the user posts + if !posts { + // Float all collection's posts + res, err = t.Exec("UPDATE posts SET collection_id = NULL WHERE collection_id = ? AND owner_id = ?", c.ID, userID) + if err != nil { + t.Rollback() + log.Error("Unable to update collection %s for posts: %v", err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Removed %d posts from collection %s", rs, c.Alias) + } + + // TODO: federate delete collection + + // Remove remote follows + res, err = t.Exec("DELETE FROM remotefollows WHERE collection_id = ?", c.ID) + if err != nil { + t.Rollback() + log.Error("Unable to delete remote follows on %s: %v", c.Alias, err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Deleted %d for %s from remotefollows", rs, c.Alias) } // Delete collections res, err = t.Exec("DELETE FROM collections WHERE owner_id = ?", userID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete collections: %v", err) - return + log.Error("Unable to delete collections: %v", err) + return err } rs, _ := res.RowsAffected() - stringLogln(l, "Deleted %d from collections", rs) + log.Info("Deleted %d from collections", rs) // Delete tokens res, err = t.Exec("DELETE FROM accesstokens WHERE user_id = ?", userID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete access tokens: %v", err) - return + log.Error("Unable to delete access tokens: %v", err) + return err } rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d from accesstokens", rs) + log.Info("Deleted %d from accesstokens", rs) // Delete posts - res, err = t.Exec("DELETE FROM posts WHERE owner_id = ?", userID) - if err != nil { - t.Rollback() - stringLogln(l, "Unable to delete posts: %v", err) - return + if posts { + // TODO: should maybe get each row so we can federate a delete + // if so needs to be outside of transaction like collections + res, err = t.Exec("DELETE FROM posts WHERE owner_id = ?", userID) + if err != nil { + t.Rollback() + log.Error("Unable to delete posts: %v", err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Deleted %d from posts", rs) } - rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d from posts", rs) + // Delete user attributes res, err = t.Exec("DELETE FROM userattributes WHERE user_id = ?", userID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete attributes: %v", err) - return + log.Error("Unable to delete attributes: %v", err) + return err } rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d from userattributes", rs) + log.Info("Deleted %d from userattributes", rs) + // Delete user invites + res, err = t.Exec("DELETE FROM userinvites WHERE owner_id = ?", userID) + if err != nil { + t.Rollback() + log.Error("Unable to delete invites: %v", err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Deleted %d from userinvites", rs) + + // Delete the user res, err = t.Exec("DELETE FROM users WHERE id = ?", userID) if err != nil { t.Rollback() - stringLogln(l, "Unable to delete user: %v", err) - return + log.Error("Unable to delete user: %v", err) + return err } rs, _ = res.RowsAffected() - stringLogln(l, "Deleted %d from users", rs) + log.Info("Deleted %d from users", rs) + // Commit all changes to the database err = t.Commit() if err != nil { t.Rollback() - stringLogln(l, "Unable to commit: %v", err) - return + log.Error("Unable to commit: %v", err) + return err } - return + // TODO: federate delete actor + + return nil } func (db *datastore) GetAPActorKeys(collectionID int64) ([]byte, []byte) { From 41166e5c356df10d36237e9296979cae65652504 Mon Sep 17 00:00:00 2001 From: Rob Loranger Date: Tue, 5 Nov 2019 09:14:20 -0800 Subject: [PATCH 2/6] CLI delete account by username and delete posts this changed the CLI flag to use the username instead of the userID leaving the underlying database function as is. also now posts are all deleted with no option to skip as this is likely never needed. --- account.go | 4 ++-- app.go | 25 ++++++++++++---------- cmd/writefreely/main.go | 7 +++--- database.go | 47 ++++++++++++++++++----------------------- 4 files changed, 39 insertions(+), 44 deletions(-) diff --git a/account.go b/account.go index c8e946a..5e5f03e 100644 --- a/account.go +++ b/account.go @@ -1069,6 +1069,6 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s return s } -func deleteAccount(app *App, userID int64, posts bool) error { - return app.db.DeleteAccount(userID, posts) +func deleteAccount(app *App, userID int64) error { + return app.db.DeleteAccount(userID) } diff --git a/app.go b/app.go index 8e8471f..e793902 100644 --- a/app.go +++ b/app.go @@ -682,21 +682,24 @@ func ResetPassword(apper Apper, username string) error { } // DoDeleteAccount runs the confirmation and account delete process. -func DoDeleteAccount(apper Apper, userID int64, posts bool) error { +func DoDeleteAccount(apper Apper, username string) error { // Connect to the database apper.LoadConfig() connectToDatabase(apper.App()) defer shutdown(apper.App()) - // do not delete the root admin account - // TODO: check for other admins and skip? - if userID == 1 { - log.Error("Can not delete admin account") + // check user exists + u, err := apper.App().db.GetUserForAuth(username) + if err != nil { + log.Error("%s", err) os.Exit(1) } - // check user exists - if _, err := apper.App().db.GetUserByID(userID); err != nil { - log.Error("%s", err) + userID := u.ID + + // do not delete the admin account + // TODO: check for other admins and skip? + if u.IsAdmin() { + log.Error("Can not delete admin account") os.Exit(1) } @@ -705,17 +708,17 @@ func DoDeleteAccount(apper Apper, userID int64, posts bool) error { Templates: &promptui.PromptTemplates{ Success: "{{ . | bold | faint }}: ", }, - Label: fmt.Sprintf("Delete user with ID: %d", userID), + Label: fmt.Sprintf("Really delete user : %s", username), IsConfirm: true, } - _, err := prompt.Run() + _, err = prompt.Run() if err != nil { log.Info("Aborted...") os.Exit(0) } log.Info("Deleting...") - err = deleteAccount(apper.App(), userID, posts) + err = deleteAccount(apper.App(), userID) if err != nil { log.Error("%s", err) os.Exit(1) diff --git a/cmd/writefreely/main.go b/cmd/writefreely/main.go index 10d8141..7fc2342 100644 --- a/cmd/writefreely/main.go +++ b/cmd/writefreely/main.go @@ -39,8 +39,7 @@ func main() { // Admin actions createAdmin := flag.String("create-admin", "", "Create an admin with the given username:password") createUser := flag.String("create-user", "", "Create a regular user with the given username:password") - deleteUserID := flag.Int64("delete-user", 0, "Delete a user with the given id, does not delete posts. Use `--delete-user id --posts`") - deletePosts := flag.Bool("posts", false, "Optionally delete the user's posts during account deletion") + deleteUsername := flag.String("delete-user", "", "Delete a user with the given username") resetPassUser := flag.String("reset-pass", "", "Reset the given user's password") outputVersion := flag.Bool("v", false, "Output the current version") flag.Parse() @@ -105,8 +104,8 @@ func main() { os.Exit(1) } os.Exit(0) - } else if *deleteUserID != 0 { - err := writefreely.DoDeleteAccount(app, *deleteUserID, *deletePosts) + } else if *deleteUsername != "" { + err := writefreely.DoDeleteAccount(app, *deleteUsername) if err != nil { log.Error(err.Error()) os.Exit(1) diff --git a/database.go b/database.go index cecd169..f2888ec 100644 --- a/database.go +++ b/database.go @@ -61,7 +61,7 @@ type writestore interface { GetAccessToken(userID int64) (string, error) GetTemporaryAccessToken(userID int64, validSecs int) (string, error) GetTemporaryOneTimeAccessToken(userID int64, validSecs int, oneTime bool) (string, error) - DeleteAccount(userID int64, posts bool) error + DeleteAccount(userID int64) error ChangeSettings(app *App, u *User, s *userSettings) error ChangePassphrase(userID int64, sudo bool, curPass string, hashedPass []byte) error @@ -2079,9 +2079,8 @@ func (db *datastore) CollectionHasAttribute(id int64, attr string) bool { return true } -// DeleteAccount will delete the entire account for userID, and if posts -// is true, also all posts associated with the userID -func (db *datastore) DeleteAccount(userID int64, posts bool) error { +// DeleteAccount will delete the entire account for userID +func (db *datastore) DeleteAccount(userID int64) error { // Get all collections rows, err := db.Query("SELECT id, alias FROM collections WHERE owner_id = ?", userID) if err != nil { @@ -2110,7 +2109,6 @@ func (db *datastore) DeleteAccount(userID int64, posts bool) error { // Clean up all collection related information var res sql.Result for _, c := range colls { - // TODO: user deleteCollection() func // Delete tokens res, err = t.Exec("DELETE FROM collectionattributes WHERE collection_id = ?", c.ID) if err != nil { @@ -2151,18 +2149,15 @@ func (db *datastore) DeleteAccount(userID int64, posts bool) error { rs, _ = res.RowsAffected() log.Info("Deleted %d for %s from collectionkeys", rs, c.Alias) - // only remove collection in posts if not deleting the user posts - if !posts { - // Float all collection's posts - res, err = t.Exec("UPDATE posts SET collection_id = NULL WHERE collection_id = ? AND owner_id = ?", c.ID, userID) - if err != nil { - t.Rollback() - log.Error("Unable to update collection %s for posts: %v", err) - return err - } - rs, _ = res.RowsAffected() - log.Info("Removed %d posts from collection %s", rs, c.Alias) + // Float all collection's posts + res, err = t.Exec("UPDATE posts SET collection_id = NULL WHERE collection_id = ? AND owner_id = ?", c.ID, userID) + if err != nil { + t.Rollback() + log.Error("Unable to update collection %s for posts: %v", c.Alias, err) + return err } + rs, _ = res.RowsAffected() + log.Info("Removed %d posts from collection %s", rs, c.Alias) // TODO: federate delete collection @@ -2198,18 +2193,16 @@ func (db *datastore) DeleteAccount(userID int64, posts bool) error { log.Info("Deleted %d from accesstokens", rs) // Delete posts - if posts { - // TODO: should maybe get each row so we can federate a delete - // if so needs to be outside of transaction like collections - res, err = t.Exec("DELETE FROM posts WHERE owner_id = ?", userID) - if err != nil { - t.Rollback() - log.Error("Unable to delete posts: %v", err) - return err - } - rs, _ = res.RowsAffected() - log.Info("Deleted %d from posts", rs) + // TODO: should maybe get each row so we can federate a delete + // if so needs to be outside of transaction like collections + res, err = t.Exec("DELETE FROM posts WHERE owner_id = ?", userID) + if err != nil { + t.Rollback() + log.Error("Unable to delete posts: %v", err) + return err } + rs, _ = res.RowsAffected() + log.Info("Deleted %d from posts", rs) // Delete user attributes res, err = t.Exec("DELETE FROM userattributes WHERE user_id = ?", userID) From b83af955c3fd1926be43968629d81ceb21edcc13 Mon Sep 17 00:00:00 2001 From: Rob Loranger Date: Tue, 5 Nov 2019 12:20:07 -0800 Subject: [PATCH 3/6] remove wrapper over db.DeleteAccount --- account.go | 4 ---- app.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/account.go b/account.go index 5e5f03e..2a66ecf 100644 --- a/account.go +++ b/account.go @@ -1068,7 +1068,3 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s // Return value return s } - -func deleteAccount(app *App, userID int64) error { - return app.db.DeleteAccount(userID) -} diff --git a/app.go b/app.go index e793902..92d5995 100644 --- a/app.go +++ b/app.go @@ -718,7 +718,7 @@ func DoDeleteAccount(apper Apper, username string) error { } log.Info("Deleting...") - err = deleteAccount(apper.App(), userID) + err = apper.App().db.DeleteAccount(userID) if err != nil { log.Error("%s", err) os.Exit(1) From c9faff178d9501554463742b0a04122279f81c7e Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Sat, 8 Feb 2020 13:51:14 -0500 Subject: [PATCH 4/6] Don't float posts on account deletion Ref T319 --- database.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/database.go b/database.go index bbec580..757a371 100644 --- a/database.go +++ b/database.go @@ -2184,16 +2184,6 @@ func (db *datastore) DeleteAccount(userID int64) error { rs, _ = res.RowsAffected() log.Info("Deleted %d for %s from collectionkeys", rs, c.Alias) - // Float all collection's posts - res, err = t.Exec("UPDATE posts SET collection_id = NULL WHERE collection_id = ? AND owner_id = ?", c.ID, userID) - if err != nil { - t.Rollback() - log.Error("Unable to update collection %s for posts: %v", c.Alias, err) - return err - } - rs, _ = res.RowsAffected() - log.Info("Removed %d posts from collection %s", rs, c.Alias) - // TODO: federate delete collection // Remove remote follows From af14bcbb7804334552a3b5d369b2a13f65b0f93f Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Sat, 8 Feb 2020 13:51:38 -0500 Subject: [PATCH 5/6] Clean up oauth_users table on account deletion Ref T319 --- database.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/database.go b/database.go index 757a371..2c44492 100644 --- a/database.go +++ b/database.go @@ -2217,6 +2217,16 @@ func (db *datastore) DeleteAccount(userID int64) error { rs, _ = res.RowsAffected() log.Info("Deleted %d from accesstokens", rs) + // Delete user attributes + res, err = t.Exec("DELETE FROM oauth_users WHERE user_id = ?", userID) + if err != nil { + t.Rollback() + log.Error("Unable to delete oauth_users: %v", err) + return err + } + rs, _ = res.RowsAffected() + log.Info("Deleted %d from oauth_users", rs) + // Delete posts // TODO: should maybe get each row so we can federate a delete // if so needs to be outside of transaction like collections From 666bd1b9d1e50fbe963dfa93616317784be6ba6c Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Sat, 8 Feb 2020 14:46:05 -0500 Subject: [PATCH 6/6] Show correct error when user not found in admin panel Previously, it would show a 500. This also logs the real reason if it's not a "not found" error --- admin.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/admin.go b/admin.go index 0a73a11..83dcc80 100644 --- a/admin.go +++ b/admin.go @@ -187,7 +187,11 @@ func handleViewAdminUser(app *App, u *User, w http.ResponseWriter, r *http.Reque var err error p.User, err = app.db.GetUserForAuth(username) if err != nil { - return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not get user: %v", err)} + if err == ErrUserNotFound { + return err + } + log.Error("Could not get user: %v", err) + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } flashes, _ := getSessionFlashes(app, w, r, nil)