firewalldb: assert strong coupling between kvstores and sessions

If the is writing to a kvstore that is namespaced by a session's ID, we
first assert that the session exists in the session store. We add a test
for this accordingly. We do this in preparation for adding a sql backend
which will error out if we try to link a kvstore to a session that does
not yet exist.

Author: ellemouton

firewalldb/kvstores_kvdb.go

@@ -60,7 +60,8 @@ func (db *BoltDB) GetKVStores(rule string, groupID session.ID,
 		db: db.DB,
 		wrapTx: func(tx *bbolt.Tx) KVStoreTx {
 			return &kvStoreTx{
-				boltTx: tx,
+				boltTx:   tx,
+				sessions: db.sessionIDIndex,
 				kvStores: &kvStores{
 					ruleName:    rule,
 					groupID:     groupID,
@@ -109,6 +110,7 @@ type getBucketFunc func(tx *bbolt.Tx, create bool) (*bbolt.Bucket, error)
 type kvStoreTx struct {
 	boltTx    *bbolt.Tx
 	getBucket getBucketFunc
+	sessions  session.IDToGroupIndex
 
 	*kvStores
 }
@@ -120,6 +122,7 @@ func (s *kvStoreTx) Global() KVStore {
 	return &kvStoreTx{
 		kvStores:  s.kvStores,
 		boltTx:    s.boltTx,
+		sessions:  s.sessions,
 		getBucket: getGlobalRuleBucket(true, s.ruleName),
 	}
 }
@@ -138,6 +141,7 @@ func (s *kvStoreTx) Local() KVStore {
 	return &kvStoreTx{
 		kvStores:  s.kvStores,
 		boltTx:    s.boltTx,
+		sessions:  s.sessions,
 		getBucket: fn,
 	}
 }
@@ -150,6 +154,7 @@ func (s *kvStoreTx) GlobalTemp() KVStore {
 	return &kvStoreTx{
 		kvStores:  s.kvStores,
 		boltTx:    s.boltTx,
+		sessions:  s.sessions,
 		getBucket: getGlobalRuleBucket(false, s.ruleName),
 	}
 }
@@ -167,6 +172,7 @@ func (s *kvStoreTx) LocalTemp() KVStore {
 	return &kvStoreTx{
 		kvStores:  s.kvStores,
 		boltTx:    s.boltTx,
+		sessions:  s.sessions,
 		getBucket: fn,
 	}
 }
@@ -294,6 +300,19 @@ func (s *kvStoreTx) getSessionRuleBucket(perm bool) getBucketFunc {
 		}
 
 		if create {
+			// NOTE: for a bbolt backend, the context is in any case
+			// dropped behind the GetSessionIDs call. So passing in
+			// a new context here is not a problem.
+			ctx := context.Background()
+
+			// If create is true, we expect this to be an existing
+			// session. So we check that now and return an error
+			// accordingly if the session does not exist.
+			_, err := s.sessions.GetSessionIDs(ctx, s.groupID)
+			if err != nil {
+				return nil, err
+			}
+
 			sessBucket, err := ruleBucket.CreateBucketIfNotExists(
 				sessKVStoreBucketKey,
 			)

firewalldb/kvstores_test.go

@@ -5,8 +5,10 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/lightninglabs/lightning-terminal/session"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -83,15 +85,21 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
 		featureName = "auto-fees"
 	}
 
-	store := NewTestDB(t)
+	sessions := session.NewTestDB(t, clock.NewDefaultClock())
+	store := NewTestDBWithSessions(t, sessions)
 	db := NewDB(store)
 	require.NoError(t, db.Start(ctx))
 
-	kvstores := db.GetKVStores(
-		"test-rule", [4]byte{1, 1, 1, 1}, featureName,
+	// Create a session that we can reference.
+	sess, err := sessions.NewSession(
+		ctx, "test", session.TypeAutopilot, time.Unix(1000, 0),
+		"something",
 	)
+	require.NoError(t, err)
+
+	kvstores := db.GetKVStores("test-rule", sess.GroupID, featureName)
 
-	err := kvstores.Update(ctx, func(ctx context.Context,
+	err = kvstores.Update(ctx, func(ctx context.Context,
 		tx KVStoreTx) error {
 
 		// Set an item in the temp store.
@@ -137,7 +145,7 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
 		require.NoError(t, db.Stop())
 	})
 
-	kvstores = db.GetKVStores("test-rule", [4]byte{1, 1, 1, 1}, featureName)
+	kvstores = db.GetKVStores("test-rule", sess.GroupID, featureName)
 
 	// The temp store should no longer have the stored value but the perm
 	// store should .
@@ -164,23 +172,31 @@ func testTempAndPermStores(t *testing.T, featureSpecificStore bool) {
 func TestKVStoreNameSpaces(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
-	db := NewTestDB(t)
 
-	var (
-		groupID1 = intToSessionID(1)
-		groupID2 = intToSessionID(2)
+	sessions := session.NewTestDB(t, clock.NewDefaultClock())
+	db := NewTestDBWithSessions(t, sessions)
+
+	// Create 2 sessions that we can reference.
+	sess1, err := sessions.NewSession(
+		ctx, "test", session.TypeAutopilot, time.Unix(1000, 0), "",
 	)
+	require.NoError(t, err)
+
+	sess2, err := sessions.NewSession(
+		ctx, "test1", session.TypeAutopilot, time.Unix(1000, 0), "",
+	)
+	require.NoError(t, err)
 
 	// Two DBs for same group but different features.
-	rulesDB1 := db.GetKVStores("test-rule", groupID1, "auto-fees")
-	rulesDB2 := db.GetKVStores("test-rule", groupID1, "re-balance")
+	rulesDB1 := db.GetKVStores("test-rule", sess1.GroupID, "auto-fees")
+	rulesDB2 := db.GetKVStores("test-rule", sess1.GroupID, "re-balance")
 
 	// The third DB is for the same rule but a different group. It is
 	// for the same feature as db 2.
-	rulesDB3 := db.GetKVStores("test-rule", groupID2, "re-balance")
+	rulesDB3 := db.GetKVStores("test-rule", sess2.GroupID, "re-balance")
 
 	// Test that the three ruleDBs share the same global space.
-	err := rulesDB1.Update(ctx, func(ctx context.Context,
+	err = rulesDB1.Update(ctx, func(ctx context.Context,
 		tx KVStoreTx) error {
 
 		return tx.Global().Set(
@@ -311,9 +327,9 @@ func TestKVStoreNameSpaces(t *testing.T) {
 	// Test that the group space is shared by the first two dbs but not
 	// the third. To do this, we re-init the DB's but leave the feature
 	// names out. This way, we will access the group storage.
-	rulesDB1 = db.GetKVStores("test-rule", groupID1, "")
-	rulesDB2 = db.GetKVStores("test-rule", groupID1, "")
-	rulesDB3 = db.GetKVStores("test-rule", groupID2, "")
+	rulesDB1 = db.GetKVStores("test-rule", sess1.GroupID, "")
+	rulesDB2 = db.GetKVStores("test-rule", sess1.GroupID, "")
+	rulesDB3 = db.GetKVStores("test-rule", sess2.GroupID, "")
 
 	err = rulesDB1.Update(ctx, func(ctx context.Context,
 		tx KVStoreTx) error {
@@ -376,6 +392,81 @@ func TestKVStoreNameSpaces(t *testing.T) {
 	require.True(t, bytes.Equal(v, []byte("thing 3")))
 }
 
+// TestKVStoreSessionCoupling tests if we attempt to write to a kvstore that
+// is namespaced by a session that does not exist, then we should get an error.
+func TestKVStoreSessionCoupling(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	sessions := session.NewTestDB(t, clock.NewDefaultClock())
+	db := NewTestDBWithSessions(t, sessions)
+
+	// Get a kvstore namespaced by a session ID for a session that does
+	// not exist.
+	store := db.GetKVStores("AutoFees", [4]byte{1, 1, 1, 1}, "auto-fees")
+
+	err := store.Update(ctx, func(ctx context.Context,
+		tx KVStoreTx) error {
+
+		// First, show that any call to the global namespace will not
+		// error since it is not namespaced by a session.
+		res, err := tx.Global().Get(ctx, "foo")
+		require.NoError(t, err)
+		require.Nil(t, res)
+
+		err = tx.Global().Set(ctx, "foo", []byte("bar"))
+		require.NoError(t, err)
+
+		res, err = tx.Global().Get(ctx, "foo")
+		require.NoError(t, err)
+		require.Equal(t, []byte("bar"), res)
+
+		// Now we switch to the local store. We don't expect the Get
+		// call to error since it should just return a nil value for
+		// key that has not been set.
+		_, err = tx.Local().Get(ctx, "foo")
+		require.NoError(t, err)
+
+		// For Set, we expect an error since the session does not exist.
+		err = tx.Local().Set(ctx, "foo", []byte("bar"))
+		require.ErrorIs(t, err, session.ErrUnknownGroup)
+
+		// We again don't expect the error for delete since we just
+		// expect it to return nil if the key is not found.
+		err = tx.Local().Del(ctx, "foo")
+		require.NoError(t, err)
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	// Now, go and create a sessions in the session DB.
+	sess, err := sessions.NewSession(
+		ctx, "test", session.TypeAutopilot, time.Unix(1000, 0),
+		"something",
+	)
+	require.NoError(t, err)
+
+	// Get a kvstore namespaced by a session ID for a session that now
+	// does exist.
+	store = db.GetKVStores("AutoFees", sess.GroupID, "auto-fees")
+
+	// Now, repeat the "Set" call for this session's kvstore to
+	// show that it no longer errors.
+	err = store.Update(ctx, func(ctx context.Context, tx KVStoreTx) error {
+		// For Set, we expect an error since the session does not exist.
+		err = tx.Local().Set(ctx, "foo", []byte("bar"))
+		require.NoError(t, err)
+
+		res, err := tx.Local().Get(ctx, "foo")
+		require.NoError(t, err)
+		require.Equal(t, []byte("bar"), res)
+
+		return nil
+	})
+	require.NoError(t, err)
+}
+
 func intToSessionID(i uint32) session.ID {
 	var id session.ID
 	byteOrder.PutUint32(id[:], i)

firewalldb/test_kvdb.go

@@ -3,6 +3,7 @@ package firewalldb
 import (
 	"testing"
 
+	"github.com/lightninglabs/lightning-terminal/session"
 	"github.com/stretchr/testify/require"
 )
 
@@ -14,7 +15,19 @@ func NewTestDB(t *testing.T) *BoltDB {
 // NewTestDBFromPath is a helper function that creates a new BoltStore with a
 // connection to an existing BBolt database for testing.
 func NewTestDBFromPath(t *testing.T, dbPath string) *BoltDB {
-	store, err := NewBoltDB(dbPath, DBFilename, nil)
+	return newDBFromPathWithSessions(t, dbPath, nil)
+}
+
+// NewTestDBWithSessions creates a new test BoltDB Store with access to an
+// existing sessions DB.
+func NewTestDBWithSessions(t *testing.T, sessStore session.Store) *BoltDB {
+	return newDBFromPathWithSessions(t, t.TempDir(), sessStore)
+}
+
+func newDBFromPathWithSessions(t *testing.T, dbPath string,
+	sessStore session.Store) *BoltDB {
+
+	store, err := NewBoltDB(dbPath, DBFilename, sessStore)
 	require.NoError(t, err)
 
 	t.Cleanup(func() {