Problem/Motivation
Google Authenticator link is broken.
Steps to reproduce
- Enable TFA
- Navigate to view profile
- Click on TFA tab
- Click on Set up application link
- Click on Google Authenticator link
Proposed resolution
- Update link to include options for Android and iOS
- Change ‘Authy’ to ‘Twilio Authy’ to avoid confusion/make sure people don’t download the wrong app
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3386910-7.patch | 1.92 KB | bhanu951 |
| #5 | drupal.org_files_issues_2023-09-13_authenticator-links-3386910-5.patch | 2.52 KB | yeniatencio |
| #3 | authenticator-links-3386910-3.patch | 2.71 KB | yeniatencio |
| #2 | authenticator-links-3386910-2.patch | 2.66 KB | yeniatencio |
Issue fork tfa-3386910
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
yeniatencio commentedPlease review patch.
Comment #3
yeniatencio commentedPlease review patch for 2.x version
Comment #4
cmlaraThis link appears to have tracking data that should not be included.
Same as the HOTP comment.
Why the removal?
Adding new records should probably be done in a separate feature request. I'm not sure what the existing entries had to go through to get on the list, but considering were talking security tokens I don't personally want to add any new entries without a discussion about them on why they should be on the list (we obviously can't and shouldn't list every token that exists in this help link list.)
Comment #5
yeniatencio commentedThanks cmlara. I have removed the tracking data from the links as point out.
Uploading new patch for version 8.x-1.x
Regarding, adding new links, completely makes sense. However, adding those for a very specific use case on my side.
Comment #6
cmlara@yeniatencio It general it is helpful if patches don't have local environment use changes in them.
Just in case you mean you are using D.O. to host patches that you use as part of your composer deployment I will note this is generally not recommended for both security and stability reasons. D.O could be compromised at any time meaning patches hosted on D.O. could pose a risk to a site, additionally there have been times during security windows where load has caused D.O. to go offline preventing deployment.
Comment #7
bhanu951 commentedI am resubmitting my patch from #3395254: Wrong domain name for Google Authenticator on TOTP and HOTP setup page as it was closed as duplicate.
Please review the patch. It has different links from the other patches submitted here earlier.
Comment #8
cmlaraThis will still need the Twillo name change from the original patch as that is in scope.
As for the links:
At first I was inclined for a single link of the first patch to keep the UI less cluttered. however reading the text on the KB page, and acknowledging it is a KB not a product page I think the links provided by @Bhanu951 are the better choice.
Side note:
We recently switched to Fully GitlabCi so we can’t actually test patch files in this project going forward.
Comment #10
bhanu951 commentedUpdated the patch addressing the review comments and created the new MR.
Comment #11
bhanu951 commentedUpdated the MR addressing the review comments please re-review.
Comment #13
cmlaraCommitted to 2.x, doesn't apply cleanly to 8.x-.1.x.
Comment #14
bhanu951 commentedComment #16
bhanu951 commentedPorted patch to 8.x-.1.x please review MR #55
Comment #18
cmlaraLGTM, merged to dev.
Thank you!