More work on cleaning up recover

- Add email layouts

auth/auth.go

@@ -38,6 +38,7 @@ type AuthPage struct {
 	ShowRecover  bool
 
 	FlashSuccess string
+	FlashError   string
 }
 
 type Auth struct {
@@ -47,7 +48,7 @@ type Auth struct {
 	logoutRedirect string
 	loginRedirect  string
 	logger         io.Writer
-	templates      *template.Template
+	templates      map[string]*template.Template
 	callbacks      *authboss.Callbacks
 
 	isRememberLoaded bool
@@ -55,7 +56,7 @@ type Auth struct {
 }
 
 func (a *Auth) Initialize(config *authboss.Config) (err error) {
-	if a.templates, err = views.Get(config.ViewsPath, pageLogin); err != nil {
+	if a.templates, err = views.Get(config.Layout, config.ViewsPath, pageLogin); err != nil {
 		return err
 	}
 
@@ -103,7 +104,9 @@ func (a *Auth) loginHandlerFunc(ctx *authboss.Context, w http.ResponseWriter, r
 			ctx.SessionStorer.Del(authboss.FlashSuccessKey)
 		}
 
-		a.templates.ExecuteTemplate(w, pageLogin, page)
+		tpl := a.templates[pageLogin]
+		tpl.Execute(w, page)
+		// tpl.ExecuteTemplate(w, tpl.Name(), page)
 	case methodPOST:
 		u, ok := ctx.FirstPostFormValue("username")
 		if !ok {
@@ -112,7 +115,9 @@ func (a *Auth) loginHandlerFunc(ctx *authboss.Context, w http.ResponseWriter, r
 
 		if err := a.callbacks.FireBefore(authboss.EventAuth, ctx); err != nil {
 			w.WriteHeader(http.StatusForbidden)
-			a.templates.ExecuteTemplate(w, pageLogin, AuthPage{err.Error(), u, a.isRememberLoaded, a.isRecoverLoaded, ""})
+
+			tpl := a.templates[pageLogin]
+			tpl.ExecuteTemplate(w, tpl.Name(), AuthPage{err.Error(), u, a.isRememberLoaded, a.isRecoverLoaded, "", ""})
 		}
 
 		p, ok := ctx.FirstPostFormValue("password")
@@ -123,7 +128,8 @@ func (a *Auth) loginHandlerFunc(ctx *authboss.Context, w http.ResponseWriter, r
 		if err := a.authenticate(ctx, u, p); err != nil {
 			fmt.Fprintln(a.logger, err)
 			w.WriteHeader(http.StatusForbidden)
-			a.templates.ExecuteTemplate(w, pageLogin, AuthPage{"invalid username and/or password", u, a.isRememberLoaded, a.isRecoverLoaded, ""})
+			tpl := a.templates[pageLogin]
+			tpl.ExecuteTemplate(w, tpl.Name(), AuthPage{"invalid username and/or password", u, a.isRememberLoaded, a.isRecoverLoaded, "", ""})
 			return
 		}
 

config.go

@@ -1,6 +1,7 @@
 package authboss
 
 import (
+	"html/template"
 	"io"
 	"io/ioutil"
 	"net/smtp"
@@ -20,6 +21,9 @@ type Config struct {
 	// BCryptPasswordCost is self explanitory.
 	BCryptCost int
 
+	Layout      *template.Template
+	LayoutEmail *template.Template
+
 	AuthLogoutRoute       string
 	AuthLoginSuccessRoute string
 

internal/flashutil/flashutil.go

@@ -0,0 +1,11 @@
+package flashutil
+
+import "gopkg.in/authboss.v0"
+
+// Pull is a convenience func to retreive then delete a flash message.  Any ok
+// checks are ignored as they don't alter the intended use.
+func Pull(storer authboss.ClientStorer, key string) string {
+	value, _ := storer.Get(key)
+	storer.Del(key)
+	return value
+}

internal/flashutil/flashutil_test.go

@@ -0,0 +1,23 @@
+package flashutil
+
+import (
+	"testing"
+
+	"gopkg.in/authboss.v0/internal/mocks"
+)
+
+func TestPull(t *testing.T) {
+	t.Parallel()
+
+	storer := mocks.MockClientStorer{"a": "1"}
+
+	v := Pull(storer, "a")
+
+	if v != "1" {
+		t.Error(`Expected value "1", got:`, v)
+	}
+
+	if len(storer) != 0 {
+		t.Error("Expected length of zero")
+	}
+}

internal/httputil/httputil.go

@@ -1,11 +0,0 @@
-package httputil
-
-import "gopkg.in/authboss.v0"
-
-// PullFlash is a convenience func to retreive then delete a flash message.  Any ok
-// checks are ignored as they don't alter the intended use.
-func PullFlash(storer authboss.ClientStorer, key string) string {
-	value, _ := storer.Get(key)
-	storer.Del(key)
-	return value
-}

internal/views/views.go

@@ -6,15 +6,38 @@ package views
 //go:generate go-bindata -pkg=views -prefix=templates templates
 
 import (
+	"errors"
 	"html/template"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 )
 
-// Get all specified templates grouped under single template.
-func Get(path string, files ...string) (*template.Template, error) {
-	root := template.New("")
+var (
+	// ErrTemplateNotFound should be returned from Get when the view is not found
+	ErrTemplateNotFound = errors.New("Template not found")
+)
+
+// Templates is a map depicting the forms a template needs wrapped within the specified layout
+type Templates map[string]*template.Template
+
+// ExecuteTemplate is a convenience wrapper for executing a template from the layout.  Returns
+// ErrTemplateNotFound when the template is missing, othwerise error.
+func (t Templates) ExecuteTemplate(w io.Writer, name string, data interface{}) error {
+	tpl, ok := t[name]
+	if !ok {
+		return ErrTemplateNotFound
+	}
+
+	return tpl.ExecuteTemplate(w, tpl.Name(), data)
+}
+
+// Get parses all speicified files located in path.  Each template is wrapped
+// in a unique clone of layout.  All templates are expecting {{authboss}} handlebars
+// for parsing.
+func Get(layout *template.Template, path string, files ...string) (Templates, error) {
+	m := make(Templates)
 
 	for _, file := range files {
 		b, err := ioutil.ReadFile(filepath.Join(path, file))
@@ -27,11 +50,18 @@ func Get(path string, files ...string) (*template.Template, error) {
 			}
 		}
 
-		_, err = root.New(file).Parse(string(b))
+		clone, err := layout.Clone()
 		if err != nil {
 			return nil, err
 		}
+
+		_, err = clone.New("authboss").Parse(string(b))
+		if err != nil {
+			return nil, err
+		}
+
+		m[file] = clone
 	}
 
-	return root, nil
+	return m, nil
 }

internal/views/views_test.go

@@ -0,0 +1,78 @@
+package views
+
+import (
+	"bytes"
+	"html/template"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestTemplates_ExecuteTemplate_ReturnsTemplateWhenFound(t *testing.T) {
+	t.Parallel()
+
+	tpl, _ := template.New("").Parse("<strong>{{.Val}}</strong>")
+	tpls := Templates{"a": tpl}
+
+	b := &bytes.Buffer{}
+	if err := tpls.ExecuteTemplate(b, "a", struct{ Val string }{"hi"}); err != nil {
+		t.Error("Unexpected error:", err)
+	}
+
+	expected := "<strong>hi</strong>"
+	actual := b.String()
+	if expected != actual {
+		t.Errorf(`Expected "%s", got %s`, expected, actual)
+	}
+}
+
+func TestTemplates_ExecuteTemplate_ReturnsErrTempalteNotFound(t *testing.T) {
+	t.Parallel()
+
+	tpls := Templates{}
+	err := tpls.ExecuteTemplate(ioutil.Discard, "shouldnotbefound", nil)
+	if err == nil {
+		t.Error("Expected error")
+	}
+
+	if err.Error() != "Template not found" {
+		t.Errorf(`Expected err.Error() to be "Template not found", got: "%s"`, err)
+	}
+
+}
+
+func TestGet(t *testing.T) {
+	t.Parallel()
+
+	file, err := ioutil.TempFile(os.TempDir(), "authboss")
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
+	if _, err := file.Write([]byte("{{.Val}}")); err != nil {
+		t.Error("Error writing to temp file", err)
+	}
+
+	layout, err := template.New("").Parse(`<strong>{{template "authboss" .}}</strong>`)
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
+
+	filename := filepath.Base(file.Name())
+
+	tpls, err := Get(layout, filepath.Dir(file.Name()), filename)
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
+
+	b := &bytes.Buffer{}
+	if err := tpls.ExecuteTemplate(b, filename, struct{ Val string }{"hi"}); err != nil {
+		t.Error("Unexpected error:", err)
+	}
+
+	expected := "<strong>hi</strong>"
+	actual := b.String()
+	if expected != actual {
+		t.Errorf(`Expected "%s", got %s`, expected, actual)
+	}
+}

recover/recover.go

@@ -7,14 +7,13 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"html/template"
 	"net/http"
 	"time"
 
 	"golang.org/x/crypto/bcrypt"
 
 	"gopkg.in/authboss.v0"
-	"gopkg.in/authboss.v0/internal/httputil"
+	"gopkg.in/authboss.v0/internal/flashutil"
 	"gopkg.in/authboss.v0/internal/views"
 )
 
@@ -43,8 +42,9 @@ func init() {
 }
 
 type RecoverModule struct {
-	templates *template.Template
-	config    *authboss.Config
+	templates      views.Templates
+	emailTemplates views.Templates
+	config         *authboss.Config
 }
 
 func (m *RecoverModule) Initialize(config *authboss.Config) (err error) {
@@ -56,7 +56,11 @@ func (m *RecoverModule) Initialize(config *authboss.Config) (err error) {
 		return errors.New("recover: RecoverStorer required for recover functionality.")
 	}
 
-	if m.templates, err = views.Get(config.ViewsPath, tplRecover, tplRecoverComplete, tplInitHTMLEmail, tplInitTextEmail); err != nil {
+	if m.templates, err = views.Get(config.Layout, config.ViewsPath, tplRecover, tplRecoverComplete); err != nil {
+		return err
+	}
+
+	if m.emailTemplates, err = views.Get(config.LayoutEmail, config.ViewsPath, tplInitHTMLEmail, tplInitTextEmail); err != nil {
 		return err
 	}
 
@@ -84,6 +88,7 @@ func (m *RecoverModule) Storage() authboss.StorageOptions {
 type pageRecover struct {
 	Username, ConfirmUsername string
 	ErrMap                    map[string][]string
+	FlashSuccess              string
 	FlashError                string
 }
 
@@ -96,7 +101,7 @@ func (m *RecoverModule) recoverHandlerFunc(ctx *authboss.Context, w http.Respons
 
 	switch r.Method {
 	case methodGET:
-		execTpl(pageRecover{FlashError: httputil.PullFlash(ctx.SessionStorer, authboss.FlashErrorKey)})
+		execTpl(pageRecover{FlashError: flashutil.Pull(ctx.SessionStorer, authboss.FlashErrorKey)})
 	case methodPOST:
 		// ignore ok checks as we validate these fields anyways
 		username, _ := ctx.FirstPostFormValue("username")
@@ -105,14 +110,14 @@ func (m *RecoverModule) recoverHandlerFunc(ctx *authboss.Context, w http.Respons
 		policies := authboss.FilterValidators(m.config.Policies, "username")
 		if validationErrs := ctx.Validate(policies, m.config.ConfirmFields...); len(validationErrs) > 0 {
 			fmt.Fprintf(m.config.LogWriter, errFormat, "validation failed", validationErrs)
-			execTpl(pageRecover{username, confirmUsername, validationErrs.Map(), ""})
+			execTpl(pageRecover{username, confirmUsername, validationErrs.Map(), "", ""})
 			return
 		}
 
 		if err := m.recover(ctx, username); err != nil {
 			// never reveal failed usernames to prevent sniffing
 			fmt.Fprintf(m.config.LogWriter, errFormat, "failed to recover", err)
-			execTpl(pageRecover{username, confirmUsername, nil, m.config.RecoverFailedErrorFlash})
+			execTpl(pageRecover{username, confirmUsername, nil, "", m.config.RecoverFailedErrorFlash})
 			return
 		}
 
@@ -154,12 +159,12 @@ func (m *RecoverModule) sendRecoverEmail(to string, token []byte) {
 	data := struct{ Link string }{fmt.Sprintf("%s/recover/complete?token=%s", m.config.HostName, base64.URLEncoding.EncodeToString(token))}
 
 	htmlEmailBody := &bytes.Buffer{}
-	if err := m.templates.ExecuteTemplate(htmlEmailBody, tplInitHTMLEmail, data); err != nil {
+	if err := m.emailTemplates.ExecuteTemplate(htmlEmailBody, tplInitHTMLEmail, data); err != nil {
 		fmt.Fprintf(m.config.LogWriter, errFormat, "failed to build html tpl", err)
 	}
 
 	textEmaiLBody := &bytes.Buffer{}
-	if err := m.templates.ExecuteTemplate(textEmaiLBody, tplInitTextEmail, data); err != nil {
+	if err := m.emailTemplates.ExecuteTemplate(textEmaiLBody, tplInitTextEmail, data); err != nil {
 		fmt.Fprintf(m.config.LogWriter, errFormat, "failed to build plaintext tpl", err)
 	}
 
@@ -176,24 +181,24 @@ func (m *RecoverModule) sendRecoverEmail(to string, token []byte) {
 }
 
 type pageRecoverComplete struct {
-	Token      string
-	ErrMap     map[string][]string
-	FlashError string
+	Token        string
+	ErrMap       map[string][]string
+	FlashSuccess string
+	FlashError   string
 }
 
 func (m *RecoverModule) recoverCompleteHandlerFunc(ctx *authboss.Context, w http.ResponseWriter, r *http.Request) {
+	execTpl := func(name string, data interface{}) {
+		if err := m.templates.ExecuteTemplate(w, name, data); err != nil {
+			fmt.Fprintf(m.config.LogWriter, errFormat, "unable to execute template", err)
+		}
+	}
+
 	switch r.Method {
 	case methodGET:
-		page := pageRecoverComplete{}
-
-		if msg, ok := ctx.SessionStorer.Get(authboss.FlashErrorKey); ok {
-			page.FlashError = msg
-			ctx.SessionStorer.Del(authboss.FlashErrorKey)
-		}
-
 		token, ok := ctx.FirstFormValue("token")
 		if !ok {
-			fmt.Fprintln(m.config.LogWriter, "recover: expected value token")
+			fmt.Fprintln(m.config.LogWriter, "recover: expected form value token")
 			http.Redirect(w, r, "/", http.StatusFound)
 			return
 		}
@@ -214,66 +219,53 @@ func (m *RecoverModule) recoverCompleteHandlerFunc(ctx *authboss.Context, w http
 			return
 		}
 
-		page.Token = token
-		if err := m.templates.ExecuteTemplate(w, tplRecoverComplete, pageRecoverComplete{Token: token}); err != nil {
-			fmt.Fprintln(m.config.LogWriter, "recover:", err)
-		}
+		execTpl(tplRecoverComplete, pageRecoverComplete{
+			FlashError: flashutil.Pull(ctx.SessionStorer, authboss.FlashErrorKey),
+			Token:      token,
+		})
 	case methodPOST:
 		token, ok := ctx.FirstFormValue("token")
 		if !ok {
-			fmt.Fprintln(m.config.LogWriter, "recover: expected value token")
+			fmt.Fprintln(m.config.LogWriter, "recover: expected form value token")
+			http.Redirect(w, r, "/", http.StatusFound)
+			return
 		}
 
 		var err error
 		ctx.User, err = m.verifyToken(token)
 		if err != nil {
-			fmt.Fprintln(m.config.LogWriter, "recover:", err)
+			fmt.Fprintln(m.config.LogWriter, "recover 1234:", err)
 			http.Redirect(w, r, "/", http.StatusFound)
 			return
 		}
 
 		policies := authboss.FilterValidators(m.config.Policies, "password")
 		if validationErrs := ctx.Validate(policies, m.config.ConfirmFields...); len(validationErrs) > 0 {
-			err := m.templates.ExecuteTemplate(w, tplRecoverComplete, pageRecoverComplete{Token: token, ErrMap: validationErrs.Map()})
-			if err != nil {
-				fmt.Fprintln(m.config.LogWriter, "recover:", err)
-			}
+			execTpl(tplRecoverComplete, pageRecoverComplete{Token: token, ErrMap: validationErrs.Map()})
 			return
 		}
 
 		password, _ := ctx.FirstFormValue("password")
-
 		encryptedPassword, err := bcrypt.GenerateFromPassword([]byte(password), m.config.BCryptCost)
 		if err != nil {
 			fmt.Fprintln(m.config.LogWriter, "recover: failed to encrypt password")
-			err := m.templates.ExecuteTemplate(w, tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
-			if err != nil {
-				fmt.Fprintln(m.config.LogWriter, "recover:", err)
-			}
+			execTpl(tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
 			return
 		}
-
 		ctx.User[attrPassword] = string(encryptedPassword)
 
 		username, ok := ctx.User.String(attrUsername)
 		if !ok {
 			fmt.Println(m.config.LogWriter, "reover: expected user attribue missing:", attrUsername)
-			err := m.templates.ExecuteTemplate(w, tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
-			if err != nil {
-				fmt.Fprintln(m.config.LogWriter, "recover:", err)
-			}
+			execTpl(tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
 			return
 		}
-
 		ctx.User[attrRecoverToken] = ""
 		ctx.User[attrRecoverTokenExpiry] = time.Now().UTC()
 
 		if err := ctx.SaveUser(username, m.config.Storer); err != nil {
-			fmt.Fprintln(m.config.LogWriter, "recover:", err)
-			err := m.templates.ExecuteTemplate(w, tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
-			if err != nil {
-				fmt.Fprintln(m.config.LogWriter, "recover:", err)
-			}
+			fmt.Fprintln(m.config.LogWriter, "recover asdf:", err)
+			execTpl(tplRecoverComplete, pageRecoverComplete{Token: token, FlashError: m.config.RecoverFailedErrorFlash})
 			return
 		}
 

storer.go

@@ -10,9 +10,9 @@ import (
 )
 
 var (
-	// UserNotFound should be returned from Get when the record is not found.
+	// ErrUserNotFound should be returned from Get when the record is not found.
 	ErrUserNotFound = errors.New("User not found")
-	// TokenNotFound should be returned from UseToken when the record is not found.
+	// ErrTokenNotFound should be returned from UseToken when the record is not found.
 	ErrTokenNotFound = errors.New("Token not found")
 )
 
@@ -29,9 +29,9 @@ type Storer interface {
 	// help serialization without using type assertions.
 	Put(key string, attr Attributes) error
 	// Get is for retrieving attributes for a given key. The return value
-	// must be a struct thot contains a field with the correct type as shown
+	// must be a struct that contains a field with the correct type as shown
 	// by attrMeta. If the key is not found in the data store simply
-	// return nil, UserNotFound.
+	// return nil, ErrUserNotFound.
 	Get(key string, attrMeta AttributeMeta) (interface{}, error)
 }
 
@@ -45,12 +45,16 @@ type TokenStorer interface {
 	DelTokens(key string) error
 	// UseToken finds the key-token pair, removes the entry in the store
 	// and returns the key that was found. If the token could not be found
-	// return "", TokenNotFound
+	// return "", ErrTokenNotFound
 	UseToken(givenKey, token string) (key string, err error)
 }
 
+// RecoverStorer must be implement in order to satisfy the recover module's
+// storage requirements
 type RecoverStorer interface {
 	Storer
+	//RecoverUser is for retrieving attributes for a given token.  If the key is
+	//not found in the data store, simply return nil, ErrUserNotFound.
 	RecoverUser(recover string) (interface{}, error)
 }