Add test cases for gopasspw#2462 and some minor cleanup (gopasspw#2465)

RELEASE_NOTES=n/a

Fixes gopasspw#2462

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

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

internal/action/otp_test.go

@@ -43,7 +43,14 @@ func TestOTP(t *testing.T) {
 		defer buf.Reset()
 		sec := secrets.NewAKV()
 		sec.SetPassword("foo")
-		_, err := sec.Write([]byte(twofactor.GenerateGoogleTOTP().URL("foo")))
+		_, err := sec.Write([]byte(twofactor.GenerateGoogleTOTP().URL("foo") + "\n"))
+		require.NoError(t, err)
+		assert.NoError(t, act.Store.Set(ctx, "bar", sec))
+
+		assert.NoError(t, act.OTP(gptest.CliCtx(ctx, t, "bar")))
+
+		// add some unrelated body material, it should still work
+		_, err = sec.Write([]byte("more body content, unrelated to otp"))
 		require.NoError(t, err)
 		assert.NoError(t, act.Store.Set(ctx, "bar", sec))
 

internal/editor/editor.go

@@ -131,9 +131,13 @@ func isVim(editor string) bool {
 	cmd := exec.Command(editor, "--version")
 	out, err := cmd.CombinedOutput()
 	if err != nil {
+		debug.Log("failed to check %s --version: %s", cmd.Path, err)
+
 		return false
 	}
 
+	debug.Log("%s --version: %s", cmd.Path, string(out))
+
 	return strings.Contains(string(out), "VIM - Vi IMproved")
 }
 

internal/store/leaf/store_test.go

@@ -28,7 +28,6 @@ func createSubStore(t *testing.T) (*Store, error) {
 		return nil, err
 	}
 
-	t.Setenv("GOPASS_CONFIG", filepath.Join(dir, ".gopass.yml"))
 	t.Setenv("GOPASS_HOMEDIR", dir)
 	t.Setenv("CHECKPOINT_DISABLE", "true")
 	t.Setenv("GOPASS_NO_NOTIFY", "true")

pkg/gopass/secrets/secparse/parse.go

@@ -46,3 +46,13 @@ func Parse(in []byte) (gopass.Secret, error) {
 
 	return s, nil
 }
+
+// MustParse parses a secret or panics. Should only be used for tests.
+func MustParse(in string) gopass.Secret {
+	sec, err := Parse([]byte(in))
+	if err != nil {
+		panic(err)
+	}
+
+	return sec
+}

pkg/otp/otp.go

@@ -17,27 +17,15 @@ import (
 //
 //nolint:ireturn
 func Calculate(name string, sec gopass.Secret) (*otp.Key, error) {
-	otpURL, found := sec.Get("otpauth")
-	if found && strings.HasPrefix(otpURL, "//") {
-		otpURL = "otpauth:" + otpURL
-	} else {
-		// check body
-		for _, line := range strings.Split(sec.Body(), "\n") {
-			if strings.HasPrefix(line, "otpauth://") {
-				otpURL = line
-
-				break
-			}
-		}
-	}
+	otpURL := getOTPURL(sec)
 
 	if otpURL != "" {
 		debug.Log("found otpauth url: %s", out.Secret(otpURL))
 
 		return otp.NewKeyFromURL(otpURL) //nolint:wrapcheck
 	}
 
-	// check yaml entry and fall back to password if we don't have one
+	// check KV entry and fall back to password if we don't have one
 
 	// TOTP
 	if secKey, found := sec.Get("totp"); found {
@@ -54,6 +42,26 @@ func Calculate(name string, sec gopass.Secret) (*otp.Key, error) {
 	return parseOTP("totp", sec.Password())
 }
 
+func getOTPURL(sec gopass.Secret) string {
+	// check if we have a key-value entry
+	if url, found := sec.Get("otpauth"); found {
+		if strings.HasPrefix(url, "//") {
+			url = "otpauth:" + url
+		}
+
+		return url
+	}
+
+	// if there is no KV entry check the body
+	for _, line := range strings.Split(sec.Body(), "\n") {
+		if strings.HasPrefix(line, "otpauth://") {
+			return line
+		}
+	}
+
+	return ""
+}
+
 func parseOTP(typ string, secKey string) (*otp.Key, error) {
 	if strings.HasPrefix(secKey, "otpauth://") {
 		debug.Log("parsing otpauth:// URL %q", out.Secret(secKey))

pkg/otp/otp_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/gopasspw/gopass/pkg/gopass"
 	"github.com/gopasspw/gopass/pkg/gopass/secrets/secparse"
 	"github.com/pquerna/otp"
 	"github.com/stretchr/testify/assert"
@@ -54,3 +55,35 @@ func TestWrite(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NoError(t, WriteQRFile(key, tf))
 }
+
+func TestGetOTPURL(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		sec  gopass.Secret
+		url  string
+	}{
+		{
+			name: "url-only-in-body",
+			sec:  secparse.MustParse(fmt.Sprintf("%s\n%s", pw, totpURL)),
+			url:  totpURL,
+		},
+		{
+			name: "url-and-other-text-in-body",
+			sec:  secparse.MustParse(fmt.Sprintf("%s\n%s\nfoo bar\nbaz\n", pw, totpURL)),
+			url:  totpURL,
+		},
+		{
+			name: "url-in-kvp",
+			sec:  secparse.MustParse(fmt.Sprintf("%s\notpauth: %s\nfoo bar\nbaz\n", pw, totpURL)),
+			url:  totpURL,
+		},
+	} {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tc.url, getOTPURL(tc.sec))
+		})
+	}
+}