libsecret: Keys() unlocks collection first

cmd/keyring/main.go

@@ -17,7 +17,7 @@ func main() {
 	listBackends := flag.Bool("list-backends", false, "Whether to list backends")
 
 	// actions to take
-	actionListKeys := flag.Bool("list-keys", false, "Whether to list backends")
+	actionListKeys := flag.Bool("list-keys", false, "Whether to list keys")
 	actionSetValue := flag.String("set", "", "The value to set")
 
 	// keychain

libsecret.go

@@ -171,25 +171,17 @@ func (k *secretsKeyring) Set(item Item) error {
 		k.collection = collection
 	}
 
-	// create the new item
-	data, err := json.Marshal(item)
-	if err != nil {
+	if err := k.ensureCollectionUnlocked(); err != nil {
 		return err
 	}
 
-	secret := libsecret.NewSecret(k.session, []byte{}, data, "application/json")
-
-	// unlock the collection first
-	locked, err := k.collection.Locked()
+	// create the new item
+	data, err := json.Marshal(item)
 	if err != nil {
 		return err
 	}
 
-	if locked {
-		if err := k.service.Unlock(k.collection); err != nil {
-			return err
-		}
-	}
+	secret := libsecret.NewSecret(k.session, []byte{}, data, "application/json")
 
 	if _, err := k.collection.CreateItem(item.Key, secret, true); err != nil {
 		return err
@@ -243,23 +235,24 @@ func (k *secretsKeyring) Keys() ([]string, error) {
 		if err == errCollectionNotFound {
 			return []string{}, nil
 		}
-		return []string{}, err
+		return nil, err
+	}
+	if err := k.ensureCollectionUnlocked(); err != nil {
+		return nil, err
 	}
-
 	items, err := k.collection.Items()
 	if err != nil {
-		return []string{}, err
+		return nil, err
 	}
-
 	keys := []string{}
-
 	for _, item := range items {
 		label, err := item.Label()
 		if err == nil {
 			keys = append(keys, label)
+		} else {
+			// err is being silently ignored here, not sure if that's good or bad
 		}
 	}
-
 	return keys, nil
 }
 
@@ -270,3 +263,15 @@ func (k *secretsKeyring) deleteCollection() error {
 	}
 	return k.collection.Delete()
 }
+
+// unlock the collection if it's locked
+func (k *secretsKeyring) ensureCollectionUnlocked() error {
+	locked, err := k.collection.Locked()
+	if err != nil {
+		return err
+	}
+	if !locked {
+		return nil
+	}
+	return k.service.Unlock(k.collection)
+}