Merge pull request #10 from crydensync/feat/password-policy2

Feat/password policy2

Author: raymondproguy

auth/breach_test.go

@@ -29,7 +29,7 @@ func TestSignUp_RejectsBreachedPassword(t *testing.T) {
 	ctx := context.Background()
 	checker := &fakeBreachChecker{breached: true}
 
-	_, err := SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, "proguy@example.com", "password123", "1.2.3.4")
+	_, err := SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, security.PasswordPolicy{}, "proguy@example.com", "password123", "1.2.3.4")
 	if err != ErrPasswordBreached {
 		t.Errorf("expected ErrPasswordBreached, got %v", err)
 	}
@@ -43,7 +43,7 @@ func TestSignUp_BreachCheckerErrorFailsOpen(t *testing.T) {
 	ctx := context.Background()
 	checker := &fakeBreachChecker{err: errors.New("simulated HIBP outage")}
 
-	_, err := SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, "proguy@example.com", "password123", "1.2.3.4")
+	_, err := SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, security.PasswordPolicy{}, "proguy@example.com", "password123", "1.2.3.4")
 	if err != nil {
 		t.Fatalf("expected signup to succeed (fail open) when the breach checker errors, got %v", err)
 	}
@@ -61,7 +61,7 @@ func TestChangePassword_RejectsBreachedNewPassword(t *testing.T) {
 	hash, _ := hasher.Hash("old-password")
 	users.Create(ctx, storeUser("user-1", "raymondproguy@dev.com", hash))
 
-	err := ChangePassword(ctx, users, sessions, hasher, checker, audit, log, "user-1", "old-password", "password123")
+	err := ChangePassword(ctx, users, sessions, hasher, checker, audit, log, security.PasswordPolicy{}, "user-1", "old-password", "password123")
 	if err != ErrPasswordBreached {
 		t.Errorf("expected ErrPasswordBreached, got %v", err)
 	}
@@ -81,7 +81,7 @@ func TestChangePassword_WrongCurrentPasswordCheckedBeforeBreach(t *testing.T) {
 	hash, _ := hasher.Hash("old-password")
 	users.Create(ctx, storeUser("user-1", "raymondproguy@dev.com", hash))
 
-	err := ChangePassword(ctx, users, sessions, hasher, checker, audit, log, "user-1", "totally-wrong", "password123")
+	err := ChangePassword(ctx, users, sessions, hasher, checker, audit, log, security.PasswordPolicy{}, "user-1", "totally-wrong", "password123")
 	if err != ErrInvalidCredentials {
 		t.Errorf("expected ErrInvalidCredentials (checked before breach check), got %v", err)
 	}
@@ -95,7 +95,7 @@ func TestSignUp_BreachRejectionIsAudited(t *testing.T) {
 	ctx := context.Background()
 	checker := &fakeBreachChecker{breached: true}
 
-	SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, "proguy@example.com", "password123", "1.2.3.4")
+	SignUp(ctx, users, hasher, ids, limiter, checker, audit, log, security.PasswordPolicy{}, "proguy@example.com", "password123", "1.2.3.4")
 
 	events, err := audit.SearchByType(ctx, store.EventPasswordBreachRejected, 10)
 	if err != nil {

security/webauthn_test.go

@@ -90,6 +90,17 @@ func TestGoWebAuthnProvider_LoginRoundTrip(t *testing.T) {
 	user.creds = append(user.creds, *cred)
 	rp := virtualwebauthn.RelyingParty{Name: testRPDisplayName, ID: testRPID, Origin: testRPOrigin}
 
+	// virtualwebauthn's simulated credential starts at counter 0 and
+	// never auto-increments — unlike many real hardware authenticators,
+	// which do. Bump it manually here to exercise the pass-through
+	// path this test actually cares about: does FinishLogin correctly
+	// surface whatever counter value the authenticator reports, so a
+	// caller can persist it and detect a future non-advancing (cloned-
+	// authenticator) value? A counter of 0 is itself a legitimate,
+	// spec-allowed value (many platform authenticators never track
+	// one at all), so asserting "must be nonzero" was simply wrong.
+	vCred.Counter = 7
+
 	assertion, session, err := provider.BeginLogin(user)
 	if err != nil {
 		t.Fatalf("BeginLogin failed: %v", err)
@@ -109,8 +120,8 @@ func TestGoWebAuthnProvider_LoginRoundTrip(t *testing.T) {
 	if err != nil {
 		t.Fatalf("FinishLogin failed: %v", err)
 	}
-	if updatedCred.Authenticator.SignCount == 0 {
-		t.Error("expected the signature counter to have advanced past zero after a real login")
+	if updatedCred.Authenticator.SignCount != 7 {
+		t.Errorf("expected the returned credential to carry the authenticator's reported counter (7), got %d", updatedCred.Authenticator.SignCount)
 	}
 }
 

token/generator.go

@@ -3,6 +3,7 @@ package token
 import (
 	"crypto/rand"
 	"encoding/hex"
+	"io"
 )
 
 // TokenGenerator defines generation of opaque, cryptographically random
@@ -21,6 +22,17 @@ type CryptoRandTokenGenerator struct {
 	// ByteLength is the number of random bytes generated per token.
 	// 32 bytes (256 bits) is the standard baseline for opaque tokens.
 	ByteLength int
+	// randReader defaults to crypto/rand.Reader (set by
+	// NewCryptoRandTokenGenerator) and is never exposed publicly.
+	// Deliberately injectable rather than calling crypto/rand.Read
+	// directly, so a failure path can be tested by swapping this
+	// field on a same-package instance — mutating the real global
+	// crypto/rand.Reader instead (the previous approach) is not
+	// portable: on at least one real platform, a failed read through
+	// the actual global triggers the Go runtime's own unrecoverable
+	// fatal-error path instead of returning a normal error, crashing
+	// the whole test binary rather than failing one test.
+	randReader io.Reader
 }
 
 // NewCryptoRandTokenGenerator constructs a generator. byteLength must
@@ -31,12 +43,12 @@ func NewCryptoRandTokenGenerator(byteLength int) (*CryptoRandTokenGenerator, err
 		// Reject anything below 128 bits — too weak for a session token.
 		return nil, ErrTokenByteLengthTooShort
 	}
-	return &CryptoRandTokenGenerator{ByteLength: byteLength}, nil
+	return &CryptoRandTokenGenerator{ByteLength: byteLength, randReader: rand.Reader}, nil
 }
 
 func (g *CryptoRandTokenGenerator) New() (string, error) {
 	buf := make([]byte, g.ByteLength)
-	if _, err := rand.Read(buf); err != nil {
+	if _, err := io.ReadFull(g.randReader, buf); err != nil {
 		return "", err
 	}
 	return hex.EncodeToString(buf), nil

token/generator_rand_error_test.go

@@ -1,15 +1,16 @@
 package token
 
 import (
-	"crypto/rand"
 	"errors"
-	"io"
 	"testing"
 )
 
-// failingReader always errors — swapped in for crypto/rand.Reader to
-// deterministically exercise the rand.Read failure path, which never
-// fails in practice under normal conditions.
+// failingReader always errors — injected directly into a
+// CryptoRandTokenGenerator instance (same package, unexported field)
+// to deterministically exercise the New() error path. This never
+// touches the real crypto/rand.Reader global — see randReader's doc
+// comment in generator.go for why mutating that global directly isn't
+// safe across platforms.
 type failingReader struct{}
 
 func (failingReader) Read(p []byte) (int, error) {
@@ -18,24 +19,19 @@ func (failingReader) Read(p []byte) (int, error) {
 
 func TestCryptoRandTokenGenerator_New_PropagatesRandReadError(t *testing.T) {
 	// Regression test: New() used to swallow a rand.Read failure and
-	// return ("", nil) — an empty string treated as a valid token
-	// with no error to catch it. It must now return the real error.
-	original := rand.Reader
-	rand.Reader = failingReader{}
-	defer func() { rand.Reader = original }()
-
+	// return ("", nil) — an empty string treated as a valid token with
+	// no error to catch it. It must now return the real error.
 	g, err := NewCryptoRandTokenGenerator(32)
 	if err != nil {
 		t.Fatalf("unexpected error constructing generator: %v", err)
 	}
+	g.randReader = failingReader{}
 
 	tok, err := g.New()
 	if err == nil {
-		t.Fatal("expected New() to return an error when rand.Read fails, got nil")
+		t.Fatal("expected New() to return an error when the entropy source fails, got nil")
 	}
 	if tok != "" {
 		t.Errorf("expected an empty token alongside the error, got %q", tok)
 	}
 }
-
-var _ io.Reader = failingReader{}