-
Notifications
You must be signed in to change notification settings - Fork 166
Allow users to opt back in to SMS (LG-3510) #5894
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
4e4f226
9ebe3eb
bf1ca7d
1ef9d29
b742293
6cf91b2
dc2fc91
49cdbc0
fee951e
1c8a1f7
6bdfdd7
cd4af05
7bd1b8a
0721a26
b51998d
2ffa2de
0437c3b
6392f14
94832dd
67d453c
9491d99
fa3ce4e
b1c8e13
68611b4
d56de54
5fb5e62
3210777
cb760a9
bbf9a1b
c54b271
0d58a93
f531499
bb47f7b
4bad8d9
9549023
173242d
045ea91
08b433d
6c40f42
77393d8
59b5264
106babb
26c828d
bb6c51f
f8fa228
080d82f
b134932
ee2f222
2245973
1d85127
19bd04a
78d8663
1a731d3
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,97 @@ | ||
| module TwoFactorAuthentication | ||
| class SmsOptInController < ApplicationController | ||
| before_action :load_phone_configuration | ||
|
|
||
| def new | ||
| @other_mfa_options_url = other_options_mfa_url | ||
| @cancel_url = cancel_url | ||
|
|
||
| analytics.track_event( | ||
| Analytics::SMS_OPT_IN_VISIT, | ||
| new_user: new_user?, | ||
| has_other_auth_methods: has_other_auth_methods?, | ||
| phone_configuration_id: @phone_configuration.id, | ||
| ) | ||
| end | ||
|
|
||
| def create | ||
| response = opt_out_manager.opt_in_phone_number(@phone_configuration.formatted_phone) | ||
|
|
||
| analytics.track_event( | ||
| Analytics::SMS_OPT_IN_SUBMITTED, | ||
| response.to_h.merge( | ||
| new_user: new_user?, | ||
| has_other_auth_methods: has_other_auth_methods?, | ||
| phone_configuration_id: @phone_configuration.id, | ||
| ), | ||
| ) | ||
|
|
||
| if response.success? | ||
| redirect_to otp_send_url(otp_delivery_selection_form: { otp_delivery_preference: :sms }) | ||
| else | ||
| @other_mfa_options_url = other_options_mfa_url | ||
| @cancel_url = cancel_url | ||
|
|
||
| if !response.error | ||
| # unsuccessful, but didn't throw an exception: already opted in last 30 days | ||
| render :error | ||
| else | ||
| # one-off error, show form so users can try again | ||
| flash[:error] = t('two_factor_authentication.opt_in.error_retry') | ||
| render :new | ||
| end | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def opt_out_manager | ||
| Telephony::Pinpoint::OptOutManager.new | ||
| end | ||
|
|
||
| def mfa_context | ||
| @mfa_context ||= MfaContext.new(current_user) | ||
| end | ||
|
|
||
| def load_phone_configuration | ||
| if user_session.present? && (unconfirmed_phone = user_session[:unconfirmed_phone]).present? | ||
| @phone_configuration = PhoneConfiguration.new(phone: unconfirmed_phone) | ||
| elsif user_session.present? && (phone_id = user_session[:phone_id]).present? | ||
| @phone_configuration = mfa_context.phone_configuration(phone_id) | ||
| else | ||
| render_not_found | ||
| end | ||
| end | ||
|
|
||
| def other_options_mfa_url | ||
| if new_user? | ||
| two_factor_options_path | ||
| elsif has_other_auth_methods? && !user_fully_authenticated? | ||
| login_two_factor_options_path | ||
| end | ||
| end | ||
|
|
||
| def cancel_url | ||
| if user_fully_authenticated? | ||
| account_path | ||
| elsif decorated_session.sp_name | ||
| return_to_sp_cancel_path | ||
| else | ||
| sign_out_path | ||
| end | ||
| end | ||
|
|
||
| def has_other_auth_methods? | ||
| two_factor_configurations. | ||
| any? { |config| config.mfa_enabled? && config != @phone_configuration } | ||
| end | ||
|
|
||
| def new_user? | ||
| two_factor_configurations.none? | ||
| end | ||
|
|
||
| def two_factor_configurations | ||
| @two_factor_configurations ||= mfa_context.two_factor_configurations | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,6 +177,12 @@ def handle_telephony_result(method:, default:) | |
| otp_make_default_number: default, | ||
| reauthn: reauthn?, | ||
| ) | ||
| elsif @telephony_result.error.is_a?(Telephony::OptOutError) && | ||
| IdentityConfig.store.sms_resubscribe_enabled | ||
| # clear message from https://github.com/18F/identity-idp/blob/7ad3feab24f6f9e0e45224d9e9be9458c0a6a648/app/controllers/users/phones_controller.rb#L40 | ||
| flash.delete(:info) | ||
| user_session[:phone_id] = phone_configuration.id if phone_configuration&.phone | ||
| redirect_to login_two_factor_sms_opt_in_path | ||
|
Comment on lines
+184
to
+185
Contributor
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. If the first line is unable to assign to the session due to a i.e.
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. There are other cases where we have In an ideal world, I'd love to throw a |
||
| else | ||
| invalid_phone_number(@telephony_result.error, action: action_name) | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| <%= image_tag asset_url('alert/fail-x.svg'), | ||
| alt: t('errors.alt.error'), | ||
| width: 54, | ||
| class: 'margin-bottom-2' %> | ||
|
|
||
| <%= render PageHeadingComponent.new.with_content(t('two_factor_authentication.opt_in.title')) %> | ||
|
|
||
| <p> | ||
| <%= t( | ||
| 'two_factor_authentication.opt_in.opted_out_last_30d_html', | ||
| phone_number: content_tag(:strong, @phone_configuration.masked_phone), | ||
| ) %> | ||
| </p> | ||
|
|
||
| <p><%= t('two_factor_authentication.opt_in.wait_30d_opt_in') %></p> | ||
|
|
||
| <%= render( | ||
| 'shared/troubleshooting_options', | ||
|
Contributor
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. More a UX question, but I sorta wonder if the "Can't use your phone?" fallback should be folded into the list of troubleshooting options.
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. I think there's a case for keeping it where it is, because that one still keeps you in what you're doing, and the troubleshooting options are more like exit opportunities
Contributor
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.
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. Oh those are good points... 📡🦇🌕 @nickttng! If you have any thoughts on combining links for the error page
Contributor
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. @zachmargolis I think combining is a great way. 👍
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. Updated in 2245973, error page looks like this now:
Contributor
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. @zachmargolis Should we update "Need immediate assistance? Here's how to get help" to "Having trouble? Here's what you can do"? With the "Choose another authentication method" text now folded into the section, the "Need immediate assistance..." might not make sense with it. And "Having trouble..." text seems to encompass more of the potential solutions.
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. Sure, updated in 19bd04a
Contributor
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. Looks good 👍🏼 |
||
| heading_tag: :h3, | ||
| heading: t('components.troubleshooting_options.default_heading'), | ||
| options: [ | ||
| @other_mfa_options_url && { | ||
| url: @other_mfa_options_url, | ||
| text: t('two_factor_authentication.login_options_link_text'), | ||
| }, | ||
| decorated_session.sp_name && { | ||
| url: return_to_sp_cancel_path(location: 'sms_resubscribe_error'), | ||
| text: t('idv.troubleshooting.options.get_help_at_sp', sp_name: decorated_session.sp_name), | ||
| }, | ||
| { | ||
| url: MarketingSite.contact_url, | ||
| text: t('links.contact_support', app_name: APP_NAME), | ||
| }, | ||
| ].select(&:present?), | ||
| ) %> | ||
|
|
||
| <%= render 'shared/cancel', link: sign_out_path %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <%= image_tag asset_url('alert/warning-lg.svg'), | ||
| alt: t('errors.alt.warning'), | ||
| width: 54, | ||
| class: 'margin-bottom-2' %> | ||
|
|
||
| <%= render PageHeadingComponent.new.with_content(t('two_factor_authentication.opt_in.title')) %> | ||
|
|
||
| <p> | ||
| <%= t( | ||
| 'two_factor_authentication.opt_in.opted_out_html', | ||
| phone_number: content_tag(:strong, @phone_configuration.masked_phone), | ||
| ) %> | ||
| </p> | ||
|
|
||
| <%= link_to t('two_factor_authentication.mobile_terms_of_service'), | ||
| MarketingSite.messaging_practices_url, | ||
| class: 'margin-top-2' %> | ||
|
|
||
| <%= button_to t('forms.buttons.send_security_code'), | ||
| login_two_factor_sms_opt_in_path, | ||
| class: 'usa-button usa-button--wide usa-button--big', | ||
| form_class: 'margin-y-5' %> | ||
|
|
||
| <% if @other_mfa_options_url %> | ||
| <h2 class="margin-top-5"><%= t('two_factor_authentication.opt_in.cant_use_phone') %></h2> | ||
|
|
||
| <%= link_to t('two_factor_authentication.login_options_link_text'), | ||
| @other_mfa_options_url %> | ||
| <% end %> | ||
|
|
||
| <%= render 'shared/cancel', link: @cancel_url %> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| module Telephony | ||
| module Pinpoint | ||
| class OptOutManager | ||
| # @return [Response] | ||
| def opt_in_phone_number(phone_number) | ||
| Telephony.config.pinpoint.sms_configs.each do |config| | ||
| client = build_client(config) | ||
| next if client.nil? | ||
|
|
||
| opt_in_response = client.opt_in_phone_number(phone_number: phone_number) | ||
|
|
||
| return Response.new(success: opt_in_response.successful?) | ||
zachmargolis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| rescue Aws::SNS::Errors::InvalidParameter | ||
| # This is thrown when the number has been opted in too recently | ||
| return Response.new(success: false) | ||
| rescue Seahorse::Client::NetworkingError, | ||
| Aws::SNS::Errors::ServiceError => error | ||
| PinpointHelper.notify_pinpoint_failover( | ||
| error: error, | ||
| region: config.region, | ||
| channel: :notification_service, | ||
| extra: {}, | ||
| ) | ||
| end | ||
|
|
||
| PinpointHelper.handle_config_failure(:notification_service) | ||
| end | ||
|
|
||
| # @api private | ||
| # @param [PinpointSmsConfig] config | ||
| # @return [nil, Aws::SNS::Client] | ||
| def build_client(config) | ||
| credentials = AwsCredentialBuilder.new(config).call | ||
| return if credentials.nil? | ||
| Aws::SNS::Client.new( | ||
| region: config.region, | ||
| retry_limit: 0, | ||
| credentials: credentials, | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| end | ||




Uh oh!
There was an error while loading. Please reload this page.