Skip to content

LG-13795: Deemphasize backup codes#10970

Merged
aduth merged 20 commits intomainfrom
aduth-lg-13795-backup-codes
Jul 30, 2024
Merged

LG-13795: Deemphasize backup codes#10970
aduth merged 20 commits intomainfrom
aduth-lg-13795-backup-codes

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jul 19, 2024

🎫 Ticket

LG-13795

🛠 Summary of changes

Updates backup codes:

  • Reorders MFA setup options to list backup codes as the last option
  • Updates backup codes setup description to discourage use
  • Reduces number of backup codes generated from 10 to 5

Draft pending confirmation of reduction of number of backup codes (see Slack discussion) Per Slack discussion, keeping count to 10.

📜 Testing Plan

  1. Go to http://localhost:3000
  2. Click "Create an account"
  3. Continue account creation until MFA selection
  4. Observe that "Backup codes" is listed last, and has updated description text
  5. Select "Backup codes"
  6. Click "Continue"
  7. Observe that you are given 5 backup codes instead of 10

👀 Screenshots

Screen Before After
MFA Setup before after
Backup codes image image

@aduth
Copy link
Contributor Author

aduth commented Jul 22, 2024

As discussed in Slack and team standup, we're going to hold off on reducing the number of backup codes for now, and limit the scope of this to content and ordering changes for backup code setup.

@aduth aduth marked this pull request as ready for review July 22, 2024 18:30
@aduth aduth requested a review from a team July 22, 2024 18:30
@aduth aduth force-pushed the aduth-lg-13795-backup-codes branch from f573e09 to 03bc73e Compare July 24, 2024 17:43
@aduth
Copy link
Contributor Author

aduth commented Jul 25, 2024

Based on failing build, numbers_and_words gem seems to be having an unexpected conflict on pluralization. Will spend some time poking at it, but could resort to reverting back to Humanize if there's not a clear solution.

@zachmargolis
Copy link
Contributor

zachmargolis commented Jul 25, 2024

Based on failing build, numbers_and_words gem seems to be having an unexpected conflict on pluralization. Will spend some time poking at it, but could resort to reverting back to Humanize if there's not a clear solution.

Looks like the the gem forces the I18n::Backend::Pluralization to be included in all I18n::Simple backends, which affects ours
https://github.com/kslazarev/numbers_and_words/blob/91f0f27886face3ea78aaae2222198e027bbc329/lib/numbers_and_words/i18n/pluralization.rb#L16

And it seems that plural rules aren't included by default so I added 488189749c411dfdf4e611f9b893d98c4498eb5f to give that a shot

@zachmargolis
Copy link
Contributor

Based on failing build, numbers_and_words gem seems to be having an unexpected conflict on pluralization. Will spend some time poking at it, but could resort to reverting back to Humanize if there's not a clear solution.

Looks like the the gem forces the I18n::Backend::Pluralization to be included in all I18n::Simple backends, which affects ours https://github.com/kslazarev/numbers_and_words/blob/91f0f27886face3ea78aaae2222198e027bbc329/lib/numbers_and_words/i18n/pluralization.rb#L16

And it seems that plural rules aren't included by default so I added 4881897 to give that a shot

To add additional clarity, here's an important snippet from the docs of I18n::Backend::Pluralization

You also need to make sure to provide pluralization algorithms to the
backend, i.e. include them to your I18n.load_path accordingly.

@aduth aduth changed the title LG-13795: Deemphasize and reduce number of backup codes LG-13795: Deemphasize backup codes Jul 30, 2024
aduth and others added 4 commits July 30, 2024 09:16
More languages, possible future use-case of gendered numbers
See: #10970 (comment)

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
**Why**: The numbers_and_words gem forces the I18n::Backend::Pluralization
which requires locales to have plural rules defined, so this commit adds
those rules
@aduth aduth force-pushed the aduth-lg-13795-backup-codes branch from 4881897 to 89b8cfe Compare July 30, 2024 13:16
@aduth
Copy link
Contributor Author

aduth commented Jul 30, 2024

Thanks for pushing up those changes @zachmargolis . Makes sense to me to include!

@aduth aduth merged commit ae28b8f into main Jul 30, 2024
@aduth aduth deleted the aduth-lg-13795-backup-codes branch July 30, 2024 13:30
mitchellhenke pushed a commit that referenced this pull request Jul 31, 2024
* Reorder backup codes to be last option

* Use NUMBER_OF_CODES to avoid magic numbers

* Reduce backup codes from 10 to 5

* Update description for backup code text

* Add changelog

changelog: User-Facing Improvements, Backup Codes, Deemphasize and reduce number of backup codes

* Fix display of odd number backup codes in setup

* Avoid persisting codes to database

* Fix hard-coded references to 10 codes

* Interpolate spelled-out backup codes count

* Revert to 10 backup codes

* Fix 10 spelled out in French

Forgot to save!

* Force string numeric for Chinese spelled-out numbers

* Mark strings used

* Interpolate numbers spelled out as string

Avoid type issues with NUMBER_OF_CODES being numeric, but typical usage of t expecting string key

* Stub faked string for odd number of codes

* Use humanize gem with service class to convert readable numbers

See: #10970 (comment)

* Swap humanize with numbers_and_words

More languages, possible future use-case of gendered numbers

* Avoid mutating reference of available_locales

* Use condition for control flow

See: #10970 (comment)

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Add plurals rules

**Why**: The numbers_and_words gem forces the I18n::Backend::Pluralization
which requires locales to have plural rules defined, so this commit adds
those rules

---------

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants