Adding support for multiple same keys (#1741)

RELEASE_NOTES=[ENHANCEMENT] KV secrets are now key-values, supporting multiple same key with different values

Fixes #1576

Signed-off-by: Yolan Romailler <yolan@romailler.ch>

internal/action/binary.go

@@ -139,21 +139,21 @@ func (s *Action) binaryCopy(ctx context.Context, c *cli.Context, from, to string
 	switch {
 	case fsutil.IsFile(from) && fsutil.IsFile(to):
 		// copying from on file to another file is not supported
-		return errors.New("ambiquity detected. Only from or to can be a file")
+		return errors.New("ambiguity detected. Only from or to can be a file")
 	case s.Store.Exists(ctx, from) && s.Store.Exists(ctx, to):
 		// copying from one secret to another secret is not supported
-		return errors.New("ambiquity detected. Either from or to must be a file")
+		return errors.New("ambiguity detected. Either from or to must be a file")
 	case fsutil.IsFile(from) && !fsutil.IsFile(to):
 		return s.binaryCopyFromFileToStore(ctx, from, to, deleteSource)
 	case !fsutil.IsFile(from):
 		return s.binaryCopyFromStoreToFile(ctx, from, to, deleteSource)
 	default:
-		return errors.Errorf("ambiquity detected. Unhandled case. Please report a bug")
+		return errors.Errorf("ambiguity detected. Unhandled case. Please report a bug")
 	}
 }
 
 func (s *Action) binaryCopyFromFileToStore(ctx context.Context, from, to string, deleteSource bool) error {
-	// if the source is a file the destination must no to avoid ambiquities
+	// if the source is a file the destination must no to avoid ambiguities
 	// if necessary this can be resolved by using a absolute path for the file
 	// and a relative one for the secret
 

internal/action/insert_test.go

@@ -87,7 +87,7 @@ func TestInsert(t *testing.T) {
 		ctx = ctxutil.WithShowSafeContent(ctx, true)
 		assert.NoError(t, act.insertYAML(ctx, "zab", "key", []byte("foobar"), nil))
 		assert.NoError(t, act.show(ctx, gptest.CliCtx(ctx, t), "zab", false))
-		assert.Contains(t, buf.String(), "key: foobar\npassword: ***")
+		assert.Contains(t, buf.String(), "key: foobar")
 		buf.Reset()
 	})
 
@@ -99,7 +99,7 @@ func TestInsert(t *testing.T) {
 	t.Run("insert key:value", func(t *testing.T) {
 		assert.NoError(t, act.Insert(gptest.CliCtxWithFlags(ctx, t, nil, "keyvaltest", "baz:val")))
 		assert.NoError(t, act.show(ctx, gptest.CliCtx(ctx, t), "keyvaltest", false))
-		assert.Contains(t, buf.String(), "baz: val\npassword: ****")
+		assert.Contains(t, buf.String(), "baz: val")
 		buf.Reset()
 	})
 
@@ -108,7 +108,7 @@ func TestInsert(t *testing.T) {
 		buf.Reset()
 
 		assert.NoError(t, act.show(ctx, gptest.CliCtx(ctx, t), "baz", false))
-		assert.Equal(t, "other: 83\npassword: *****\nuser: name\n", buf.String())
+		assert.Equal(t, "other: 83\nuser: name\n", buf.String())
 		buf.Reset()
 	})
 

internal/action/show.go

@@ -55,7 +55,7 @@ func (s *Action) Show(c *cli.Context) error {
 	ctx := showParseArgs(c)
 
 	if key := c.Args().Get(1); key != "" {
-		debug.Log("Setting key: %s", key)
+		debug.Log("Adding key to ctx: %s", key)
 		ctx = WithKey(ctx, key)
 	}
 
@@ -158,27 +158,31 @@ func (s *Action) showHandleOutput(ctx context.Context, name string, sec gopass.S
 		return nil
 	}
 
-	ctx = out.WithNewline(ctx, ctxutil.IsTerminal(ctx) && !strings.HasSuffix(body, "\n"))
+	ctx = out.WithNewline(ctx, ctxutil.IsTerminal(ctx))
 	if ctxutil.IsTerminal(ctx) {
-		out.Print(ctx, "Secret: %s\n\n", name)
+		header := fmt.Sprintf("Secret: %s\n", name)
+		if HasKey(ctx) {
+			header += fmt.Sprintf("Key: %s\n", GetKey(ctx))
+		}
+		out.Print(ctx, "%s", header)
 	}
-	// output the actual secret
+	// output the actual secret, newlines are handled by ctx and Print
 	out.Print(ctx, body)
-	if ctxutil.IsTerminal(ctx) {
-		out.Print(ctx, "\n")
-	}
+
 	return nil
 }
 
 func (s *Action) showGetContent(ctx context.Context, sec gopass.Secret) (string, string, error) {
 	// YAML key
 	if HasKey(ctx) && ctxutil.IsShowParsing(ctx) {
 		key := GetKey(ctx)
-		val, found := sec.Get(key)
+		values, found := sec.Values(key)
 		if !found {
 			return "", "", ExitError(ExitNotFound, store.ErrNoKey, store.ErrNoKey.Error())
 		}
-		debug.Log("got(found: %t) key %s: %s", found, key, val)
+		val := strings.Join(values, "\n")
+		//TODO: consider if we really want to have the value of the keys output in the debug logs
+		debug.Log("got(found: %t) key %s: %v", found, key, values)
 		return val, val, nil
 	} else if HasKey(ctx) {
 		out.Warning(ctx, "Parsing is disabled but a key was provided.")
@@ -203,22 +207,20 @@ func (s *Action) showGetContent(ctx context.Context, sec gopass.Secret) (string,
 		for _, k := range sec.Keys() {
 			sb.WriteString(k)
 			sb.WriteString(": ")
-			// check is this key should be obstructed
+			// check if this key should be obstructed
 			if isUnsafeKey(k, sec) {
 				debug.Log("obstructing unsafe key %s", k)
 				sb.WriteString(randAsterisk())
 			} else {
-				v, found := sec.Get(k)
+				v, found := sec.Values(k)
 				if !found {
 					continue
 				}
-				sb.WriteString(v)
+				sb.WriteString(strings.Join(v, "\n"+k+": "))
 			}
 			sb.WriteString("\n")
 		}
-		if len(sec.Keys()) > 0 && len(sec.Body()) > 0 {
-			sb.WriteString("\n")
-		}
+
 		sb.WriteString(sec.Body())
 		if IsAlsoClip(ctx) {
 			return pw, sb.String(), nil

internal/action/show_test.go

@@ -72,7 +72,8 @@ func TestShowMulti(t *testing.T) {
 
 		assert.NoError(t, act.Show(c))
 		assert.Contains(t, buf.String(), "bar: zab")
-		assert.Contains(t, buf.String(), "password: ***")
+		assert.NotContains(t, buf.String(), "password: ***")
+		assert.NotContains(t, buf.String(), "123")
 		buf.Reset()
 	})
 
@@ -124,7 +125,8 @@ func TestShowMulti(t *testing.T) {
 
 		assert.NoError(t, act.Show(c))
 		assert.Contains(t, buf.String(), "bar: zab")
-		assert.Contains(t, buf.String(), "password: ***")
+		assert.NotContains(t, buf.String(), "password: ***")
+		assert.NotContains(t, buf.String(), "123")
 		buf.Reset()
 	})
 

internal/tpl/funcs.go

@@ -30,6 +30,7 @@ const (
 	FuncGet         = "get"
 	FuncGetPassword = "getpw"
 	FuncGetValue    = "getval"
+	FuncGetValues   = "getvals"
 	FuncArgon2i     = "argon2i"
 	FuncArgon2id    = "argon2id"
 	FuncBcrypt      = "bcrypt"
@@ -169,11 +170,32 @@ func getValue(ctx context.Context, kv kvstore) func(...string) (string, error) {
 	}
 }
 
+func getValues(ctx context.Context, kv kvstore) func(...string) ([]string, error) {
+	return func(s ...string) ([]string, error) {
+		if len(s) < 2 {
+			return nil, nil
+		}
+		if kv == nil {
+			return nil, errors.Errorf("KV is nil")
+		}
+		sec, err := kv.Get(ctx, s[0])
+		if err != nil {
+			return nil, err
+		}
+		values, found := sec.Values(s[1])
+		if !found {
+			return nil, fmt.Errorf("key %q not found", s[1])
+		}
+		return values, nil
+	}
+}
+
 func funcMap(ctx context.Context, kv kvstore) template.FuncMap {
 	return template.FuncMap{
 		FuncGet:         get(ctx, kv),
 		FuncGetPassword: getPassword(ctx, kv),
 		FuncGetValue:    getValue(ctx, kv),
+		FuncGetValues:   getValues(ctx, kv),
 		FuncMd5sum:      md5sum(),
 		FuncSha1sum:     sha1sum(),
 		FuncMd5Crypt:    md5cryptFunc(),

pkg/gopass/secrets/kv.go

@@ -17,15 +17,15 @@ var _ gopass.Secret = &KV{}
 // NewKV creates a new KV secret
 func NewKV() *KV {
 	return &KV{
-		data: make(map[string]string, 10),
+		data: make(map[string][]string, 10),
 	}
 }
 
 // NewKVWithData returns a new KV secret populated with data
-func NewKVWithData(pw string, kvps map[string]string, body string, converted bool) *KV {
+func NewKVWithData(pw string, kvps map[string][]string, body string, converted bool) *KV {
 	kv := &KV{
 		password: pw,
-		data:     make(map[string]string, len(kvps)),
+		data:     make(map[string][]string, len(kvps)),
 		body:     body,
 		fromMime: converted,
 	}
@@ -72,7 +72,7 @@ func NewKVWithData(pw string, kvps map[string]string, body string, converted boo
 //   - body: "Yo\nHi"
 type KV struct {
 	password string
-	data     map[string]string
+	data     map[string][]string
 	body     string
 	fromMime bool
 }
@@ -87,10 +87,12 @@ func (k *KV) Bytes() []byte {
 		if !ok {
 			continue
 		}
-		_, _ = buf.WriteString(key)
-		_, _ = buf.WriteString(": ")
-		_, _ = buf.WriteString(sv)
-		_, _ = buf.WriteString("\n")
+		for _, v := range sv {
+			_, _ = buf.WriteString(key)
+			_, _ = buf.WriteString(": ")
+			_, _ = buf.WriteString(v)
+			_, _ = buf.WriteString("\n")
+		}
 	}
 	buf.WriteString(k.body)
 	return buf.Bytes()
@@ -102,28 +104,46 @@ func (k *KV) Keys() []string {
 	for key := range k.data {
 		keys = append(keys, key)
 	}
-	if _, found := k.data["password"]; !found {
-		keys = append(keys, "password")
-	}
 	sort.Strings(keys)
 	return keys
 }
 
-// Get returns a single key
+// Get returns the first value of that key
 func (k *KV) Get(key string) (string, bool) {
 	key = strings.ToLower(key)
+
+	if v, found := k.data[key]; found {
+		return v[0], true
+	}
+
+	return "", false
+}
+
+// Values returns all values for that key
+func (k *KV) Values(key string) ([]string, bool) {
+	key = strings.ToLower(key)
 	v, found := k.data[key]
 	return v, found
 }
 
 // Set writes a single key
 func (k *KV) Set(key string, value interface{}) error {
 	key = strings.ToLower(key)
-	k.data[key] = fmt.Sprintf("%s", value)
+	if v, ok := k.data[key]; ok && len(v) > 1 {
+		return fmt.Errorf("cannot set key %s: this entry contains multiple same keys. Please use 'gopass edit' instead", key)
+	}
+	k.data[key] = []string{fmt.Sprintf("%s", value)}
 	return nil
 }
 
-// Del removes a key
+// Add appends data to a given key
+func (k *KV) Add(key string, value interface{}) error {
+	key = strings.ToLower(key)
+	k.data[key] = append(k.data[key], fmt.Sprintf("%s", value))
+	return nil
+}
+
+// Del removes a given key and all of its values
 func (k *KV) Del(key string) bool {
 	key = strings.ToLower(key)
 	_, found := k.data[key]
@@ -149,7 +169,7 @@ func (k *KV) SetPassword(p string) {
 // ParseKV tries to parse a KV secret
 func ParseKV(in []byte) (*KV, error) {
 	k := &KV{
-		data: make(map[string]string, 10),
+		data: make(map[string][]string, 10),
 	}
 	r := bufio.NewReader(bytes.NewReader(in))
 	line, err := r.ReadString('\n')
@@ -184,14 +204,14 @@ func ParseKV(in []byte) (*KV, error) {
 		}
 		// preserve key only entries
 		if len(parts) < 2 {
-			k.data[parts[0]] = ""
+			k.data[parts[0]] = append(k.data[parts[0]], "")
 			continue
 		}
-		k.data[parts[0]] = parts[1]
+
+		k.data[parts[0]] = append(k.data[parts[0]], parts[1])
 	}
 	if len(k.data) < 1 {
 		debug.Log("no KV entries")
-		//return nil, fmt.Errorf("no KV entries")
 	}
 	k.body = sb.String()
 	return k, nil
@@ -203,7 +223,7 @@ func (k *KV) Write(buf []byte) (int, error) {
 	return len(buf), nil
 }
 
-// FromMime returns which whether this secret was converted from a Mime secret of not
+// FromMime returns whether this secret was converted from a Mime secret of not
 func (k *KV) FromMime() bool {
 	return k.fromMime
 }

pkg/gopass/secrets/kv_test.go

@@ -80,3 +80,18 @@ ab: cd`
 	v, _ := s.Get("ab")
 	assert.Equal(t, "cd", v)
 }
+
+func TestMultiKeyKVMIME(t *testing.T) {
+	in := `passw0rd
+foo: baz
+foo: bar
+zab: 123`
+	out := `passw0rd
+foo: baz
+foo: bar
+zab: 123
+`
+	sec, err := ParseKV([]byte(in))
+	require.NoError(t, err)
+	assert.Equal(t, out, string(sec.Bytes()))
+}

pkg/gopass/secrets/plain.go

@@ -53,12 +53,18 @@ func (p *Plain) Keys() []string {
 	return nil
 }
 
-// Get returns the first line (for password) or the empty string
+// Get returns the empty string for Plain secrets
 func (p *Plain) Get(key string) (string, bool) {
 	debug.Log("Trying to access key %q on a Plain secret", key)
 	return "", false
 }
 
+// Values returns the empty string for Plain secrets
+func (p *Plain) Values(key string) ([]string, bool) {
+	debug.Log("Trying to access key %q on a Plain secret", key)
+	return []string{""}, false
+}
+
 // Password returns the first line
 func (p *Plain) Password() string {
 	br := bufio.NewReader(bytes.NewReader(p.buf))
@@ -71,6 +77,11 @@ func (p *Plain) Set(_ string, _ interface{}) error {
 	return fmt.Errorf("not supported for PLAIN")
 }
 
+// Add does nothing
+func (p *Plain) Add(_ string, _ interface{}) error {
+	return fmt.Errorf("not supported for PLAIN")
+}
+
 // SetPassword updates the first line
 func (p *Plain) SetPassword(value string) {
 	buf := &bytes.Buffer{}

pkg/gopass/secrets/secparse/mime.go

@@ -41,9 +41,9 @@ func parseLegacyMIME(buf []byte) (*secrets.KV, error) {
 		hdr.Del("Password")
 	}
 
-	data := make(map[string]string, len(hdr))
+	data := make(map[string][]string, len(hdr))
 	for k := range hdr {
-		data[strings.ToLower(k)] = hdr.Get(k)
+		data[strings.ToLower(k)] = hdr.Values(k)
 	}
 
 	return secrets.NewKVWithData(pw, data, body.String(), true), nil