Password reset done

# HG changeset patch # User Sascha L. Teichmann # Date 1563275053 -7200 # Node ID 66a421f3e3a6f8e384682ac349a1f4954328fefc # Parent 7df9ef183985b069c90ac3a5defd6aaa60eaa5d8# Parent 6dd9741d6ff792702d1a47c69d5170b6e0793750 Merge pwrest-branch into default. diff -r 7df9ef183985 -r 66a421f3e3a6 pkg/controllers/pwreset.go --- a/pkg/controllers/pwreset.go Tue Jul 16 11:37:47 2019 +0200 +++ b/pkg/controllers/pwreset.go Tue Jul 16 13:04:13 2019 +0200 @@ -21,14 +21,17 @@ "database/sql" "encoding/hex" "errors" + "io" "log" "net/http" "os/exec" "strconv" "strings" - "text/template" "time" + htmlTemplate "html/template" + textTemplate "text/template" + "github.com/gorilla/mux" "gemma.intevation.de/gemma/pkg/auth" @@ -40,17 +43,15 @@ const ( insertRequestSQL = `INSERT INTO sys_admin.password_reset_requests - (hash, username) VALUES ($1, $2)` + (hash, username) VALUES ($1, $2) + ON CONFLICT (username) DO UPDATE SET hash = $1` countRequestsSQL = `SELECT count(*) FROM sys_admin.password_reset_requests` - countRequestsUserSQL = `SELECT count(*) FROM sys_admin.password_reset_requests - WHERE username = $1` - deleteRequestSQL = `DELETE FROM sys_admin.password_reset_requests WHERE hash = $1` - findRequestSQL = `SELECT lu.email_address, lu.username + findRequestSQL = `SELECT lu.username FROM sys_admin.password_reset_requests prr JOIN users.list_users lu on prr.username = lu.username WHERE prr.hash = $1` @@ -63,6 +64,10 @@ updatePasswordSQL = `UPDATE users.list_users SET pw = $1 WHERE username = $2` + + deletePasswordResetRequestSQL = ` + DELETE FROM sys_admin.password_reset_requests + WHERE username = $1` ) const ( @@ -77,21 +82,20 @@ const pwResetRole = "sys_admin" var ( - errTooMuchPasswordResets = errors.New("Too many password resets") - errTooMuchPasswordResetsPerUser = errors.New("Too many password resets per user") - errNoSuchUser = errors.New("User does not exist") - errInvalidUser = errors.New("Invalid user") + errTooMuchPasswordResets = errors.New("Too many password resets") + errNoSuchUser = errors.New("User does not exist") + errInvalidUser = errors.New("Invalid user") ) var ( - passwordResetRequestMailTmpl = template.Must( - template.New("request").Parse(`You or someone else has requested a password change + passwordResetRequestMailTmpl = textTemplate.Must( + textTemplate.New("request").Parse(`You or someone else has requested a password change for your account {{ .User }} on -{{ .HTTPS }}://{{ .Server }} +{{ .Server }} -Please follow this link to have a new password generated and mailed to you: +Please follow this link to have a new password generated: -{{ .HTTPS }}://{{ .Server }}/api/users/passwordreset/{{ .Hash }} +{{ .Server }}/api/users/passwordreset/{{ .Hash }} The link is only valid for 12 hours. @@ -101,17 +105,20 @@ Best regards Your service team`)) - passwordResetMailTmpl = template.Must( - template.New("reset").Parse(`Your password for your account {{ .User }} on -{{ .HTTPS }}://{{ .Server }} - -has been changed to - {{ .Password }} - -Change it as soon as possible. - -Best regards - Your service team`)) + passwordResetPage = htmlTemplate.Must( + htmlTemplate.New("page").Parse(` + + + + Password reset done + + +

The password reset for user {{ .User }} successfully done.

New password: {{ .Password }}

Go to login page.

+ + +`)) ) func init() { @@ -137,15 +144,13 @@ } } -func requestMessageBody(https, user, hash, server string) string { +func requestMessageBody(user, hash, server string) string { var content = struct { User string - HTTPS string Server string Hash string }{ User: user, - HTTPS: https, Server: server, Hash: hash, } @@ -156,31 +161,15 @@ return buf.String() } -func changedMessageBody(https, user, password, server string) string { +func changedMessageBody(w io.Writer, user, password string) error { var content = struct { User string - HTTPS string - Server string Password string }{ User: user, - HTTPS: https, - Server: server, Password: password, } - var buf bytes.Buffer - if err := passwordResetMailTmpl.Execute(&buf, &content); err != nil { - log.Printf("error: %v\n", err) - } - return buf.String() -} - -func useHTTPS(req *http.Request) string { - if req.Header.Get("X-Use-Protocol") == "https" || - req.URL.Scheme == "https" { - return "https" - } - return "http" + return passwordResetPage.Execute(w, &content) } func generateHash() string { @@ -198,7 +187,7 @@ return common.RandomString(passwordLength) } -func backgroundRequest(https, host string, user *models.PWResetUser) error { +func backgroundRequest(host string, user *models.PWResetUser) error { if user.User == "" { return errInvalidUser @@ -232,16 +221,6 @@ return err } - if err := conn.QueryRowContext( - ctx, countRequestsUserSQL, user.User).Scan(&count); err != nil { - return err - } - - // Limit requests per user - if count >= maxPasswordRequestsPerUser { - return errTooMuchPasswordResetsPerUser - } - hash = generateHash() _, err = conn.ExecContext(ctx, insertRequestSQL, hash, user.User) return err @@ -250,20 +229,11 @@ return err } - body := requestMessageBody(https, user.User, hash, host) + body := requestMessageBody(user.User, hash, host) return misc.SendMail(email, "Password Reset Link", body) } -// host checks if we are behind a proxy and returns the name -// of the up-front server. -func host(req *http.Request) string { - if fwd := req.Header.Get("X-Forwarded-Host"); fwd != "" { - return fwd - } - return req.Host -} - func passwordResetRequest( input interface{}, req *http.Request, @@ -272,11 +242,13 @@ // We do the checks and the emailing in background // no reduce the risks of timing attacks. - go func(https, host string, user *models.PWResetUser) { - if err := backgroundRequest(https, host, user); err != nil { + go func(user *models.PWResetUser) { + config.WaitReady() + host := config.ExternalURL() + if err := backgroundRequest(host, user); err != nil { log.Printf("error: %v\n", err) } - }(useHTTPS(req), host(req), input.(*models.PWResetUser)) + }(input.(*models.PWResetUser)) // Send a neutral message to avoid being an user oracle. const neutralMessage = "If this account exists, a reset link will be mailed." @@ -296,42 +268,65 @@ return } - var email, user, password string + var user, password string ctx := req.Context() - if err := auth.RunAs( - ctx, pwResetRole, func(conn *sql.Conn) error { - err := conn.QueryRowContext(ctx, findRequestSQL, hash).Scan(&email, &user) + err := auth.RunAs( + ctx, pwResetRole, + func(conn *sql.Conn) error { + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + err = tx.QueryRowContext(ctx, findRequestSQL, hash).Scan(&user) switch { case err == sql.ErrNoRows: - return JSONError{http.StatusNotFound, "No such hash"} + return errors.New("This URL is no longer valid.") case err != nil: return err } password = generateNewPassword() - res, err := conn.ExecContext(ctx, updatePasswordSQL, password, user) + res, err := tx.ExecContext(ctx, updatePasswordSQL, password, user) if err != nil { return err } if n, err2 := res.RowsAffected(); err2 == nil && n == 0 { - return JSONError{http.StatusNotFound, "User not found"} + return errors.New("User not found") + } + if _, err = tx.ExecContext(ctx, deleteRequestSQL, hash); err != nil { + return err } - _, err = conn.ExecContext(ctx, deleteRequestSQL, hash) - return err - }); err == nil { - https := useHTTPS(req) - server := host(req) - body := changedMessageBody(https, user, password, server) - if err = misc.SendMail(email, "Password Reset Done", body); err != nil { - log.Printf("error: %v\n", err) - http.Error( - rw, - http.StatusText(http.StatusInternalServerError), - http.StatusInternalServerError) - return - } - var url = https + "://" + server - http.Redirect(rw, req, url, http.StatusSeeOther) + return tx.Commit() + }, + ) + + switch { + case err == sql.ErrNoRows: + http.Error(rw, "No such request", http.StatusNotFound) + return + case err != nil: + http.Error(rw, "Error: "+err.Error(), http.StatusInternalServerError) + return + } + + if err := changedMessageBody(rw, user, password); err != nil { + log.Printf("error: %v\n", err) } } + +func deletePasswordResetRequest(user string) { + ctx := context.Background() + if err := auth.RunAs( + ctx, + pwResetRole, + func(conn *sql.Conn) error { + _, err := conn.ExecContext(ctx, deletePasswordResetRequestSQL, user) + return err + }, + ); err != nil { + log.Printf("error: %v\n", err) + } +} diff -r 7df9ef183985 -r 66a421f3e3a6 pkg/controllers/token.go --- a/pkg/controllers/token.go Tue Jul 16 11:37:47 2019 +0200 +++ b/pkg/controllers/token.go Tue Jul 16 13:04:13 2019 +0200 @@ -102,5 +102,7 @@ Roles: session.Roles, } + go deletePasswordResetRequest(session.User) + SendJSON(rw, http.StatusCreated, &result) } diff -r 7df9ef183985 -r 66a421f3e3a6 schema/auth.sql --- a/schema/auth.sql Tue Jul 16 11:37:47 2019 +0200 +++ b/schema/auth.sql Tue Jul 16 13:04:13 2019 +0200 @@ -66,7 +66,7 @@ GRANT INSERT, UPDATE ON sys_admin.system_config TO sys_admin; GRANT UPDATE ON systemconf.feature_colours TO sys_admin; GRANT UPDATE ON sys_admin.published_services TO sys_admin; -GRANT INSERT, DELETE ON sys_admin.password_reset_requests TO sys_admin; +GRANT INSERT, DELETE, UPDATE ON sys_admin.password_reset_requests TO sys_admin; -- -- Privileges assigned directly to metamorph diff -r 7df9ef183985 -r 66a421f3e3a6 schema/gemma.sql --- a/schema/gemma.sql Tue Jul 16 11:37:47 2019 +0200 +++ b/schema/gemma.sql Tue Jul 16 13:04:13 2019 +0200 @@ -232,7 +232,7 @@ CREATE TABLE password_reset_requests ( hash varchar(32) PRIMARY KEY, issued timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, - username varchar NOT NULL + username varchar NOT NULL UNIQUE REFERENCES internal.user_profiles(username) ON DELETE CASCADE ON UPDATE CASCADE )