diff --git a/README.md b/README.md index 1447e3a..65dde60 100644 --- a/README.md +++ b/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: "test@example.com", + }) + 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: diff --git a/totp/totp.go b/totp/totp.go index f33458c..b2e86b4 100644 --- a/totp/totp.go +++ b/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 +} diff --git a/totp/totp_test.go b/totp/totp_test.go index f56e36a..47e8817 100644 --- a/totp/totp_test.go +++ b/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:test_head@example.com", + uriExpect: "otpauth://totp/example.com:test_head@example.com", + }, + { + title: "empty query (require be as is)", + uriInput: "otpauth://totp/example.com:test_head@example.com?", + uriExpect: "otpauth://totp/example.com:test_head@example.com?", + }, + { + title: "secret only (minimum info)", + uriInput: "otpauth://totp/example.com:test_head@example.com?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X", + uriExpect: "otpauth://totp/example.com:test_head@example.com?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&", + }, + { + title: "heading secret", + uriInput: "otpauth://totp/example.com:test_head@example.com?" + + "secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=example.com&period=45", + uriExpect: "otpauth://totp/example.com:test_head@example.com?" + + "secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=example.com&period=45", + }, + { + title: "middle secret", + uriInput: "otpauth://totp/example.com:test_middle@example.com?" + + "algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=example.com&period=45", + uriExpect: "otpauth://totp/example.com:test_middle@example.com?" + + "algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=example.com&period=45", + }, + { + title: "tailing secret (require '&' to be added)", + uriInput: "otpauth://totp/example.com:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X", + uriExpect: "otpauth://totp/example.com:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&", + }, + { + title: "tailing secret (fixed w/& only)", + uriInput: "otpauth://totp/example.com:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&", + uriExpect: "otpauth://totp/example.com:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&", + }, + { + title: "tailing secret (fixed w/single param)", + uriInput: "otpauth://totp/example.com:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo", + uriExpect: "otpauth://totp/example.com:test_tail@example.com?" + + "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:test_tail@example.com?" + + "algorithm=SHA256&digits=8&issuer=example.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo=bar", + uriExpect: "otpauth://totp/example.com:test_tail@example.com?" + + "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:test_tail@example.com?\n") + + require.Error(t, err, "malformed uri should return error") + require.Empty(t, out, "returned value should be empty on error") + }) +}