Merge pull request #969 from ellemouton/sql11Sessions3

[sql-11] sessions: interface massage

session/interface.go

@@ -71,11 +71,12 @@ type Session struct {
 	GroupID ID
 }
 
-// NewSession creates a new session with the given user-defined parameters.
-func NewSession(id ID, localPrivKey *btcec.PrivateKey, label string, typ Type,
-	expiry time.Time, serverAddr string, devServer bool, perms []bakery.Op,
-	caveats []macaroon.Caveat, featureConfig FeaturesConfig,
-	privacy bool, linkedGroupID *ID, flags PrivacyFlags) (*Session, error) {
+// buildSession creates a new session with the given user-defined parameters.
+func buildSession(id ID, localPrivKey *btcec.PrivateKey, label string, typ Type,
+	created, expiry time.Time, serverAddr string, devServer bool,
+	perms []bakery.Op, caveats []macaroon.Caveat,
+	featureConfig FeaturesConfig, privacy bool, linkedGroupID *ID,
+	flags PrivacyFlags) (*Session, error) {
 
 	_, pairingSecret, err := mailbox.NewPassphraseEntropy()
 	if err != nil {
@@ -98,8 +99,8 @@ func NewSession(id ID, localPrivKey *btcec.PrivateKey, label string, typ Type,
 		Label:             label,
 		State:             StateCreated,
 		Type:              typ,
-		Expiry:            expiry,
-		CreatedAt:         time.Now(),
+		Expiry:            expiry.UTC(),
+		CreatedAt:         created.UTC(),
 		ServerAddr:        serverAddr,
 		DevServer:         devServer,
 		MacaroonRootKey:   macRootKey,
@@ -139,6 +140,18 @@ type IDToGroupIndex interface {
 // Store is the interface a persistent storage must implement for storing and
 // retrieving Terminal Connect sessions.
 type Store interface {
+	// NewSession creates a new session with the given user-defined
+	// parameters.
+	//
+	// NOTE: currently this purely a constructor of the Session type and
+	// does not make any database calls. This will be changed in a future
+	// commit.
+	NewSession(id ID, localPrivKey *btcec.PrivateKey, label string,
+		typ Type, expiry time.Time, serverAddr string, devServer bool,
+		perms []bakery.Op, caveats []macaroon.Caveat,
+		featureConfig FeaturesConfig, privacy bool, linkedGroupID *ID,
+		flags PrivacyFlags) (*Session, error)
+
 	// CreateSession adds a new session to the store. If a session with the
 	// same local public key already exists an error is returned. This
 	// can only be called with a Session with an ID that the Store has

session/kvdb_store.go

@@ -10,7 +10,10 @@ import (
 	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
+	"github.com/lightningnetwork/lnd/clock"
 	"go.etcd.io/bbolt"
+	"gopkg.in/macaroon-bakery.v2/bakery"
+	"gopkg.in/macaroon.v2"
 )
 
 var (
@@ -77,13 +80,15 @@ const (
 // BoltStore is a bolt-backed persistent store.
 type BoltStore struct {
 	*bbolt.DB
+
+	clock clock.Clock
 }
 
 // A compile-time check to ensure that BoltStore implements the Store interface.
 var _ Store = (*BoltStore)(nil)
 
 // NewDB creates a new bolt database that can be found at the given directory.
-func NewDB(dir, fileName string) (*BoltStore, error) {
+func NewDB(dir, fileName string, clock clock.Clock) (*BoltStore, error) {
 	firstInit := false
 	path := filepath.Join(dir, fileName)
 
@@ -106,7 +111,10 @@ func NewDB(dir, fileName string) (*BoltStore, error) {
 		return nil, err
 	}
 
-	return &BoltStore{DB: db}, nil
+	return &BoltStore{
+		DB:    db,
+		clock: clock,
+	}, nil
 }
 
 // fileExists reports whether the named file or directory exists.
@@ -173,6 +181,25 @@ func getSessionKey(session *Session) []byte {
 	return session.LocalPublicKey.SerializeCompressed()
 }
 
+// NewSession creates a new session with the given user-defined parameters.
+//
+// NOTE: currently this purely a constructor of the Session type and does not
+// make any database calls. This will be changed in a future commit.
+//
+// NOTE: this is part of the Store interface.
+func (db *BoltStore) NewSession(id ID, localPrivKey *btcec.PrivateKey,
+	label string, typ Type, expiry time.Time, serverAddr string,
+	devServer bool, perms []bakery.Op, caveats []macaroon.Caveat,
+	featureConfig FeaturesConfig, privacy bool, linkedGroupID *ID,
+	flags PrivacyFlags) (*Session, error) {
+
+	return buildSession(
+		id, localPrivKey, label, typ, db.clock.Now(), expiry,
+		serverAddr, devServer, perms, caveats, featureConfig, privacy,
+		linkedGroupID, flags,
+	)
+}
+
 // CreateSession adds a new session to the store. If a session with the same
 // local public key already exists an error is returned.
 //
@@ -398,7 +425,7 @@ func (db *BoltStore) RevokeSession(key *btcec.PublicKey) error {
 		}
 
 		session.State = StateRevoked
-		session.RevokedAt = time.Now()
+		session.RevokedAt = db.clock.Now().UTC()
 
 		var buf bytes.Buffer
 		if err := SerializeSession(&buf, session); err != nil {

session/store_test.go

@@ -6,24 +6,28 @@ import (
 	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/stretchr/testify/require"
 )
 
+var testTime = time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+
 // TestBasicSessionStore tests the basic getters and setters of the session
 // store.
 func TestBasicSessionStore(t *testing.T) {
 	// Set up a new DB.
-	db, err := NewDB(t.TempDir(), "test.db")
+	clock := clock.NewTestClock(testTime)
+	db, err := NewDB(t.TempDir(), "test.db", clock)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = db.Close()
 	})
 
 	// Create a few sessions.
-	s1 := newSession(t, db, "session 1", nil)
-	s2 := newSession(t, db, "session 2", nil)
-	s3 := newSession(t, db, "session 3", nil)
-	s4 := newSession(t, db, "session 4", nil)
+	s1 := newSession(t, db, clock, "session 1", nil)
+	s2 := newSession(t, db, clock, "session 2", nil)
+	s3 := newSession(t, db, clock, "session 3", nil)
+	s4 := newSession(t, db, clock, "session 4", nil)
 
 	// Persist session 1. This should now succeed.
 	require.NoError(t, db.CreateSession(s1))
@@ -51,11 +55,11 @@ func TestBasicSessionStore(t *testing.T) {
 	for _, s := range []*Session{s1, s2, s3} {
 		session, err := db.GetSession(s.LocalPublicKey)
 		require.NoError(t, err)
-		require.Equal(t, s.Label, session.Label)
+		assertEqualSessions(t, s, session)
 
 		session, err = db.GetSessionByID(s.ID)
 		require.NoError(t, err)
-		require.Equal(t, s.Label, session.Label)
+		assertEqualSessions(t, s, session)
 	}
 
 	// Fetch session 1 and assert that it currently has no remote pub key.
@@ -89,17 +93,18 @@ func TestBasicSessionStore(t *testing.T) {
 // TestLinkingSessions tests that session linking works as expected.
 func TestLinkingSessions(t *testing.T) {
 	// Set up a new DB.
-	db, err := NewDB(t.TempDir(), "test.db")
+	clock := clock.NewTestClock(testTime)
+	db, err := NewDB(t.TempDir(), "test.db", clock)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = db.Close()
 	})
 
 	// Create a new session with no previous link.
-	s1 := newSession(t, db, "session 1", nil)
+	s1 := newSession(t, db, clock, "session 1", nil)
 
 	// Create another session and link it to the first.
-	s2 := newSession(t, db, "session 2", &s1.GroupID)
+	s2 := newSession(t, db, clock, "session 2", &s1.GroupID)
 
 	// Try to persist the second session and assert that it fails due to the
 	// linked session not existing in the DB yet.
@@ -125,7 +130,8 @@ func TestLinkingSessions(t *testing.T) {
 // of the GetGroupID and GetSessionIDs methods.
 func TestLinkedSessions(t *testing.T) {
 	// Set up a new DB.
-	db, err := NewDB(t.TempDir(), "test.db")
+	clock := clock.NewTestClock(testTime)
+	db, err := NewDB(t.TempDir(), "test.db", clock)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = db.Close()
@@ -135,9 +141,9 @@ func TestLinkedSessions(t *testing.T) {
 	// after are all linked to the prior one. All these sessions belong to
 	// the same group. The group ID is equivalent to the session ID of the
 	// first session.
-	s1 := newSession(t, db, "session 1", nil)
-	s2 := newSession(t, db, "session 2", &s1.GroupID)
-	s3 := newSession(t, db, "session 3", &s2.GroupID)
+	s1 := newSession(t, db, clock, "session 1", nil)
+	s2 := newSession(t, db, clock, "session 2", &s1.GroupID)
+	s3 := newSession(t, db, clock, "session 3", &s2.GroupID)
 
 	// Persist the sessions.
 	require.NoError(t, db.CreateSession(s1))
@@ -163,8 +169,8 @@ func TestLinkedSessions(t *testing.T) {
 
 	// To ensure that different groups don't interfere with each other,
 	// let's add another set of linked sessions not linked to the first.
-	s4 := newSession(t, db, "session 4", nil)
-	s5 := newSession(t, db, "session 5", &s4.GroupID)
+	s4 := newSession(t, db, clock, "session 4", nil)
+	s5 := newSession(t, db, clock, "session 5", &s4.GroupID)
 
 	require.NotEqual(t, s4.GroupID, s1.GroupID)
 
@@ -192,7 +198,8 @@ func TestLinkedSessions(t *testing.T) {
 // method correctly checks if each session in a group passes a predicate.
 func TestCheckSessionGroupPredicate(t *testing.T) {
 	// Set up a new DB.
-	db, err := NewDB(t.TempDir(), "test.db")
+	clock := clock.NewTestClock(testTime)
+	db, err := NewDB(t.TempDir(), "test.db", clock)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = db.Close()
@@ -202,7 +209,7 @@ func TestCheckSessionGroupPredicate(t *testing.T) {
 	// function is checked correctly.
 
 	// Add a new session to the DB.
-	s1 := newSession(t, db, "label 1", nil)
+	s1 := newSession(t, db, clock, "label 1", nil)
 	require.NoError(t, db.CreateSession(s1))
 
 	// Check that the group passes against an appropriate predicate.
@@ -227,7 +234,7 @@ func TestCheckSessionGroupPredicate(t *testing.T) {
 	require.NoError(t, db.RevokeSession(s1.LocalPublicKey))
 
 	// Add a new session to the same group as the first one.
-	s2 := newSession(t, db, "label 2", &s1.GroupID)
+	s2 := newSession(t, db, clock, "label 2", &s1.GroupID)
 	require.NoError(t, db.CreateSession(s2))
 
 	// Check that the group passes against an appropriate predicate.
@@ -249,7 +256,7 @@ func TestCheckSessionGroupPredicate(t *testing.T) {
 	require.False(t, ok)
 
 	// Add a new session that is not linked to the first one.
-	s3 := newSession(t, db, "completely different", nil)
+	s3 := newSession(t, db, clock, "completely different", nil)
 	require.NoError(t, db.CreateSession(s3))
 
 	// Ensure that the first group is unaffected.
@@ -279,14 +286,15 @@ func TestCheckSessionGroupPredicate(t *testing.T) {
 	require.True(t, ok)
 }
 
-func newSession(t *testing.T, db Store, label string,
+func newSession(t *testing.T, db Store, clock clock.Clock, label string,
 	linkedGroupID *ID) *Session {
 
 	id, priv, err := db.GetUnusedIDAndKeyPair()
 	require.NoError(t, err)
 
-	session, err := NewSession(
+	session, err := buildSession(
 		id, priv, label, TypeMacaroonAdmin,
+		clock.Now(),
 		time.Date(99999, 1, 1, 0, 0, 0, 0, time.UTC),
 		"foo.bar.baz:1234", true, nil, nil, nil, true, linkedGroupID,
 		[]PrivacyFlag{ClearPubkeys},
@@ -295,3 +303,32 @@ func newSession(t *testing.T, db Store, label string,
 
 	return session
 }
+
+func assertEqualSessions(t *testing.T, expected, actual *Session) {
+	expectedExpiry := expected.Expiry
+	actualExpiry := actual.Expiry
+	expectedRevoked := expected.RevokedAt
+	actualRevoked := actual.RevokedAt
+	expectedCreated := expected.CreatedAt
+	actualCreated := actual.CreatedAt
+
+	expected.Expiry = time.Time{}
+	expected.RevokedAt = time.Time{}
+	expected.CreatedAt = time.Time{}
+	actual.Expiry = time.Time{}
+	actual.RevokedAt = time.Time{}
+	actual.CreatedAt = time.Time{}
+
+	require.Equal(t, expected, actual)
+	require.Equal(t, expectedExpiry.Unix(), actualExpiry.Unix())
+	require.Equal(t, expectedRevoked.Unix(), actualRevoked.Unix())
+	require.Equal(t, expectedCreated.Unix(), actualCreated.Unix())
+
+	// Restore the old values to not influence the tests.
+	expected.Expiry = expectedExpiry
+	expected.RevokedAt = expectedRevoked
+	expected.CreatedAt = expectedCreated
+	actual.Expiry = actualExpiry
+	actual.RevokedAt = actualRevoked
+	actual.CreatedAt = actualCreated
+}

session/tlv.go

@@ -237,8 +237,8 @@ func DeserializeSession(r io.Reader) (*Session, error) {
 	session.Label = string(label)
 	session.State = State(state)
 	session.Type = Type(typ)
-	session.Expiry = time.Unix(int64(expiry), 0)
-	session.CreatedAt = time.Unix(int64(createdAt), 0)
+	session.Expiry = time.Unix(int64(expiry), 0).UTC()
+	session.CreatedAt = time.Unix(int64(createdAt), 0).UTC()
 	session.ServerAddr = string(serverAddr)
 	session.DevServer = devServer == 1
 	session.WithPrivacyMapper = privacy == 1
@@ -248,7 +248,7 @@ func DeserializeSession(r io.Reader) (*Session, error) {
 	}
 
 	if revokedAt != 0 {
-		session.RevokedAt = time.Unix(int64(revokedAt), 0)
+		session.RevokedAt = time.Unix(int64(revokedAt), 0).UTC()
 	}
 
 	if t, ok := parsedTypes[typeMacaroonRecipe]; ok && t == nil {

session/tlv_test.go

@@ -129,8 +129,9 @@ func TestSerializeDeserializeSession(t *testing.T) {
 			priv, id, err := NewSessionPrivKeyAndID()
 			require.NoError(t, err)
 
-			session, err := NewSession(
+			session, err := buildSession(
 				id, priv, test.name, test.sessType,
+				time.Now(),
 				time.Date(99999, 1, 1, 0, 0, 0, 0, time.UTC),
 				"foo.bar.baz:1234", true, test.perms,
 				test.caveats, test.featureConfig, true,
@@ -183,8 +184,9 @@ func TestGroupIDForOlderSessions(t *testing.T) {
 	priv, id, err := NewSessionPrivKeyAndID()
 	require.NoError(t, err)
 
-	session, err := NewSession(
+	session, err := buildSession(
 		id, priv, "test-session", TypeMacaroonAdmin,
+		time.Now(),
 		time.Date(99999, 1, 1, 0, 0, 0, 0, time.UTC),
 		"foo.bar.baz:1234", true, nil, nil, nil, false, nil,
 		PrivacyFlags{},
@@ -218,8 +220,9 @@ func TestGroupID(t *testing.T) {
 	require.NoError(t, err)
 
 	// Create session 1 which is not linked to any previous session.
-	session1, err := NewSession(
+	session1, err := buildSession(
 		id, priv, "test-session", TypeMacaroonAdmin,
+		time.Now(),
 		time.Date(99999, 1, 1, 0, 0, 0, 0, time.UTC),
 		"foo.bar.baz:1234", true, nil, nil, nil, false, nil,
 		PrivacyFlags{},
@@ -232,8 +235,9 @@ func TestGroupID(t *testing.T) {
 	// Create session 2 and link it to session 1.
 	priv, id, err = NewSessionPrivKeyAndID()
 	require.NoError(t, err)
-	session2, err := NewSession(
+	session2, err := buildSession(
 		id, priv, "test-session", TypeMacaroonAdmin,
+		time.Now(),
 		time.Date(99999, 1, 1, 0, 0, 0, 0, time.UTC),
 		"foo.bar.baz:1234", true, nil, nil, nil, false,
 		&session1.GroupID, PrivacyFlags{},

session_rpcserver.go

@@ -318,7 +318,7 @@ func (s *sessionRpcServer) AddSession(ctx context.Context,
 		return nil, err
 	}
 
-	sess, err := session.NewSession(
+	sess, err := s.cfg.db.NewSession(
 		id, localPrivKey, req.Label, typ, expiry, req.MailboxServerAddr,
 		req.DevServer, uniquePermissions, caveats, nil, false, nil,
 		session.PrivacyFlags{},
@@ -1148,7 +1148,7 @@ func (s *sessionRpcServer) AddAutopilotSession(ctx context.Context,
 		return nil, err
 	}
 
-	sess, err := session.NewSession(
+	sess, err := s.cfg.db.NewSession(
 		id, localPrivKey, req.Label, session.TypeAutopilot, expiry,
 		req.MailboxServerAddr, req.DevServer, perms, caveats,
 		clientConfig, privacy, linkedGroupID, privacyFlags,