Skip to content

Remove unused Authentication Presenter data #7868

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.

Sign up for GitHub

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

Merged
mitchellhenke merged 3 commits into from
Feb 22, 2023
Merged

Remove unused Authentication Presenter data #7868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c86a614
Remove unused Authentication Presenter data
Feb 22, 2023
f0addec
remove unused pivcac email data
Feb 22, 2023
455b986
remove more unused methods
Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions app/controllers/concerns/two_factor_authenticatable_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,6 @@ def otp_expiration
current_user.direct_otp_sent_at + TwoFactorAuthenticatable::DIRECT_OTP_VALID_FOR_SECONDS
end

def personal_key_unavailable?
current_user.encrypted_recovery_code_digest.blank?
end

def user_opted_remember_device_cookie
cookies.encrypted[:user_opted_remember_device_preference]
end
Expand All @@ -305,15 +301,14 @@ def unconfirmed_phone?
end

def phone_view_data
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was like "oh nothing seems to call this" and then I remembered what mixin we're in with dynamic sends everywhere 😎 😭

identity-idp/app/controllers/concerns/two_factor_authenticatable_methods.rb

Line 372 in 38b37f8

data = send("#{type}_view_data".to_sym)

Copy link
Copy Markdown
Contributor Author

@mitchellhenke mitchellhenke Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, would like to redo this whole thing

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIG SAME

{ confirmation_for_add_phone: confirmation_for_add_phone?,
{
confirmation_for_add_phone: confirmation_for_add_phone?,
phone_number: display_phone_to_deliver_to,
code_value: direct_otp_code,
otp_expiration: otp_expiration,
otp_delivery_preference: two_factor_authentication_method,
otp_make_default_number: selected_otp_make_default_number,
voice_otp_delivery_unsupported: voice_otp_delivery_unsupported?,
unconfirmed_phone: unconfirmed_phone?,
account_reset_token: account_reset_token }.merge(generic_data)
Comment on lines -314 to -316
Copy link
Copy Markdown
Contributor

@aduth aduth Feb 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • unconfirmed_phone was still used in PhoneDeliveryPresenter (source), follow-up at LG-9037: Unconfirmed Phone fix #7884
  • voice_otp_delivery_unsupported? and account_reset_token methods appear to be unused in this class now

}.merge(generic_data)
end

def selected_otp_make_default_number
Expand All @@ -333,8 +328,6 @@ def authenticator_view_data

def generic_data
{
personal_key_unavailable: personal_key_unavailable?,
reauthn: reauthn?,
user_opted_remember_device_cookie: user_opted_remember_device_cookie,
}
end
Expand Down
1 change: 0 additions & 1 deletion app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ def piv_cac_view_data
{
two_factor_authentication_method: two_factor_authentication_method,
hide_fallback_question: service_provider_mfa_policy.piv_cac_required?,
user_email: current_user.email_addresses.take.email,
}.merge(generic_data)
end

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/two_factor_auth_code/generic_delivery_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ def service_provider_mfa_policy
)
end

attr_reader :personal_key_unavailable, :view, :user_opted_remember_device_cookie
attr_reader :view, :user_opted_remember_device_cookie
end
end
2 changes: 0 additions & 2 deletions app/presenters/two_factor_auth_code/phone_delivery_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@ def troubleshoot_change_phone_or_method_option

attr_reader(
:phone_number,
:account_reset_token,
:confirmation_for_add_phone,
:voice_otp_delivery_unsupported,
)
end
end
3 changes: 0 additions & 3 deletions spec/presenters/two_factor_auth_code/phone_delivery_presenter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
phone_number: '5555559876',
code_value: '999999',
otp_delivery_preference: 'sms',
unconfirmed_phone: true,
totp_enabled: false,
personal_key_unavailable: true,
reauthn: false,
}
end
let(:presenter) do
Expand Down