Conversation
39497c3 to
0f38c2a
Compare
|
Additionally, It would be worth changing this from just a QR image, to also linking to the otpauth url. Ie. -<p id="two-factor-qr-placeholder">QR</p>
+<a id="two-factor-qr-placeholder" href="otpauth://....">QR</a>I've tested this on mobile and desktop, and works as expected with FreeOTP, Yubico Authenticator, and the Apple Password Manager. |
That's a good idea, done in 68ed4f2 |
Co-authored-by: Dion Hulse <dion@wordpress.org>
6f33e71 to
8e01621
Compare
| <p> | ||
| <img src="<?php echo esc_url( $this->get_google_qr_code( $totp_title, $key, $site_name ) ); ?>" id="two-factor-totp-qrcode" /> | ||
| <p id="two-factor-qr-code"> | ||
| <a href="<?php echo $totp_url; ?>"> |
There was a problem hiding this comment.
This needs to be escaped with esc_url().
There was a problem hiding this comment.
It is escaped above (same w/ the other). Normally I'd escape as late as possible, but this one needs the extra param for the otpauth protocol, which would make those echos fairly long and less readable, and duplicated instead of DRY.
I don't feel strongly either way though. I'm happy to change it if you'd prefer.
Fixes #42
Closes #388 - This is an alternate approach, which addresses the concerns I raised there.
Closes #443 - There won't be an external URL to filter anymore. This has filters for the local URL, though
I don't think a PHP fallback is needed, since disabling JS is extremely rare, and the manual secret already serves that purpose.