fix(verify): close service-level typed-nil store + adapter-author guidance

auth.go

@@ -27,15 +27,15 @@ type Client struct {
 
 // Service provides higher level wrapper allowing to construct everything and get back token middleware
 type Service struct {
-	logger             logger.L
-	opts               Opts
-	jwtService         *token.Service
-	providers          []provider.Service
-	authMiddleware     middleware.Authenticator
-	avatarProxy        *avatar.Proxy
-	issuer             string
-	useGravatar        bool
-	verifConfirmStore  provider.VerifConfirmationStore
+	logger                logger.L
+	opts                  Opts
+	jwtService            *token.Service
+	providers             []provider.Service
+	authMiddleware        middleware.Authenticator
+	avatarProxy           *avatar.Proxy
+	issuer                string
+	useGravatar           bool
+	verifConfirmStore     provider.VerifConfirmationStore
 	verifConfirmStoreOnce sync.Once
 }
 
@@ -447,8 +447,16 @@ func (s *Service) AddDirectProviderWithUserIDFunc(name string, credChecker provi
 // AddVerifProvider adds provider user's verification sent by sender
 func (s *Service) AddVerifProvider(name, msgTmpl string, sender provider.Sender) {
 	s.verifConfirmStoreOnce.Do(func() {
-		if s.opts.VerifConfirmationStore != nil {
-			s.verifConfirmStore = s.opts.VerifConfirmationStore
+		store := s.opts.VerifConfirmationStore
+		// guard against a typed-nil VerifConfirmationStoreFunc: a non-nil
+		// interface wrapping a nil func would survive the != nil check below
+		// and silently disable replay protection (the handler-level guard
+		// at LoginHandler then normalizes it to nil).
+		if fn, ok := store.(provider.VerifConfirmationStoreFunc); ok && fn == nil {
+			store = nil
+		}
+		if store != nil {
+			s.verifConfirmStore = store
 			return
 		}
 		s.verifConfirmStore = provider.NewInMemoryVerifStore()

auth_test.go

@@ -129,6 +129,25 @@ func TestService_AddVerifProvider_UsesCustomConfirmationStore(t *testing.T) {
 	assert.Same(t, custom, svc.verifConfirmStore, "custom store from Opts must be used, not the in-memory default")
 }
 
+func TestService_AddVerifProvider_TypedNilStoreFuncFallsBackToDefault(t *testing.T) {
+	// Opts.VerifConfirmationStore set to a typed-nil VerifConfirmationStoreFunc
+	// is a non-nil interface wrapping a nil func. Without a service-level
+	// guard the != nil check at AddVerifProvider succeeds, NewInMemoryVerifStore
+	// is skipped, and the handler-level guard then normalizes the typed-nil to
+	// nil at redemption -- net result is replay protection silently disabled.
+	var nilFn provider.VerifConfirmationStoreFunc
+	svc := NewService(Opts{
+		SecretReader:           token.SecretFunc(func(string) (string, error) { return "secret", nil }),
+		URL:                    "http://127.0.0.1",
+		Logger:                 logger.Std,
+		VerifConfirmationStore: nilFn,
+	})
+	svc.AddVerifProvider("email", "{{.Token}}", provider.SenderFunc(func(string, string) error { return nil }))
+	require.NotNil(t, svc.verifConfirmStore, "typed-nil func must fall back to in-memory default, not silently disable protection")
+	_, ok := svc.verifConfirmStore.(provider.VerifConfirmationStoreFunc)
+	assert.False(t, ok, "fallback must be the in-memory default, not the typed-nil func itself")
+}
+
 func TestService_AddVerifProvider_DefaultsToInMemory(t *testing.T) {
 	svc := NewService(Opts{
 		SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),

provider/verify.go

@@ -70,6 +70,13 @@ type VerifConfirmationStore interface {
 	// valid reopens the replay window the store is meant to close. err
 	// signals a backend failure (network, disk, capacity, etc.); callers
 	// MUST treat a non-nil err as fail-closed (reject the redemption).
+	//
+	// Adapter authors: do NOT embed key (or any caller-supplied data) in
+	// returned errors. The handler logs err on the fail-closed branch, and
+	// although key is the SHA-256 of the raw token rather than the token
+	// itself, it still uniquely identifies the live, unredeemed JWT in
+	// log destinations. Wrap the underlying backend error with a generic
+	// description (e.g. "redis SET failed: %w") instead.
 	MarkUsed(key string, ttl time.Duration) (alreadyUsed bool, err error)
 }
 

provider/verify_test.go

@@ -338,6 +338,26 @@ func TestInMemoryVerifStore(t *testing.T) {
 	})
 }
 
+func TestScrubTokenFromRequest(t *testing.T) {
+	t.Run("token query is replaced with redacted sentinel", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/login?token=secret-jwt&sess=1", http.NoBody)
+		require.NoError(t, err)
+		out := scrubTokenFromRequest(req)
+		assert.Equal(t, "secret-jwt", req.URL.Query().Get("token"), "original request must not be mutated")
+		assert.Equal(t, "<redacted>", out.URL.Query().Get("token"))
+		assert.Equal(t, "1", out.URL.Query().Get("sess"), "other query params preserved")
+	})
+	t.Run("missing token query returns request unchanged", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/login?sess=1", http.NoBody)
+		require.NoError(t, err)
+		out := scrubTokenFromRequest(req)
+		assert.Same(t, req, out)
+	})
+	t.Run("nil request returns nil", func(t *testing.T) {
+		assert.Nil(t, scrubTokenFromRequest(nil))
+	})
+}
+
 func TestVerifConfirmationStoreFunc(t *testing.T) {
 	calls := 0
 	var lastKey string

v2/auth.go

@@ -27,15 +27,15 @@ type Client struct {
 
 // Service provides higher level wrapper allowing to construct everything and get back token middleware
 type Service struct {
-	logger             logger.L
-	opts               Opts
-	jwtService         *token.Service
-	providers          []provider.Service
-	authMiddleware     middleware.Authenticator
-	avatarProxy        *avatar.Proxy
-	issuer             string
-	useGravatar        bool
-	verifConfirmStore  provider.VerifConfirmationStore
+	logger                logger.L
+	opts                  Opts
+	jwtService            *token.Service
+	providers             []provider.Service
+	authMiddleware        middleware.Authenticator
+	avatarProxy           *avatar.Proxy
+	issuer                string
+	useGravatar           bool
+	verifConfirmStore     provider.VerifConfirmationStore
 	verifConfirmStoreOnce sync.Once
 }
 
@@ -448,8 +448,16 @@ func (s *Service) AddDirectProviderWithUserIDFunc(name string, credChecker provi
 // AddVerifProvider adds provider user's verification sent by sender
 func (s *Service) AddVerifProvider(name, msgTmpl string, sender provider.Sender) {
 	s.verifConfirmStoreOnce.Do(func() {
-		if s.opts.VerifConfirmationStore != nil {
-			s.verifConfirmStore = s.opts.VerifConfirmationStore
+		store := s.opts.VerifConfirmationStore
+		// guard against a typed-nil VerifConfirmationStoreFunc: a non-nil
+		// interface wrapping a nil func would survive the != nil check below
+		// and silently disable replay protection (the handler-level guard
+		// at LoginHandler then normalizes it to nil).
+		if fn, ok := store.(provider.VerifConfirmationStoreFunc); ok && fn == nil {
+			store = nil
+		}
+		if store != nil {
+			s.verifConfirmStore = store
 			return
 		}
 		s.verifConfirmStore = provider.NewInMemoryVerifStore()

v2/auth_test.go

@@ -129,6 +129,25 @@ func TestService_AddVerifProvider_UsesCustomConfirmationStore(t *testing.T) {
 	assert.Same(t, custom, svc.verifConfirmStore, "custom store from Opts must be used, not the in-memory default")
 }
 
+func TestService_AddVerifProvider_TypedNilStoreFuncFallsBackToDefault(t *testing.T) {
+	// Opts.VerifConfirmationStore set to a typed-nil VerifConfirmationStoreFunc
+	// is a non-nil interface wrapping a nil func. Without a service-level
+	// guard the != nil check at AddVerifProvider succeeds, NewInMemoryVerifStore
+	// is skipped, and the handler-level guard then normalizes the typed-nil to
+	// nil at redemption -- net result is replay protection silently disabled.
+	var nilFn provider.VerifConfirmationStoreFunc
+	svc := NewService(Opts{
+		SecretReader:           token.SecretFunc(func(string) (string, error) { return "secret", nil }),
+		URL:                    "http://127.0.0.1",
+		Logger:                 logger.Std,
+		VerifConfirmationStore: nilFn,
+	})
+	svc.AddVerifProvider("email", "{{.Token}}", provider.SenderFunc(func(string, string) error { return nil }))
+	require.NotNil(t, svc.verifConfirmStore, "typed-nil func must fall back to in-memory default, not silently disable protection")
+	_, ok := svc.verifConfirmStore.(provider.VerifConfirmationStoreFunc)
+	assert.False(t, ok, "fallback must be the in-memory default, not the typed-nil func itself")
+}
+
 func TestService_AddVerifProvider_DefaultsToInMemory(t *testing.T) {
 	svc := NewService(Opts{
 		SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),

v2/provider/verify.go

@@ -70,6 +70,13 @@ type VerifConfirmationStore interface {
 	// valid reopens the replay window the store is meant to close. err
 	// signals a backend failure (network, disk, capacity, etc.); callers
 	// MUST treat a non-nil err as fail-closed (reject the redemption).
+	//
+	// Adapter authors: do NOT embed key (or any caller-supplied data) in
+	// returned errors. The handler logs err on the fail-closed branch, and
+	// although key is the SHA-256 of the raw token rather than the token
+	// itself, it still uniquely identifies the live, unredeemed JWT in
+	// log destinations. Wrap the underlying backend error with a generic
+	// description (e.g. "redis SET failed: %w") instead.
 	MarkUsed(key string, ttl time.Duration) (alreadyUsed bool, err error)
 }
 

v2/provider/verify_test.go

@@ -259,6 +259,26 @@ func TestInMemoryVerifStore(t *testing.T) {
 	})
 }
 
+func TestScrubTokenFromRequest(t *testing.T) {
+	t.Run("token query is replaced with redacted sentinel", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/login?token=secret-jwt&sess=1", http.NoBody)
+		require.NoError(t, err)
+		out := scrubTokenFromRequest(req)
+		assert.Equal(t, "secret-jwt", req.URL.Query().Get("token"), "original request must not be mutated")
+		assert.Equal(t, "<redacted>", out.URL.Query().Get("token"))
+		assert.Equal(t, "1", out.URL.Query().Get("sess"), "other query params preserved")
+	})
+	t.Run("missing token query returns request unchanged", func(t *testing.T) {
+		req, err := http.NewRequest("GET", "/login?sess=1", http.NoBody)
+		require.NoError(t, err)
+		out := scrubTokenFromRequest(req)
+		assert.Same(t, req, out)
+	})
+	t.Run("nil request returns nil", func(t *testing.T) {
+		assert.Nil(t, scrubTokenFromRequest(nil))
+	})
+}
+
 func TestVerifConfirmationStoreFunc(t *testing.T) {
 	calls := 0
 	var lastKey string