feat: FixGAuthBug function to avoid #94

README.md

@@ -44,12 +44,41 @@ For an example of a working enrollment work flow, [GitHub has documented theirs]
 1. Retrieve the User's TOTP Secret from your backend.
 1. Validate the user's passcode. `totp.Validate(...)`
 
-
 ### Recovery Codes
 
 When a user loses access to their TOTP device, they would no longer have access to their account.  Because TOTPs are often configured on mobile devices that can be lost, stolen or damaged, this is a common problem. For this reason many providers give their users "backup codes" or "recovery codes".  These are a set of one time use codes that can be used instead of the TOTP.  These can simply be randomly generated strings that you store in your backend.  [Github's documentation provides an overview of the user experience](
 https://help.github.com/articles/downloading-your-two-factor-authentication-recovery-codes/).
 
+### Google Authenticator Bugfix
+
+There is a [known bug](https://github.com/pquerna/otp/issues/94) in Google Authenticator that causes QR code scanning to fail, due to the combination of standards and parameters used to generate the QR code image.
+
+This library provides a workaround function `FixGAuthBug` that appends an extra parameter delimiter "&" to the URI if necessary.
+
+If you plan to use external tools (e.g. "qrencode") to generate QR codes, use this function to ensure compatibility with Google Authenticator.
+
+```go
+func main() {
+  key, err := totp.Generate(totp.GenerateOpts{
+    Issuer:      "localhost:8000",
+    AccountName: "[email protected]",
+  })
+  if err != nil {
+    panic(err)
+  }
+
+  // Generate a URI for the key.
+  uri := key.URL()
+
+  // Ensure the URI be compatible with Google Authenticator.
+  uri, err = totp.FixGAuthBug(uri)
+  if err != nil {
+    panic(err)
+  }
+
+  fmt.Println("Use this URI to generate the QR code:", uri)
+}
+```
 
 ## Improvements, bugs, adding feature, etc:
 

totp/totp.go

@@ -18,10 +18,12 @@
 package totp
 
 import (
+	"io"
+	"strings"
+
 	"github.com/pquerna/otp"
 	"github.com/pquerna/otp/hotp"
 	"github.com/pquerna/otp/internal"
-	"io"
 
 	"crypto/rand"
 	"encoding/base32"
@@ -208,3 +210,29 @@ func Generate(opts GenerateOpts) (*otp.Key, error) {
 
 	return otp.NewKeyFromURL(u.String())
 }
+
+// FixGAuthBug appends an extra parameter delimiter "&" to the URI if necessary.
+//
+// This function provides a workaround for a known bug in Google Authenticator.
+// (See issue #94: https://github.com/pquerna/otp/issues/94#issuecomment-2524954588)
+//
+// The bug affects QR codes generated with certain combinations of standards
+// and parameters, causing them to fail when scanned by Google Authenticator.
+//
+// Use this function if you need to generate a QR code using an external tool
+// (outside this library) and require the URI to be compatible with Google Authenticator.
+// It adjusts the URI to ensure proper functionality.
+func FixGAuthBug(uri string) (string, error) {
+	parsedURL, err := url.Parse(uri)
+	if err != nil {
+		return "", err
+	}
+
+	chunks := strings.Split(parsedURL.RawQuery, "&")
+
+	if !strings.HasPrefix(chunks[len(chunks)-1], "secret=") {
+		return uri, nil // last param is not a secret
+	}
+
+	return uri + "&", nil // avoid GoogleAuthenticator bug
+}

totp/totp_test.go

@@ -61,14 +61,14 @@ var (
 	}
 )
 
-//
 // Test vectors from http://tools.ietf.org/html/rfc6238#appendix-B
 // NOTE -- the test vectors are documented as having the SAME
 // secret -- this is WRONG -- they have a variable secret
 // depending upon the hmac algorithm:
-// 		http://www.rfc-editor.org/errata_search.php?rfc=6238
-// this only took a few hours of head/desk interaction to figure out.
 //
+//	http://www.rfc-editor.org/errata_search.php?rfc=6238
+//
+// this only took a few hours of head/desk interaction to figure out.
 func TestValidateRFCMatrix(t *testing.T) {
 	for _, tx := range rfcMatrixTCs {
 		valid, err := ValidateCustom(tx.TOTP, tx.Secret, time.Unix(tx.TS, 0).UTC(),
@@ -198,3 +198,82 @@ func TestSteamSecret(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, valid)
 }
+
+func TestFixGAuthBug_fix94(t *testing.T) {
+	for index, test := range []struct {
+		title     string
+		uriInput  string
+		uriExpect string
+	}{
+		{
+			title:     "no query (require be as is)",
+			uriInput:  "otpauth://totp/example.com:[email protected]",
+			uriExpect: "otpauth://totp/example.com:[email protected]",
+		},
+		{
+			title:     "empty query (require be as is)",
+			uriInput:  "otpauth://totp/example.com:[email protected]?",
+			uriExpect: "otpauth://totp/example.com:[email protected]?",
+		},
+		{
+			title:     "secret only (minimum info)",
+			uriInput:  "otpauth://totp/example.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X",
+			uriExpect: "otpauth://totp/example.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
+		},
+		{
+			title: "heading secret",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=example.com&period=45",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=example.com&period=45",
+		},
+		{
+			title: "middle secret",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=example.com&period=45",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=example.com&period=45",
+		},
+		{
+			title: "tailing secret (require '&' to be added)",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
+		},
+		{
+			title: "tailing secret (fixed w/& only)",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
+		},
+		{
+			title: "tailing secret (fixed w/single param)",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo",
+		},
+		{
+			title: "tailing secret (fixed w/key-value param)",
+			uriInput: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo=bar",
+			uriExpect: "otpauth://totp/example.com:[email protected]?" +
+				"algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo=bar",
+		},
+	} {
+		expect := test.uriExpect
+		actual, err := FixGAuthBug(test.uriInput) // Apply bug fix
+
+		require.NoError(t, err, "test case #%d: %s", index+1, test.title)
+		require.Equal(t, expect, actual, "test case #%d: %s (failed)", index+1, test.title)
+	}
+
+	t.Run("malformed uri (contains CTL byte)", func(t *testing.T) {
+		out, err := FixGAuthBug("otpauth://totp/example.com:[email protected]?\n")
+
+		require.Error(t, err, "malformed uri should return error")
+		require.Empty(t, out, "returned value should be empty on error")
+	})
+}