Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3caace0
changelog: User-Facing Improvements, Authentication, MFA redirect for…
mdiarra3 Dec 28, 2023
3470cbb
include application helper
mdiarra3 Dec 28, 2023
ba686e3
add ft unlock check in session
mdiarra3 Dec 29, 2023
b4d68f8
add spec for authentication controller
mdiarra3 Jan 2, 2024
1244e69
spec and address comments
mdiarra3 Jan 3, 2024
4f8b8a1
linting
mdiarra3 Jan 3, 2024
6880fc3
fix spec
mdiarra3 Jan 3, 2024
da26d72
hidden spec
mdiarra3 Jan 3, 2024
2fff708
add to session
mdiarra3 Jan 3, 2024
ccee0cd
mock browser cache for specs
mdiarra3 Jan 4, 2024
0f388f5
new session spec
mdiarra3 Jan 4, 2024
d7ed538
create component
mdiarra3 Jan 4, 2024
d6a13bb
check support
mdiarra3 Jan 5, 2024
f428f03
use form for hidden
mdiarra3 Jan 5, 2024
2c79e27
rename and refactor package
mdiarra3 Jan 5, 2024
d3a129e
move verification
mdiarra3 Jan 8, 2024
86649ec
chang back to hidden field tag
mdiarra3 Jan 8, 2024
64371de
Merge remote-tracking branch 'origin/main' into LG-11433-default-ft-u…
mdiarra3 Jan 8, 2024
d7df22c
set text
mdiarra3 Jan 8, 2024
6cd7e83
LG-11433: default
mdiarra3 Jan 8, 2024
ac3b3bf
only check if has key
mdiarra3 Jan 8, 2024
de4541c
make sure to properly check session
mdiarra3 Jan 8, 2024
48e61df
make sure to sign in eligible for webauthn platform
mdiarra3 Jan 9, 2024
c986489
remember device set back
mdiarra3 Jan 9, 2024
d894294
webauthn spec js enabled
mdiarra3 Jan 9, 2024
4b12998
default hidden value in frontend
mdiarra3 Jan 9, 2024
4920ed3
use existing method to set hidden field
mdiarra3 Jan 10, 2024
b0398e4
Merge remote-tracking branch 'origin/main' into LG-11433-default-ft-u…
mdiarra3 Jan 10, 2024
5207b09
set hidden field added to session helper
mdiarra3 Jan 10, 2024
4b7b486
include proper javascript enabled
mdiarra3 Jan 10, 2024
46c2497
put back tests in right format
mdiarra3 Jan 10, 2024
717f2da
address comments
mdiarra3 Jan 11, 2024
5236b87
Merge remote-tracking branch 'origin/main' into LG-11433-default-ft-u…
mdiarra3 Jan 16, 2024
c29c71d
fix lint
mdiarra3 Jan 16, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class WebauthnVerificationController < ApplicationController
include TwoFactorAuthenticatable

before_action :check_sp_required_mfa
before_action :check_if_device_supports_platform_auth, only: :show
before_action :confirm_webauthn_enabled, only: :show

def show
Expand Down Expand Up @@ -33,6 +34,17 @@ def confirm

private

def check_if_device_supports_platform_auth
return unless user_session.has_key?(:platform_authenticator_available)
if platform_authenticator? && !device_supports_webauthn_platform?
redirect_to login_two_factor_options_url
end
end

def device_supports_webauthn_platform?
user_session.delete(:platform_authenticator_available) == true
end

def handle_webauthn_result(result)
if result.success?
handle_valid_webauthn
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ def handle_valid_authentication
user_id: current_user.id,
email: auth_params[:email],
)
user_session[:platform_authenticator_available] =
params[:platform_authenticator_available] == 'true'
redirect_to next_url_after_valid_authentication
end

Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: There are many ways that a user might be prompted for MFA, such as reauthentication. Can we always rely on the session value being present in those other scenarios? My thinking is yes since someone's session should always begin from the sign-in page, but I wanted to ask to double-check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should for sure, but I dont know if in reauthenthication we want this? Maybe we can remove it when its directed.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module Users
class TwoFactorAuthenticationController < ApplicationController
include TwoFactorAuthenticatable
include ApplicationHelper
include ActionView::Helpers::DateHelper

before_action :check_remember_device_preference
Expand Down
2 changes: 2 additions & 0 deletions app/javascript/packages/webauthn/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ export { default as enrollWebauthnDevice } from './enroll-webauthn-device';
export { default as extractCredentials } from './extract-credentials';
export { default as verifyWebauthnDevice } from './verify-webauthn-device';
export { default as isExpectedWebauthnError } from './is-expected-error';
export { default as isWebauthnPlatformAuthenticatorAvailable } from './is-webauthn-platform-authenticator-available';
export { default as isWebauthnPasskeySupported } from './is-webauthn-passkey-supported';
export * from './converters';

export type { VerifyCredentialDescriptor } from './verify-webauthn-device';
20 changes: 20 additions & 0 deletions app/javascript/packs/platform-authenticator-available.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {
isWebauthnPlatformAuthenticatorAvailable,
isWebauthnPasskeySupported,
} from '@18f/identity-webauthn';

async function platformAuthenticatorAvailable() {
const platformAuthenticatorAvailableInput = document.getElementById(
'platform_authenticator_available',
) as HTMLInputElement;
if (!platformAuthenticatorAvailableInput) {
return;
}
if (isWebauthnPasskeySupported() && (await isWebauthnPlatformAuthenticatorAvailable())) {
platformAuthenticatorAvailableInput.value = 'true';
} else {
platformAuthenticatorAvailableInput.value = 'false';
}
}

platformAuthenticatorAvailable();
3 changes: 3 additions & 0 deletions app/views/devise/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
},
},
) %>
<%= hidden_field_tag :platform_authenticator_available, id: 'platform_authenticator_available' %>
<%= f.submit t('links.sign_in'), full_width: true, wide: false %>
<% end %>
<% if @ial && desktop_device? %>
Expand Down Expand Up @@ -86,3 +87,5 @@
<% end %>

<noscript><link rel="stylesheet" href="<%= no_js_detect_css_path(location: :sign_in) %>"></noscript>
<%= javascript_packs_tag_once('platform-authenticator-available') %>

Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,17 @@ def index
expect(response).to redirect_to login_two_factor_webauthn_path(platform: false)
end

it 'passes the platform parameter if the user has a platform autheticator' do
controller.current_user.webauthn_configurations.first.update!(platform_authenticator: true)
context 'when platform_authenticator' do
before do
controller.current_user.webauthn_configurations.
first.update!(platform_authenticator: true)
end

get :show
it 'passes the platform parameter if the user has a platform autheticator' do
get :show

expect(response).to redirect_to login_two_factor_webauthn_path(platform: true)
expect(response).to redirect_to login_two_factor_webauthn_path(platform: true)
end
end
end

Expand Down
1 change: 0 additions & 1 deletion spec/features/two_factor_authentication/sign_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,6 @@ def attempt_to_bypass_2fa
context 'sign in' do
it 'allows user to be signed in without issue' do
mock_webauthn_verification_challenge

sign_in_user(webauthn_configuration.user)
mock_successful_webauthn_authentication { click_webauthn_authenticate_button }

Expand Down
24 changes: 20 additions & 4 deletions spec/features/webauthn/hidden_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,27 @@
let(:user) { create(:user, :fully_registered, :with_webauthn_platform) }

context 'with javascript enabled', :js do
it 'displays the authenticator option' do
sign_in_user(user)
click_on t('two_factor_authentication.login_options_link_text')
context ' with device that supports authenticator' do
it 'displays the authenticator option' do
sign_in_user(user)
click_on t('two_factor_authentication.login_options_link_text')

expect(webauthn_option_hidden?).to eq(false)
expect(webauthn_option_hidden?).to eq(false)
end
end

context 'with device that doesnt support authenticator' do
it 'redirects to options page on sign in and shows the option' do
email ||= user.email_addresses.first.email
password = user.password
allow(UserMailer).to receive(:new_device_sign_in).and_call_original
visit new_user_session_path
set_hidden_field('platform_authenticator_available', 'false')
fill_in_credentials_and_submit(email, password)
continue_as(email, password)
expect(current_path).to eq(login_two_factor_options_path)
expect(webauthn_option_hidden?).to eq(false)
end
end
end

Expand Down
24 changes: 24 additions & 0 deletions spec/features/webauthn/sign_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,33 @@
mock_webauthn_verification_challenge

sign_in_user(user)
expect(current_url).to eq(login_two_factor_webauthn_url(platform: true))
mock_cancelled_webauthn_authentication { click_webauthn_authenticate_button }

expect(page).to have_content(t('two_factor_authentication.webauthn_platform_header_text'))
end

context 'with device that doesnt support authenticator' do
before do
email ||= user.email_addresses.first.email
password = user.password
allow(UserMailer).to receive(:new_device_sign_in).and_call_original
visit new_user_session_path
set_hidden_field('platform_authenticator_available', 'false')
fill_in_credentials_and_submit(email, password)
continue_as(email, password)
end

it 'redirects to options page on sign in' do
expect(current_path).to eq(login_two_factor_options_path)
end

it 'allows user to go to options page and still select webauthn as their option' do
expect(current_path).to eq(login_two_factor_options_path)
select_2fa_option('webauthn_platform', visible: :all)
click_continue
expect(current_url).to eq(login_two_factor_webauthn_url(platform: true))
end
end
end
end
11 changes: 11 additions & 0 deletions spec/support/features/session_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

module Features
module SessionHelper
include JavascriptDriverHelper
include PersonalKeyHelper

VALID_PASSWORD = 'Val!d Pass w0rd'.freeze
Expand Down Expand Up @@ -50,6 +51,7 @@ def sign_up_and_2fa_ial1_user
def signin(email, password)
allow(UserMailer).to receive(:new_device_sign_in).and_call_original
visit new_user_session_path
set_hidden_field('platform_authenticator_available', 'true')
fill_in_credentials_and_submit(email, password)
continue_as(email, password)
end
Expand Down Expand Up @@ -729,5 +731,14 @@ def expect_branded_experience
def acknowledge_backup_code_confirmation
click_on t('two_factor_authentication.backup_codes.saved_backup_codes')
end

def set_hidden_field(id, value)
input = first("input##{id}", visible: false)
if javascript_enabled?
input.execute_script("this.value = #{value.to_json}")
else
input.set(value)
end
end
end
end