Skip to content

Commit

Permalink
Merge pull request #11472 from 18F/stages/rc-2024-11-07
Browse files Browse the repository at this point in the history
Deploy RC 429 to Production
  • Loading branch information
jmhooper authored Nov 7, 2024
2 parents 7fc5046 + cc00194 commit 9f98746
Show file tree
Hide file tree
Showing 50 changed files with 310 additions and 86 deletions.
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

0 comments on commit 9f98746

Please sign in to comment.