From 95eb1ea67218c5b39c08a87baf3931bab47a674e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Szafra=C5=84ski?= Date: Tue, 13 Dec 2022 10:04:22 +0100 Subject: [PATCH 1/2] Fix typos --- doc.go | 3 ++- hotp/hotp.go | 2 +- hotp/hotp_test.go | 10 +++++----- totp/totp_test.go | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/doc.go b/doc.go index b8b4c8c..bb19545 100644 --- a/doc.go +++ b/doc.go @@ -19,7 +19,7 @@ // one time passcodes in a Google Authenticator compatible manner. // // When adding a TOTP for a user, you must store the "secret" value -// persistently. It is recommend to store the secret in an encrypted field in your +// persistently. It is recommended to store the secret in an encrypted field in your // datastore. Due to how TOTP works, it is not possible to store a hash // for the secret value like you would a password. // @@ -57,6 +57,7 @@ // // Validating a TOTP passcode is very easy, just prompt the user for a passcode // and retrieve the associated user's previously stored secret. +// // import "github.com/pquerna/otp/totp" // // passcode := promptForPasscode() diff --git a/hotp/hotp.go b/hotp/hotp.go index 5c9b8bb..081c063 100644 --- a/hotp/hotp.go +++ b/hotp/hotp.go @@ -186,7 +186,7 @@ func Generate(opts GenerateOpts) (*otp.Key, error) { opts.Rand = rand.Reader } - // otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example + // otpauth://hotp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example v := url.Values{} if len(opts.Secret) != 0 { diff --git a/hotp/hotp_test.go b/hotp/hotp_test.go index 3e425d4..bbab193 100644 --- a/hotp/hotp_test.go +++ b/hotp/hotp_test.go @@ -148,7 +148,7 @@ func TestGenerate(t *testing.T) { Issuer: "SnakeOil", AccountName: "alice@example.com", }) - require.NoError(t, err, "generate basic TOTP") + require.NoError(t, err, "generate basic HOTP") require.Equal(t, "SnakeOil", k.Issuer(), "Extracting Issuer") require.Equal(t, "alice@example.com", k.AccountName(), "Extracting Account Name") require.Equal(t, 16, len(k.Secret()), "Secret is 16 bytes long as base32.") @@ -158,7 +158,7 @@ func TestGenerate(t *testing.T) { AccountName: "alice@example.com", SecretSize: 20, }) - require.NoError(t, err, "generate larger TOTP") + require.NoError(t, err, "generate larger HOTP") require.Equal(t, 32, len(k.Secret()), "Secret is 32 bytes long as base32.") k, err = Generate(GenerateOpts{ @@ -178,9 +178,9 @@ func TestGenerate(t *testing.T) { k, err = Generate(GenerateOpts{ Issuer: "SnakeOil", AccountName: "alice@example.com", - SecretSize: 17, // anything that is not divisable by 5, really + SecretSize: 17, // anything that is not divisible by 5, really }) - require.NoError(t, err, "Secret size is valid when length not divisable by 5.") + require.NoError(t, err, "Secret size is valid when length not divisible by 5.") require.NotContains(t, k.Secret(), "=", "Secret has no escaped characters.") k, err = Generate(GenerateOpts{ @@ -190,6 +190,6 @@ func TestGenerate(t *testing.T) { }) require.NoError(t, err, "Secret generation failed") sec, err := b32NoPadding.DecodeString(k.Secret()) - require.NoError(t, err, "Secret wa not valid base32") + require.NoError(t, err, "Secret was not valid base32") require.Equal(t, sec, []byte("helloworld"), "Specified Secret was not kept") } diff --git a/totp/totp_test.go b/totp/totp_test.go index c71961b..9b2b637 100644 --- a/totp/totp_test.go +++ b/totp/totp_test.go @@ -139,9 +139,9 @@ func TestGenerate(t *testing.T) { k, err = Generate(GenerateOpts{ Issuer: "SnakeOil", AccountName: "alice@example.com", - SecretSize: 13, // anything that is not divisable by 5, really + SecretSize: 13, // anything that is not divisible by 5, really }) - require.NoError(t, err, "Secret size is valid when length not divisable by 5.") + require.NoError(t, err, "Secret size is valid when length not divisible by 5.") require.NotContains(t, k.Secret(), "=", "Secret has no escaped characters.") k, err = Generate(GenerateOpts{ From 5bfb6b8015a9f4ab592f1809ceda8e7e75bf3cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Szafra=C5=84ski?= Date: Tue, 13 Dec 2022 10:53:47 +0100 Subject: [PATCH 2/2] Fix encoding spaces in issuer name --- go.sum | 2 -- hotp/hotp.go | 3 ++- hotp/hotp_test.go | 7 +++++++ internal/encode.go | 35 +++++++++++++++++++++++++++++++++++ totp/totp.go | 3 ++- totp/totp_test.go | 7 +++++++ 6 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 internal/encode.go diff --git a/go.sum b/go.sum index 6848b56..6db19d5 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/boombuler/barcode v1.0.0 h1:s1TvRnXwL2xJRaccrdcBQMZxq6X7DvsMogtmJeHDdrc= -github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc h1:biVzkmvwrH8WK8raXaxBx6fRVTlJILwEwQGL1I/ByEI= github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= diff --git a/hotp/hotp.go b/hotp/hotp.go index 081c063..13a193e 100644 --- a/hotp/hotp.go +++ b/hotp/hotp.go @@ -19,6 +19,7 @@ package hotp import ( "github.com/pquerna/otp" + "github.com/pquerna/otp/internal" "io" "crypto/hmac" @@ -208,7 +209,7 @@ func Generate(opts GenerateOpts) (*otp.Key, error) { Scheme: "otpauth", Host: "hotp", Path: "/" + opts.Issuer + ":" + opts.AccountName, - RawQuery: v.Encode(), + RawQuery: internal.EncodeQuery(v), } return otp.NewKeyFromURL(u.String()) diff --git a/hotp/hotp_test.go b/hotp/hotp_test.go index bbab193..ddb760a 100644 --- a/hotp/hotp_test.go +++ b/hotp/hotp_test.go @@ -153,6 +153,13 @@ func TestGenerate(t *testing.T) { require.Equal(t, "alice@example.com", k.AccountName(), "Extracting Account Name") require.Equal(t, 16, len(k.Secret()), "Secret is 16 bytes long as base32.") + k, err = Generate(GenerateOpts{ + Issuer: "Snake Oil", + AccountName: "alice@example.com", + }) + require.NoError(t, err, "issuer with a space in the name") + require.Contains(t, k.String(), "issuer=Snake%20Oil") + k, err = Generate(GenerateOpts{ Issuer: "SnakeOil", AccountName: "alice@example.com", diff --git a/internal/encode.go b/internal/encode.go new file mode 100644 index 0000000..2af3c8b --- /dev/null +++ b/internal/encode.go @@ -0,0 +1,35 @@ +package internal + +import ( + "net/url" + "sort" + "strings" +) + +// EncodeQuery is a copy-paste of url.Values.Encode, except it uses %20 instead +// of + to encode spaces. This is necessary to correctly render spaces in some +// authenticator apps, like Google Authenticator. +func EncodeQuery(v url.Values) string { + if v == nil { + return "" + } + var buf strings.Builder + keys := make([]string, 0, len(v)) + for k := range v { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + vs := v[k] + keyEscaped := url.PathEscape(k) // changed from url.QueryEscape + for _, v := range vs { + if buf.Len() > 0 { + buf.WriteByte('&') + } + buf.WriteString(keyEscaped) + buf.WriteByte('=') + buf.WriteString(url.PathEscape(v)) // changed from url.QueryEscape + } + } + return buf.String() +} diff --git a/totp/totp.go b/totp/totp.go index db5ed36..a2fb7d5 100644 --- a/totp/totp.go +++ b/totp/totp.go @@ -20,6 +20,7 @@ package totp import ( "github.com/pquerna/otp" "github.com/pquerna/otp/hotp" + "github.com/pquerna/otp/internal" "io" "crypto/rand" @@ -199,7 +200,7 @@ func Generate(opts GenerateOpts) (*otp.Key, error) { Scheme: "otpauth", Host: "totp", Path: "/" + opts.Issuer + ":" + opts.AccountName, - RawQuery: v.Encode(), + RawQuery: internal.EncodeQuery(v), } return otp.NewKeyFromURL(u.String()) diff --git a/totp/totp_test.go b/totp/totp_test.go index 9b2b637..1f854b7 100644 --- a/totp/totp_test.go +++ b/totp/totp_test.go @@ -128,6 +128,13 @@ func TestGenerate(t *testing.T) { require.Equal(t, "alice@example.com", k.AccountName(), "Extracting Account Name") require.Equal(t, 32, len(k.Secret()), "Secret is 32 bytes long as base32.") + k, err = Generate(GenerateOpts{ + Issuer: "Snake Oil", + AccountName: "alice@example.com", + }) + require.NoError(t, err, "issuer with a space in the name") + require.Contains(t, k.String(), "issuer=Snake%20Oil") + k, err = Generate(GenerateOpts{ Issuer: "SnakeOil", AccountName: "alice@example.com",