Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
The sms tfa_user_setting is a hold over from when the source of the ga_login module spun off from tfa_basic which had a SMS plugin.
As we have never had this feature it is not a setting that belongs in the tfa_user_settings. Even if we did add a SMS plugin it would belong as a plugin with the plugin storing its own user data.
Steps to reproduce
https://git.drupalcode.org/project/tfa/-/blob/44f4dd3d0f9fd1594dfca6d106...
Proposed resolution
Remove the sms setting from user data, including an update hook.
Remaining tasks
Patch
User interface changes
None
API changes
None
Data model changes
Removal of the sms setting from tfa_user_settings user data.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | after-fixes_tfa.mp4 | 485.8 KB | tirupati_singh |
| #11 | before-fixes_tfa.mp4 | 304.47 KB | tirupati_singh |
Issue fork tfa-3386547
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:
changes
Comments
Comment #4
sarwan_verma commentedHi @cmlara,
I have fixed this issue Remove 'sms' from tfa_user_settings also created the patch ,
please review and verify.
Comment #5
sarwan_verma commentedComment #6
cmlara@kalash-j This still needs an update hook.
@sarwan I'm not sure how you generated your patch, however it appears to have regressions present.
Comment #7
kalash-j commented@cmlara created update hook for MR!37
Comment #8
cmlaraI have a few concerns, especially around the new PHPStan warnings, the lack of batch usage in the update hook and the lack of testing to go with it.
I apparently had a version of this already in my local repo that I never pushed that resolved most of the concerns. I've pushed it up to the MR, however I don't honestly recall if there was anything left to do with it.
I may have been distracted by the 8.x-1.5 security fix, or the other security code issues or I may have been distracted working on the Core bug that UserData returns strings instead of integers.
Comment #11
tirupati_singh commentedHi, I've applied the MR !37 as a patch and it applied with no error. The module is working fine after applying the patch and not getting any issue while using two factor authentication for login. As the MR is resolving the issue, hence moving the issue status to RTBC. I'm also attaching the video clips for before and after fixes of the issue.
Comment #12
cmlaraCommiting to dev.
As this is minor I'm going to skip back-porting to 1.x.