Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make QR code for TOTP more specific, informative #5894

Open
brainwane opened this issue May 21, 2019 · 6 comments
Open

Make QR code for TOTP more specific, informative #5894

brainwane opened this issue May 21, 2019 · 6 comments

Comments

@brainwane
Copy link
Contributor

What's the problem this feature will solve?
The QR code (for Duo Mobile, at least) produces an entry with just the username, which is ambiguous to the user.

Describe the solution you'd like
More helpful would be, e.g., test.pypi.org-$username to show which PyPI (and really which site) the account is for, AND which account it is.

Additional context
Submitted at the PyCon North America sprints by signop who is probably @signop; thank you for testing!

cc @steiza

@brainwane
Copy link
Contributor Author

@di @woodruffw If this is easy, let's try to get it in before we wrap up the OTF milestone?

@signop
Copy link

signop commented May 23, 2019

Just confirming over here too that #5809 is the same thing I was after with my request. Thanks all!

@woodruffw
Copy link
Member

woodruffw commented May 24, 2019

Hmm, so this is how we're generating the TOTP provisioning URI:

otp.generate_totp_provisioning_uri(
    totp_secret,
    self.request.user.username,
    issuer_name=self.request.registry.settings["site.name"],
)

Which yields a URI like this:

otpauth://totp/Warehouse:foobar?digits=6&secret=TOTP_SECRET_HERE&algorithm=SHA1&issuer=Warehouse&period=30

So, TOTP applications should have enough information to unambiguously identify (Warehouse, username) tuples. For example, this is what I get with Google Authenticator:

IMG_1123

Whereas for Duo:

IMG_1124

Fixing this is a bit of a double-edged sword: we could change the otpauth username field to something like Warehouse-$username, but that would probably lead to some confusing presentations on TOTP applications that parse the otpauth:// scheme correctly (like Google Authenticator and FreeOTP).

Edit: For reference: https://github.com/google/google-authenticator/wiki/Key-Uri-Format

@woodruffw
Copy link
Member

Somewhat related: we can change the issuer name from site.name to warehouse.domain/request.domain to present to the user as "pypi.org" or "test.pypi.org" instead of just "Warehouse". But that doesn't improve things for TOTP applications that don't respect the issuer name/parameter.

@brainwane
Copy link
Contributor Author

@steiza do I recall correctly that there's an update on the Duo Mobile side that partially addresses this issue?

@steiza
Copy link

steiza commented Jul 19, 2019

Oops! Sorry, I'm not sure how I missed this back in May.

I haven't worked on the mobile app, but my understanding is that if the URI contains issuer=PyPI (as https://pypi.org currently does), then it will show up in Duo Mobile with:

  • the PyPI logo
  • the first line will read "PYPI" (sorry for the capitalization - the first line in the Duo Mobile app is always all uppercase)
  • the second line will be the username

If the issuer is set to any other value, like TestPyPI (as http://test.pypi.org did when I last tested it), or Warehouse, then the first line will read "THIRD-PARTY" and there won't be a logo.

This isn't ideal, but it shouldn't be confusing to users of pypi.org. I can definitely see how this would be confusing to users of test.pypi.org.

@brainwane brainwane removed this from the OTF Security work milestone Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants