From 482e632ca970e2dcd961531be32c2e0edf5ebea6 Mon Sep 17 00:00:00 2001 From: Rob Loranger Date: Tue, 5 Nov 2019 12:21:18 -0800 Subject: [PATCH 1/6] add user account delete UI --- account.go | 17 +++++++++++++++++ routes.go | 1 + templates/user/settings.tmpl | 12 ++++++++++++ 3 files changed, 30 insertions(+) diff --git a/account.go b/account.go index 2a66ecf..a8057d4 100644 --- a/account.go +++ b/account.go @@ -1068,3 +1068,20 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s // Return value return s } + +func handleUserDelete(app *App, u *User, w http.ResponseWriter, r *http.Request) error { + confirmUsername := r.PostFormValue("confirm-username") + if u.Username != confirmUsername { + return impart.HTTPError{http.StatusBadRequest, "Confirmation username must match your username exactly."} + } + + // TODO: prevent admin delete themselves? + err := app.db.DeleteAccount(u.ID) + if err != nil { + log.Error("user delete account: %v", err) + return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not delete account: %v", err)} + } + + _ = addSessionFlash(app, w, r, "Account deleted successfully, sorry to see you go.", nil) + return impart.HTTPError{http.StatusFound, "/me/logout"} +} diff --git a/routes.go b/routes.go index 0113e93..ab0a854 100644 --- a/routes.go +++ b/routes.go @@ -87,6 +87,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { me.HandleFunc("/c/", handler.User(viewCollections)).Methods("GET") me.HandleFunc("/c/{collection}", handler.User(viewEditCollection)).Methods("GET") me.HandleFunc("/c/{collection}/stats", handler.User(viewStats)).Methods("GET") + me.HandleFunc("/delete", handler.User(handleUserDelete)).Methods("POST") me.HandleFunc("/posts", handler.Redirect("/me/posts/", UserLevelUser)).Methods("GET") me.HandleFunc("/posts/", handler.User(viewArticles)).Methods("GET") me.HandleFunc("/posts/export.csv", handler.Download(viewExportPosts, UserLevelUser)).Methods("GET") diff --git a/templates/user/settings.tmpl b/templates/user/settings.tmpl index fd204a5..cd45179 100644 --- a/templates/user/settings.tmpl +++ b/templates/user/settings.tmpl @@ -63,6 +63,18 @@ h3 { font-weight: normal; } + + {{ if not .IsAdmin }} +
+

Delete Account

+

Danger Zone - This cannot be undone

+

This will delete your account and all your blogs and posts. Before continuing make sure to export your data.

+
+

Type your username to confirm deletion.

+ + +

+ {{end}} + {{template "footer" .}} From b092421f6ebc6ae20ca68f4cd0768d2c3034a612 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Thu, 22 Apr 2021 12:41:54 -0400 Subject: [PATCH 4/6] Add Cross-Site Request Forgery (CSRF) protection on account deletion This requires admins to generate a new encryption key with: writefreely keys generate Ref T319 --- account.go | 3 +++ app.go | 28 ++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 ++++ key/key.go | 10 ++++++++-- keys.go | 2 ++ routes.go | 5 +++-- templates/user/settings.tmpl | 1 + 8 files changed, 50 insertions(+), 5 deletions(-) diff --git a/account.go b/account.go index b93168f..66a9d24 100644 --- a/account.go +++ b/account.go @@ -20,6 +20,7 @@ import ( "sync" "time" + "github.com/gorilla/csrf" "github.com/gorilla/mux" "github.com/gorilla/sessions" "github.com/guregu/null/zero" @@ -1082,6 +1083,7 @@ func viewSettings(app *App, u *User, w http.ResponseWriter, r *http.Request) err HasPass bool IsLogOut bool Silenced bool + CSRFField template.HTML OauthSection bool OauthAccounts []oauthAccountInfo OauthSlack bool @@ -1098,6 +1100,7 @@ func viewSettings(app *App, u *User, w http.ResponseWriter, r *http.Request) err HasPass: passIsSet, IsLogOut: r.FormValue("logout") == "1", Silenced: fullUser.IsSilenced(), + CSRFField: csrf.TemplateField(r), OauthSection: displayOauthSection, OauthAccounts: oauthAccounts, OauthSlack: enableOauthSlack, diff --git a/app.go b/app.go index ed4e096..f665166 100644 --- a/app.go +++ b/app.go @@ -166,6 +166,14 @@ func (app *App) LoadKeys() error { if debugging { log.Info(" %s", emailKeyPath) } + + executable, err := os.Executable() + if err != nil { + executable = "writefreely" + } else { + executable = filepath.Base(executable) + } + app.keys.EmailKey, err = ioutil.ReadFile(emailKeyPath) if err != nil { return err @@ -187,6 +195,22 @@ func (app *App) LoadKeys() error { return err } + if debugging { + log.Info(" %s", csrfKeyPath) + } + app.keys.CSRFKey, err = ioutil.ReadFile(csrfKeyPath) + if err != nil { + if os.IsNotExist(err) { + log.Error(`Missing key: %s. + + Run this command to generate missing keys: + %s keys generate + +`, csrfKeyPath, executable) + } + return err + } + return nil } @@ -637,6 +661,10 @@ func GenerateKeyFiles(app *App) error { if err != nil { keyErrs = err } + err = generateKey(csrfKeyPath) + if err != nil { + keyErrs = err + } return keyErrs } diff --git a/go.mod b/go.mod index 3006fa6..bb69895 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/go-sql-driver/mysql v1.6.0 github.com/go-test/deep v1.0.1 // indirect github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e // indirect + github.com/gorilla/csrf v1.7.0 github.com/gorilla/feeds v1.1.1 github.com/gorilla/mux v1.8.0 github.com/gorilla/schema v1.2.0 @@ -22,7 +23,6 @@ require ( github.com/microcosm-cc/bluemonday v1.0.5 github.com/mitchellh/go-wordwrap v1.0.1 github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d - github.com/pkg/errors v0.8.1 // indirect github.com/prologic/go-gopher v0.0.0-20200721020712-3e11dcff0469 github.com/rainycape/unidecode v0.0.0-20150907023854-cb7f23ec59be // indirect github.com/smartystreets/assertions v0.0.0-20190116191733-b6c0e53d7304 // indirect diff --git a/go.sum b/go.sum index b8adf04..354a1dd 100644 --- a/go.sum +++ b/go.sum @@ -44,6 +44,8 @@ github.com/gologme/log v1.2.0 h1:Ya5Ip/KD6FX7uH0S31QO87nCCSucKtF44TLbTtO7V4c= github.com/gologme/log v1.2.0/go.mod h1:gq31gQ8wEHkR+WekdWsqDuf8pXTUZA9BnnzTuPz1Y9U= github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e h1:JKmoR8x90Iww1ks85zJ1lfDGgIiMDuIptTOhJq+zKyg= github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= +github.com/gorilla/csrf v1.7.0 h1:mMPjV5/3Zd460xCavIkppUdvnl5fPXMpv2uz2Zyg7/Y= +github.com/gorilla/csrf v1.7.0/go.mod h1:+a/4tCmqhG6/w4oafeAZ9pEa3/NZOWYVbD9fV0FwIQA= github.com/gorilla/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY= github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl60c= github.com/gorilla/feeds v1.1.1 h1:HwKXxqzcRNg9to+BbvJog4+f3s/xzvtZXICcQGutYfY= @@ -99,6 +101,8 @@ github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d h1:VhgPp6v9qf9Agr/ github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d/go.mod h1:YUTz3bUH2ZwIWBy3CJBeOBEugqcmXREj14T+iG/4k4U= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prologic/go-gopher v0.0.0-20200721020712-3e11dcff0469 h1:rAbv2gekFbUcjhUkruwo0vMJ0JqhUgg9tz7t+bxHbN4= diff --git a/key/key.go b/key/key.go index 1cb3cf8..25490ca 100644 --- a/key/key.go +++ b/key/key.go @@ -1,5 +1,5 @@ /* - * Copyright © 2019 A Bunch Tell LLC. + * Copyright © 2019, 2021 A Bunch Tell LLC. * * This file is part of WriteFreely. * @@ -20,7 +20,7 @@ const ( ) type Keychain struct { - EmailKey, CookieAuthKey, CookieKey []byte + EmailKey, CookieAuthKey, CookieKey, CSRFKey []byte } // GenerateKeys generates necessary keys for the app on the given Keychain, @@ -47,6 +47,12 @@ func (keys *Keychain) GenerateKeys() error { keyErrs = err } } + if len(keys.CSRFKey) == 0 { + keys.CSRFKey, err = GenerateBytes(EncKeysBytes) + if err != nil { + keyErrs = err + } + } return keyErrs } diff --git a/keys.go b/keys.go index e53d811..1b43d0b 100644 --- a/keys.go +++ b/keys.go @@ -26,6 +26,7 @@ var ( emailKeyPath = filepath.Join(keysDir, "email.aes256") cookieAuthKeyPath = filepath.Join(keysDir, "cookies_auth.aes256") cookieKeyPath = filepath.Join(keysDir, "cookies_enc.aes256") + csrfKeyPath = filepath.Join(keysDir, "csrf.aes256") ) // InitKeys loads encryption keys into memory via the given Apper interface @@ -42,6 +43,7 @@ func initKeyPaths(app *App) { emailKeyPath = filepath.Join(app.cfg.Server.KeysParentDir, emailKeyPath) cookieAuthKeyPath = filepath.Join(app.cfg.Server.KeysParentDir, cookieAuthKeyPath) cookieKeyPath = filepath.Join(app.cfg.Server.KeysParentDir, cookieKeyPath) + csrfKeyPath = filepath.Join(app.cfg.Server.KeysParentDir, csrfKeyPath) } // generateKey generates a key at the given path used for the encryption of diff --git a/routes.go b/routes.go index c4e7c07..2b23bd1 100644 --- a/routes.go +++ b/routes.go @@ -16,6 +16,7 @@ import ( "path/filepath" "strings" + "github.com/gorilla/csrf" "github.com/gorilla/mux" "github.com/writeas/go-webfinger" "github.com/writeas/web-core/log" @@ -98,7 +99,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { me.HandleFunc("/c/", handler.User(viewCollections)).Methods("GET") me.HandleFunc("/c/{collection}", handler.User(viewEditCollection)).Methods("GET") me.HandleFunc("/c/{collection}/stats", handler.User(viewStats)).Methods("GET") - me.HandleFunc("/delete", handler.User(handleUserDelete)).Methods("POST") + me.Path("/delete").Handler(csrf.Protect(apper.App().keys.CSRFKey)(handler.User(handleUserDelete))).Methods("POST") me.HandleFunc("/posts", handler.Redirect("/me/posts/", UserLevelUser)).Methods("GET") me.HandleFunc("/posts/", handler.User(viewArticles)).Methods("GET") me.HandleFunc("/posts/export.csv", handler.Download(viewExportPosts, UserLevelUser)).Methods("GET") @@ -107,7 +108,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { me.HandleFunc("/export", handler.User(viewExportOptions)).Methods("GET") me.HandleFunc("/export.json", handler.Download(viewExportFull, UserLevelUser)).Methods("GET") me.HandleFunc("/import", handler.User(viewImport)).Methods("GET") - me.HandleFunc("/settings", handler.User(viewSettings)).Methods("GET") + me.Path("/settings").Handler(csrf.Protect(apper.App().keys.CSRFKey)(handler.User(viewSettings))).Methods("GET") me.HandleFunc("/invites", handler.User(handleViewUserInvites)).Methods("GET") me.HandleFunc("/logout", handler.Web(viewLogout, UserLevelNone)).Methods("GET") diff --git a/templates/user/settings.tmpl b/templates/user/settings.tmpl index c17a99f..7a90fb7 100644 --- a/templates/user/settings.tmpl +++ b/templates/user/settings.tmpl @@ -181,6 +181,7 @@ h3 { font-weight: normal; }
+ {{ .CSRFField }}
Cancel From 7c1c1218b1fcbd7670848c0b544a000efd8eeb0f Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Thu, 22 Apr 2021 12:45:55 -0400 Subject: [PATCH 5/6] Tweak "deletion success" message and note it doesn't work Ref T319 --- account.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/account.go b/account.go index 66a9d24..6369d47 100644 --- a/account.go +++ b/account.go @@ -1172,7 +1172,8 @@ func handleUserDelete(app *App, u *User, w http.ResponseWriter, r *http.Request) return impart.HTTPError{http.StatusInternalServerError, fmt.Sprintf("Could not delete account: %v", err)} } - _ = addSessionFlash(app, w, r, "Account deleted successfully, sorry to see you go.", nil) + // FIXME: This doesn't ever appear to the user, as (I believe) the value is erased when the session cookie is reset + _ = addSessionFlash(app, w, r, "Thanks for writing with us! You account was deleted successfully.", nil) return impart.HTTPError{http.StatusFound, "/me/logout"} } From d3d77cee54e65c908b306ec7cd4c89302dafca94 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Thu, 22 Apr 2021 13:13:47 -0400 Subject: [PATCH 6/6] Make open account deletion configurable This adds a configuration option to the [app] section: open_deletion. When true, users can delete their account on their own. Ref T319 --- account.go | 4 ++++ admin.go | 1 + config/config.go | 3 ++- templates/user/admin/app-settings.tmpl | 8 ++++++++ templates/user/settings.tmpl | 4 ++-- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/account.go b/account.go index 6369d47..cbd7cde 100644 --- a/account.go +++ b/account.go @@ -1156,6 +1156,10 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s } func handleUserDelete(app *App, u *User, w http.ResponseWriter, r *http.Request) error { + if !app.cfg.App.OpenDeletion { + return impart.HTTPError{http.StatusForbidden, "Open account deletion is disabled on this instance."} + } + confirmUsername := r.PostFormValue("confirm-username") if u.Username != confirmUsername { return impart.HTTPError{http.StatusBadRequest, "Confirmation username must match your username exactly."} diff --git a/admin.go b/admin.go index 6a1a3b5..2b757ca 100644 --- a/admin.go +++ b/admin.go @@ -555,6 +555,7 @@ func handleAdminUpdateConfig(apper Apper, u *User, w http.ResponseWriter, r *htt apper.App().cfg.App.SiteDesc = r.FormValue("site_desc") apper.App().cfg.App.Landing = r.FormValue("landing") apper.App().cfg.App.OpenRegistration = r.FormValue("open_registration") == "on" + apper.App().cfg.App.OpenDeletion = r.FormValue("open_deletion") == "on" mul, err := strconv.Atoi(r.FormValue("min_username_len")) if err == nil { apper.App().cfg.App.MinUsernameLen = mul diff --git a/config/config.go b/config/config.go index 8ee03ba..c0e5255 100644 --- a/config/config.go +++ b/config/config.go @@ -1,5 +1,5 @@ /* - * Copyright © 2018-2020 A Bunch Tell LLC. + * Copyright © 2018-2021 A Bunch Tell LLC. * * This file is part of WriteFreely. * @@ -139,6 +139,7 @@ type ( // Users SingleUser bool `ini:"single_user"` OpenRegistration bool `ini:"open_registration"` + OpenDeletion bool `ini:"open_deletion"` MinUsernameLen int `ini:"min_username_len"` MaxBlogs int `ini:"max_blogs"` diff --git a/templates/user/admin/app-settings.tmpl b/templates/user/admin/app-settings.tmpl index 9142dcc..50c50ec 100644 --- a/templates/user/admin/app-settings.tmpl +++ b/templates/user/admin/app-settings.tmpl @@ -75,6 +75,14 @@ select {
+
+
+