Skip to content

Commit

Permalink
Fix URL query encoding (#78)
Browse files Browse the repository at this point in the history
* Fix typos

* Fix encoding spaces in issuer name
  • Loading branch information
kszafran authored Dec 13, 2022
1 parent 45fc76e commit 87d222c
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 13 deletions.
3 changes: 2 additions & 1 deletion doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -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=
Expand Down
5 changes: 3 additions & 2 deletions hotp/hotp.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package hotp

import (
"github.com/pquerna/otp"
"github.com/pquerna/otp/internal"
"io"

"crypto/hmac"
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
Expand Down
17 changes: 12 additions & 5 deletions hotp/hotp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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")
}
35 changes: 35 additions & 0 deletions internal/encode.go
Original file line number Diff line number Diff line change
@@ -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()
}
3 changes: 2 additions & 1 deletion totp/totp.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package totp
import (
"github.com/pquerna/otp"
"github.com/pquerna/otp/hotp"
"github.com/pquerna/otp/internal"
"io"

"crypto/rand"
Expand Down Expand Up @@ -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())
Expand Down
11 changes: 9 additions & 2 deletions totp/totp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand All @@ -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{
Expand Down

0 comments on commit 87d222c

Please sign in to comment.