session: introduce Reserve->Create pattern

TODO: flesh out commit message

Author: ellemouton

session/interface.go

@@ -27,15 +27,15 @@ const (
 type State uint8
 
 /*
-		/---> StateExpired
-StateCreated ---
-       		\---> StateRevoked
+				   /---> StateExpired
+StateReserved ---> StateCreated ---
+       				   \---> StateRevoked
 */
 
 const (
 	// StateCreated is the state of a session once it has been fully
-	// committed to the Store and is ready to be used. This is the first
-	// state of a session.
+	// committed to the BoltStore and is ready to be used. This is the
+	// first state after StateReserved.
 	StateCreated State = 0
 
 	// StateInUse is the state of a session that is currently being used.
@@ -52,10 +52,10 @@ const (
 	// date.
 	StateExpired State = 3
 
-	// StateReserved is a temporary initial state of a session. On start-up,
-	// any sessions in this state should be cleaned up.
-	//
-	// NOTE: this isn't used yet.
+	// StateReserved is a temporary initial state of a session. This is used
+	// to reserve a unique ID and private key pair for a session before it
+	// is fully created. On start-up, any sessions in this state should be
+	// cleaned up.
 	StateReserved State = 4
 )
 
@@ -123,7 +123,7 @@ func buildSession(id ID, localPrivKey *btcec.PrivateKey, label string, typ Type,
 	sess := &Session{
 		ID:                id,
 		Label:             label,
-		State:             StateCreated,
+		State:             StateReserved,
 		Type:              typ,
 		Expiry:            expiry.UTC(),
 		CreatedAt:         created.UTC(),
@@ -167,22 +167,16 @@ type IDToGroupIndex interface {
 // 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,
+	// parameters. The session will remain in the StateReserved state until
+	// CreateSession is called for the session.
+	NewSession(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
-	// reserved.
-	CreateSession(*Session) error
+	// CreateSession moves the given session from the StateReserved state to
+	// the StateCreated state.
+	CreateSession(ID) (*Session, error)
 
 	// GetSession fetches the session with the given key.
 	GetSession(key *btcec.PublicKey) (*Session, error)
@@ -206,12 +200,6 @@ type Store interface {
 	UpdateSessionRemotePubKey(localPubKey,
 		remotePubKey *btcec.PublicKey) error
 
-	// GetUnusedIDAndKeyPair can be used to generate a new, unused, local
-	// private key and session ID pair. Care must be taken to ensure that no
-	// other thread calls this before the returned ID and key pair from this
-	// method are either used or discarded.
-	GetUnusedIDAndKeyPair() (ID, *btcec.PrivateKey, error)
-
 	// GetSessionByID fetches the session with the given ID.
 	GetSessionByID(id ID) (*Session, error)
 

session/kvdb_store.go

@@ -182,41 +182,39 @@ 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.
+// NewSession creates and persists a new session with the given user-defined
+// parameters. The initial state of the session will be Reserved until
+// CreateSession is called.
 //
 // 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,
-	)
-}
+func (db *BoltStore) NewSession(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.
-//
-// NOTE: this is part of the Store interface.
-func (db *BoltStore) CreateSession(session *Session) error {
-	sessionKey := getSessionKey(session)
-
-	return db.Update(func(tx *bbolt.Tx) error {
+	var session *Session
+	err := db.Update(func(tx *bbolt.Tx) error {
 		sessionBucket, err := getBucket(tx, sessionBucketKey)
 		if err != nil {
 			return err
 		}
 
+		id, localPrivKey, err := getUnusedIDAndKeyPair(sessionBucket)
+		if err != nil {
+			return err
+		}
+
+		session, err = buildSession(
+			id, localPrivKey, label, typ, db.clock.Now(), expiry,
+			serverAddr, devServer, perms, caveats, featureConfig,
+			privacy, linkedGroupID, flags,
+		)
+
+		sessionKey := getSessionKey(session)
+
 		if len(sessionBucket.Get(sessionKey)) != 0 {
-			return fmt.Errorf("session with local public "+
-				"key(%x) already exists",
+			return fmt.Errorf("session with local public key(%x) "+
+				"already exists",
 				session.LocalPublicKey.SerializeCompressed())
 		}
 
@@ -275,6 +273,46 @@ func (db *BoltStore) CreateSession(session *Session) error {
 
 		return putSession(sessionBucket, session)
 	})
+	if err != nil {
+		return nil, err
+	}
+
+	return session, nil
+}
+
+// CreateSession moves the session with the given ID from the Reserved state to
+// the Created state.
+//
+// NOTE: this is part of the Store interface.
+func (db *BoltStore) CreateSession(id ID) (*Session, error) {
+	var session *Session
+	err := db.Update(func(tx *bbolt.Tx) error {
+		sessionBucket, err := getBucket(tx, sessionBucketKey)
+		if err != nil {
+			return err
+		}
+
+		session, err = getSessionByID(sessionBucket, id)
+		if err != nil {
+			return err
+		}
+
+		// The session MUST be in the Reserved state.
+		if session.State != StateReserved {
+			return fmt.Errorf("session must be in the Reserved " +
+				"state for it to move to the Created state")
+		}
+
+		// Move the session to the CreatedState.
+		session.State = StateCreated
+
+		return putSession(sessionBucket, session)
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return session, nil
 }
 
 // UpdateSessionRemotePubKey can be used to add the given remote pub key
@@ -565,53 +603,35 @@ func (db *BoltStore) GetSessionByID(id ID) (*Session, error) {
 	return session, nil
 }
 
-// GetUnusedIDAndKeyPair can be used to generate a new, unused, local private
+// getUnusedIDAndKeyPair can be used to generate a new, unused, local private
 // key and session ID pair. Care must be taken to ensure that no other thread
 // calls this before the returned ID and key pair from this method are either
 // used or discarded.
-//
-// NOTE: this is part of the Store interface.
-func (db *BoltStore) GetUnusedIDAndKeyPair() (ID, *btcec.PrivateKey, error) {
-	var (
-		id      ID
-		privKey *btcec.PrivateKey
-	)
-	err := db.Update(func(tx *bbolt.Tx) error {
-		sessionBucket, err := getBucket(tx, sessionBucketKey)
-		if err != nil {
-			return err
-		}
-
-		idIndexBkt := sessionBucket.Bucket(idIndexKey)
-		if idIndexBkt == nil {
-			return ErrDBInitErr
-		}
+func getUnusedIDAndKeyPair(bucket *bbolt.Bucket) (ID, *btcec.PrivateKey,
+	error) {
 
-		// Spin until we find a key with an ID that does not collide
-		// with any of our existing IDs.
-		for {
-			// Generate a new private key and ID pair.
-			privKey, id, err = NewSessionPrivKeyAndID()
-			if err != nil {
-				return err
-			}
+	idIndexBkt := bucket.Bucket(idIndexKey)
+	if idIndexBkt == nil {
+		return ID{}, nil, ErrDBInitErr
+	}
 
-			// Check that no such ID exits in our id-to-key index.
-			idBkt := idIndexBkt.Bucket(id[:])
-			if idBkt != nil {
-				continue
-			}
+	// Spin until we find a key with an ID that does not collide with any of
+	// our existing IDs.
+	for {
+		// Generate a new private key and ID pair.
+		privKey, id, err := NewSessionPrivKeyAndID()
+		if err != nil {
+			return ID{}, nil, err
+		}
 
-			break
+		// Check that no such ID exits in our id-to-key index.
+		idBkt := idIndexBkt.Bucket(id[:])
+		if idBkt != nil {
+			continue
 		}
 
-		return nil
-	})
-	if err != nil {
-		return id, nil, err
+		return id, privKey, nil
 	}
-
-	return id, privKey, nil
 }
 
 // GetGroupID will return the group ID for the given session ID.