diff --git a/app/controllers/users/verify_account_controller.rb b/app/controllers/users/verify_account_controller.rb index 513ba7e9d98..4d2a5feb2df 100644 --- a/app/controllers/users/verify_account_controller.rb +++ b/app/controllers/users/verify_account_controller.rb @@ -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 @@ -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 @@ -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 diff --git a/app/controllers/verify/come_back_later_controller.rb b/app/controllers/verify/come_back_later_controller.rb index 1aab4f04b13..c854758ad30 100644 --- a/app/controllers/verify/come_back_later_controller.rb +++ b/app/controllers/verify/come_back_later_controller.rb @@ -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 diff --git a/app/controllers/verify/review_controller.rb b/app/controllers/verify/review_controller.rb index efb27ee78f4..7bd7bed0675 100644 --- a/app/controllers/verify/review_controller.rb +++ b/app/controllers/verify/review_controller.rb @@ -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 diff --git a/app/controllers/verify/usps_controller.rb b/app/controllers/verify/usps_controller.rb index 642810fbe3e..e134d645c4c 100644 --- a/app/controllers/verify/usps_controller.rb +++ b/app/controllers/verify/usps_controller.rb @@ -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 @@ -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 diff --git a/app/forms/verify_account_form.rb b/app/forms/verify_account_form.rb index 90aebb5782b..b1b268fd4c9 100644 --- a/app/forms/verify_account_form.rb +++ b/app/forms/verify_account_form.rb @@ -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 @@ -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 @@ -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 diff --git a/app/models/profile.rb b/app/models/profile.rb index 7f74da26ef7..cae60c6893d 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -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? } diff --git a/app/models/usps_confirmation_code.rb b/app/models/usps_confirmation_code.rb new file mode 100644 index 00000000000..0881adb5bf3 --- /dev/null +++ b/app/models/usps_confirmation_code.rb @@ -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 diff --git a/app/services/idv/profile_maker.rb b/app/services/idv/profile_maker.rb index 983e5601640..797f9af2aff 100644 --- a/app/services/idv/profile_maker.rb +++ b/app/services/idv/profile_maker.rb @@ -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 diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index a1c35ec9274..64230b9bcdc 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -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 @@ -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? diff --git a/app/services/idv/usps_mail.rb b/app/services/idv/usps_mail.rb index 07337ebb74d..26b9193049f 100644 --- a/app/services/idv/usps_mail.rb +++ b/app/services/idv/usps_mail.rb @@ -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 @@ -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 diff --git a/app/services/pii/attributes.rb b/app/services/pii/attributes.rb index 239e8134de4..ba0c5e28c6c 100644 --- a/app/services/pii/attributes.rb +++ b/app/services/pii/attributes.rb @@ -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) diff --git a/app/services/usps_confirmation_maker.rb b/app/services/usps_confirmation_maker.rb index e9a3ddf8af5..1d604a7ed8b 100644 --- a/app/services/usps_confirmation_maker.rb +++ b/app/services/usps_confirmation_maker.rb @@ -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 @@ -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, @@ -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 diff --git a/db/migrate/20170905144239_create_usps_confirmation_codes.rb b/db/migrate/20170905144239_create_usps_confirmation_codes.rb new file mode 100644 index 00000000000..7b6f406040a --- /dev/null +++ b/db/migrate/20170905144239_create_usps_confirmation_codes.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 08da593db47..6cc72a30e39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,4 +1,3 @@ -# encoding: UTF-8 # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -11,189 +10,189 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170626205402) do +ActiveRecord::Schema.define(version: 20170905144239) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "app_settings", force: :cascade do |t| - t.string "name", limit: 255 - t.string "value", limit: 255 + t.string "name", limit: 255 + t.string "value", limit: 255 t.datetime "created_at" t.datetime "updated_at" + t.index ["name"], name: "index_app_settings_on_name" end - add_index "app_settings", ["name"], name: "index_app_settings_on_name", using: :btree - create_table "authorizations", force: :cascade do |t| - t.string "provider", limit: 255 - t.string "uid", limit: 255 - t.integer "user_id" + t.string "provider", limit: 255 + t.string "uid", limit: 255 + t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" t.datetime "authorized_at" + t.index ["provider", "uid"], name: "index_authorizations_on_provider_and_uid" + t.index ["user_id"], name: "index_authorizations_on_user_id" end - add_index "authorizations", ["provider", "uid"], name: "index_authorizations_on_provider_and_uid", using: :btree - add_index "authorizations", ["user_id"], name: "index_authorizations_on_user_id", using: :btree - create_table "events", force: :cascade do |t| - t.integer "user_id", null: false - t.integer "event_type", null: false + t.integer "user_id", null: false + t.integer "event_type", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_events_on_user_id" end - add_index "events", ["user_id"], name: "index_events_on_user_id", using: :btree - create_table "identities", force: :cascade do |t| - t.string "service_provider", limit: 255 + t.string "service_provider", limit: 255 t.datetime "last_authenticated_at" - t.integer "user_id" + t.integer "user_id" t.datetime "created_at" t.datetime "updated_at" - t.string "session_uuid", limit: 255 - t.string "uuid", null: false - t.string "nonce" - t.integer "ial", default: 1 - t.string "access_token" - t.string "scope" - t.string "code_challenge" - t.string "rails_session_id" + t.string "session_uuid", limit: 255 + t.string "uuid", null: false + t.string "nonce" + t.integer "ial", default: 1 + t.string "access_token" + t.string "scope" + t.string "code_challenge" + t.string "rails_session_id" + t.index ["access_token"], name: "index_identities_on_access_token", unique: true + t.index ["session_uuid"], name: "index_identities_on_session_uuid", unique: true + t.index ["user_id", "service_provider"], name: "index_identities_on_user_id_and_service_provider" + t.index ["user_id"], name: "index_identities_on_user_id" + t.index ["uuid"], name: "index_identities_on_uuid", unique: true end - add_index "identities", ["access_token"], name: "index_identities_on_access_token", unique: true, using: :btree - add_index "identities", ["session_uuid"], name: "index_identities_on_session_uuid", unique: true, using: :btree - add_index "identities", ["user_id", "service_provider"], name: "index_identities_on_user_id_and_service_provider", using: :btree - add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree - add_index "identities", ["uuid"], name: "index_identities_on_uuid", unique: true, using: :btree - create_table "otp_requests_trackers", force: :cascade do |t| t.datetime "otp_last_sent_at" - t.integer "otp_send_count", default: 0 - t.string "attribute_cost" - t.string "phone_fingerprint", default: "", null: false + t.integer "otp_send_count", default: 0 + t.string "attribute_cost" + t.string "phone_fingerprint", default: "", null: false t.datetime "created_at" t.datetime "updated_at" + t.index ["phone_fingerprint"], name: "index_otp_requests_trackers_on_phone_fingerprint", unique: true + t.index ["updated_at"], name: "index_otp_requests_trackers_on_updated_at" end - add_index "otp_requests_trackers", ["phone_fingerprint"], name: "index_otp_requests_trackers_on_phone_fingerprint", unique: true, using: :btree - add_index "otp_requests_trackers", ["updated_at"], name: "index_otp_requests_trackers_on_updated_at", using: :btree - create_table "profiles", force: :cascade do |t| - t.integer "user_id", null: false - t.boolean "active", default: false, null: false + t.integer "user_id", null: false + t.boolean "active", default: false, null: false t.datetime "verified_at" t.datetime "activated_at" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "vendor" - t.text "encrypted_pii" - t.string "ssn_signature", limit: 64 - t.text "encrypted_pii_recovery" - t.integer "deactivation_reason" - t.boolean "phone_confirmed", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "vendor" + t.text "encrypted_pii" + t.string "ssn_signature", limit: 64 + t.text "encrypted_pii_recovery" + t.integer "deactivation_reason" + t.boolean "phone_confirmed", default: false, null: false + t.index ["ssn_signature", "active"], name: "index_profiles_on_ssn_signature_and_active", unique: true, where: "(active = true)" + t.index ["ssn_signature"], name: "index_profiles_on_ssn_signature" + t.index ["user_id", "active"], name: "index_profiles_on_user_id_and_active", unique: true, where: "(active = true)" + t.index ["user_id", "ssn_signature", "active"], name: "index_profiles_on_user_id_and_ssn_signature_and_active", unique: true, where: "(active = true)" + t.index ["user_id"], name: "index_profiles_on_user_id" end - add_index "profiles", ["ssn_signature", "active"], name: "index_profiles_on_ssn_signature_and_active", unique: true, where: "(active = true)", using: :btree - add_index "profiles", ["ssn_signature"], name: "index_profiles_on_ssn_signature", using: :btree - add_index "profiles", ["user_id", "active"], name: "index_profiles_on_user_id_and_active", unique: true, where: "(active = true)", using: :btree - add_index "profiles", ["user_id", "ssn_signature", "active"], name: "index_profiles_on_user_id_and_ssn_signature_and_active", unique: true, where: "(active = true)", using: :btree - add_index "profiles", ["user_id"], name: "index_profiles_on_user_id", using: :btree - create_table "service_provider_requests", force: :cascade do |t| - t.string "issuer", null: false - t.string "loa", null: false - t.string "url", null: false - t.string "uuid", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "requested_attributes", default: [], array: true + t.string "issuer", null: false + t.string "loa", null: false + t.string "url", null: false + t.string "uuid", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "requested_attributes", default: [], array: true + t.index ["uuid"], name: "index_service_provider_requests_on_uuid", unique: true end - add_index "service_provider_requests", ["uuid"], name: "index_service_provider_requests_on_uuid", unique: true, using: :btree - create_table "service_providers", force: :cascade do |t| - t.string "issuer", null: false - t.string "friendly_name" - t.text "description" - t.text "metadata_url" - t.text "acs_url" - t.text "assertion_consumer_logout_service_url" - t.text "cert" - t.text "logo" - t.string "fingerprint" - t.string "signature" - t.string "block_encryption", default: "aes256-cbc", null: false - t.text "sp_initiated_login_url" - t.text "return_to_sp_url" - t.string "agency" - t.json "attribute_bundle" - t.string "redirect_uri" + t.string "issuer", null: false + t.string "friendly_name" + t.text "description" + t.text "metadata_url" + t.text "acs_url" + t.text "assertion_consumer_logout_service_url" + t.text "cert" + t.text "logo" + t.string "fingerprint" + t.string "signature" + t.string "block_encryption", default: "aes256-cbc", null: false + t.text "sp_initiated_login_url" + t.text "return_to_sp_url" + t.string "agency" + t.json "attribute_bundle" + t.string "redirect_uri" t.datetime "created_at" t.datetime "updated_at" - t.boolean "active", default: false, null: false - t.boolean "approved", default: false, null: false - t.boolean "native", default: false, null: false - t.string "redirect_uris", default: [], array: true + t.boolean "active", default: false, null: false + t.boolean "approved", default: false, null: false + t.boolean "native", default: false, null: false + t.string "redirect_uris", default: [], array: true + t.index ["issuer"], name: "index_service_providers_on_issuer", unique: true end - add_index "service_providers", ["issuer"], name: "index_service_providers_on_issuer", unique: true, using: :btree - create_table "users", force: :cascade do |t| - t.string "encrypted_password", limit: 255, default: "" - t.string "reset_password_token", limit: 255 + t.string "encrypted_password", limit: 255, default: "" + t.string "reset_password_token", limit: 255 t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" - t.string "current_sign_in_ip", limit: 255 - t.string "last_sign_in_ip", limit: 255 + t.string "current_sign_in_ip", limit: 255 + t.string "last_sign_in_ip", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "confirmation_token", limit: 255 + t.string "confirmation_token", limit: 255 t.datetime "confirmed_at" t.datetime "confirmation_sent_at" - t.string "unconfirmed_email", limit: 255 - t.integer "role" - t.integer "second_factor_attempts_count", default: 0 - t.string "uuid", limit: 255, null: false + t.string "unconfirmed_email", limit: 255 + t.integer "role" + t.integer "second_factor_attempts_count", default: 0 + t.string "uuid", limit: 255, null: false t.datetime "reset_requested_at" t.datetime "second_factor_locked_at" t.datetime "locked_at" - t.integer "failed_attempts", default: 0 - t.string "unlock_token", limit: 255 + t.integer "failed_attempts", default: 0 + t.string "unlock_token", limit: 255 t.datetime "phone_confirmed_at" - t.text "encrypted_otp_secret_key" - t.string "direct_otp" + t.text "encrypted_otp_secret_key" + t.string "direct_otp" t.datetime "direct_otp_sent_at" t.datetime "idv_attempted_at" - t.integer "idv_attempts", default: 0 - t.string "recovery_code" - t.string "password_salt" - t.string "encryption_key" - t.string "unique_session_id" - t.string "recovery_salt" - t.string "password_cost" - t.string "recovery_cost" - t.string "email_fingerprint", default: "", null: false - t.text "encrypted_email", default: "", null: false - t.string "attribute_cost" - t.text "encrypted_phone" - t.integer "otp_delivery_preference", default: 0, null: false + t.integer "idv_attempts", default: 0 + t.string "recovery_code" + t.string "password_salt" + t.string "encryption_key" + t.string "unique_session_id" + t.string "recovery_salt" + t.string "password_cost" + t.string "recovery_cost" + t.string "email_fingerprint", default: "", null: false + t.text "encrypted_email", default: "", null: false + t.string "attribute_cost" + t.text "encrypted_phone" + t.integer "otp_delivery_preference", default: 0, null: false + t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true + t.index ["email_fingerprint"], name: "index_users_on_email_fingerprint", unique: true + t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true + t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true + t.index ["unconfirmed_email"], name: "index_users_on_unconfirmed_email" + t.index ["unlock_token"], name: "index_users_on_unlock_token" + t.index ["uuid"], name: "index_users_on_uuid", unique: true end - add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree - add_index "users", ["email_fingerprint"], name: "index_users_on_email_fingerprint", unique: true, using: :btree - add_index "users", ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true, using: :btree - add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree - add_index "users", ["unconfirmed_email"], name: "index_users_on_unconfirmed_email", using: :btree - add_index "users", ["unlock_token"], name: "index_users_on_unlock_token", using: :btree - add_index "users", ["uuid"], name: "index_users_on_uuid", unique: true, using: :btree + create_table "usps_confirmation_codes", force: :cascade do |t| + t.integer "profile_id", null: false + t.string "otp_fingerprint", null: false + t.datetime "code_sent_at", default: -> { "now()" }, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["profile_id"], name: "index_usps_confirmation_codes_on_profile_id" + end create_table "usps_confirmations", force: :cascade do |t| - t.text "entry", null: false + t.text "entry", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index f5a30f62989..9e55dc50bf7 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -327,7 +327,7 @@ :profile, deactivation_reason: :verification_pending, phone_confirmed: false, - pii: { otp: 'abc123', ssn: '6666', dob: '1920-01-01' } + pii: { ssn: '6666', dob: '1920-01-01' } ) user = profile.user diff --git a/spec/controllers/users/verify_account_controller_spec.rb b/spec/controllers/users/verify_account_controller_spec.rb index 06894fd0270..16fb4af70ee 100644 --- a/spec/controllers/users/verify_account_controller_spec.rb +++ b/spec/controllers/users/verify_account_controller_spec.rb @@ -5,18 +5,21 @@ let(:has_pending_profile) { true } let(:success) { true } - let(:otp) { 'abc123' } + let(:otp) { 'ABC123' } let(:submitted_otp) { otp } - let(:pii_attributes) { Pii::Attributes.new_from_hash(otp: otp) } let(:pending_profile) { build(:profile) } before do user = stub_sign_in decorated_user = stub_decorated_user_with_pending_profile(user) + create( + :usps_confirmation_code, + profile: pending_profile, + otp_fingerprint: Pii::Fingerprinter.fingerprint(otp) + ) allow(decorated_user).to receive(:needs_profile_phone_verification?).and_return(false) allow(decorated_user).to receive(:needs_profile_usps_verification?). and_return(has_pending_profile) - allow(controller).to receive(:decrypted_pii).and_return(pii_attributes) end describe '#index' do diff --git a/spec/controllers/verify/come_back_later_controller_spec.rb b/spec/controllers/verify/come_back_later_controller_spec.rb index 35cf9dbe442..bec8055623c 100644 --- a/spec/controllers/verify/come_back_later_controller_spec.rb +++ b/spec/controllers/verify/come_back_later_controller_spec.rb @@ -2,25 +2,17 @@ describe Verify::ComeBackLaterController do let(:user) { build_stubbed(:user, :signed_up) } - let(:address_verification_mechanism) { 'usps' } - let(:profile) { build_stubbed(:profile, user: user) } - let(:idv_session) do - Idv::Session.new( - user_session: { context: :idv }, - current_user: user, - issuer: nil - ) - end + let(:needs_profile_usps_verification) { true } before do - allow(idv_session).to receive(:address_verification_mechanism). - and_return(address_verification_mechanism) - allow(idv_session).to receive(:profile). - and_return(profile) - allow(subject).to receive(:idv_session).and_return(idv_session) + user_decorator = instance_double(UserDecorator) + allow(user_decorator).to receive(:needs_profile_usps_verification?). + and_return(needs_profile_usps_verification) + allow(user).to receive(:decorate).and_return(user_decorator) + allow(subject).to receive(:current_user).and_return(user) end - context 'user has selected USPS address verification and has a complete profile' do + context 'user needs USPS address verification' do it 'renders the show template' do get :show @@ -28,18 +20,8 @@ end end - context 'user has not selected USPS address verification' do - let(:address_verification_mechanism) { 'phone' } - - it 'redirects to the account path' do - get :show - - expect(response).to redirect_to account_path - end - end - - context 'does not have a complete profile' do - let(:profile) { nil } + context 'user does not need USPS address verification' do + let(:needs_profile_usps_verification) { false } it 'redirects to the account path' do get :show diff --git a/spec/controllers/verify/usps_controller_spec.rb b/spec/controllers/verify/usps_controller_spec.rb index ddf705a41f5..35a14b55dbf 100644 --- a/spec/controllers/verify/usps_controller_spec.rb +++ b/spec/controllers/verify/usps_controller_spec.rb @@ -45,17 +45,50 @@ end describe '#create' do - before do - stub_verify_steps_one_and_two(user) + context 'first time through the idv process' do + before do + stub_verify_steps_one_and_two(user) + end + + it 'sets session to :usps and redirects' do + expect(subject.idv_session.address_verification_mechanism).to be_nil + + put :create + + expect(response).to redirect_to verify_review_path + expect(subject.idv_session.address_verification_mechanism).to eq :usps + end end - it 'sets session to :usps and redirects' do - expect(subject.idv_session.address_verification_mechanism).to be_nil + context 'resending a letter' do + let(:has_pending_profile) { true } + let(:pending_profile) { create(:profile, phone_confirmed: false) } - put :create + before do + stub_sign_in(user) + stub_decorated_user_with_pending_profile(user) + allow(user.decorate).to receive(:needs_profile_usps_verification?).and_return(true) + end - expect(response).to redirect_to verify_review_path - expect(subject.idv_session.address_verification_mechanism).to eq :usps + it 'calls the UspsConfirmationMaker to send another letter and redirects' do + pii = { first_name: 'Samuel', last_name: 'Sampson' } + pii_cacher = instance_double(Pii::Cacher) + allow(pii_cacher).to receive(:fetch).and_return(pii) + allow(Pii::Cacher).to receive(:new).and_return(pii_cacher) + + session[:sp] = { issuer: '123abc' } + + usps_confirmation_maker = instance_double(UspsConfirmationMaker) + allow(UspsConfirmationMaker).to receive(:new). + with(pii: pii, issuer: '123abc', profile: pending_profile). + and_return(usps_confirmation_maker) + + expect(usps_confirmation_maker).to receive(:perform) + + put :create + + expect(response).to redirect_to verify_come_back_later_path + end end end end diff --git a/spec/factories/usps_confirmation_codes.rb b/spec/factories/usps_confirmation_codes.rb new file mode 100644 index 00000000000..fde6215e92c --- /dev/null +++ b/spec/factories/usps_confirmation_codes.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + Faker::Config.locale = :en + + factory :usps_confirmation_code do + profile + otp_fingerprint Pii::Fingerprinter.fingerprint('ABCDE12345') + end +end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index b24b07de74a..8519934b3d7 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -251,7 +251,7 @@ :profile, deactivation_reason: :verification_pending, phone_confirmed: phone_confirmed, - pii: { otp: otp, ssn: '6666', dob: '1920-01-01' } + pii: { ssn: '6666', dob: '1920-01-01' } ) end let(:oidc_auth_url) do diff --git a/spec/features/saml/loa3_sso_spec.rb b/spec/features/saml/loa3_sso_spec.rb index c713daec8a8..f81e6cfcec7 100644 --- a/spec/features/saml/loa3_sso_spec.rb +++ b/spec/features/saml/loa3_sso_spec.rb @@ -116,13 +116,12 @@ def sign_out_user context 'continuing verification' do let(:user) { profile.user } - let(:otp) { 'abc123' } let(:profile) do create( :profile, deactivation_reason: :verification_pending, phone_confirmed: phone_confirmed, - pii: { otp: otp, ssn: '6666', dob: '1920-01-01' } + pii: { ssn: '6666', dob: '1920-01-01' } ) end @@ -148,7 +147,7 @@ def sign_out_user click_button(t('idv.buttons.mail.resend')) - expect(current_path).to eq(account_path) + expect(current_path).to eq(verify_come_back_later_path) end it 'after signing out' do @@ -168,7 +167,7 @@ def sign_out_user click_button(t('idv.buttons.mail.resend')) - expect(current_path).to eq(account_path) + expect(current_path).to eq(verify_come_back_later_path) end end end diff --git a/spec/features/users/verify_profile_spec.rb b/spec/features/users/verify_profile_spec.rb index 9babf534b43..52e347d6eb0 100644 --- a/spec/features/users/verify_profile_spec.rb +++ b/spec/features/users/verify_profile_spec.rb @@ -2,23 +2,32 @@ feature 'verify profile with OTP' do let(:user) { create(:user, :signed_up) } - let(:otp) { 'abc123' } + let(:otp) { 'ABC123' } before do - create( + profile = create( :profile, deactivation_reason: :verification_pending, - pii: { otp: otp, ssn: '666-66-1234', dob: '1920-01-01', phone: '555-555-9999' }, + pii: { ssn: '666-66-1234', dob: '1920-01-01', phone: '555-555-9999' }, phone_confirmed: phone_confirmed, user: user ) + otp_fingerprint = Pii::Fingerprinter.fingerprint(otp) + create(:usps_confirmation_code, profile: profile, otp_fingerprint: otp_fingerprint) end context 'USPS letter' do let(:phone_confirmed) { false } - xscenario 'OTP has expired' do - # see https://github.com/18F/identity-private/issues/1108#issuecomment-293328267 + scenario 'OTP has expired' do + UspsConfirmationCode.first.update(code_sent_at: 11.days.ago) + + sign_in_live_with_2fa(user) + fill_in t('forms.verify_profile.name'), with: otp + click_button t('forms.verify_profile.submit') + + expect(page).to have_content t('errors.messages.usps_otp_expired') + expect(current_path).to eq verify_account_path end scenario 'wrong OTP used' do diff --git a/spec/forms/verify_account_form_spec.rb b/spec/forms/verify_account_form_spec.rb index 6efd0bc1c72..30a766bc2a9 100644 --- a/spec/forms/verify_account_form_spec.rb +++ b/spec/forms/verify_account_form_spec.rb @@ -2,15 +2,26 @@ describe VerifyAccountForm do subject(:form) do - VerifyAccountForm.new(user: user, otp: entered_otp, pii_attributes: pii_attributes) + VerifyAccountForm.new(user: user, otp: entered_otp) end let(:user) { pending_profile.user } let(:entered_otp) { otp } - let(:otp) { 'abc123' } - let(:pii_attributes) { Pii::Attributes.new_from_hash(otp: otp) } + let(:otp) { 'ABC123' } + let(:code_sent_at) { Time.zone.now } let(:pending_profile) { create(:profile, deactivation_reason: :verification_pending) } + before do + next if pending_profile.blank? + + create( + :usps_confirmation_code, + otp_fingerprint: Pii::Fingerprinter.fingerprint(otp), + code_sent_at: code_sent_at, + profile: pending_profile + ) + end + describe '#valid?' do context 'when required attributes are not present' do let(:entered_otp) { nil } @@ -61,11 +72,7 @@ end context 'when OTP is expired' do - before do - usps_mail = double(Idv::UspsMail) - allow(usps_mail).to receive(:most_recent_otp_expired?).and_return(true) - allow(Idv::UspsMail).to receive(:new).with(user).and_return(usps_mail) - end + let(:code_sent_at) { 11.days.ago } it 'is invalid' do expect(subject).to_not be_valid diff --git a/spec/models/usps_confirmation_code_spec.rb b/spec/models/usps_confirmation_code_spec.rb new file mode 100644 index 00000000000..f6a2be9987d --- /dev/null +++ b/spec/models/usps_confirmation_code_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +RSpec.describe UspsConfirmationCode do + let(:otp) { 'ABC123' } + let(:profile) { build(:profile) } + + describe '.first_with_otp' do + it 'return the record with the matching OTP' do + create(:usps_confirmation_code) + good_confirmation_code = create( + :usps_confirmation_code, + otp_fingerprint: Pii::Fingerprinter.fingerprint(otp) + ) + + expect(described_class.first_with_otp(otp)).to eq(good_confirmation_code) + end + + it 'normalizes the entered otp before searching' do + confirmation_code = create( + :usps_confirmation_code, + otp_fingerprint: Pii::Fingerprinter.fingerprint('ABC000') + ) + + expect(described_class.first_with_otp('abcooo')).to eq(confirmation_code) + end + + it 'returns nil if no record matches the OTP' do + create(:usps_confirmation_code) + + expect(described_class.first_with_otp(otp)).to be_nil + end + end + + describe '#expired?' do + it 'returns false for a valid otp' do + confirmation_code = build( + :usps_confirmation_code, + code_sent_at: Time.zone.now + ) + + expect(confirmation_code.expired?).to eq(false) + end + + it 'returns true for an expired otp' do + confirmation_code = build( + :usps_confirmation_code, + code_sent_at: (Figaro.env.usps_confirmation_max_days.to_i + 1).days.ago + ) + + expect(confirmation_code.expired?).to eq(true) + end + end +end diff --git a/spec/services/idv/profile_maker_spec.rb b/spec/services/idv/profile_maker_spec.rb index 13041c3b950..494e76f05b5 100644 --- a/spec/services/idv/profile_maker_spec.rb +++ b/spec/services/idv/profile_maker_spec.rb @@ -27,10 +27,6 @@ expect(pii).to be_a Pii::Attributes expect(pii.first_name.raw).to eq 'Some' expect(pii.first_name.norm).to eq 'Somebody' - - otp = pii.otp.raw - expect(otp.length).to eq(10) - expect(otp).to eq(Base32::Crockford.normalize(otp)) end end end diff --git a/spec/services/idv/usps_mail_spec.rb b/spec/services/idv/usps_mail_spec.rb index 7c4540a7b2a..576a952ec1e 100644 --- a/spec/services/idv/usps_mail_spec.rb +++ b/spec/services/idv/usps_mail_spec.rb @@ -44,30 +44,4 @@ end end end - - describe '#most_recent_otp_expired?' do - context 'when no mail has been sent' do - it 'returns false' do - expect(subject.most_recent_otp_expired?).to eq false - end - end - - context 'when the most recent mail was sent less than 10 days ago' do - it 'returns false' do - Event.create(event_type: :usps_mail_sent, user: user, updated_at: 5.days.ago) - Event.create(event_type: :usps_mail_sent, user: user, updated_at: 12.days.ago) - - expect(subject.most_recent_otp_expired?).to eq false - end - end - - context 'when the most recent mail was sent more than 10 days ago' do - it 'returns true' do - Event.create(event_type: :usps_mail_sent, user: user, updated_at: 11.days.ago) - Event.create(event_type: :usps_mail_sent, user: user, updated_at: 12.days.ago) - - expect(subject.most_recent_otp_expired?).to eq true - end - end - end end diff --git a/spec/services/usps_confirmation_maker_spec.rb b/spec/services/usps_confirmation_maker_spec.rb new file mode 100644 index 00000000000..a82d6ec7384 --- /dev/null +++ b/spec/services/usps_confirmation_maker_spec.rb @@ -0,0 +1,56 @@ +require 'rails_helper' + +describe UspsConfirmationMaker do + let(:otp) { '123ABC' } + let(:issuer) { 'this-is-an-issuer' } + let(:decrypted_attributes) do + { + address1: '123 main st', address2: '', + city: 'Baton Rouge', state: 'Louisiana', zipcode: '12345', + first_name: 'Robert', last_name: 'Robertson', + otp: otp, issuer: issuer + } + end + let(:pii) do + attributes = Pii::Attributes.new + decrypted_attributes.each do |key, value| + next unless attributes.respond_to? key + attributes[key] = Pii::Attribute.new(raw: value, norm: value) + end + attributes + end + let(:profile) { create(:profile) } + + subject { described_class.new(pii: pii, issuer: issuer, profile: profile) } + + describe '#perform' do + before do + allow(Base32::Crockford).to receive(:encode).and_return(otp) + end + + it 'should create a UspsConfirmation with the encrypted attributes' do + expect { subject.perform }.to change { UspsConfirmation.count }.from(0).to(1) + + usps_confirmation = UspsConfirmation.first + expect(usps_confirmation.decrypted_entry.to_h).to eq decrypted_attributes + end + + it 'should create a UspsConfrimationCode with the profile and the encrypted OTP' do + expect { subject.perform }.to change { UspsConfirmationCode.count }.from(0).to(1) + + usps_confirmation_code = UspsConfirmationCode.first + + expect(usps_confirmation_code.profile).to eq profile + expect(usps_confirmation_code.otp_fingerprint).to eq Pii::Fingerprinter.fingerprint(otp) + end + end + + describe '#otp' do + it 'should return a normalized, 10 digit code' do + otp = subject.otp + + expect(otp.length).to eq 10 + expect(Base32::Crockford.normalize(otp)).to eq otp + end + end +end diff --git a/spec/services/usps_exporter_spec.rb b/spec/services/usps_exporter_spec.rb index a2da4deee36..0bf42a90a99 100644 --- a/spec/services/usps_exporter_spec.rb +++ b/spec/services/usps_exporter_spec.rb @@ -2,6 +2,7 @@ describe UspsExporter do let(:export_file) { Tempfile.new('usps_export.psv') } + let(:otp) { 'ABC123' } let(:pii_attributes) do Pii::Attributes.new_from_hash( first_name: { raw: 'Söme', norm: 'Some' }, @@ -10,8 +11,7 @@ address2: { raw: 'Sté 123', norm: 'Ste 123' }, city: { raw: 'Sömewhere', norm: 'Somewhere' }, state: { raw: 'KS', norm: 'KS' }, - zipcode: { raw: '66666-1234', norm: '66666-1234' }, - otp: { raw: 123, norm: 123 } + zipcode: { raw: '66666-1234', norm: '66666-1234' } ) end let(:service_provider) { ServiceProvider.from_issuer('http://localhost:3000') } @@ -28,7 +28,7 @@ pii_attributes.city.norm, pii_attributes.state.norm, pii_attributes.zipcode.norm, - pii_attributes.otp.norm, + otp, "#{current_date}, #{now.year}", "#{due_date}, #{due.year}", service_provider.friendly_name, @@ -52,7 +52,13 @@ describe '#run' do before do - UspsConfirmationMaker.new(pii: pii_attributes, issuer: service_provider.issuer).perform + confirmation_maker = UspsConfirmationMaker.new( + pii: pii_attributes, + issuer: service_provider.issuer, + profile: build(:profile) + ) + allow(confirmation_maker).to receive(:otp).and_return(otp) + confirmation_maker.perform end it 'creates encrypted file' do diff --git a/spec/support/idv_examples/usps_verification.rb b/spec/support/idv_examples/usps_verification.rb index e60db3c14d8..d35619ce079 100644 --- a/spec/support/idv_examples/usps_verification.rb +++ b/spec/support/idv_examples/usps_verification.rb @@ -1,11 +1,18 @@ shared_examples 'signing in with pending USPS verification' do |sp| - let(:otp) { 'abc123' } + let(:otp) { 'ABC123' } let(:profile) do create( :profile, deactivation_reason: :verification_pending, phone_confirmed: false, - pii: { otp: otp, ssn: '123-45-6789', dob: '1970-01-01' } + pii: { ssn: '123-45-6789', dob: '1970-01-01' } + ) + end + let(:usps_confirmation_code) do + create( + :usps_confirmation_code, + profile: profile, + otp_fingerprint: Pii::Fingerprinter.fingerprint(otp) ) end let(:user) { profile.user } @@ -16,6 +23,7 @@ expect(current_path).to eq verify_account_path expect(page).to have_content t('idv.messages.usps.resend') + usps_confirmation_code fill_in t('forms.verify_profile.name'), with: otp click_button t('forms.verify_profile.submit') @@ -42,7 +50,7 @@ it 'renders an error for an expired USPS OTP' do sign_in_from_sp(sp) - Event.create(event_type: :usps_mail_sent, user: user, updated_at: 11.days.ago) + usps_confirmation_code.update(code_sent_at: 11.days.ago) fill_in t('forms.verify_profile.name'), with: otp click_button t('forms.verify_profile.submit') @@ -56,6 +64,28 @@ expect(user.active_profile).to be_nil end + it 'allows a user to resend a letter' do + allow(Base32::Crockford).to receive(:encode).and_return(otp) + + sign_in_from_sp(sp) + + expect(UspsConfirmation.count).to eq(0) + expect(UspsConfirmationCode.count).to eq(0) + + click_on t('idv.messages.usps.resend') + click_on t('idv.buttons.mail.send') + + expect(UspsConfirmation.count).to eq(1) + expect(UspsConfirmationCode.count).to eq(1) + expect(current_path).to eq verify_come_back_later_path + + confirmation_code = UspsConfirmationCode.first + otp_fingerprint = Pii::Fingerprinter.fingerprint(otp) + + expect(confirmation_code.otp_fingerprint).to eq(otp_fingerprint) + expect(confirmation_code.profile).to eq(profile) + end + def sign_in_from_sp(sp) visit_idp_from_sp_with_loa3(sp) diff --git a/spec/support/idv_examples/usps_verification_selection.rb b/spec/support/idv_examples/usps_verification_selection.rb index 8254140b306..f03553c18d3 100644 --- a/spec/support/idv_examples/usps_verification_selection.rb +++ b/spec/support/idv_examples/usps_verification_selection.rb @@ -50,4 +50,52 @@ to eq('urn:gov:gsa:openidconnect:sp:server') end end + + describe 'USPS OTP prefilling' do + it 'prefills USPS OTP if the reveal_usps_code feature flag is set', email: true do + visit_idp_from_sp_with_loa3(sp) + register_user + + usps_confirmation_maker = instance_double(UspsConfirmationMaker) + allow(usps_confirmation_maker).to receive(:otp).and_return('123ABC') + allow(usps_confirmation_maker).to receive(:perform) + allow(UspsConfirmationMaker).to receive(:new).and_return(usps_confirmation_maker) + allow(FeatureManagement).to receive(:reveal_usps_code?).and_return(true) + + complete_idv_profile_ok_with_usps + + visit verify_account_path + + expect(page.find('#verify_account_form_otp').value).to eq '123ABC' + end + + it 'does not prefill USPS OTP if the reveal_usps_code feature flag is not set', email: true do + visit_idp_from_sp_with_loa3(sp) + register_user + + usps_confirmation_maker = instance_double(UspsConfirmationMaker) + allow(usps_confirmation_maker).to receive(:otp).and_return('123ABC') + allow(usps_confirmation_maker).to receive(:perform) + allow(UspsConfirmationMaker).to receive(:new).and_return(usps_confirmation_maker) + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(false) + + complete_idv_profile_ok_with_usps + + visit verify_account_path + + expect(page.find('#verify_account_form_otp').value).to be_nil + end + end + + def complete_idv_profile_ok_with_usps + click_idv_begin + fill_out_idv_form_ok + click_idv_continue + fill_out_financial_form_ok + click_idv_continue + click_idv_address_choose_usps + click_on t('idv.buttons.mail.send') + fill_in :user_password, with: user_password + click_submit_default + end end