fix: propagate session unmarshal errors instead of silently discarding

Author: hrntknr

pkg/repository/interface.go

@@ -3,6 +3,7 @@ package repository
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/oauth2"
@@ -29,10 +30,12 @@ type AuthorizeRequestStorage interface {
 	DeleteAuthorizeRequest(ctx context.Context, requestID string) error
 }
 
-func restoreSession(req *fosite.Request, sessionData json.RawMessage, sess fosite.Session) {
+func restoreSession(req *fosite.Request, sessionData json.RawMessage, sess fosite.Session) error {
 	if len(sessionData) > 0 && sess != nil {
-		if json.Unmarshal(sessionData, sess) == nil {
-			req.SetSession(sess)
+		if err := json.Unmarshal(sessionData, sess); err != nil {
+			return fmt.Errorf("failed to unmarshal session data: %w", err)
 		}
+		req.SetSession(sess)
 	}
+	return nil
 }

pkg/repository/kvs.go

@@ -89,7 +89,9 @@ func (r *kvsRepository) GetAuthorizeCodeSession(ctx context.Context, code string
 		return nil, err
 	}
 	fositeReq := req.ToFositeReq()
-	restoreSession(fositeReq, req.SessionData, sess)
+	if err := restoreSession(fositeReq, req.SessionData, sess); err != nil {
+		return nil, err
+	}
 	return fositeReq, nil
 }
 
@@ -107,7 +109,9 @@ func (r *kvsRepository) GetAccessTokenSession(ctx context.Context, signature str
 		return nil, err
 	}
 	fositeReq := req.ToFositeReq()
-	restoreSession(fositeReq, req.SessionData, sess)
+	if err := restoreSession(fositeReq, req.SessionData, sess); err != nil {
+		return nil, err
+	}
 	return fositeReq, nil
 }
 
@@ -125,7 +129,9 @@ func (r *kvsRepository) GetRefreshTokenSession(ctx context.Context, signature st
 		return nil, err
 	}
 	fositeReq := req.ToFositeReq()
-	restoreSession(fositeReq, req.SessionData, sess)
+	if err := restoreSession(fositeReq, req.SessionData, sess); err != nil {
+		return nil, err
+	}
 	return fositeReq, nil
 }
 
@@ -200,7 +206,9 @@ func (r *kvsRepository) GetPKCERequestSession(ctx context.Context, signature str
 		return nil, err
 	}
 	fositeReq := req.ToFositeReq()
-	restoreSession(fositeReq, req.SessionData, sess)
+	if err := restoreSession(fositeReq, req.SessionData, sess); err != nil {
+		return nil, err
+	}
 	return fositeReq, nil
 }
 

pkg/repository/sql.go

@@ -356,7 +356,9 @@ func unmarshalRequest(data []byte, sess fosite.Session) (fosite.Requester, error
 		return nil, fmt.Errorf("failed to unmarshal request: %w", err)
 	}
 	fositeReq := req.ToFositeReq()
-	restoreSession(fositeReq, req.SessionData, sess)
+	if err := restoreSession(fositeReq, req.SessionData, sess); err != nil {
+		return nil, err
+	}
 	return fositeReq, nil
 }
 

pkg/repository/sql_test.go

@@ -98,7 +98,7 @@ func TestSQLRepositorySessionPersistence(t *testing.T) {
 	}
 }
 
-func TestSQLRepositorySessionPersistence_BackwardsCompatible(t *testing.T) {
+func TestSQLRepositorySessionPersistence_NilSessionStored(t *testing.T) {
 	repo, err := NewSQLRepository("sqlite", "file::memory:?cache=shared&_busy_timeout=5000")
 	if err != nil {
 		t.Fatalf("failed to create sql repository: %v", err)
@@ -112,7 +112,6 @@ func TestSQLRepositorySessionPersistence_BackwardsCompatible(t *testing.T) {
 		RedirectURIs: []string{"https://example.com/callback"},
 	}
 
-	// Simulate old data without session
 	req := &fosite.Request{
 		ID:             "req-old",
 		RequestedAt:    time.Now().UTC().Round(time.Second),
@@ -130,9 +129,8 @@ func TestSQLRepositorySessionPersistence_BackwardsCompatible(t *testing.T) {
 		t.Fatalf("GetAuthorizeCodeSession failed: %v", err)
 	}
 
-	// Session should be nil (no data to restore, no fallback)
 	if result.GetSession() != nil {
-		t.Fatalf("expected nil session for old data, got %v", result.GetSession())
+		t.Fatalf("expected nil session when no session was stored, got %v", result.GetSession())
 	}
 }