Skip to content

Fix Broken Personal Keys (LG-5525)#6010

Merged
zachmargolis merged 15 commits intomainfrom
margolis-fix-broken-personal-keys-LG-5525
Mar 2, 2022
Merged

Fix Broken Personal Keys (LG-5525)#6010
zachmargolis merged 15 commits intomainfrom
margolis-fix-broken-personal-keys-LG-5525

Conversation

@zachmargolis
Copy link
Contributor

Between #5230 and #5433, users who went through the IDV flow received broken personal keys.

This PR introduces a flow that gives these users a new personal key, so they can successfully recover their data if they forget their password.

To test this out locally or in a sandbox environment, update the user like this and then log in anywhere

window_start = IdentityConfig.store.broken_personal_key_window_start
user.active_profile.update(verified_at: window_start); user.update(encrypted_recovery_code_digest_generated_at: nil)
screenshot
Screen Shot 2022-02-28 at 4 38 01 PM

Comment on lines +191 to +210
def fix_broken_personal_key_url
if current_user.broken_personal_key?
pii_unlocked = user_session[:decrypted_pii].present?

if pii_unlocked
cacher = Pii::Cacher.new(current_user, user_session)
user_session[:personal_key] = current_user.active_profile.encrypt_recovery_pii(cacher.fetch)

flash[:info] = t('account.personal_key.needs_new')
analytics.track_event(Analytics::BROKEN_PERSONAL_KEY_REGENERATED)

manage_personal_key_url
else
flash[:info] = t('account.personal_key.needs_new_password_prompt')
user_session[:needs_new_personal_key] = true

capture_password_url
end
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not in love with a plain "_url" method having a ton of side effects, especially in such a hot path... but I wasn't sure if we had better places to insert something into the "flow" after signing in but before getting redirected elsewhere

changelog: Bug Fixes, Account Recovery, Add flow to help accounts replace personal keys
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

I ran through the flow locally, but when testing the password reset flow, reactivating the profile with the new key produced an "Incorrect personal key" error for me.

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
@zachmargolis
Copy link
Contributor Author

I ran through the flow locally, but when testing the password reset flow, reactivating the profile with the new key produced an "Incorrect personal key" error for me.

Which order did you go through the password reset flow?

The problem with resetting the password, is that for these broken personal keys, the password is the only viable way to decrypt the user's data, because the old personal key is broken. So resetting the password means that we're ophaning the data... probably need to have additional warning screens or link directly to the re-verify flow

@aduth
Copy link
Contributor

aduth commented Mar 1, 2022

Which order did you go through the password reset flow?

  1. Verify account
  2. Change broken_personal_key_window_finish to time after verification, before now
  3. Log out
  4. Log in
  5. See new screen with new personal key
  6. Copy key
  7. Log out
  8. Reset password
  9. Start reactivation
  10. Enter key from Step 5
  11. See error "Incorrect personal key"

@zachmargolis
Copy link
Contributor Author

  • Copy key
  • Reset password
  • Start reactivation

Did you reset password from the account page? Or from a signed out page?

@aduth
Copy link
Contributor

aduth commented Mar 1, 2022

  • Copy key
  • Reset password
  • Start reactivation

Did you reset password from the account page? Or from a signed out page?

Signed out. Sorry, missed that in steps. Updated them.

@zachmargolis
Copy link
Contributor Author

zachmargolis commented Mar 1, 2022

I ran through the flow locally, but when testing the password reset flow, reactivating the profile with the new key produced an "Incorrect personal key" error for me.

Turns out there was a missing #save!, fixed in 3400003, thanks for the help reproducing (spec in d653a72, f733904)

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

👍

Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>
@zachmargolis zachmargolis merged commit 060e6fc into main Mar 2, 2022
@zachmargolis zachmargolis deleted the margolis-fix-broken-personal-keys-LG-5525 branch March 2, 2022 01:46
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.

2 participants