Skip to content

Commit

Permalink
Allow multiple USPS confirmation codes
Browse files Browse the repository at this point in the history
**Why**: So that when a user resends a letter, it does not invalidate
the USPS confirmation code in preceding letters. This means that if a
user resends a letter before receiving their first letter, the code in
the first letter can still be used to verify their account.

**How**: This commit adds a new model named `UspsConfirmationCode`. This
model is associated with a profile and has an asymmetrically encrypted
OTP. When the user enters an OTP, the record with the matching OTP
fingerprint is found. This is used to determine whether the entered OTP
is valid for verifying their account.
  • Loading branch information
jmhooper committed Sep 8, 2017
1 parent 073261f commit 5c3ea90
Show file tree
Hide file tree
Showing 30 changed files with 499 additions and 258 deletions.
12 changes: 2 additions & 10 deletions app/controllers/users/verify_account_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def index
@verify_account_form = VerifyAccountForm.new(user: current_user)

return unless FeatureManagement.reveal_usps_code?
@code = JSON.parse(user_session[:decrypted_pii])['otp']['raw']
@code = session[:last_usps_confirmation_code]
end

def create
Expand All @@ -28,8 +28,7 @@ def create
def build_verify_account_form
VerifyAccountForm.new(
user: current_user,
otp: params_otp,
pii_attributes: decrypted_pii
otp: params_otp
)
end

Expand All @@ -41,12 +40,5 @@ def confirm_verification_needed
return if current_user.decorate.pending_profile_requires_verification?
redirect_to account_url
end

def decrypted_pii
@_decrypted_pii ||= begin
cacher = Pii::Cacher.new(current_user, user_session)
cacher.fetch
end
end
end
end
11 changes: 3 additions & 8 deletions app/controllers/verify/come_back_later_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,14 @@ module Verify
class ComeBackLaterController < ApplicationController
include IdvSession

before_action :confirm_idv_session_completed
before_action :confirm_usps_verification_method_chosen
before_action :confirm_user_needs_usps_confirmation

def show; end

private

def confirm_idv_session_completed
redirect_to account_path if idv_session.profile.blank?
end

def confirm_usps_verification_method_chosen
redirect_to account_path unless idv_session.address_verification_mechanism == 'usps'
def confirm_user_needs_usps_confirmation
redirect_to account_path unless current_user.decorate.needs_profile_usps_verification?
end
end
end
3 changes: 3 additions & 0 deletions app/controllers/verify/review_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ def create
init_profile
redirect_to verify_confirmations_path
analytics.track_event(Analytics::IDV_REVIEW_COMPLETE)

return unless FeatureManagement.reveal_usps_code?
session[:last_usps_confirmation_code] = idv_session.usps_otp
end

private
Expand Down
15 changes: 14 additions & 1 deletion app/controllers/verify/usps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ def create
idv_session.address_verification_mechanism = :usps

if current_user.decorate.needs_profile_usps_verification?
redirect_to account_path
resend_letter
redirect_to verify_come_back_later_url
else
redirect_to verify_review_url
end
Expand All @@ -29,5 +30,17 @@ def confirm_mail_not_spammed
redirect_to verify_review_path if idv_session.address_mechanism_chosen? &&
usps_mail_service.mail_spammed?
end

def resend_letter
confirmation_maker = UspsConfirmationMaker.new(
pii: Pii::Cacher.new(current_user, user_session).fetch,
issuer: sp_session[:issuer],
profile: current_user.decorate.pending_profile
)
confirmation_maker.perform

return unless FeatureManagement.reveal_usps_code?
session[:last_usps_confirmation_code] = confirmation_maker.otp
end
end
end
15 changes: 9 additions & 6 deletions app/forms/verify_account_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ class VerifyAccountForm
attr_accessor :otp, :pii_attributes
attr_reader :user

def initialize(user:, otp: nil, pii_attributes: nil)
def initialize(user:, otp: nil)
@user = user
@otp = otp
@pii_attributes = pii_attributes
end

def submit
Expand All @@ -31,8 +30,14 @@ def pending_profile
@_pending_profile ||= user.decorate.pending_profile
end

def usps_confirmation_code
return if otp.blank? || pending_profile.blank?

pending_profile.usps_confirmation_codes.first_with_otp(otp)
end

def validate_otp_not_expired
return unless Idv::UspsMail.new(user).most_recent_otp_expired?
return unless usps_confirmation_code.present? && usps_confirmation_code.expired?

errors.add :otp, :usps_otp_expired
end
Expand All @@ -47,9 +52,7 @@ def validate_otp
end

def valid_otp?
otp.present? && ActiveSupport::SecurityUtils.secure_compare(
Base32::Crockford.normalize(otp), Base32::Crockford.normalize(pii_attributes.otp.to_s)
)
otp.present? && usps_confirmation_code.present?
end

def reset_sensitive_fields
Expand Down
1 change: 1 addition & 0 deletions app/models/profile.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class Profile < ApplicationRecord
belongs_to :user
has_many :usps_confirmation_codes

validates :active, uniqueness: { scope: :user_id, if: :active? }
validates :ssn_signature, uniqueness: { scope: :active, if: :active? }
Expand Down
16 changes: 16 additions & 0 deletions app/models/usps_confirmation_code.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class UspsConfirmationCode < ApplicationRecord
belongs_to :profile

def self.first_with_otp(otp)
find do |usps_confirmation_code|
Pii::Fingerprinter.verify(
Base32::Crockford.normalize(otp),
usps_confirmation_code.otp_fingerprint
)
end
end

def expired?
code_sent_at < Figaro.env.usps_confirmation_max_days.to_i.days.ago
end
end
8 changes: 1 addition & 7 deletions app/services/idv/profile_maker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,9 @@ def pii_from_applicant(appl, norm_appl)
zipcode: Pii::Attribute.new(raw: appl.zipcode, norm: norm_appl.zipcode),
dob: Pii::Attribute.new(raw: appl.dob, norm: norm_appl.dob),
ssn: Pii::Attribute.new(raw: appl.ssn, norm: norm_appl.ssn),
phone: Pii::Attribute.new(raw: appl.phone, norm: norm_appl.phone),
otp: generate_otp
phone: Pii::Attribute.new(raw: appl.phone, norm: norm_appl.phone)
)
end
# rubocop:enable MethodLength, AbcSize

def generate_otp(length: 10)
# Crockford encoding is 5 bits per character
Base32::Crockford.encode(SecureRandom.random_number(2**(5 * length)), length: length)
end
end
end
6 changes: 4 additions & 2 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Session
vendor_session_id
].freeze

attr_reader :current_user
attr_reader :current_user, :usps_otp

def initialize(user_session:, current_user:, issuer:)
@user_session = user_session
Expand Down Expand Up @@ -91,8 +91,10 @@ def create_usps_entry
if pii.is_a?(String)
self.pii = Pii::Attributes.new_from_json(user_session[:decrypted_pii])
end
confirmation_maker = UspsConfirmationMaker.new(pii: pii, issuer: issuer, profile: profile)
confirmation_maker.perform

UspsConfirmationMaker.new(pii: pii, issuer: issuer).perform
@usps_otp = confirmation_maker.otp
end

def alive?
Expand Down
7 changes: 0 additions & 7 deletions app/services/idv/usps_mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module Idv
class UspsMail
MAX_MAIL_EVENTS = Figaro.env.max_mail_events.to_i
MAIL_EVENTS_WINDOW_DAYS = Figaro.env.max_mail_events_window_in_days.to_i
USPS_CONFIRMATION_WINDOW_DAYS = Figaro.env.usps_confirmation_max_days.to_i

def initialize(current_user)
@current_user = current_user
Expand All @@ -17,12 +16,6 @@ def any_mail_sent?
user_mail_events.any?
end

def most_recent_otp_expired?
return false unless any_mail_sent?

user_mail_events.first.updated_at < USPS_CONFIRMATION_WINDOW_DAYS.days.ago
end

private

attr_reader :current_user
Expand Down
2 changes: 1 addition & 1 deletion app/services/pii/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Pii
Attributes = Struct.new(
:first_name, :middle_name, :last_name,
:address1, :address2, :city, :state, :zipcode,
:ssn, :dob, :phone, :otp,
:ssn, :dob, :phone,
:prev_address1, :prev_address2, :prev_city, :prev_state, :prev_zipcode
) do
def self.new_from_hash(hash)
Expand Down
20 changes: 17 additions & 3 deletions app/services/usps_confirmation_maker.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
class UspsConfirmationMaker
def initialize(pii:, issuer:)
def initialize(pii:, issuer:, profile:)
@pii = pii
@issuer = issuer
@profile = profile
end

def otp
@otp ||= generate_otp
end

def perform
entry = UspsConfirmationEntry.new_from_hash(attributes)
UspsConfirmation.create!(entry: entry.encrypted)
UspsConfirmationCode.create!(
profile: profile,
otp_fingerprint: Pii::Fingerprinter.fingerprint(otp)
)
end

private

attr_reader :pii, :issuer
attr_reader :pii, :issuer, :profile

# rubocop:disable AbcSize, MethodLength
# This method is single statement spread across many lines for readability
Expand All @@ -20,7 +29,7 @@ def attributes
address1: pii[:address1].norm,
address2: pii[:address2].norm,
city: pii[:city].norm,
otp: pii[:otp].norm,
otp: otp,
first_name: pii[:first_name].norm,
last_name: pii[:last_name].norm,
state: pii[:state].norm,
Expand All @@ -29,4 +38,9 @@ def attributes
}
end
# rubocop:enable AbcSize, MethodLength

def generate_otp
# Crockford encoding is 5 bits per character
Base32::Crockford.encode(SecureRandom.random_number(2**(5 * 10)), length: 10)
end
end
12 changes: 12 additions & 0 deletions db/migrate/20170905144239_create_usps_confirmation_codes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreateUspsConfirmationCodes < ActiveRecord::Migration[5.1]
def change
create_table :usps_confirmation_codes do |t|
t.integer :profile_id, null: false
t.string :otp_fingerprint, null: false
t.datetime :code_sent_at, null: false, default: ->{ 'CURRENT_TIMESTAMP' }
t.index :profile_id, using: :btree

t.timestamps
end
end
end
Loading

0 comments on commit 5c3ea90

Please sign in to comment.