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
4 changes: 3 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ review-app:
stage: review
allow_failure: true
needs:
- job: build-review-image
- job: build-idp-image
resource_group: $CI_ENVIRONMENT_SLUG.reviewapps.identitysandbox.gov
extends: .deploy
environment:
Expand Down Expand Up @@ -503,6 +503,7 @@ include:

secret_detection:
allow_failure: false
needs: []
variables:
SECRET_DETECTION_EXCLUDED_PATHS: 'keys.example,config/artifacts.example,public/acuant/*/opencv.min.js,tmp/0.0.0.0-3000.key'
SECRET_DETECTION_REPORT_FILE: 'gl-secret-detection-report.json'
Expand All @@ -527,6 +528,7 @@ secret_detection:
# check if '{ "vulnerabilities": [], ..' is empty in the report file if it exists
if [ "$(jq ".vulnerabilities | length" $SECRET_DETECTION_REPORT_FILE)" -gt 0 ]; then
echo "Vulnerabilities detected. Please analyze the artifact $SECRET_DETECTION_REPORT_FILE produced by the 'secret-detection' job."
echo "Check the \"Security\" tab on the overall pipeline run to download the report for more information."
exit 80
fi
else
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ def show
user: current_user,
locked_for_session: pii_locked_for_session?(current_user),
)
if session.delete(:from_select_email_flow)
flash.now[:success] = t(
'account.emails.confirmed_html',
url: account_connected_accounts_url,
)
end
end

def reauthentication
Expand Down
2 changes: 0 additions & 2 deletions app/controllers/concerns/idv/verify_info_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ def shared_update
Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]).
call('verify', :update, true)

set_state_id_type

ssn_rate_limiter.increment!

document_capture_session = DocumentCaptureSession.create(
Expand Down
8 changes: 0 additions & 8 deletions app/controllers/idv/in_person/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,6 @@ def flow_param
'in_person'
end

# state_id_type is hard-coded here because it's required for proofing against
# AAMVA. We're sticking with driver's license because most states don't discern
# between various ID types and driver's license is the most common one that will
# be supported. See also LG-3852 and related findings document.
def set_state_id_type
pii_from_user[:state_id_type] = 'drivers_license' unless invalid_state?
end

def invalid_state?
pii_from_user.blank?
end
Expand Down
3 changes: 0 additions & 3 deletions app/controllers/idv/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ def self.step_info

def flow_param; end

# state ID type isn't manually set for Idv::VerifyInfoController
def set_state_id_type; end

def prev_url
idv_ssn_url
end
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/users/email_confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def email_address_already_confirmed?

def process_successful_confirmation(email_address)
confirm_and_notify(email_address)
store_from_select_email_flow_in_session
if current_user
flash[:success] = t('devise.confirmations.confirmed')
redirect_to account_url
Expand Down Expand Up @@ -98,5 +99,9 @@ def email_address_already_confirmed_by_current_user?
def confirmation_params
params.permit(:confirmation_token)
end

def store_from_select_email_flow_in_session
session[:from_select_email_flow] = params[:from_select_email_flow].to_s == 'true'
end
end
end
17 changes: 13 additions & 4 deletions app/controllers/users/emails_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,19 @@ class EmailsController < ApplicationController

def show
analytics.add_email_visit
session[:in_select_email_flow] = params[:in_select_email_flow]
@add_user_email_form = AddUserEmailForm.new
@pending_completions_consent = pending_completions_consent?
end

def add
@add_user_email_form = AddUserEmailForm.new
@add_user_email_form = AddUserEmailForm.new(
session[:in_select_email_flow],
)

result = @add_user_email_form.submit(current_user, permitted_params)
result = @add_user_email_form.submit(
current_user, permitted_params
)
analytics.add_email_request(**result.to_h)

if result.success?
Expand Down Expand Up @@ -71,7 +76,8 @@ def verify
if session_email.blank?
redirect_to add_email_url
else
render :verify, locals: { email: session_email }
render :verify,
locals: { email: session_email, in_select_email_flow: params[:in_select_email_flow] }
end
end

Expand All @@ -97,7 +103,10 @@ def process_successful_creation
resend_confirmation = params[:user][:resend]
session[:email] = @add_user_email_form.email

redirect_to add_email_verify_email_url(resend: resend_confirmation)
redirect_to add_email_verify_email_url(
resend: resend_confirmation,
in_select_email_flow: session.delete(:in_select_email_flow),
)
end

def session_email
Expand Down
14 changes: 7 additions & 7 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def create
return process_rate_limited if session_bad_password_count_max_exceeded?
return process_locked_out_user if current_user && user_locked_out?(current_user)
return process_rate_limited if rate_limited?
return process_failed_captcha unless valid_captcha_result? || log_captcha_failures_only?
return process_failed_captcha unless recaptcha_response.success? || log_captcha_failures_only?

rate_limit_password_failure = true
self.resource = warden.authenticate!(auth_options)
Expand Down Expand Up @@ -100,11 +100,10 @@ def locked_out_time_remaining
distance_of_time_in_words(Time.zone.now, time_lockout_expires, true)
end

def valid_captcha_result?
return @valid_captcha_result if defined?(@valid_captcha_result)
@valid_captcha_result = recaptcha_form.submit(
def recaptcha_response
@recaptcha_response ||= recaptcha_form.submit(
recaptcha_token: params.require(:user)[:recaptcha_token],
).success?
)
end

def recaptcha_form
Expand Down Expand Up @@ -206,15 +205,16 @@ def track_authentication_attempt

success = current_user.present? &&
!user_locked_out?(user) &&
(valid_captcha_result? || log_captcha_failures_only?)
(recaptcha_response.success? || log_captcha_failures_only?)

analytics.email_and_password_auth(
**recaptcha_response.to_h,
success: success,
user_id: user.uuid,
user_locked_out: user_locked_out?(user),
rate_limited: rate_limited?,
captcha_validation_performed: captcha_validation_performed?,
valid_captcha_result: valid_captcha_result?,
valid_captcha_result: recaptcha_response.success?,
bad_password_count: session[:bad_password_count].to_i,
sp_request_url_present: sp_session[:request_url].present?,
remember_device: remember_device_cookie.present?,
Expand Down
8 changes: 6 additions & 2 deletions app/forms/add_user_email_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ class AddUserEmailForm
include FormAddEmailValidator
include ActionView::Helpers::TranslationHelper

attr_reader :email
attr_reader :email, :in_select_email_flow

def self.model_name
ActiveModel::Name.new(self, nil, 'User')
end

def initialize(in_select_email_flow = nil)
@in_select_email_flow = in_select_email_flow
end

def user
@user ||= User.new
end
Expand Down Expand Up @@ -47,7 +51,7 @@ def email_address_record(email)
def process_successful_submission
@success = true
email_address.save!
SendAddEmailConfirmation.new(user).call(email_address)
SendAddEmailConfirmation.new(user).call(email_address, in_select_email_flow)
end

def extra_analytics_attributes
Expand Down
4 changes: 4 additions & 0 deletions app/javascript/packages/address-search/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# `Change Log`

## v3.3.0 (2024-11-05)

- Remove obsolete `is_pilot` field from PostOffice and FormattedLocation types

## v3.2.0 (2024-10-01)

### New feature
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('InPersonLocations', () => {
saturdayHours: '9 AM - 6 PM',
sundayHours: 'Closed',
id: 1,
isPilot: false,
},
{
name: 'test name',
Expand All @@ -36,7 +35,6 @@ describe('InPersonLocations', () => {
saturdayHours: '10 AM - 5 PM',
sundayHours: 'Closed',
id: 1,
isPilot: false,
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export interface FormattedLocation {
streetAddress: string;
sundayHours: string;
weekdayHours: string;
isPilot: boolean;
}

function InPersonLocations({
Expand All @@ -22,20 +21,17 @@ function InPersonLocations({
noInPersonLocationsDisplay: NoInPersonLocationsDisplay,
resultsHeaderComponent: HeaderComponent,
}: InPersonLocationsProps) {
const isPilot = locations?.some((l) => l.isPilot);

if (locations?.length === 0) {
return <NoInPersonLocationsDisplay address={address} />;
}

return (
<>
<h3 role="status">
{!isPilot &&
t('in_person_proofing.body.location.po_search.results_description', {
address,
count: locations?.length,
})}
{t('in_person_proofing.body.location.po_search.results_description', {
address,
count: locations?.length,
})}
</h3>
{HeaderComponent && <HeaderComponent />}
{onSelect && <p>{t('in_person_proofing.body.location.po_search.results_instructions')}</p>}
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/packages/address-search/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@18f/identity-address-search",
"version": "3.2.0",
"version": "3.3.0",
"type": "module",
"private": false,
"files": [
Expand Down
2 changes: 0 additions & 2 deletions app/javascript/packages/address-search/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ interface FormattedLocation {
streetAddress: string;
sundayHours: string;
weekdayHours: string;
isPilot: boolean;
}

interface PostOffice {
Expand All @@ -24,7 +23,6 @@ interface PostOffice {
weekday_hours: string;
zip_code_4: string;
zip_code_5: string;
is_pilot: boolean;
}

interface LocationQuery {
Expand Down
1 change: 0 additions & 1 deletion app/javascript/packages/address-search/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const formatLocations = (postOffices: PostOffice[]): FormattedLocation[]
streetAddress: po.address,
sundayHours: po.sunday_hours,
weekdayHours: po.weekday_hours,
isPilot: !!po.is_pilot,
}));

export const snakeCase = (value: string) =>
Expand Down
2 changes: 2 additions & 0 deletions app/javascript/packages/analytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export function trackEvent(event: string, payload?: object) {
* Logs an error.
*
* @param error Error object.
* @param event Error event, if error is caught using an `error` event handler. Including this can
* add additional resolution to the logged error, notably the filename where the error occurred.
*/
export const trackError = ({ name, message, stack }: Error, event?: ErrorEvent) =>
trackEvent('Frontend Error', { name, message, stack, filename: event?.filename });
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
import quibble from 'quibble';
import type { SinonStub } from 'sinon';
import userEvent from '@testing-library/user-event';
import { screen, waitFor, fireEvent } from '@testing-library/dom';
import { useSandbox, useDefineProperty } from '@18f/identity-test-helpers';
import '@18f/identity-spinner-button/spinner-button-element';
import './captcha-submit-button-element';

describe('CaptchaSubmitButtonElement', () => {
const sandbox = useSandbox();
const trackError = sandbox.stub();

before(async () => {
quibble('@18f/identity-analytics', { trackError });
await import('./captcha-submit-button-element');
});

afterEach(() => {
trackError.reset();
});

after(() => {
quibble.reset();
});

context('without ancestor form element', () => {
beforeEach(() => {
Expand Down Expand Up @@ -157,6 +171,35 @@ describe('CaptchaSubmitButtonElement', () => {
await waitFor(() => expect(didSubmit).to.be.true());
});
});

context('when recaptcha fails to execute', () => {
let error: Error;

beforeEach(() => {
error = new Error('Invalid site key or not loaded in api.js: badkey');
((global as any).grecaptcha.execute as SinonStub).throws(error);
});

it('does not prevent default form submission', async () => {
const button = screen.getByRole('button', { name: 'Submit' });
const form = document.querySelector('form')!;
sandbox.stub(form, 'submit');

await userEvent.click(button);

await expect(form.submit).to.eventually.be.called();
});

it('tracks error', async () => {
const button = screen.getByRole('button', { name: 'Submit' });
const form = document.querySelector('form')!;
sandbox.stub(form, 'submit');

await userEvent.click(button);

await expect(trackError).to.eventually.be.calledWith(error);
});
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { trackError } from '@18f/identity-analytics';

class CaptchaSubmitButtonElement extends HTMLElement {
form: HTMLFormElement | null;

Expand Down Expand Up @@ -46,7 +48,14 @@ class CaptchaSubmitButtonElement extends HTMLElement {
invokeChallenge() {
this.recaptchaClient!.ready(async () => {
const { recaptchaSiteKey: siteKey, recaptchaAction: action } = this;
const token = await this.recaptchaClient!.execute(siteKey!, { action });

let token;
try {
token = await this.recaptchaClient!.execute(siteKey!, { action });
} catch (error) {
trackError(error);
}

this.tokenInput.value = token;
this.submit();
});
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/data_warehouse/daily_sensitive_column_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def generate_column_data(columns, table)
end

def bucket_name
bucket_name = IdentityConfig.store.s3_idp_internal_dw_tasks
bucket_name = IdentityConfig.store.s3_idp_dw_tasks
env = Identity::Hostdata.env
aws_account_id = Identity::Hostdata.aws_account_id
aws_region = Identity::Hostdata.aws_region
Expand Down
Loading