pypi / warehouse

The Python Package Index
https://pypi.org
Apache License 2.0
3.58k stars 964 forks source link

Make QR code for TOTP more specific, informative #5894

Open brainwane opened 5 years ago

brainwane commented 5 years ago

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 commented 5 years ago

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

signop commented 5 years ago

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

woodruffw commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

steiza commented 5 years ago

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:

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.