Add form validation for present TOTP name#6267
Conversation
**Why**: For consistency with other MFA setup, and for consistency with the JS-enabled user experience, where the input is rendered as required. changelog: Bug Fixes, MFA Setup, Consistently validate TOTP name
Do we know what the previous version was allowing a blank name? Any thoughts on adding making the name required by the validator? Additionally, do we have auth app configs without a name? |
Yeah, my worry is that since we update the |
|
Yeah, but we could backfill them with something generic like "Authentication app 1" and then add the validation |
|
Oh wait, I totally got the red and green in this diff backwards! Since this is the setup form and the validation is not on the model it shouldn't create a problem for configurations when we update them. We should probably still backfill though so we aren't surprised to encounter a blank name |
|
Yeah this is mostly a UI consistency problem. The records themselves always have and will continue to have names associated with them, but previously we weren't requiring this to be provided by the form, and instead would default to the current date when not provided by the form. In practice, I expect this would rarely have happened in the wild, since the field is "required" and enforced by browser validation. The fact that we weren't validating previously seemed to be more for the convenience of our own specs, which typically don't (or at least didn't, prior to #6229) run with JavaScript enabled.
|
should be unique user / MFA setup between specs
* Add form validation for present TOTP name **Why**: For consistency with other MFA setup, and for consistency with the JS-enabled user experience, where the input is rendered as required. changelog: Bug Fixes, MFA Setup, Consistently validate TOTP name * Pass name parameter for controller specs * Improve specs for TOTP setup controller * Fill in name * More forms! * Try non-unique app names in specs should be unique user / MFA setup between specs
3 participants
Extracted from #6229
Why: For consistency with other MFA setup, and for consistency with the JS-enabled user experience, where the input is rendered as required.
This was discovered in #6229, where the
set_up_2fa_with_authenticator_appspec helper would not work as expected in a JavaScript-enabled browser, since the browser validation would prevent submission without a valid name.The previous implementation seemed to be intentionally allowing an absent name. While this may be reasonable if applied consistently, the revisions bring alignment to client-side and server-side validation, and across different MFA types (e.g. PIV/CAC).