Skip to content

Commit

Permalink
Include issuer in TOTP provisioning URL as well as query string
Browse files Browse the repository at this point in the history 
According the google authenticator docs the issuer should be
included both in the URL itself as well as in the query
string. See:
https://github.com/google/google-authenticator/wiki/Key-Uri-Format#issuer
  • Loading branch information
@sbc100
sbc100 committed Apr 18, 2016
1 parent ff5ebaf commit 07825ec
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
9 changes: 7 additions & 2 deletions lib/rotp/totp.rb
View file
Expand Up @@ -51,9 +51,14 @@ def verify_with_drift(otp, drift, time = Time.now)
# This can then be encoded in a QR Code and used
# to provision the Google Authenticator app
# @param [String] name of the account
# @return [String] provisioning uri
# @return [String] provisioning URI
def provisioning_uri(name)
encode_params("otpauth://totp/#{URI.encode(name)}",
# The format of this URI is documented at:
# https://github.com/google/google-authenticator/wiki/Key-Uri-Format
# For compatibility the issuer appears both before that account name and also in the
# query string.
issuer_string = issuer.nil? ? "" : "#{URI.encode(issuer)}:"

This comment has been minimized.

Sign in to view

Copy link
@reedloden

reedloden May 18, 2016

This looks much better with an ending space. Can we add one? I guess %20 directly since it's not going to be encoded.

This comment has been minimized.

Sign in to view

Copy link
@sbc100

sbc100 May 19, 2016

Author Contributor

Sorry, a space where? Any where does it look better? In the Google Authenticator UI? I've not seen %20 in URI in the wild, have you?

This comment has been minimized.

Sign in to view

Copy link
@reedloden

reedloden May 19, 2016

A space after the colon. Basically, if the issuer is Foo, the label would be "Foo: bar@example.com"

This looks much better than "Foo:bar@example.com", especially since it looks like the "Foo:" is part of the e-mail address (or username or whatever the label is).

As far as %20 in URI, I have in a few places. Slack would be one example.

Here's Google Authenticator's UI (random image search found this... just see how it would look). Just visualize what "Foo:bar@example.com" looks like compared to "Foo: bar@example.com".

Example

This comment has been minimized.

Sign in to view

Copy link
@sbc100

sbc100 May 20, 2016

Author Contributor

When I tested this, the Google Authenticator app will split the string, so the "Foo" will apear above the code and the bare email address will appear below. For example the URL used to see your google account about would be Google:steve@sterndata.com (with no space). We I don't think we need/want the space.

This comment has been minimized.

Sign in to view

Copy link
@reedloden

reedloden May 20, 2016

I'm not sure what you mean... Google Authenticator isn't doing any splitting of the string as far as I can tell in my testing locally. The part above the code is the issuer (set as a query parameter) and the bottom part is the label (which you're setting to be ":".

otpauth://totp/Foo:bar@example.com?secret=abcdefghij&issuer=Foo
... results in 'Foo' above the code and 'Foo:bar@example.com' below the code.

Are you seeing something different? I'm testing with Google Authenticator on iOS.

This comment has been minimized.

Sign in to view

Copy link
@mdp

mdp May 20, 2016

Owner

OK, so I played around with this on Google Authenticator for iOS. It's a bit weird, but with the default ROTP provisioning URL it looks OK to me.

Here's three different provisioning URLs

972ed285-2894-48b3-9284-a37ed8ff2411

  1. Provisioning URL with an issuer param and issuer prefix, both the same.
  2. Provisioning URL with an issue prefix, no issuer param.
  3. Provisioning URL with an issuer params and issuer prefix, both different values

Using ROTP with the default settings, you get the the first case which seems fine in my version of Google Authenticator (v3.0.0) for iOS

This comment has been minimized.

Sign in to view

Copy link
@sbc100

sbc100 May 21, 2016 •
edited

Author Contributor

Yes, the first case it what we want and what is recommended and used by Google themselves:
https://github.com/google/google-authenticator/wiki/Key-Uri-Format

In other words the issuer should be in the URL both as a prefix and as a parameter and the value should be the same. That is what this change implements. It always does (1), right?

(2) and (3) are both wrong and should never be generated by this code.
encode_params("otpauth://totp/#{issuer_string}#{URI.encode(name)}",
:secret => secret, :period => (interval==30 ? nil : interval), :issuer => issuer)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/lib/rotp/totp_spec.rb
View file
Expand Up @@ -101,7 +101,7 @@
let(:totp) { ROTP::TOTP.new 'JBSWY3DPEHPK3PXP', issuer: 'FooCo' }

it 'has the correct format' do
expect(uri).to match %r{\Aotpauth:\/\/totp.+}
expect(uri).to match %r{\Aotpauth:\/\/totp/FooCo:.+}
end

it 'includes the secret as parameter' do
Expand Down

0 comments on commit 07825ec

Please sign in to comment.