Skip to content

Remove unreachable "otp_code" auth method#9638

Merged
aduth merged 2 commits intomainfrom
aduth-rm-otp-code-auth-method
Nov 22, 2023
Merged

Remove unreachable "otp_code" auth method#9638
aduth merged 2 commits intomainfrom
aduth-rm-otp-code-auth-method

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Nov 21, 2023

🛠 Summary of changes

Removes an unused reference to "otp_code" as a multi_factor_auth_method value, in order to avoid confusion for a value that's never logged.

I stumbled across this when searching possible multi_factor_auth_method values in the codebase. After verifying there are no results in CloudWatch for filter properties.event_properties.multi_factor_auth_method = 'otp_code', I noticed that this would be unused, due to how the form's extra analytics are overridden:

properties = result.to_h.merge(analytics_properties)

def analytics_properties
parsed_phone = Phonelib.parse(phone)
{
context: context,
multi_factor_auth_method: params[:otp_delivery_preference],

📜 Testing Plan

This is a refactor expected to be covered by existing test coverage.

rspec spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb spec/forms/otp_verification_form_spec.rb

You could also verify that MFA'ing with a phone continues to log events with multi_factor_auth_method equal to the delivery method of your authenticator ("sms" or "voice").

changelog: Internal, Analytics, Normalize auth method for phone OTP submission
@aduth aduth requested a review from a team November 21, 2023 21:50
@aduth aduth merged commit 7411e8f into main Nov 22, 2023
@aduth aduth deleted the aduth-rm-otp-code-auth-method branch November 22, 2023 14:13
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