Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .mocharc.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ module.exports = /** @type {import('mocha').MochaOptions} */ ({
require: ['./spec/javascript/support/mocha.js'],
file: 'spec/javascript/spec_helper.js',
extension: ['js', 'jsx', 'ts', 'tsx'],
loader: ['quibble'],
conditions: ['source'],
});
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ GEM
factory_bot_rails (6.4.3)
factory_bot (~> 6.4)
railties (>= 5.0.0)
faker (2.19.0)
i18n (>= 1.6, < 2)
faker (3.4.2)
i18n (>= 1.8.11, < 2)
faraday (2.10.0)
faraday-net_http (>= 2.0, < 3.2)
logger
Expand Down
1 change: 0 additions & 1 deletion app/controllers/idv/agreement_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ def update
)

if result.success?
idv_session.idv_consent_given = true
idv_session.idv_consent_given_at = Time.zone.now

if IdentityConfig.store.in_person_proofing_opt_in_enabled &&
Expand Down
6 changes: 0 additions & 6 deletions app/controllers/idv/in_person/state_id_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class StateIdController < ApplicationController
include Idv::AvailabilityConcern
include IdvStepConcern

before_action :render_404_if_controller_not_enabled
before_action :redirect_unless_enrollment # confirm previous step is complete
before_action :set_usps_form_presenter

Expand Down Expand Up @@ -92,11 +91,6 @@ def self.step_info

private

def render_404_if_controller_not_enabled
render_not_found unless
IdentityConfig.store.in_person_state_id_controller_enabled
end

def redirect_unless_enrollment
redirect_to idv_document_capture_url unless current_user.establishing_in_person_enrollment
end
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/idv/in_person/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ def prev_url

def pii
pii_from_user = user_session.dig('idv/in_person', :pii_from_user) || {}
pii_from_user.merge(ssn: idv_session.ssn)
pii_from_user.merge(
consent_given_at: idv_session.idv_consent_given_at,
ssn: idv_session.ssn,
)
end

# override IdvSessionConcern
Expand Down
1 change: 1 addition & 0 deletions app/controllers/idv/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ def analytics_arguments
def pii
idv_session.pii_from_doc.to_h.merge(
ssn: idv_session.ssn,
consent_given_at: idv_session.idv_consent_given_at,
**idv_session.updated_user_address.to_h,
).with_indifferent_access
end
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def capture_analytics
finish_profile: user_has_pending_profile?,
requested_ial: requested_ial,
request_signed: saml_request.signed?,
matching_cert_serial: saml_request.service_provider.matching_cert&.serial&.to_s,
matching_cert_serial:,
requested_nameid_format: saml_request.name_id_format,
)

Expand All @@ -140,6 +140,12 @@ def capture_analytics
analytics.saml_auth(**analytics_payload)
end

def matching_cert_serial
saml_request.matching_cert&.serial&.to_s
rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError
nil
end

def log_external_saml_auth_request
return unless external_saml_request?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ class WebauthnVerificationController < ApplicationController
include NewDeviceConcern

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 All @@ -23,17 +22,6 @@ 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)
handle_verification_for_authentication_context(
result:,
Expand Down
12 changes: 7 additions & 5 deletions app/controllers/users/two_factor_authentication_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -338,17 +338,19 @@ def otp_rate_limiter
def redirect_url
if !mobile? && TwoFactorAuthentication::PivCacPolicy.new(current_user).enabled?
login_two_factor_piv_cac_url
elsif TwoFactorAuthentication::WebauthnPolicy.new(current_user).platform_enabled?
if user_session[:platform_authenticator_available] == false
login_two_factor_options_url
else
login_two_factor_webauthn_url(platform: true)
end
elsif TwoFactorAuthentication::WebauthnPolicy.new(current_user).enabled?
login_two_factor_webauthn_url(webauthn_params)
login_two_factor_webauthn_url
elsif TwoFactorAuthentication::AuthAppPolicy.new(current_user).enabled?
login_two_factor_authenticator_url
end
end

def webauthn_params
{ platform: current_user.webauthn_configurations.platform_authenticators.present? }
end

def handle_too_many_short_term_otp_sends(method:, default:)
analytics.rate_limit_reached(
limiter_type: short_term_otp_rate_limiter.rate_limit_type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,57 @@ import { render, screen, within } from '@testing-library/react';
import DocumentCaptureReviewIssues from '@18f/identity-document-capture/components/document-capture-review-issues';
import { InPersonContext } from '@18f/identity-document-capture/context';
import { toFormEntryError } from '@18f/identity-document-capture/services/upload';
import { I18nContext } from '@18f/identity-react-i18n';
import { I18n } from '@18f/identity-i18n';
import { expect } from 'chai';
import { composeComponents } from '@18f/identity-compose-components';

describe('DocumentCaptureReviewIssues', () => {
const DEFAULT_OPTIONS = {
registerField: () => undefined,
captureHints: true,
remainingAttempts: 2,
value: {},
onChange: () => undefined,
onError: () => undefined,
isFailedSelfie: false,
isFailedDocType: false,
isFailedSelfieLivenessOrQuality: false,
remainingSubmitAttempts: Infinity,
unknownFieldErrors: [],
errors: [],
hasDismissed: false,
toPreviousStep: () => undefined,
};

context('with default props', () => {
it('does not display infinity remaining attempts', () => {
const { queryByText } = render(
<I18nContext.Provider
value={
new I18n({
strings: {
'idv.failure.attempts_html': 'You have %{count} attempts remaining.',
},
})
}
>
<DocumentCaptureReviewIssues {...DEFAULT_OPTIONS} />
</I18nContext.Provider>,
);

expect(queryByText('You have Infinity attempts remaining.')).to.equal(null);
});
});

context('with doc error', () => {
it('renders for non doc type failure', () => {
const props = {
isFailedDocType: false,
remainingSubmitAttempts: 2,
unknownFieldErrors: [
{
field: 'general',
error: toFormEntryError({ field: 'general', message: 'general error' }),
error: toFormEntryError({ field: 'network', message: 'general error' }),
},
],
errors: [
Expand All @@ -44,6 +75,16 @@ describe('DocumentCaptureReviewIssues', () => {
},
},
],
[
I18nContext.Provider,
{
value: new I18n({
strings: {
'idv.failure.attempts_html': 'You have %{count} attempts remaining.',
},
}),
},
],
[
DocumentCaptureReviewIssues,
{
Expand All @@ -58,6 +99,8 @@ describe('DocumentCaptureReviewIssues', () => {

expect(getByText('general error')).to.be.ok();

expect(getByText('You have 2 attempts remaining.')).to.be.ok();

// tips header
expect(getByText('doc_auth.tips.review_issues_id_header_text')).to.be.ok();
const lists = getAllByRole('list');
Expand All @@ -80,31 +123,37 @@ describe('DocumentCaptureReviewIssues', () => {
});

it('renders for a doc type failure', () => {
const props = {
isFailedDocType: true,
unknownFieldErrors: [
{
field: 'general',
error: toFormEntryError({ field: 'general', message: 'general error' }),
},
],
errors: [
{
field: 'front',
error: toFormEntryError({ field: 'front', message: 'front side doc type error' }),
},
{
field: 'back',
error: toFormEntryError({ field: 'back', message: 'back side doc type error' }),
},
],
};
const { getByText, getByLabelText, getByRole } = render(
<InPersonContext.Provider value={{ inPersonURL: '/verify/doc_capture' }}>
<InPersonContext.Provider
value={{
inPersonURL: '/verify/doc_capture',
locationsURL: '',
addressSearchURL: '',
inPersonFullAddressEntryEnabled: false,
inPersonOutageMessageEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [['Los Angeles', 'NY']],
}}
>
<DocumentCaptureReviewIssues
{...{
...DEFAULT_OPTIONS,
...props,
isFailedDocType: true,
unknownFieldErrors: [
{
error: toFormEntryError({ field: 'network', message: 'general error' }),
},
],
errors: [
{
field: 'front',
error: toFormEntryError({ field: 'front', message: 'front side doc type error' }),
},
{
field: 'back',
error: toFormEntryError({ field: 'back', message: 'back side doc type error' }),
},
],
}}
/>
</InPersonContext.Provider>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ function DocumentCaptureReviewIssues({
altFailedDocTypeMsg={isFailedDocType ? t('doc_auth.errors.doc.wrong_id_type_html') : null}
hasDismissed={hasDismissed}
/>
<p>
<HtmlTextWithStrongNoWrap
text={t('idv.failure.attempts_html', { count: remainingSubmitAttempts })}
/>
</p>
{Number.isFinite(remainingSubmitAttempts) && (
<p>
<HtmlTextWithStrongNoWrap
text={t('idv.failure.attempts_html', { count: remainingSubmitAttempts })}
/>
</p>
)}
{!isFailedDocType && captureHints && (
<TipList
titleClassName="margin-bottom-0 margin-top-2"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ describe('enrollWebauthnDevice', () => {
user,
challenge,
excludeCredentials,
authenticatorAttachment: 'cross-platform',
hints: ['security-key'],
});

expect(navigator.credentials.create).to.have.been.calledWith({
Expand Down Expand Up @@ -130,16 +128,15 @@ describe('enrollWebauthnDevice', () => {
context('platform authenticator', () => {
it('enrolls a device with correct authenticatorAttachment', async () => {
await enrollWebauthnDevice({
platformAuthenticator: true,
user,
challenge,
excludeCredentials,
authenticatorAttachment: 'platform',
hints: ['client-device'],
});

expect(navigator.credentials.create).to.have.been.calledWithMatch({
publicKey: {
hints: ['client-device'],
hints: undefined,
authenticatorSelection: {
authenticatorAttachment: 'platform',
},
Expand All @@ -162,7 +159,6 @@ describe('enrollWebauthnDevice', () => {
user,
challenge,
excludeCredentials,
authenticatorAttachment: 'cross-platform',
});

expect(result.transports).to.equal(undefined);
Expand All @@ -182,7 +178,6 @@ describe('enrollWebauthnDevice', () => {
user,
challenge,
excludeCredentials,
authenticatorAttachment: 'cross-platform',
});

expect(result.authenticatorDataFlagsValue).to.equal(undefined);
Expand Down
20 changes: 11 additions & 9 deletions app/javascript/packages/webauthn/enroll-webauthn-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ interface AuthenticatorAttestationResponseBrowserSupport
type PublicKeyCredentialHintType = 'client-device' | 'security-key' | 'hybrid';

interface EnrollOptions {
platformAuthenticator?: boolean;

user: PublicKeyCredentialUserEntity;

challenge: BufferSource;

excludeCredentials: PublicKeyCredentialDescriptor[];

authenticatorAttachment?: AuthenticatorAttachment;

hints?: Array<PublicKeyCredentialHintType>;
}

interface EnrollResult {
Expand Down Expand Up @@ -80,11 +78,10 @@ const SUPPORTED_ALGORITHMS: COSEAlgorithm[] = [
];

async function enrollWebauthnDevice({
platformAuthenticator = false,
user,
challenge,
excludeCredentials,
authenticatorAttachment,
hints,
}: EnrollOptions): Promise<EnrollResult> {
const credential = (await navigator.credentials.create({
publicKey: {
Expand All @@ -94,11 +91,16 @@ async function enrollWebauthnDevice({
pubKeyCredParams: SUPPORTED_ALGORITHMS.map((alg) => ({ alg, type: 'public-key' })),
timeout: 800000,
attestation: 'none',
hints,
hints: platformAuthenticator ? undefined : ['security-key'],
authenticatorSelection: {
// Prevents user from needing to use PIN with Security Key
// A user is assumed to be AAL2 recently authenticated before being permitted to add an
// authentication method to their account. Additionally, unless explicitly discouraged,
// Windows devices will prompt the user to add a PIN to their security key. When used as a
// single-factor authenticator in combination with a memorized secret (password), proving
// possession is sufficient, and a PIN ("something you know") is unnecessary friction that
// contributes to abandonment or loss of access.
userVerification: 'discouraged',
authenticatorAttachment,
authenticatorAttachment: platformAuthenticator ? 'platform' : 'cross-platform',
},
excludeCredentials,
} as PublicKeyCredentialCreationOptionsWithHints,
Expand Down
Loading