Mercurial > gemma

changeset 3959:66a421f3e3a6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Merge pwrest-branch into default.
author Sascha L. Teichmann <sascha.teichmann@intevation.de>
date Tue, 16 Jul 2019 13:04:13 +0200
parents 7df9ef183985 (current diff) 6dd9741d6ff7 (diff)
children 4268bb8e2839
files
diffstat 4 files changed, 93 insertions(+), 96 deletions(-) [+]
line wrap: on
line diff
--- 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(`<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8" />
+	<title>Password reset done</title>
+  </head>
+  <body>
+    <p>The password reset for user <strong><tt>{{ .User }}</tt></strong> successfully done.</p>
+	<p>New password: <strong><tt>{{ .Password }}</tt></strong></p>
+	<p><a href="/">Go to login page.</a></p>
+  </body>
+</html>
+`))
 )
 
 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)
+	}
+}
--- 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)
 }
--- 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
--- 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
     )