Always copy passwords to clipboard is autoclip is set

Fixes gopasspw#1255
Fixes gopasspw#1317

Signed-off-by: Dominik Schulz <[email protected]>

pkg/action/commands.go

@@ -741,8 +741,9 @@ func (s *Action) GetCommands() []*cli.Command {
 					Usage:   "Copy the first line of the secret into the clipboard",
 				},
 				&cli.BoolFlag{
-					Name:  "alsoclip, C",
-					Usage: "Copy the first line of the secret and show everything",
+					Name:    "alsoclip",
+					Aliases: []string{"C"},
+					Usage:   "Copy the first line of the secret and show everything",
 				},
 				&cli.BoolFlag{
 					Name:  "qr",

pkg/action/copy_test.go

@@ -23,6 +23,7 @@ func TestCopy(t *testing.T) {
 
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)

pkg/action/find.go

@@ -24,9 +24,9 @@ func (s *Action) FindNoFuzzy(c *cli.Context) error {
 func (s *Action) Find(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	if c.IsSet("clip") {
+		ctx = WithOnlyClip(ctx, c.Bool("clip"))
 		ctx = WithClip(ctx, c.Bool("clip"))
 	}
-
 	if c.IsSet("force") {
 		ctx = WithForce(ctx, c.Bool("force"))
 	}

pkg/action/find_test.go

@@ -27,6 +27,7 @@ func TestFind(t *testing.T) {
 
 	ctx := context.Background()
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)
@@ -68,12 +69,7 @@ func TestFind(t *testing.T) {
 	// testing the safecontent case
 	ctx = ctxutil.WithShowSafeContent(ctx, true)
 	c.Context = ctx
-	assert.NoError(t, act.Find(c))
-
-	out := strings.TrimSpace(buf.String())
-	assert.Contains(t, out, "Found exact match in 'foo'")
-	assert.Contains(t, out, "with -f")
-	assert.Contains(t, out, "Copying password instead.")
+	assert.Error(t, act.Find(c))
 	buf.Reset()
 
 	// testing with the clip flag set
@@ -87,7 +83,7 @@ func TestFind(t *testing.T) {
 	c.Context = ctx
 
 	assert.NoError(t, act.Find(c))
-	out = strings.TrimSpace(buf.String())
+	out := strings.TrimSpace(buf.String())
 	assert.Contains(t, out, "Found exact match in 'foo'")
 	buf.Reset()
 

pkg/action/generate.go

@@ -35,6 +35,7 @@ var (
 // Generate and save a password
 func (s *Action) Generate(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
+	ctx = WithClip(ctx, c.Bool("clip"))
 	force := c.Bool("force")
 	edit := c.Bool("edit")
 
@@ -116,13 +117,13 @@ func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name,
 		)
 	}
 
-	if ctxutil.IsAutoClip(ctx) || c.Bool("clip") {
+	if ctxutil.IsAutoClip(ctx) || IsClip(ctx) {
 		if err := clipboard.CopyTo(ctx, name, []byte(password)); err != nil {
 			return ExitError(ctx, ExitIO, err, "failed to copy to clipboard: %s", err)
 		}
 	}
 
-	if c.Bool("print") || c.Bool("clip") {
+	if c.Bool("print") || IsClip(ctx) {
 		return nil
 	}
 
@@ -166,17 +167,7 @@ func (s *Action) generatePassword(ctx context.Context, c *cli.Context, length st
 		return "", ExitError(ctx, ExitUsage, nil, "password length must not be zero")
 	}
 
-	var corp bool
-	if c.IsSet("strict") || c.Bool("force") {
-		corp = c.Bool("strict")
-	} else {
-		var err error
-		corp, err = termio.AskForBool(ctx, "Do you have strict rules to include different character classes?", false)
-		if err != nil {
-			return "", err
-		}
-	}
-	if corp {
+	if c.Bool("strict") {
 		return pwgen.GeneratePasswordWithAllClasses(pwlen)
 	}
 

pkg/action/history.go

@@ -31,7 +31,7 @@ func (s *Action) History(c *cli.Context) error {
 	for _, rev := range revs {
 		pw := ""
 		if showPassword {
-			sec, err := s.Store.GetRevision(ctx, name, rev.Hash)
+			ctx, sec, err := s.Store.GetRevision(ctx, name, rev.Hash)
 			if err != nil {
 				out.Debug(ctx, "Failed to get revision '%s' of '%s': %s", rev.Hash, name, err)
 			}

pkg/action/insert_test.go

@@ -24,6 +24,7 @@ func TestInsert(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)

pkg/action/show.go

@@ -22,18 +22,33 @@ const (
 	BinarySuffix = ".b64"
 )
 
+func showParseArgs(c *cli.Context) context.Context {
+	ctx := ctxutil.WithGlobalFlags(c)
+	if c.IsSet("clip") {
+		ctx = WithOnlyClip(ctx, c.Bool("clip"))
+	}
+	if c.IsSet("force") {
+		ctx = WithForce(ctx, c.Bool("force"))
+	}
+	if c.IsSet("qr") {
+		ctx = WithPrintQR(ctx, c.Bool("qr"))
+	}
+	if c.IsSet("password") {
+		ctx = WithPasswordOnly(ctx, c.Bool("password"))
+	}
+	if c.IsSet("revision") {
+		ctx = WithRevision(ctx, c.String("revision"))
+	}
+	ctx = WithClip(ctx, IsOnlyClip(ctx) || c.Bool("alsoclip"))
+	return ctx
+}
+
 // Show the content of a secret file
 func (s *Action) Show(c *cli.Context) error {
-	ctx := ctxutil.WithGlobalFlags(c)
 	name := c.Args().First()
 
-	ctx = s.Store.WithConfig(ctx, name)
-	ctx = WithOnlyClip(ctx, c.Bool("clip"))
-	ctx = WithClip(ctx, c.Bool("alsoclip"))
-	ctx = WithForce(ctx, c.Bool("force"))
-	ctx = WithPrintQR(ctx, c.Bool("qr"))
-	ctx = WithPasswordOnly(ctx, c.Bool("password"))
-	ctx = WithRevision(ctx, c.String("revision"))
+	ctx := showParseArgs(c)
+
 	if key := c.Args().Get(1); key != "" {
 		ctx = WithKey(ctx, key)
 	}
@@ -82,7 +97,7 @@ func (s *Action) show(ctx context.Context, c *cli.Context, name string, recurse
 
 // showHandleRevision displays a single revision
 func (s *Action) showHandleRevision(ctx context.Context, c *cli.Context, name, revision string) error {
-	sec, err := s.Store.GetRevision(ctx, name, revision)
+	ctx, sec, err := s.Store.GetRevision(ctx, name, revision)
 	if err != nil {
 		return s.showHandleError(ctx, c, name, false, err)
 	}
@@ -92,65 +107,79 @@ func (s *Action) showHandleRevision(ctx context.Context, c *cli.Context, name, r
 
 // showHandleOutput displays a secret
 func (s *Action) showHandleOutput(ctx context.Context, name string, sec store.Secret) error {
-	var content string
-	var key string
-	if HasKey(ctx) {
-		key = GetKey(ctx)
+	pw, body, err := s.showGetContent(ctx, name, sec)
+	if err != nil {
+		return err
 	}
 
-	switch {
-	case key != "":
-		val, err := sec.Value(key)
-		if err != nil {
-			return s.showHandleYAMLError(ctx, name, key, err)
-		}
-		if IsClip(ctx) {
-			if err := clipboard.CopyTo(ctx, name, []byte(val)); err != nil {
-				return err
-			}
-			if IsOnlyClip(ctx) {
-				return nil
-			}
-		}
-		content = val
-	case IsPrintQR(ctx):
-		return s.showPrintQR(ctx, name, sec.Password())
-	case IsClip(ctx):
-		if err := clipboard.CopyTo(ctx, name, []byte(sec.Password())); err != nil {
+	// we need to set AutoClip here because it's a per store config option
+	if ctxutil.IsAutoClip(ctx) {
+		ctx = WithClip(ctx, true)
+	}
+
+	if pw == "" && body == "" {
+		return ExitError(ctx, ExitNotFound, store.ErrNoBody, store.ErrNoBody.Error())
+	}
+
+	if IsPrintQR(ctx) && pw != "" {
+		return s.showPrintQR(ctx, name, pw)
+	}
+
+	if IsClip(ctx) && pw != "" {
+		if err := clipboard.CopyTo(ctx, name, []byte(pw)); err != nil {
 			return err
 		}
-		if IsOnlyClip(ctx) {
-			return nil
-		}
-		fallthrough
-	default:
-		switch {
-		case IsPasswordOnly(ctx):
-			content = sec.Password()
-		case ctxutil.IsShowSafeContent(ctx) && !IsForce(ctx):
-			content = sec.Body()
-			if content == "" {
-				if ctxutil.IsAutoClip(ctx) {
-					out.Yellow(ctx, "Info: %s.", store.ErrNoBody.Error())
-					out.Yellow(ctx, "Copying password instead.")
-					return clipboard.CopyTo(ctx, name, []byte(sec.Password()))
-				}
-				return ExitError(ctx, ExitNotFound, store.ErrNoBody, store.ErrNoBody.Error())
-			}
-		default:
-			buf, err := sec.Bytes()
-			if err != nil {
-				return ExitError(ctx, ExitUnknown, err, "failed to encode secret: %s", err)
-			}
-			content = string(buf)
-		}
 	}
 
-	ctx = out.WithNewline(ctx, ctxutil.IsTerminal(ctx) && !strings.HasSuffix(content, "\n"))
-	out.Yellow(ctx, content)
+	if body == "" {
+		return nil
+	}
+
+	ctx = out.WithNewline(ctx, ctxutil.IsTerminal(ctx) && !strings.HasSuffix(body, "\n"))
+	out.Yellow(ctx, body)
 	return nil
 }
 
+func (s *Action) showGetContent(ctx context.Context, name string, sec store.Secret) (string, string, error) {
+	// YAML key
+	if HasKey(ctx) {
+		key := GetKey(ctx)
+		val, err := sec.Value(key)
+		if err != nil {
+			return "", "", s.showHandleYAMLError(ctx, name, key, err)
+		}
+		return val, "", nil
+	}
+
+	// first line of the secret only
+	if IsPrintQR(ctx) || IsOnlyClip(ctx) {
+		fmt.Println("1")
+		return sec.Password(), "", nil
+	}
+	if IsPasswordOnly(ctx) {
+		fmt.Println("2")
+		return sec.Password(), sec.Password(), nil
+	}
+	if ctxutil.IsAutoClip(ctx) && !IsForce(ctx) {
+		fmt.Println("3")
+		return sec.Password(), "", nil
+	}
+
+	// everything but the first line
+	if ctxutil.IsShowSafeContent(ctx) && !IsForce(ctx) {
+		fmt.Println("4")
+		return "", sec.Body(), nil
+	}
+
+	// everything (default)
+	buf, err := sec.Bytes()
+	if err != nil {
+		return "", "", ExitError(ctx, ExitUnknown, err, "failed to encode secret: %s", err)
+	}
+	fmt.Println("5")
+	return sec.Password(), string(buf), nil
+}
+
 // showHandleError handles errors retrieving secrets
 func (s *Action) showHandleError(ctx context.Context, c *cli.Context, name string, recurse bool, err error) error {
 	if err != store.ErrNotFound || !recurse || !ctxutil.IsTerminal(ctx) {

pkg/action/show_test.go

@@ -26,6 +26,7 @@ func TestShow(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)
@@ -90,14 +91,13 @@ func TestShow(t *testing.T) {
 	assert.Equal(t, "---\nbar: zab", buf.String())
 	buf.Reset()
 
-	// show foo with safecontent enabled, should warn and copy the stuff
+	// show foo with safecontent enabled, should error out
 	fs = flag.NewFlagSet("default", flag.ContinueOnError)
 	assert.NoError(t, fs.Parse([]string{"foo"}))
 	c = cli.NewContext(app, fs, nil)
 	c.Context = ctx
 
-	assert.NoError(t, act.Show(c))
-	assert.Contains(t, buf.String(), "no safe content to display, you can force display with -f.")
+	assert.Error(t, act.Show(c))
 	buf.Reset()
 
 	// show foo with safecontent enabled, with the force flag
@@ -139,6 +139,7 @@ func TestShowHandleRevision(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)
@@ -159,7 +160,7 @@ func TestShowHandleRevision(t *testing.T) {
 	c := cli.NewContext(app, fs, nil)
 	c.Context = ctx
 
-	assert.NoError(t, act.showHandleRevision(ctx, c, "foo", "baz"))
+	assert.NoError(t, act.showHandleRevision(ctx, c, "foo", "HEAD"))
 	buf.Reset()
 }
 
@@ -170,6 +171,7 @@ func TestShowHandleError(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)
@@ -201,6 +203,7 @@ func TestShowHandleYAMLError(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)
@@ -225,6 +228,7 @@ func TestShowPrintQR(t *testing.T) {
 	ctx := context.Background()
 	ctx = ctxutil.WithAlwaysYes(ctx, true)
 	ctx = ctxutil.WithTerminal(ctx, false)
+	ctx = ctxutil.WithAutoClip(ctx, false)
 	act, err := newMock(ctx, u)
 	require.NoError(t, err)
 	require.NotNil(t, act)

pkg/backend/rcs/noop/backend.go

@@ -79,7 +79,7 @@ func (g *Noop) Revisions(context.Context, string) ([]backend.Revision, error) {
 
 // GetRevision is not implemented
 func (g *Noop) GetRevision(context.Context, string, string) ([]byte, error) {
-	return []byte(""), nil
+	return []byte("foo\nbar"), nil
 }
 
 // Status is not implemented

pkg/backend/rcs/noop/backend_test.go

@@ -27,6 +27,6 @@ func TestNoop(t *testing.T) {
 	assert.Error(t, err)
 	body, err := g.GetRevision(ctx, "foo", "bar")
 	require.NoError(t, err)
-	assert.Equal(t, "", string(body))
+	assert.Equal(t, "foo\nbar", string(body))
 	assert.NoError(t, g.RemoveRemote(ctx, "foo"))
 }

pkg/config/config.go

@@ -40,7 +40,7 @@ func New() *Config {
 		Path: configLocation(),
 		Root: &StoreConfig{
 			AskForMore:    false,
-			AutoClip:      true,
+			AutoClip:      false,
 			AutoImport:    true,
 			AutoSync:      true,
 			ClipTimeout:   45,