Skip to content

Log Face/Touch setup A/B test from session value#11549

Merged
aduth merged 1 commit intomainfrom
aduth-lg-14191-ft-ab-session
Nov 25, 2024
Merged

Log Face/Touch setup A/B test from session value#11549
aduth merged 1 commit intomainfrom
aduth-lg-14191-ft-ab-session

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Nov 25, 2024

🎫 Ticket

Related to LG-14191

🛠 Summary of changes

Updates logging to include an additional property in the "Multi-Factor Authentication Setup" event corresponding to whether the user was presented the recommendation for SMS users to set up Face or Touch Unlock.

A planned CloudWatch query was proposed in the content of #11496, but it did not account for all of the additional conditions that apply for whether the user was part of the test group. The approach here adds a session value to accurately compute the denominators.

Revised query:

Recommendation in account creation:

filter name in ['webauthn_platform_recommended_visited', 'Multi-Factor Authentication Setup']
| fields (name = 'webauthn_platform_recommended_visited' and properties.ab_tests.recommend_webauthn_platform_for_sms_user.bucket = 'recommend_for_account_creation') as visited,
(name = 'Multi-Factor Authentication Setup' and properties.event_properties.multi_factor_auth_method = 'webauthn_platform' and properties.event_properties.success and properties.event_properties.webauthn_platform_recommended = 'account_creation') as setup
| stats sum(visited) > 0 as visited_once, sum(setup) > 0 as setup_once by properties.user_id
| stats sum(setup_once)/sum(visited_once)

Recommendation in authentication:

filter name in ['webauthn_platform_recommended_visited', 'Multi-Factor Authentication Setup']
| fields (name = 'webauthn_platform_recommended_visited' and properties.ab_tests.recommend_webauthn_platform_for_sms_user.bucket = 'recommend_for_authentication') as visited,
(name = 'Multi-Factor Authentication Setup' and properties.event_properties.multi_factor_auth_method = 'webauthn_platform' and properties.event_properties.success and properties.event_properties.webauthn_platform_recommended = 'authentication') as setup
| stats sum(visited) > 0 as visited_once, sum(setup) > 0 as setup_once by properties.user_id
| stats sum(setup_once)/sum(visited_once)

📜 Testing Plan

rspec spec/controllers/users/webauthn_platform_recommended_controller_spec.rb spec/controllers/users/webauthn_setup_controller_spec.rb

Verify that A/B test value is included in "Multi-Factor Authentication Setup" if and only if you were actually presented recommendation:

Configure A/B test:

# config/application.yml
recommend_webauthn_platform_for_sms_ab_test_account_creation_percent: 100
  1. Run make watch_events in a separate terminal process
  2. Go to http://localhost:3000
  3. Create account up to MFA selection
  4. (Optional) Simulate a supported device using Chrome device simulation for a simulated device and reload the page
  5. Select phone as your only MFA
  6. Setup phone MFA (SMS)
  7. Observe that you see Face/Touch recommendation if you are on a supported device from Step 3. Otherwise, you'll see the standard 2nd MFA recommendation
  8. In either outcome, choose to setup Face/Touch Unlock as a second MFA method
  9. After successfully setting up Face/Touch, observe log event for "Multi-Factor Authentication Setup" in make watch_events terminal process. There should be a webauthn_platform_recommended: 'account_creation' value only if you saw the Face/Touch recommendation at Step 7

changelog: Internal, A/B Tests, Fix logging for A/B test to recommend platform authenticator to SMS users
@aduth aduth requested a review from a team November 25, 2024 13:29
Copy link
Copy Markdown
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.

Local testing behaves as promised.
Looks good!

@aduth aduth merged commit d1f9c01 into main Nov 25, 2024
@aduth aduth deleted the aduth-lg-14191-ft-ab-session branch November 25, 2024 15:39
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