diff --git a/internal/action/otp_test.go b/internal/action/otp_test.go index 4a3eec781f..87990a1f3a 100644 --- a/internal/action/otp_test.go +++ b/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)) diff --git a/internal/editor/editor.go b/internal/editor/editor.go index 1e2a6b00bd..6ccec9d585 100644 --- a/internal/editor/editor.go +++ b/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") } diff --git a/internal/store/leaf/store_test.go b/internal/store/leaf/store_test.go index ea6cff582f..32665a8682 100644 --- a/internal/store/leaf/store_test.go +++ b/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") diff --git a/pkg/gopass/secrets/secparse/parse.go b/pkg/gopass/secrets/secparse/parse.go index dfbb611619..a474c23b59 100644 --- a/pkg/gopass/secrets/secparse/parse.go +++ b/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 +} diff --git a/pkg/otp/otp.go b/pkg/otp/otp.go index f9246abb2c..f293424963 100644 --- a/pkg/otp/otp.go +++ b/pkg/otp/otp.go @@ -17,19 +17,7 @@ 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)) @@ -37,7 +25,7 @@ func Calculate(name string, sec gopass.Secret) (*otp.Key, error) { 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)) diff --git a/pkg/otp/otp_test.go b/pkg/otp/otp_test.go index 5505b21125..52941d22d7 100644 --- a/pkg/otp/otp_test.go +++ b/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)) + }) + } +}