Add backup codes option to account page #2819
Conversation
**How:** This commit adds a Backup Codes optiopn in the 2FA section of the account screen. A "generated" status is shown if the user has generated backup codes as a 2FA option, otherwise a "not generated" status is shown. A Manage link is provided that takes the user to the generate backup codes screen.
paolov18F
requested review from
jmhooper,
jgsmith-usds,
clantosswett,
stevegsa and
cpbgsa
|
Doesn't need to happen in this PR, but it'd be nice to be able to remove backup codes from an account. |
github says branch it out of date with master. Hope it doesn't accidentally merge my branch into master instead.
- Tests added to click Generate/Regenerate - Backup codes sedction refactored to allow for Generate/Regenerate buttons - Replace hardcoded Xpaths with translate calls
…ithub.com/18F/identity-idp into paolov18F-add-backup-codes-option-account
|
Whoa GitHub, I didn't "dismiss" any teammates contributions. |
|
It automatically dismiss them when you push new changes. The idea is that it requires the most recent set of changes to be approved. |
| context 'user clicks regenerate backup codes' do | ||
| let(:user) { create(:user, :with_backup_code, :with_piv_or_cac) } | ||
|
|
||
| it 'user can click regenerate backup codes' do |
There was a problem hiding this comment.
I think you could get rid of this context block, and move this test into the with backup codes context, right? You may even be able to save some time on running the specs if you combined this test with the shows backup code section to make shows backup code section and allows used to regenerate codes.
Also, thoughts on testing that backup codes were actually generated? You save one of the user's backup codes, and then test that it had been deleted after clicking the button, e.g.
old_backup_code = user.backup_code_configurations.sample
# do the stuff to regenerate
expect(BackupCodeConfiguration.where(id: old_backup_code.id).any?).to be_false
expect(user.reload.backup_code_configurations.count).to eq(10)
There was a problem hiding this comment.
Thanks, I like the idea of combining the shows backup code section and user can click regenerate backup codes section.
As far as testing the generated backup codes section, that seems to be already done in spec/features/two_factor_authentication/backup_code_sign_up_spec.rb. The generate and regenerate link both take the user to the page that is tested by that file. But if you feel this needs to be tested in backup_codes_spec as well, I can do that.
For now I modified the test to at least check that the user is actually taken to the backup code setup page upon click of Regenerate, after the check for old backup code being deleted. User has to click Continue to generate 10 new codes, but as mentioned that is covered in the other feature test.
There was a problem hiding this comment.
Yeah, they go to the same place now, but they are not guaranteed to do that forever. We also aren't guaranteed that it won't have some sort of bug when called outside the sign up flow. I think we want some coverage here to make sure we are confident this works at the feature level.
|
I had one comment on how to shuffle some things around in the tests and get some extra coverage. After that, I think this one is ready to go 🚀 |
* **Why:** So that users can generate backup codes as a 2FA option. **How:** This commit adds a Backup Codes optiopn in the 2FA section of the account screen. A "generated" status is shown if the user has generated backup codes as a 2FA option, otherwise a "not generated" status is shown. A Manage link is provided that takes the user to the generate backup codes screen.
Why: So that users can generate backup codes as a 2FA option.
How: This commit adds a Backup Codes optiopn in the 2FA section of the
account screen. A "generated" status is shown if the user has generated
backup codes as a 2FA option, otherwise a "not generated" status is shown.
A Manage link is provided that takes the user to the generate backup codes
screen.