Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor backup code verification to follow conventional form pattern #11089

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Aug 15, 2024

🎫 Ticket

Supports LG-14078

🛠 Summary of changes

Updates backup code verification to use the conventional form pattern:

  • Validation should use ActiveModel#valid? in combination with validation methods appending errors as relevant
  • Include errors in FormResponse return value
  • Use errors from form response result to display in user interface
  • Remove unused BackupCodeGenerator#verify method and update specs to ensure coverage for actively-used methods

To simplify review, consider hiding whitespace changes: https://github.com/18F/identity-idp/pull/11089/files?w=1

📜 Testing Plan

Verify that you can sign in with backup codes:

  1. Prerequisite: Have an account with backup codes where you have a record of a usable code
  2. Go to http://localhost:3000
  3. SIgn in
  4. If not prompted for MFA, click "Forget all browsers" from account dashboard, confirm, sign out, then start from Step 3
  5. When prompted for MFA, if prompted for an MFA other than backup codes, select "Choose another authentication method" and select "Backup codes"
  6. When prompted for backup codes, enter a valid backup code and submit
  7. Observe no regressions in being signed in

Verify that updated specs pass:

rspec spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb spec/forms/backup_code_verification_form_spec.rb spec/services/backup_code_generator_spec.rb

changelog: Internal, Code Quality, Refactor backup code verification to follow conventional form pattern
@aduth aduth requested a review from a team August 15, 2024 15:55
Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Local testing OK - no errors.

@aduth aduth merged commit cf00082 into main Aug 15, 2024
2 checks passed
@aduth aduth deleted the aduth-backup-code-verification-form-consistency branch August 15, 2024 20:11
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