Fix URL query encoding (pquerna#78)

* Fix typos

* Fix encoding spaces in issuer name

Author: kszafran

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()

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=

hotp/hotp.go

@@ -19,6 +19,7 @@ package hotp
 
 import (
 	"github.com/pquerna/otp"
+	"github.com/pquerna/otp/internal"
 	"io"
 
 	"crypto/hmac"
@@ -186,7 +187,7 @@ func Generate(opts GenerateOpts) (*otp.Key, error) {
 		opts.Rand = rand.Reader
 	}
 
-	// otpauth://totp/Example:[email protected]?secret=JBSWY3DPEHPK3PXP&issuer=Example
+	// otpauth://hotp/Example:[email protected]?secret=JBSWY3DPEHPK3PXP&issuer=Example
 
 	v := url.Values{}
 	if len(opts.Secret) != 0 {
@@ -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())

hotp/hotp_test.go

@@ -148,17 +148,24 @@ func TestGenerate(t *testing.T) {
 		Issuer:      "SnakeOil",
 		AccountName: "[email protected]",
 	})
-	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, "[email protected]", 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: "[email protected]",
+	})
+	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: "[email protected]",
 		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 +185,9 @@ func TestGenerate(t *testing.T) {
 	k, err = Generate(GenerateOpts{
 		Issuer:      "SnakeOil",
 		AccountName: "[email protected]",
-		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 +197,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")
 }

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()
+}

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())

totp/totp_test.go

@@ -128,6 +128,13 @@ func TestGenerate(t *testing.T) {
 	require.Equal(t, "[email protected]", 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: "[email protected]",
+	})
+	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: "[email protected]",
@@ -139,9 +146,9 @@ func TestGenerate(t *testing.T) {
 	k, err = Generate(GenerateOpts{
 		Issuer:      "SnakeOil",
 		AccountName: "[email protected]",
-		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{