diff --git a/internal/action/binary.go b/internal/action/binary.go index 613a4a7ef9..35ce112393 100644 --- a/internal/action/binary.go +++ b/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 diff --git a/internal/action/insert_test.go b/internal/action/insert_test.go index 6f2b2ec283..bebc3a45e8 100644 --- a/internal/action/insert_test.go +++ b/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() }) diff --git a/internal/action/show.go b/internal/action/show.go index e405aeee0e..5c4ef3f731 100644 --- a/internal/action/show.go +++ b/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,15 +158,17 @@ 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 } @@ -174,11 +176,13 @@ func (s *Action) showGetContent(ctx context.Context, sec gopass.Secret) (string, // 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 diff --git a/internal/action/show_test.go b/internal/action/show_test.go index a5cc90a047..399520283c 100644 --- a/internal/action/show_test.go +++ b/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() }) diff --git a/internal/tpl/funcs.go b/internal/tpl/funcs.go index e8fa5a370f..6c748737fa 100644 --- a/internal/tpl/funcs.go +++ b/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(), diff --git a/pkg/gopass/secrets/kv.go b/pkg/gopass/secrets/kv.go index e7c7b59515..cf68df9040 100644 --- a/pkg/gopass/secrets/kv.go +++ b/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,16 +104,24 @@ 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 } @@ -119,11 +129,21 @@ func (k *KV) Get(key string) (string, bool) { // 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 } diff --git a/pkg/gopass/secrets/kv_test.go b/pkg/gopass/secrets/kv_test.go index 25d05aa4bd..834d93cf44 100644 --- a/pkg/gopass/secrets/kv_test.go +++ b/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())) +} diff --git a/pkg/gopass/secrets/plain.go b/pkg/gopass/secrets/plain.go index be89b5a614..faa6ab49ec 100644 --- a/pkg/gopass/secrets/plain.go +++ b/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{} diff --git a/pkg/gopass/secrets/secparse/mime.go b/pkg/gopass/secrets/secparse/mime.go index da0308afaf..7e01e45da1 100644 --- a/pkg/gopass/secrets/secparse/mime.go +++ b/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 diff --git a/pkg/gopass/secrets/secparse/parse_test.go b/pkg/gopass/secrets/secparse/parse_test.go index 3e94aff321..f8cb1ef1d6 100644 --- a/pkg/gopass/secrets/secparse/parse_test.go +++ b/pkg/gopass/secrets/secparse/parse_test.go @@ -1,6 +1,7 @@ package secparse import ( + "fmt" "testing" "github.com/gopasspw/gopass/pkg/gopass/secrets" @@ -32,6 +33,7 @@ func TestParsedIsSerialized(t *testing.T) { } { sec, err := Parse([]byte(tc)) require.NoError(t, err) + fmt.Println() assert.Equal(t, tc, string(sec.Bytes())) } } diff --git a/pkg/gopass/secrets/yaml.go b/pkg/gopass/secrets/yaml.go index 894b1c2b5c..49b1ff20dc 100644 --- a/pkg/gopass/secrets/yaml.go +++ b/pkg/gopass/secrets/yaml.go @@ -41,14 +41,11 @@ func (y *YAML) Keys() []string { for key := range y.data { keys = append(keys, key) } - if _, found := y.data["password"]; !found { - keys = append(keys, "password") - } sort.Strings(keys) return keys } -// Get returns the value of a single key +// Get returns the first value of a single key func (y *YAML) Get(key string) (string, bool) { if y.data == nil { y.data = make(map[string]interface{}) @@ -62,6 +59,12 @@ func (y *YAML) Get(key string) (string, bool) { return "", false } +// Values returns Get since as per YAML specification keys must be unique +func (y *YAML) Values(key string) ([]string, bool) { + data, found := y.Get(key) + return []string{data}, found +} + // Set sets a key to a given value func (y *YAML) Set(key string, value interface{}) error { if y.data == nil { @@ -71,6 +74,11 @@ func (y *YAML) Set(key string, value interface{}) error { return nil } +// Add doesn't work since as per YAML specification keys must be unique +func (y *YAML) Add(key string, value interface{}) error { + return fmt.Errorf("not supported for YAML") +} + // Del removes a single key func (y *YAML) Del(key string) bool { _, found := y.data[key] @@ -155,14 +163,14 @@ func (y *YAML) Bytes() []byte { }() buf := &bytes.Buffer{} buf.WriteString(y.password) - buf.WriteString("\n") if y.body != "" { + buf.WriteString("\n") buf.WriteString(y.body) + } + if len(y.data) > 0 { if !strings.HasSuffix(y.body, "\n") { buf.WriteString("\n") } - } - if len(y.data) > 0 { buf.WriteString("---\n") if err := yaml.NewEncoder(buf).Encode(y.data); err != nil { debug.Log("failed to encode YAML: %s", err) diff --git a/pkg/gopass/secrets/yaml_test.go b/pkg/gopass/secrets/yaml_test.go index 2444780595..76f2d62065 100644 --- a/pkg/gopass/secrets/yaml_test.go +++ b/pkg/gopass/secrets/yaml_test.go @@ -238,5 +238,5 @@ sub: assert.Equal(t, "hallo", get("login")) assert.Equal(t, "42", get("number")) assert.Equal(t, "map[subentry:123]", get("sub")) - assert.Equal(t, []string{"login", "number", "password", "sub"}, s.Keys()) + assert.Equal(t, []string{"login", "number", "sub"}, s.Keys()) } diff --git a/pkg/gopass/store.go b/pkg/gopass/store.go index 3ec11b0bc0..0b82323616 100644 --- a/pkg/gopass/store.go +++ b/pkg/gopass/store.go @@ -15,10 +15,14 @@ type Secret interface { Byter Keys() []string - // Get returns a single header value, use Password() to get the password value. + // Get returns a single value for that key, use Password() to get the password value. Get(key string) (string, bool) + // Values returns all values for that key, use Password() to get the password value. + Values(key string) ([]string, bool) // Set sets a single header value, use SetPassword() to set the password value. Set(key string, value interface{}) error + // Add appends the value to that key, use SetPassword() to set the password value. + Add(key string, value interface{}) error // Del removes a single header value Del(key string) bool diff --git a/tests/show_test.go b/tests/show_test.go index 8f1a3f26b5..4634719ed4 100644 --- a/tests/show_test.go +++ b/tests/show_test.go @@ -77,10 +77,7 @@ func TestShow(t *testing.T) { out, err = ts.run("show fixed/twoliner") assert.NoError(t, err) - assert.Contains(t, out, "password: ***") - - out, err = ts.run("show fixed/twoliner") - assert.NoError(t, err) + assert.NotContains(t, out, "password: ***") assert.Contains(t, out, "more stuff") assert.NotContains(t, out, "and") }) @@ -108,11 +105,13 @@ func TestShow(t *testing.T) { out, err = ts.run("show -c fixed/twoliner") assert.NoError(t, err) assert.NotContains(t, out, "***") + assert.NotContains(t, out, "safecontent=true") assert.NotContains(t, out, "and") + assert.NotContains(t, out, "more stuff") out, err = ts.run("show -C fixed/twoliner") assert.NoError(t, err) - assert.Contains(t, out, "***") + assert.Contains(t, out, "more stuff") assert.NotContains(t, out, "and") }) @@ -123,12 +122,13 @@ func TestShow(t *testing.T) { _, err := ts.run("generate fo2 5") assert.NoError(t, err) - // TODO fix this - //out, err := ts.run("show fo2") - //assert.Error(t, err) - //assert.Contains(t, out, "Warning: safecontent=true") + out, err := ts.run("show fo2") + assert.Error(t, err) + assert.NotContains(t, out, "password: *****") + assert.NotContains(t, out, "aaaaa") + assert.Contains(t, out, "safecontent=true") - out, err := ts.run("show -u fo2") + out, err = ts.run("show -u fo2") assert.NoError(t, err) assert.Equal(t, out, "aaaaa") @@ -136,10 +136,10 @@ func TestShow(t *testing.T) { assert.NoError(t, err) out, err = ts.run("show fo6") - assert.NoError(t, err) - assert.Contains(t, out, "password: ***") + assert.Error(t, err) + assert.NotContains(t, out, "password: ***") assert.NotContains(t, out, "aaaaa") - assert.NotContains(t, out, "\n\n") + assert.Contains(t, out, "safecontent=true") out, err = ts.run("show -u fo6") assert.NoError(t, err) diff --git a/tests/yaml_test.go b/tests/yaml_test.go index 09b18be653..8c90b66da3 100644 --- a/tests/yaml_test.go +++ b/tests/yaml_test.go @@ -57,7 +57,7 @@ func TestInvalidYAML(t *testing.T) { --- Test / test.com username: myuser@test.com -password: somepasswd +password: someotherpasswd url: http://www.test.com/` ts := newTester(t)