-
Notifications
You must be signed in to change notification settings - Fork 166
LG-14655: A/B test to recommend platform authenticator to SMS users #11402
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
Changes from all commits
1126830
f3250dc
ea89838
3bb9f72
cd2f95b
78bee11
2cd8ef5
640f98f
4cb2340
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module RecommendWebauthnPlatformConcern | ||
| def recommend_webauthn_platform_for_sms_user?(bucket) | ||
| # Only consider for A/B test if: | ||
| # 1. Option would be offered for setup | ||
| # 2. User is viewing content in English | ||
| # 3. Other recommendations have not already been offered (e.g. PIV/CAC for federal emails) | ||
| # 4. User selected to setup phone or authenticated with phone | ||
| # 5. User has not already set up a platform authenticator | ||
| return false if !device_supports_platform_authenticator_setup? | ||
| return false if I18n.locale != :en | ||
| return false if current_user.webauthn_platform_recommended_dismissed_at? | ||
| return false if !user_set_up_or_authenticated_with_phone? | ||
| return false if current_user.webauthn_configurations.platform_authenticators.present? | ||
| ab_test_bucket(:RECOMMEND_WEBAUTHN_PLATFORM_FOR_SMS_USER) == bucket | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def device_supports_platform_authenticator_setup? | ||
| user_session[:platform_authenticator_available] == true | ||
| end | ||
|
|
||
| def in_account_creation_flow? | ||
| user_session[:in_account_creation_flow] == true | ||
| end | ||
|
|
||
| def user_set_up_or_authenticated_with_phone? | ||
| if in_account_creation_flow? | ||
| current_user.phone_configurations.any? do |phone_configuration| | ||
| phone_configuration.mfa_enabled? && phone_configuration.delivery_preference == 'sms' | ||
| end | ||
| else | ||
| auth_methods_session.auth_events.pluck(:auth_method). | ||
| include?(TwoFactorAuthenticatable::AuthMethod::SMS) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Users | ||
| class WebauthnPlatformRecommendedController < ApplicationController | ||
| include SecureHeadersConcern | ||
| include MfaSetupConcern | ||
|
|
||
| before_action :confirm_two_factor_authenticated | ||
| before_action :apply_secure_headers_override | ||
|
|
||
| def new | ||
| @sign_in_flow = session[:sign_in_flow] | ||
| analytics.webauthn_platform_recommended_visited | ||
| end | ||
|
|
||
| def create | ||
| analytics.webauthn_platform_recommended_submitted(opted_to_add: opted_to_add?) | ||
| current_user.update(webauthn_platform_recommended_dismissed_at: Time.zone.now) | ||
| redirect_to dismiss_redirect_path | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def opted_to_add? | ||
| params[:add_method].present? | ||
| end | ||
|
|
||
| def dismiss_redirect_path | ||
| if opted_to_add? | ||
| webauthn_setup_path(platform: true) | ||
| elsif in_account_creation_flow? | ||
| next_setup_path | ||
| else | ||
| after_sign_in_path_for(current_user) | ||
| end | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <% self.title = t('webauthn_platform_recommended.heading') %> | ||
|
|
||
| <%= render StatusPageComponent.new(status: :info, icon: :question) do |c| %> | ||
| <% c.with_header { t('webauthn_platform_recommended.heading') } %> | ||
|
|
||
| <p><%= t('webauthn_platform_recommended.description_secure_account') %></p> | ||
|
|
||
| <p> | ||
| <%= t( | ||
| 'webauthn_platform_recommended.description_private_html', | ||
| phishing_resistant_link_html: new_tab_link_to( | ||
| t('webauthn_platform_recommended.phishing_resistant'), | ||
| help_center_redirect_path( | ||
| category: 'get-started', | ||
| article: 'authentication-methods', | ||
| anchor: 'face-or-touch-unlock', | ||
| flow: @sign_in_flow, | ||
| step: :webauthn_platform_recommended, | ||
| ), | ||
| ), | ||
| ) %> | ||
| </p> | ||
|
|
||
| <div class="grid-row margin-top-5"> | ||
| <div class="tablet:grid-col-9"> | ||
| <%= render ButtonComponent.new( | ||
| url: webauthn_platform_recommended_url, | ||
| method: :post, | ||
| params: { add_method: true }, | ||
| big: true, | ||
| full_width: true, | ||
| class: 'margin-bottom-2', | ||
| ).with_content(t('webauthn_platform_recommended.cta')) %> | ||
| <%= render ButtonComponent.new( | ||
| url: webauthn_platform_recommended_url, | ||
| method: :post, | ||
| unstyled: true, | ||
| ).with_content(t('webauthn_platform_recommended.skip')) %> | ||
| </div> | ||
| </div> | ||
| <% end %> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,4 +87,19 @@ def self.all | |
| should_log: ['Password Reset: Password Submitted'].to_set, | ||
| buckets: { log: IdentityConfig.store.log_password_reset_matches_existing_ab_test_percent }, | ||
| ).freeze | ||
|
|
||
| RECOMMEND_WEBAUTHN_PLATFORM_FOR_SMS_USER = AbTest.new( | ||
| experiment_name: 'Recommend Face or Touch Unlock for SMS users', | ||
| should_log: [ | ||
| 'Multi-Factor Authentication', | ||
| 'User Registration: MFA Setup Complete', | ||
| 'User Registration: 2FA Setup', | ||
| ].to_set, | ||
| buckets: { | ||
| recommend_for_account_creation: | ||
| IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_account_creation_percent, | ||
| recommend_for_authentication: | ||
| IdentityConfig.store.recommend_webauthn_platform_for_sms_ab_test_authentication_percent, | ||
|
Comment on lines
+99
to
+102
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noting that having separate configurations per buckets makes local testing a little cumbersome, since they can't both be maxed out at the same time, otherwise initialization fails with "buckets exceeding 100". The solution is to set one or the other to 100 at a time and test each individually. In production, we'd have a smaller percentage, and using buckets this way allows for test candidates which are mutually exclusive from each other, which isn't strictly a requirement, but also not an issue. |
||
| }, | ||
| ).freeze | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddWebauthnPlatformRecommendedDismissedAtToUser < ActiveRecord::Migration[7.2] | ||
| def change | ||
| add_column :users, :webauthn_platform_recommended_dismissed_at, :datetime, default: nil, comment: 'sensitive=false' | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice like having this split out