Page MenuHomePhabricator

QR code displayed inconsistently
Closed, ResolvedPublic

Description

When enabling two-factor authentication, the QR code which is scanned for enrollment occasionally does not render. There appears to be in issue with the client-side code used to draw the code. This is confusing for users who are new to the use of Google Authenticator and other such tools as part of their authentication flow.

Here are reproduction steps that seem to work fairly consistently. They require that you not already have two-factor enabled for the account used for testing.

  1. Clear your browser cache.
  2. Login to https://en.wikipedia.org
  3. Click "Preferences"
  4. Click "Enable two-factor authentication"
  5. Observe that the QR code is displayed
  6. Click "Preferences"
  7. Click "Enable two-factor authentication"
  8. Observe that the QR code is not displayed
  9. Shift-Refresh the page
  10. Observe that the QR code is displayed
  11. Click "Preferences"
  12. Click "Enable two-factor authentication"
  13. Observe that the QR code is not displayed
  14. Click "Preferences"
  15. Click "Enable two-factor authentication"
  16. Observe that the QR code is not displayed
  17. Click "Preferences"
  18. Click "Enable two-factor authentication"
  19. Observe that the QR code is not displayed

So, my hypothesis is that this is a timing issue and has to do with loading from browser cache. It seems that the QR code is not rendered when the JavaScript need is available in local cache. Maybe. Sometimes it does render when loading from local cache. so I'm not certain.

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 3 2016, 9:37 PM
dpatrick triaged this task as Unbreak Now! priority.Jun 3 2016, 9:38 PM
Restricted Application added subscribers: Luke081515, TerraCodes, Urbanecm. · View Herald TranscriptJun 3 2016, 9:38 PM

Is there a scenario in which this can be reproduced? Or is it seemingly random?

It seems to be random, and I've experienced it in both Firefox and Chrome.

@Jalexander mentioned yesterday that he encountered this during enrollment as well, but he knew to enter the key manually. Many users will not (which is an issue to be emphasized in documentation resulting from a separate phab ticket).

Restricted Application added a subscriber: Poyekhali. · View Herald TranscriptJun 8 2016, 5:39 PM
dpatrick renamed this task from QR code displayed inconsistenly to QR code displayed inconsistently.Jun 9 2016, 1:18 AM

Could this be caused by the JavaScript creating the QRCode running before the DOM is fully loaded?

@Krinkle, do you have any ideas about this?

Bawolff claimed this task.Jun 14 2016, 8:37 PM
Tgr added a subscriber: Tgr.Jun 16 2016, 11:39 AM

Can't reproduce in Chrome nor in Firefox. Code looks sane too - the DOM placeholder is generated by serverside code, the QR image generator is loaded via ResourceLoader::makeInlineScript which places the script into the RL queue which is processed on DOM ready.

Can't reproduce in Chrome nor in Firefox. Code looks sane too - the DOM placeholder is generated by serverside code, the QR image generator is loaded via ResourceLoader::makeInlineScript which places the script into the RL queue which is processed on DOM ready.

RL queue is not necessarily processed on DOM ready. It only garuntees that RL is ready. In fact, I suspect what is happening is that on requests where the startup module is locally cached, RL is ready before <div id="qrcode"> is parsed by the browser, so $( '#qrcode' ) at the time of call returns 0 results, resulting in nothing happening.

I wonder if the reason this is impossible to reproduce locally, is that from localhost, there is no network delay and a much larger MTU (and due to output buffering, the page comes all at once), so the page wouldn't be split over multiple packets. Hence there is no time between the browser getting the instruction to load the startup module and the rest of the page.

Change 295191 had a related patch set uploaded (by Brian Wolff):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295191

Change 295191 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295191

@Bawolff Thanks a lot Brian. This is schedule for deployment in the early SWAT on June 22nd.

Change 295510 had a related patch set uploaded (by Thcipriani):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295510

Change 295511 had a related patch set uploaded (by Thcipriani):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295511

Change 295511 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295511

Change 295510 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295510

This has been deployed and appears to be working well!

Bawolff closed this task as Resolved.Jun 23 2016, 6:53 PM