Fix the remember module.

lock/lock.go

@@ -14,6 +14,10 @@ const (
 	StoreLocked        = "locked"
 )
 
+var (
+	errUserMissing = errors.New("lock: user not loaded in BeforeAuth callback")
+)
+
 // L is the singleton instance of the lock module which will have been
 // configured and ready to use after authboss.Init()
 var L *Lock
@@ -55,7 +59,7 @@ func (l *Lock) Storage() authboss.StorageOptions {
 // BeforeAuth ensures the account is not locked.
 func (l *Lock) BeforeAuth(ctx *authboss.Context) (authboss.Interrupt, error) {
 	if ctx.User == nil {
-		return authboss.InterruptNone, errors.New("lock: user not loaded in BeforeAuth callback")
+		return authboss.InterruptNone, errUserMissing
 	}
 
 	if locked, ok := ctx.User.Bool(StoreLocked); ok && locked {
@@ -68,7 +72,7 @@ func (l *Lock) BeforeAuth(ctx *authboss.Context) (authboss.Interrupt, error) {
 // AfterAuth resets the attempt number field.
 func (l *Lock) AfterAuth(ctx *authboss.Context) error {
 	if ctx.User == nil {
-		return errors.New("lock: user not loaded in AfterAuth callback")
+		return errUserMissing
 	}
 
 	ctx.User[StoreAttemptNumber] = 0
@@ -84,7 +88,7 @@ func (l *Lock) AfterAuth(ctx *authboss.Context) error {
 // AfterAuthFail adjusts the attempt number and time.
 func (l *Lock) AfterAuthFail(ctx *authboss.Context) error {
 	if ctx.User == nil {
-		return errors.New("lock: user not loaded in AfterAuth callback")
+		return errUserMissing
 	}
 
 	lastAttempt := time.Now().UTC()

lock/lock_test.go

@@ -23,8 +23,8 @@ func TestBeforeAuth(t *testing.T) {
 	authboss.NewConfig()
 	ctx := authboss.NewContext()
 
-	if interrupt, err := L.BeforeAuth(ctx); err == nil {
-		t.Error("Want death because user not loaded:", err)
+	if interrupt, err := L.BeforeAuth(ctx); err != errUserMissing {
+		t.Error("Expected an error because of missing user:", err)
 	} else if interrupt != authboss.InterruptNone {
 		t.Error("Interrupt should not be set:", interrupt)
 	}
@@ -43,8 +43,8 @@ func TestAfterAuth(t *testing.T) {
 	lock := Lock{}
 	ctx := authboss.NewContext()
 
-	if err := lock.AfterAuth(ctx); err == nil {
-		t.Error("Expected an error because of missing user.")
+	if err := lock.AfterAuth(ctx); err != errUserMissing {
+		t.Error("Expected an error because of missing user:", err)
 	}
 
 	storer := mocks.NewMockStorer()

remember/remember.go

@@ -15,8 +15,12 @@ import (
 )
 
 const (
-	// ValueKey is used for cookies and form input names.
-	ValueKey = "rm"
+	// RememberKey is used for cookies and form input names.
+	RememberKey = "rm"
+)
+
+var (
+	errUserMissing = errors.New("remember: User not loaded in callback")
 )
 
 const nRandBytes = 32
@@ -34,11 +38,11 @@ type Remember struct{}
 
 func (r *Remember) Initialize() error {
 	if authboss.Cfg.Storer == nil {
-		return errors.New("remember: Need a TokenStorer.")
+		return errors.New("remember: Need a TokenStorer")
 	}
 
 	if _, ok := authboss.Cfg.Storer.(authboss.TokenStorer); !ok {
-		return errors.New("remember: TokenStorer required for remember me functionality.")
+		return errors.New("remember: TokenStorer required for remember me functionality")
 	}
 
 	authboss.Cfg.Callbacks.After(authboss.EventAuth, r.AfterAuth)
@@ -55,31 +59,25 @@ func (r *Remember) Storage() authboss.StorageOptions {
 }
 
 // AfterAuth is called after authentication is successful.
-func (r *Remember) AfterAuth(ctx *authboss.Context) {
-	if val, ok := ctx.FirstPostFormValue(ValueKey); !ok || val != "true" {
-		return
+func (r *Remember) AfterAuth(ctx *authboss.Context) error {
+	if val, ok := ctx.FirstPostFormValue(RememberKey); !ok || val != "true" {
+		return nil
 	}
 
 	if ctx.User == nil {
-		fmt.Fprintf(authboss.Cfg.LogWriter, "remember: AfterAuth no user loaded")
-		return
+		return errUserMissing
 	}
 
-	keyIntf, ok := ctx.User["username"]
-	if !ok {
-		fmt.Fprintf(authboss.Cfg.LogWriter, "remember: username not present")
-		return
-	}
-
-	key, ok := keyIntf.(string)
-	if !ok {
-		fmt.Fprintf(authboss.Cfg.LogWriter, "remember: username not a string")
-		return
+	key, err := ctx.User.StringErr("username")
+	if err != nil {
+		return err
 	}
 
 	if _, err := r.New(ctx.CookieStorer, key); err != nil {
-		fmt.Fprintf(authboss.Cfg.LogWriter, "remember: Failed to create remember token: %v", err)
+		return fmt.Errorf("remember: Failed to create remember token: %v", err)
 	}
+
+	return nil
 }
 
 // New generates a new remember token and stores it in the configured TokenStorer.
@@ -104,7 +102,7 @@ func (r *Remember) New(cstorer authboss.ClientStorer, storageKey string) (string
 	}
 
 	// Write the finalToken to the cookie
-	cstorer.Put(ValueKey, finalToken)
+	cstorer.Put(RememberKey, finalToken)
 
 	return finalToken, nil
 }

remember/remember_test.go

@@ -54,9 +54,11 @@ func TestAfterAuth(t *testing.T) {
 	ctx.CookieStorer = cookies
 	ctx.User = authboss.Attributes{"username": "testuser"}
 
-	R.AfterAuth(ctx)
+	if err := R.AfterAuth(ctx); err != nil {
+		t.Error(err)
+	}
 
-	if _, ok := cookies.Values[ValueKey]; !ok {
+	if _, ok := cookies.Values[RememberKey]; !ok {
 		t.Error("Expected a cookie to have been set.")
 	}
 }
@@ -84,7 +86,7 @@ func TestNew(t *testing.T) {
 		t.Error("Expected a token to be saved.")
 	}
 
-	if token != cookies.Values[ValueKey] {
+	if token != cookies.Values[RememberKey] {
 		t.Error("Expected a cookie set with the token.")
 	}
 }