Address @jkroll-cf's feedback on keycache interface.

+ persistLabels moved from cryptor to persist package global.
+ Restore now explicitly checks for the case where there aren't enough
  shares to return `ErrRestoreDelegations`.
+ The users responsible for restoring the cache are now logged.

cryptor/cryptor.go

@@ -12,8 +12,10 @@ import (
 	"crypto/sha1"
 	"encoding/json"
 	"errors"
+	"log"
 	"sort"
 	"strconv"
+	"strings"
 
 	"github.com/cloudflare/redoctober/config"
 	"github.com/cloudflare/redoctober/keycache"
@@ -705,8 +707,6 @@ func (c *Cryptor) DelegateStatus(name string, labels, admins []string) (adminsDe
 	return c.cache.DelegateStatus(name, labels, admins)
 }
 
-var persistLabels = []string{"restore"}
-
 // store serialises the key cache, encrypts it, and writes it to disk.
 func (c *Cryptor) store() error {
 	// If the store isn't currently active, we shouldn't attempt
@@ -726,7 +726,7 @@ func (c *Cryptor) store() error {
 		Predicate: c.persist.Policy(),
 	}
 
-	cache, err = c.Encrypt(cache, persistLabels, access)
+	cache, err = c.Encrypt(cache, persist.Labels, access)
 	if err != nil {
 		return err
 	}
@@ -747,19 +747,25 @@ func (c *Cryptor) Restore(name, password string, uses int, slot, durationString
 		return errors.New("Missing user on disk")
 	}
 
-	err := c.persist.Delegate(record, name, password, c.persist.Users(), persistLabels, uses, slot, durationString)
+	err := c.persist.Delegate(record, name, password, c.persist.Users(), persist.Labels, uses, slot, durationString)
 	if err != nil {
 		return err
 	}
 
-	// A failure to decrypt isn't an error, it just means there
-	// aren't enough delegations yet; the sentinal value
-	// ErrRestoreDelegations is returned to indicate this.
-	cache, _, _, _, err := c.decrypt(c.persist.Cache(), c.persist.Blob(), name)
+	// A failure to decrypt isn't a restore error, it (most often)
+	// just means there aren't enough delegations yet; the
+	// sentinal value ErrRestoreDelegations is returned to
+	// indicate this. However, the error
+	cache, _, names, _, err := c.decrypt(c.persist.Cache(), c.persist.Blob(), name)
 	if err != nil {
-		return ErrRestoreDelegations
+		if err == msp.ErrNotEnoughShares {
+			return ErrRestoreDelegations
+		}
+		return err
 	}
 
+	log.Printf("cryptor.restore success: names=%s", strings.Join(names, ","))
+
 	var uk map[string]keycache.ActiveUser
 	err = json.Unmarshal(cache, &uk)
 	if err != nil {

cryptor/cryptor_test.go

@@ -293,6 +293,11 @@ func TestRestore(t *testing.T) {
 			persist.Inactive, status.State)
 	}
 
+	err = c.Restore("Carl", "weakpassword", 0, "", "0h")
+	if err != ErrRestoreDelegations {
+		t.Fatal(err)
+	}
+
 	err = c.Restore("Bob", "weakpassword", 2, "", "1h")
 	if err != nil {
 		t.Fatal(err)

msp/msp.go

@@ -211,6 +211,10 @@ func (m MSP) DistributeShares(sec []byte, db UserDatabase) (map[string][][]byte,
 	return out, nil
 }
 
+// ErrNotEnoughShares is returned if there aren't enough shares to
+// decrypt the secret.
+var ErrNotEnoughShares = errors.New("Not enough shares to recover.")
+
 // RecoverSecret takes a user database storing secret shares as input and returns the original secret.
 func (m MSP) RecoverSecret(db UserDatabase) ([]byte, error) {
 	cache := make(map[string][][]byte, 0) // Caches un-used shares for a user.
@@ -225,7 +229,7 @@ func (m MSP) recoverSecret(db UserDatabase, cache map[string][][]byte) ([]byte,
 
 	ok, names, locs, _ := m.DerivePath(db)
 	if !ok {
-		return nil, errors.New("Not enough shares to recover.")
+		return nil, ErrNotEnoughShares
 	}
 
 	for _, name := range names {

persist/persist.go

@@ -13,9 +13,12 @@ import (
 
 var defaultStore Store = &File{}
 
+// Labels are the labels that the keycache should be encrypted with.
+var Labels = []string{"restore"}
+
 const (
-	// NeverPersist indicates that the persistence store will
-	// never persist active delegations.
+	// Disabled indicates that the persistence store will never
+	// persist active delegations.
 	Disabled = "disabled"
 
 	// Inactive indicates that the persistence store requires
@@ -52,6 +55,7 @@ type Store interface {
 	Cache() *keycache.Cache
 }
 
+// FileMechanism indicates that the persistence mechanism is a file.
 const FileMechanism = "file"
 
 type mechanism func(*config.Delegations) (Store, error)
@@ -61,6 +65,8 @@ var stores = map[string]mechanism{
 	FileMechanism: newFile,
 }
 
+// New attempts to create a new persistence store from the
+// configuration.
 func New(config *config.Delegations) (Store, error) {
 	if config == nil {
 		return nil, errors.New("persist: nil configuration")
@@ -78,4 +84,6 @@ func New(config *config.Delegations) (Store, error) {
 	return constructor(config)
 }
 
+// ErrInvalidConfig is returned when the configuration is invalid for
+// the type of persistence store in use.
 var ErrInvalidConfig = errors.New("persist: invalid configuration")