Skip to content
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

LG 11140 Break up webauthn selection presenter #9467

Merged
5 changes: 3 additions & 2 deletions app/models/webauthn_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ def mfa_enabled?

def selection_presenters
if platform_authenticator?
[TwoFactorAuthentication::WebauthnPlatformSelectionPresenter.new(user:, configuration: self)]
[TwoFactorAuthentication::SignInWebauthnPlatformSelectionPresenter.
new(user:, configuration: self)]
else
[TwoFactorAuthentication::WebauthnSelectionPresenter.new(user:, configuration: self)]
[TwoFactorAuthentication::SignInWebauthnSelectionPresenter.new(user:, configuration: self)]
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ def login_label(type)
t('two_factor_authentication.login_options.sms')
when 'voice'
t('two_factor_authentication.login_options.voice')
when 'webauthn'
t('two_factor_authentication.login_options.webauthn')
when 'webauthn_platform'
t('two_factor_authentication.login_options.webauthn_platform')
else
raise "Unsupported login method: #{type}"
end
Expand All @@ -100,10 +96,6 @@ def setup_label(type)
t('two_factor_authentication.two_factor_choice_options.sms')
when 'voice'
t('two_factor_authentication.two_factor_choice_options.voice')
when 'webauthn'
t('two_factor_authentication.two_factor_choice_options.webauthn')
when 'webauthn_platform'
t('two_factor_authentication.two_factor_choice_options.webauthn_platform')
else
raise "Unsupported setup method: #{type}"
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
module TwoFactorAuthentication
class WebauthnPlatformSelectionPresenter < SelectionPresenter
class SetUpWebauthnPlatformSelectionPresenter < SetUpSelectionPresenter
def initialize(user:, configuration: nil)
@user = user
@configuration = configuration
end

def method
:webauthn_platform
end
Expand All @@ -8,15 +13,25 @@ def render_in(view_context, &block)
view_context.render(
WebauthnInputComponent.new(
platform: true,
passkey_supported_only: configuration.blank?,
passkey_supported_only: true,
show_unsupported_passkey:
configuration.blank? &&
IdentityConfig.store.show_unsupported_passkey_platform_authentication_setup,
),
&block
)
end

def label
t('two_factor_authentication.two_factor_choice_options.webauthn_platform')
end

def info
t(
'two_factor_authentication.two_factor_choice_options.webauthn_platform_info',
app_name: APP_NAME,
)
end

def single_configuration_only?
true
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module TwoFactorAuthentication
class WebauthnSelectionPresenter < SelectionPresenter
class SetUpWebauthnSelectionPresenter < SetUpSelectionPresenter
def method
:webauthn
end
Expand All @@ -8,6 +8,14 @@ def render_in(view_context, &block)
view_context.render(WebauthnInputComponent.new, &block)
end

def label
t('two_factor_authentication.two_factor_choice_options.webauthn')
end

def info
t('two_factor_authentication.login_options.webauthn_info')
end

def mfa_configuration_count
user.webauthn_configurations.where(platform_authenticator: [false, nil]).count
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module TwoFactorAuthentication
class SignInWebauthnPlatformSelectionPresenter < SelectionPresenter
def method
:webauthn_platform
end
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove checks below to configuration.blank?, since I would expect that the configuration should never be blank in the "Sign in" classes. They should be replaced with false.


def render_in(view_context, &block)
view_context.render(
WebauthnInputComponent.new(
platform: true,
passkey_supported_only: false,
show_unsupported_passkey: false,
),
&block
)
end

def label
t('two_factor_authentication.login_options.webauthn_platform')
end

def info
t('two_factor_authentication.login_options.webauthn_platform_info', app_name: APP_NAME)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module TwoFactorAuthentication
class SignInWebauthnSelectionPresenter < SignInSelectionPresenter
def method
:webauthn
end

def render_in(view_context, &block)
view_context.render(WebauthnInputComponent.new, &block)
end

def label
t('two_factor_authentication.login_options.webauthn')
end

def info
t('two_factor_authentication.login_options.webauthn_info')
end
end
end
8 changes: 4 additions & 4 deletions app/presenters/two_factor_options_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ def options
# add additional MFA page
def all_user_selected_options
[
TwoFactorAuthentication::WebauthnPlatformSelectionPresenter.new(user: user),
TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter.new(user: user),
TwoFactorAuthentication::SetUpAuthAppSelectionPresenter.new(user: user),
TwoFactorAuthentication::PhoneSelectionPresenter.new(user: user),
TwoFactorAuthentication::BackupCodeSelectionPresenter.new(user: user),
TwoFactorAuthentication::WebauthnSelectionPresenter.new(user: user),
TwoFactorAuthentication::SetUpWebauthnSelectionPresenter.new(user: user),
TwoFactorAuthentication::PivCacSelectionPresenter.new(user: user),
]
end
Expand Down Expand Up @@ -104,12 +104,12 @@ def piv_cac_option

def webauthn_option
return [] if piv_cac_required?
[TwoFactorAuthentication::WebauthnSelectionPresenter.new(user: user)]
[TwoFactorAuthentication::SetUpWebauthnSelectionPresenter.new(user: user)]
end

def webauthn_platform_option
return [] if piv_cac_required? || !IdentityConfig.store.platform_auth_set_up_enabled
[TwoFactorAuthentication::WebauthnPlatformSelectionPresenter.new(user: user)]
[TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter.new(user: user)]
end

def phone_options
Expand Down
4 changes: 2 additions & 2 deletions spec/models/webauthn_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
presenters = subject.selection_presenters
expect(presenters.count).to eq 1
expect(presenters.first).to be_instance_of(
TwoFactorAuthentication::WebauthnSelectionPresenter,
TwoFactorAuthentication::SignInWebauthnSelectionPresenter,
)
end
end
Expand All @@ -27,7 +27,7 @@
presenters = subject.selection_presenters
expect(presenters.count).to eq 1
expect(presenters.first).to be_instance_of(
TwoFactorAuthentication::WebauthnPlatformSelectionPresenter,
TwoFactorAuthentication::SignInWebauthnPlatformSelectionPresenter,
)
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'rails_helper'

RSpec.describe TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter do
let(:user_without_mfa) { create(:user) }
let(:user_with_mfa) { create(:user) }
let(:configuration) {}
let(:presenter_without_mfa) do
described_class.new(user: user_without_mfa)
end
let(:presenter_with_mfa) do
described_class.new(user: user_with_mfa)
end

describe '#type' do
it 'returns webauthn_platform' do
expect(presenter_without_mfa.type).to eq 'webauthn_platform'
end
end

describe '#mfa_configuration' do
it 'returns an empty string when user has not configured this authenticator' do
expect(presenter_without_mfa.mfa_configuration_description).to eq('')
end

it 'returns an # added when user has configured this authenticator' do
create(:webauthn_configuration, platform_authenticator: true, user: user_with_mfa)
expect(presenter_with_mfa.mfa_configuration_description).to eq(
t(
'two_factor_authentication.two_factor_choice_options.no_count_configuration_added',
count: 1,
),
)
end
end
end
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
require 'rails_helper'

RSpec.describe TwoFactorAuthentication::WebauthnSelectionPresenter do
RSpec.describe TwoFactorAuthentication::SetUpWebauthnSelectionPresenter do
let(:user_without_mfa) { create(:user) }
let(:user_with_mfa) { create(:user) }
let(:configuration) {}
let(:presenter_without_mfa) do
described_class.new(configuration: configuration, user: user_without_mfa)
described_class.new(user: user_without_mfa)
end
let(:presenter_with_mfa) do
described_class.new(configuration: configuration, user: user_with_mfa)
described_class.new(user: user_with_mfa)
end

describe '#type' do
Expand All @@ -17,19 +17,6 @@
end
end

describe '#render_in' do
Copy link
Member

Choose a reason for hiding this comment

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

I think we lost test coverage here, we still have a SetUpWebauthnSelectionPresenter#render_in method.

it 'renders a WebauthnInputComponent' do
view_context = ActionController::Base.new.view_context

expect(view_context).to receive(:render) do |component, &block|
expect(component).to be_instance_of(WebauthnInputComponent)
expect(block.call).to eq('content')
end

presenter_without_mfa.render_in(view_context) { 'content' }
end
end

describe '#mfa_configuration' do
it 'returns an empty string when user has not configured this authenticator' do
expect(presenter_without_mfa.mfa_configuration_description).to eq('')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require 'rails_helper'

RSpec.describe TwoFactorAuthentication::SignInWebauthnPlatformSelectionPresenter do
let(:user) { create(:user) }
let(:configuration) { create(:webauthn_configuration, user: user) }

let(:presenter) do
described_class.new(user: user, configuration: configuration)
end

describe '#type' do
it 'returns webauthn_platform' do
expect(presenter.type).to eq 'webauthn_platform'
end
end

describe '#label' do
it 'raises with missing translation' do
expect(presenter.label).to eq(
t('two_factor_authentication.two_factor_choice_options.webauthn_platform'),
)
end
end

describe '#info' do
it 'raises with missing translation' do
Comment on lines +18 to +26
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed these as I start work on a related ticket. These methods don't raise, so I think we should revise the expectation description accordingly.

Suggested change
it 'raises with missing translation' do
expect(presenter.label).to eq(
t('two_factor_authentication.two_factor_choice_options.webauthn_platform'),
)
end
end
describe '#info' do
it 'raises with missing translation' do
it 'returns the label text' do
expect(presenter.label).to eq(
t('two_factor_authentication.two_factor_choice_options.webauthn_platform'),
)
end
end
describe '#info' do
it 'returns the info text' do

expect(presenter.info).to eq(
t('two_factor_authentication.login_options.webauthn_platform_info'),
)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require 'rails_helper'

RSpec.describe TwoFactorAuthentication::SignInWebauthnSelectionPresenter do
let(:user) { create(:user) }
let(:configuration) { create(:webauthn_configuration, user: user) }

let(:presenter) do
described_class.new(user: user, configuration: configuration)
end

describe '#type' do
it 'returns webauthn' do
expect(presenter.type).to eq 'webauthn'
end
end

describe '#label' do
it 'raises with missing translation' do
expect(presenter.label).to eq(
t('two_factor_authentication.two_factor_choice_options.webauthn'),
)
end
end

describe '#info' do
it 'raises with missing translation' do
expect(presenter.info).to eq(
t('two_factor_authentication.login_options.webauthn_info'),
)
end
end
end

This file was deleted.

Loading