Skip to content
Merged
49 changes: 49 additions & 0 deletions app/jobs/expire_account_reset_requests_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

class ExpireAccountResetRequestsJob < ApplicationJob
queue_as :long_running

def perform(now)
resets = 0
expired_days = (
IdentityConfig.store.account_reset_wait_period_days +
IdentityConfig.store.account_reset_token_valid_for_days
).days
AccountResetRequest.where(
sql_query_for_users_with_expired_requests,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I referred to prior art with this worker job https://github.com/18F/identity-idp/blob/main/app/jobs/grant_account_reset_requests_and_send_emails_job.rb#L31-L39 using SQL. I assumed it was more efficient to use a SQL query.

tvalue: now - expired_days,
).order('requested_at ASC').limit(1_000).each do |arr|
resets += 1 if expire_request(arr)
end

analytics.account_reset_request_expired(count: resets)

resets
end

private

def analytics
@analytics ||= Analytics.new(
user: AnonymousUser.new,
request: nil,
sp: nil,
session: {},
)
end

def sql_query_for_users_with_expired_requests
<<~SQL
request_token IS NOT NULL AND
cancelled_at IS NULL AND
granted_at < :tvalue
SQL
end

def expire_request(arr)
arr.update!(
request_token: nil,
granted_token: nil,
)
end
end
6 changes: 6 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ def account_reset_request(
)
end

# Tracks expiration of account reset requests
# @param [Integer] count number of requests expired
def account_reset_request_expired(count:, **extra)
track_event(:account_reset_request_expired, count: count, **extra)
end

# User visited the account deletion and reset page
def account_reset_visit
track_event('Account deletion and reset visited')
Expand Down
6 changes: 6 additions & 0 deletions config/initializers/job_configurations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
cron: cron_5m,
args: -> { [Time.zone.now] },
},
# Cancel expired account reset requests
expire_account_reset_requests: {
class: 'ExpireAccountResetRequestsJob',
cron: cron_5m,
args: -> { [Time.zone.now] },
},
# Send Total Monthly Auths Report to S3
total_monthly_auths: {
class: 'Reports::TotalMonthlyAuthsReport',
Expand Down
32 changes: 32 additions & 0 deletions spec/controllers/account_reset/delete_account_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,38 @@
expect(response).to redirect_to account_reset_confirm_delete_account_url
end

it 'allows deletion after expire reset job has run and reset still qualifies' do
user = create(:user, :fully_registered, :with_backup_code, confirmed_at: Time.zone.now.round)
create(:phone_configuration, user: user, phone: Faker::PhoneNumber.cell_phone)
create_list(:webauthn_configuration, 2, user: user)
create_account_reset_request_for(user)
grant_request(user)
session[:granted_token] = AccountResetRequest.first.granted_token

ExpireAccountResetRequestsJob.new.perform(Time.zone.now)
expect(@attempts_api_tracker).to receive(:account_reset_account_deleted).with(
success: true,
failure_reason: nil,
)

delete :delete

expect(@analytics).to have_logged_event(
'Account Reset: delete',
user_id: user.uuid,
success: true,
mfa_method_counts: {
backup_codes: BackupCodeGenerator::NUMBER_OF_CODES,
webauthn: 2,
phone: 2,
},
identity_verified: false,
account_age_in_days: 0,
account_confirmed_at: user.confirmed_at,
)
expect(response).to redirect_to account_reset_confirm_delete_account_url
end

it 'redirects to root if the token does not match one in the DB' do
session[:granted_token] = 'foo'
expect(@attempts_api_tracker).to receive(:account_reset_account_deleted).with(
Expand Down
43 changes: 43 additions & 0 deletions spec/jobs/expire_account_reset_requests_job_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'rails_helper'

RSpec.describe ExpireAccountResetRequestsJob do
include AccountResetHelper
describe '#perform' do
subject(:perform) { job.perform(now) }
let(:job) { ExpireAccountResetRequestsJob.new }
let(:job_analytics) { FakeAnalytics.new }
let(:now) { Time.zone.now }

before do
allow(IdentityConfig.store).to receive(:account_reset_token_valid_for_days)
.and_return(0)
allow(Analytics).to receive(:new).and_return(job_analytics)
end

it 'it expires requests with expired grant tokens and ignores valid grant tokens' do
user = create(:user, :fully_registered, confirmed_at: Time.zone.now.round)
create_account_reset_request_for(user)
grant_request(user)

travel_to(Time.zone.now + 2.days) do
user2 = create(
:user, :fully_registered,
confirmed_at: Time.zone.now.round
)
create_account_reset_request_for(user2)
grant_request(user2)

notification_sent = perform

expect(job_analytics).to have_logged_event(
:account_reset_request_expired,
count: 1,
)
expect(notification_sent).to eq(1)

expect(AccountResetRequest.first.request_token).to be(nil)
expect(AccountResetRequest.second.request_token).to_not be(nil)
end
end
end
end