Merge remote-tracking branch 'stephenafamo/feature/configurable-hashi…

…ng-and-encoding'

auth/auth.go

@@ -5,8 +5,6 @@ import (
 	"context"
 	"net/http"
 
-	"golang.org/x/crypto/bcrypt"
-
 	"github.com/volatiletech/authboss/v3"
 )
 
@@ -77,7 +75,7 @@ func (a *Auth) LoginPost(w http.ResponseWriter, r *http.Request) error {
 	r = r.WithContext(context.WithValue(r.Context(), authboss.CTXKeyUser, pidUser))
 
 	var handled bool
-	err = bcrypt.CompareHashAndPassword([]byte(password), []byte(creds.GetPassword()))
+	err = a.Authboss.Core.Hasher.CompareHashAndPassword(password, creds.GetPassword())
 	if err != nil {
 		handled, err = a.Authboss.Events.FireAfter(authboss.EventAuthFail, w, r)
 		if err != nil {

auth/auth_test.go

@@ -91,6 +91,7 @@ func testSetup() *testHarness {
 
 	harness.ab.Config.Core.BodyReader = harness.bodyReader
 	harness.ab.Config.Core.Logger = mocks.Logger{}
+	harness.ab.Config.Core.Hasher = mocks.Hasher{}
 	harness.ab.Config.Core.Responder = harness.responder
 	harness.ab.Config.Core.Redirector = harness.redirector
 	harness.ab.Config.Storage.SessionState = harness.session

authboss.go

@@ -39,6 +39,15 @@ func New() *Authboss {
 
 // Init authboss, modules, renderers
 func (a *Authboss) Init(modulesToLoad ...string) error {
+	// Create the hasher
+	// Have to do it in Init for backwards compatibility.
+	// If a user did not previously use defaults.SetCore() then they will
+	// suddenly start getting panics
+	// We also cannot use config.Defaults() so we can respect the user's BCryptCost
+	if a.Config.Core.Hasher == nil {
+		a.Config.Core.Hasher = NewBCryptHasher(a.Config.Modules.BCryptCost)
+	}
+
 	if len(modulesToLoad) == 0 {
 		modulesToLoad = RegisteredModules()
 	}
@@ -65,12 +74,12 @@ func (a *Authboss) Init(modulesToLoad ...string) error {
 // in sessions for a user requires special mechanisms not currently provided
 // by authboss.
 func (a *Authboss) UpdatePassword(ctx context.Context, user AuthableUser, newPassword string) error {
-	pass, err := bcrypt.GenerateFromPassword([]byte(newPassword), a.Config.Modules.BCryptCost)
+	pass, err := a.Config.Core.Hasher.GenerateHash(newPassword)
 	if err != nil {
 		return err
 	}
 
-	user.PutPassword(string(pass))
+	user.PutPassword(pass)
 
 	storer := a.Config.Storage.Server
 	if err := storer.Save(ctx, user); err != nil {
@@ -85,9 +94,20 @@ func (a *Authboss) UpdatePassword(ctx context.Context, user AuthableUser, newPas
 	return rmStorer.DelRememberTokens(ctx, user.GetPID())
 }
 
+// VerifyPassword check that the provided password for the user is correct.
+// Returns nil on success otherwise there will be an error.
+// Simply a wrapper for [a.Core.Hasher.CompareHashAndPassword]
+func (a *Authboss) VerifyPassword(user AuthableUser, password string) error {
+	return a.Core.Hasher.CompareHashAndPassword(user.GetPassword(), password)
+}
+
 // VerifyPassword uses authboss mechanisms to check that a password is correct.
 // Returns nil on success otherwise there will be an error. Simply a helper
 // to do the bcrypt comparison.
+//
+// NOTE: This function will work ONLY if no custom hasher was configured in global ab.config
+//
+// Deperecated: use [a.VerifyPassword] instead
 func VerifyPassword(user AuthableUser, password string) error {
 	return bcrypt.CompareHashAndPassword([]byte(user.GetPassword()), []byte(password))
 }
@@ -96,7 +116,7 @@ func VerifyPassword(user AuthableUser, password string) error {
 // in order to access the routes in protects. Requirements is a bit-set integer
 // to be able to easily combine requirements like so:
 //
-//   authboss.RequireFullAuth | authboss.Require2FA
+//	authboss.RequireFullAuth | authboss.Require2FA
 type MWRequirements int
 
 // MWRespondOnFailure tells authboss.Middleware how to respond to
@@ -153,7 +173,7 @@ func MountedMiddleware(ab *Authboss, mountPathed, redirectToLogin, forceFullAuth
 //
 // requirements are set by logical or'ing together requirements. eg:
 //
-//   authboss.RequireFullAuth | authboss.Require2FA
+//	authboss.RequireFullAuth | authboss.Require2FA
 //
 // failureResponse is how the middleware rejects the users that don't meet
 // the criteria. This should be chosen from the MWRespondOnFailure constants.

authboss_test.go

@@ -25,6 +25,7 @@ func TestAuthbossUpdatePassword(t *testing.T) {
 
 	ab := New()
 	ab.Config.Storage.Server = storer
+	ab.Config.Core.Hasher = mockHasher{}
 
 	if err := ab.UpdatePassword(context.Background(), user, "hello world"); err != nil {
 		t.Error(err)

config.go

@@ -61,6 +61,7 @@ type Config struct {
 
 	Modules struct {
 		// BCryptCost is the cost of the bcrypt password hashing function.
+		// Deprecated: Use Hasher instead.
 		BCryptCost int
 
 		// ConfirmMethod IS DEPRECATED! See MailRouteMethod instead.
@@ -239,6 +240,12 @@ type Config struct {
 		// Mailer is the mailer being used to send e-mails out via smtp
 		Mailer Mailer
 
+		// Hasher hashes passwords into hashes
+		Hasher Hasher
+
+		// OneTimeTokenGenerator generates credentials (selector+verified+token)
+		OneTimeTokenGenerator OneTimeTokenGenerator
+
 		// Logger implies just a few log levels for use, can optionally
 		// also implement the ContextLogger to be able to upgrade to a
 		// request specific logger.
@@ -272,4 +279,6 @@ func (c *Config) Defaults() {
 	c.Modules.MailRouteMethod = http.MethodGet
 	c.Modules.RecoverLoginAfterRecovery = false
 	c.Modules.RecoverTokenDuration = 24 * time.Hour
+
+	c.Core.OneTimeTokenGenerator = NewSha512TokenGenerator()
 }

confirm/confirm.go

@@ -14,7 +14,6 @@ import (
 	"path"
 
 	"github.com/friendsofgo/errors"
-
 	"github.com/volatiletech/authboss/v3"
 )
 
@@ -33,9 +32,6 @@ const (
 	// DataConfirmURL is the name of the e-mail template variable
 	// that gives the url to send to the user for confirmation.
 	DataConfirmURL = "url"
-
-	confirmTokenSize  = 64
-	confirmTokenSplit = confirmTokenSize / 2
 )
 
 func init() {
@@ -128,7 +124,7 @@ func (c *Confirm) StartConfirmationWeb(w http.ResponseWriter, r *http.Request, h
 func (c *Confirm) StartConfirmation(ctx context.Context, user authboss.ConfirmableUser, sendEmail bool) error {
 	logger := c.Authboss.Logger(ctx)
 
-	selector, verifier, token, err := GenerateConfirmCreds()
+	selector, verifier, token, err := c.Authboss.Core.OneTimeTokenGenerator.GenerateToken()
 	if err != nil {
 		return err
 	}
@@ -198,13 +194,14 @@ func (c *Confirm) Get(w http.ResponseWriter, r *http.Request) error {
 		return c.invalidToken(w, r)
 	}
 
-	if len(rawToken) != confirmTokenSize {
+	credsGenerator := c.Authboss.Core.OneTimeTokenGenerator
+
+	if len(rawToken) != credsGenerator.TokenSize() {
 		logger.Infof("invalid confirm token submitted, size was wrong: %d", len(rawToken))
 		return c.invalidToken(w, r)
 	}
 
-	selectorBytes := sha512.Sum512(rawToken[:confirmTokenSplit])
-	verifierBytes := sha512.Sum512(rawToken[confirmTokenSplit:])
+	selectorBytes, verifierBytes := credsGenerator.ParseToken(string(rawToken))
 	selector := base64.StdEncoding.EncodeToString(selectorBytes[:])
 
 	storer := authboss.EnsureCanConfirm(c.Authboss.Config.Storage.Server)
@@ -303,11 +300,17 @@ func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {
 // verifier: hash of the second half of a 64 byte value
 // (to be stored in database but never used in SELECT query)
 // token: the user-facing base64 encoded selector+verifier
+//
+// Deprecated: use [authboss.OneTimeTokenGenerator] instead.
 func GenerateConfirmCreds() (selector, verifier, token string, err error) {
+	confirmTokenSize := 64
+	confirmTokenSplit := confirmTokenSize / 2
+
 	rawToken := make([]byte, confirmTokenSize)
 	if _, err = io.ReadFull(rand.Reader, rawToken); err != nil {
 		return "", "", "", err
 	}
+
 	selectorBytes := sha512.Sum512(rawToken[:confirmTokenSplit])
 	verifierBytes := sha512.Sum512(rawToken[confirmTokenSplit:])
 

confirm/confirm_test.go

@@ -71,6 +71,7 @@ func testSetup() *testHarness {
 
 	harness.ab.Config.Core.BodyReader = harness.bodyReader
 	harness.ab.Config.Core.Logger = mocks.Logger{}
+	harness.ab.Config.Core.Hasher = mocks.Hasher{}
 	harness.ab.Config.Core.Mailer = harness.mailer
 	harness.ab.Config.Core.Redirector = harness.redirector
 	harness.ab.Config.Core.MailRenderer = harness.renderer
@@ -176,7 +177,7 @@ func TestGetSuccess(t *testing.T) {
 
 	harness := testSetup()
 
-	selector, verifier, token, err := GenerateConfirmCreds()
+	selector, verifier, token, err := harness.ab.Config.Core.OneTimeTokenGenerator.GenerateToken()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -271,7 +272,7 @@ func TestGetUserNotFoundFailure(t *testing.T) {
 
 	harness := testSetup()
 
-	_, _, token, err := GenerateConfirmCreds()
+	_, _, token, err := harness.ab.Config.Core.OneTimeTokenGenerator.GenerateToken()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -380,7 +381,9 @@ func TestMailURL(t *testing.T) {
 func TestGenerateRecoverCreds(t *testing.T) {
 	t.Parallel()
 
-	selector, verifier, token, err := GenerateConfirmCreds()
+	credsGenerator := authboss.NewSha512TokenGenerator()
+
+	selector, verifier, token, err := credsGenerator.GenerateToken()
 	if err != nil {
 		t.Error(err)
 	}
@@ -389,6 +392,8 @@ func TestGenerateRecoverCreds(t *testing.T) {
 		t.Error("the verifier and selector should be different")
 	}
 
+	confirmTokenSplit := credsGenerator.TokenSize() / 2
+
 	// base64 length: n = 64; 4*(64/3) = 85.3; round to nearest 4: 88
 	if len(verifier) != 88 {
 		t.Errorf("verifier length was wrong (%d): %s", len(verifier), verifier)

go.mod

@@ -1,11 +1,11 @@
 module github.com/volatiletech/authboss/v3
 
-go 1.19
+go 1.20
 
 require (
 	github.com/friendsofgo/errors v0.9.2
 	github.com/pquerna/otp v1.4.0
-	golang.org/x/crypto v0.7.0
+	golang.org/x/crypto v0.14.0
 	golang.org/x/oauth2 v0.6.0
 )
 
@@ -14,7 +14,7 @@ require (
 	github.com/boombuler/barcode v1.0.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
-	golang.org/x/net v0.8.0 // indirect
+	golang.org/x/net v0.10.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.29.1 // indirect

go.sum

@@ -22,11 +22,11 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.7.0 h1:AvwMYaRytfdeVt3u6mLaxYtErKYjxA2OXjJ1HHq6t3A=
-golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU=
+golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc=
+golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4=
 golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
-golang.org/x/net v0.8.0 h1:Zrh2ngAOFYneWTAIAPethzeaQLuHwhuBkuV6ZiRnUaQ=
-golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc=
+golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
+golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
 golang.org/x/oauth2 v0.6.0 h1:Lh8GPgSKBfWSwFvtuWOfeI3aAAnbXTSutYxJiOJFgIw=
 golang.org/x/oauth2 v0.6.0/go.mod h1:ycmewcwgD4Rpr3eZJLSB4Kyyljb3qDh40vJ8STE5HKw=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

hasher.go

@@ -0,0 +1,33 @@
+package authboss
+
+import (
+	"golang.org/x/crypto/bcrypt"
+)
+
+// Hasher is the interface that wraps the hashing and comparison of passwords
+type Hasher interface {
+	CompareHashAndPassword(hash, password string) error
+	GenerateHash(password string) (string, error)
+}
+
+// NewBCryptHasher creates a new bcrypt hasher with the given cost
+func NewBCryptHasher(cost int) *bcryptHasher {
+	return &bcryptHasher{cost: cost}
+}
+
+type bcryptHasher struct {
+	cost int
+}
+
+func (h *bcryptHasher) GenerateHash(password string) (string, error) {
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), h.cost)
+	if err != nil {
+		return "", err
+	}
+
+	return string(hash), nil
+}
+
+func (h *bcryptHasher) CompareHashAndPassword(hashedPassword, password string) error {
+	return bcrypt.CompareHashAndPassword([]byte(hashedPassword), []byte(password))
+}

hasher_test.go

@@ -0,0 +1,37 @@
+package authboss
+
+import (
+	"strings"
+	"testing"
+
+	"golang.org/x/crypto/bcrypt"
+)
+
+func TestBcryptHasher(t *testing.T) {
+	t.Parallel()
+
+	hasher := NewBCryptHasher(bcrypt.DefaultCost)
+
+	hash, err := hasher.GenerateHash("qwerty")
+	if err != nil {
+		t.Error(err)
+	}
+
+	if hash == "" {
+		t.Error("Result Hash must be not empty")
+	}
+	if len(hash) != 60 {
+		t.Error("hash was invalid length", len(hash))
+	}
+	if !strings.HasPrefix(hash, "$2a$10$") {
+		t.Error("hash was wrong", hash)
+	}
+
+	if err := hasher.CompareHashAndPassword(hash, "qwerty"); err != nil {
+		t.Error("compare-hash-and-password for valid password must be ok", err)
+	}
+
+	if err := hasher.CompareHashAndPassword(hash, "qwerty-invalid"); err == nil {
+		t.Error("compare-hash-and-password for invalid password must fail")
+	}
+}

mocks/mocks.go

@@ -3,6 +3,7 @@ package mocks
 
 import (
 	"context"
+	"golang.org/x/crypto/bcrypt"
 	"io"
 	"net/http"
 	"net/url"
@@ -751,3 +752,19 @@ func (e *ErrorHandler) Wrap(handler func(w http.ResponseWriter, r *http.Request)
 		}
 	})
 }
+
+// Hasher is actually just a normal bcrypt hasher
+type Hasher struct{}
+
+func (m Hasher) GenerateHash(s string) (string, error) {
+	hash, err := bcrypt.GenerateFromPassword([]byte(s), bcrypt.DefaultCost)
+	if err != nil {
+		return "", err
+	}
+
+	return string(hash), nil
+}
+
+func (m Hasher) CompareHashAndPassword(hashedPassword, password string) error {
+	return bcrypt.CompareHashAndPassword([]byte(hashedPassword), []byte(password))
+}