Skip to content

Log errors for profile reactivate password submitted#11822

Merged
aduth merged 1 commit intomainfrom
aduth-verify-password-form-pkey
Feb 10, 2025
Merged

Log errors for profile reactivate password submitted#11822
aduth merged 1 commit intomainfrom
aduth-verify-password-form-pkey

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 30, 2025

🛠 Summary of changes

This pull request changes VerifyPasswordForm to avoid assigning personal_key on FormResponse#extra and instead assign as an instance variable accessible through a new attribute reader.

Why?

  • extra is intended to be used for analytics logging and should exclude sensitive information (docs). Personal key is a very sensitive value, and while we've taken steps to ensure that it isn't logged, the previous implementation creates a lot of unnecessary risk that it could accidentally be logged in future changes.
  • The aforementioned "taken steps to ensure that it isn't logged" meant that we were only logging success values for this analytics event. By removing personal key from extra, we can log the entire response using FormResponse#to_h, including errors.

📜 Testing Plan

Verify that the build passes.

Verify using make watch_events that submitting password after reactivating a profile disabled due to password reset logs the reactivate_account_verify_password_submitted event (a) without personal_key and (b) with error_details if the submission is unsuccessful.

@aduth
Copy link
Contributor Author

aduth commented Jan 30, 2025

This approach is also exactly what's documented in the aforementioned Forms documentation:

For sensitive properties, or results that are not meant to be logged, add properties to the Form object that get written during #submit

@aduth aduth changed the title Log errors for profile reactivity password submitted Log errors for profile reactivate password submitted Jan 31, 2025
changelog: Internal, Analytics, Log errors for profile reactivity password submitted
@aduth aduth force-pushed the aduth-verify-password-form-pkey branch from e074ab2 to f5cf243 Compare February 10, 2025 19:17
@aduth aduth merged commit 583006d into main Feb 10, 2025
2 checks passed
@aduth aduth deleted the aduth-verify-password-form-pkey branch February 10, 2025 19:51
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