Feature: Enables email TFA and adds override for password reset form #86
Conversation
nathanielwoodland
commented
Dec 21, 2023
•
nathanielwoodland
commented
- Enables email TFA and adds override for password reset form
- Removes default reroute_email override from settings.php and moves them into config split
| @@ -5,7 +5,7 @@ subject: 'One Time Password' | |||
| body: "Dear [user:name],\r\n \r\nYour One Time Passcode for completing your NY Senate TFA Verification is: [user:email_tfa]\r\n \r\nPlease use this Passcode to complete your transaction. Do not share this Passcode with anyone.\r\n \r\nThank you,\r\n[site:name] team" | |||
| routes: "email_tfa.verifiy\r\nuser.logout" | |||
| timeouts: 300 | |||
| status: false | |||
| status: true | |||
There was a problem hiding this comment.
Is this turning on TFA for local environments?
There was a problem hiding this comment.
@routinet It seems that way to me. I wonder if it was just a mistake testing locally. I think it would be safe to change to false. Would you like me to make that change?
There was a problem hiding this comment.
Our goal is to ensure TFA is always enabled on live for all privileged users (e.g., LC, MCP, admins). test should probably have TFA enabled, though the reroute_email address should be set to our dev gmail account ([email protected]). For all other environments, TFA should be disabled.
web/modules/custom/nys_registration/src/Controller/NysUserController.php
Show resolved
Hide resolved
| * {@inheritdoc} | ||
| */ | ||
| public static function create(ContainerInterface $container) { | ||
| $instance = new static( |
There was a problem hiding this comment.
can replace with $instance = parent::create($container); ?
There was a problem hiding this comment.
FWIW I believe I meant to do this, but I'd directly copied the parent create logic into the new create method while debugging something. Anyway, great catch! Future me thanks you for not letting me introduce a future upgrade bug.
|
Okay @routinet and @aheaphy, this PR has been updated with everything discussed in Slack. Feel free to add your reviews. |
There was a problem hiding this comment.
Is this turning TFA on for local environments?
There was a problem hiding this comment.
@routinet we should probably delete the develop multi-dev and anything associated with this environment ... can you think of any reason to keep?
There was a problem hiding this comment.
only the warm+fuzzy feelings I have when I think about it.
kzalewski
deleted the
feature/aten-nys-58--enable-email-tfa-and-add-override-for-pass-reset
branch
