-
Notifications
You must be signed in to change notification settings - Fork 166
LG-15437 Update PII validation #12027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
07aac59
097c7b5
f3bf070
5e7bf6f
05353e4
a46a102
7b2ec54
f44ec60
da36a22
f078528
7c89e78
1827084
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,35 +6,17 @@ class DocPiiForm | |
|
|
||
| validate :name_valid? | ||
| validate :dob_valid? | ||
| validates_presence_of :address1, { message: proc { | ||
| I18n.t('doc_auth.errors.alerts.address_check') | ||
| } } | ||
| validate :zipcode_valid? | ||
| validates :jurisdiction, :state, inclusion: { in: Idp::Constants::STATE_AND_TERRITORY_CODES, | ||
| message: proc { | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| } } | ||
|
|
||
| validates_presence_of :state_id_number, { message: proc { | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| } } | ||
| validate :state_id_expired? | ||
| validate :state_id_or_passport | ||
|
|
||
| attr_reader :first_name, :last_name, :dob, :address1, :state, :zipcode, :attention_with_barcode, | ||
| :jurisdiction, :state_id_number, :state_id_expiration | ||
| attr_reader :first_name, :last_name, :dob, :state_id_type, :attention_with_barcode | ||
| alias_method :attention_with_barcode?, :attention_with_barcode | ||
|
|
||
| def initialize(pii:, attention_with_barcode: false) | ||
| @pii_from_doc = pii | ||
| @first_name = pii[:first_name] | ||
| @last_name = pii[:last_name] | ||
| @dob = pii[:dob] | ||
| @address1 = pii[:address1] | ||
| @state = pii[:state] | ||
| @zipcode = pii[:zipcode] | ||
| @jurisdiction = pii[:state_id_jurisdiction] | ||
| @state_id_number = pii[:state_id_number] | ||
| @state_id_expiration = pii[:state_id_expiration] | ||
| @state_id_type = pii[:state_id_type] | ||
| @attention_with_barcode = attention_with_barcode | ||
| end | ||
|
|
||
|
|
@@ -43,19 +25,27 @@ def submit | |
| success: valid?, | ||
| errors: errors, | ||
| extra: { | ||
| pii_like_keypaths: self.class.pii_like_keypaths, | ||
| pii_like_keypaths: self.class.pii_like_keypaths(document_type: state_id_type), | ||
| attention_with_barcode: attention_with_barcode?, | ||
| id_issued_status: pii_from_doc[:state_id_issued].present? ? 'present' : 'missing', | ||
| id_expiration_status: pii_from_doc[:state_id_expiration].present? ? 'present' : 'missing', | ||
| passport_issued_status: pii_from_doc[:passport_issued].present? ? 'present' : 'missing', | ||
| passport_expiration_status: pii_from_doc[:passport_expiration].present? ? | ||
| 'present' : 'missing', | ||
| }, | ||
| ) | ||
| response.pii_from_doc = pii_from_doc | ||
| response | ||
| end | ||
|
|
||
| def self.pii_like_keypaths | ||
| def self.pii_like_keypaths(document_type:) | ||
| keypaths = [[:pii]] | ||
| attrs = %i[name dob dob_min_age address1 state zipcode jurisdiction state_id_number] | ||
| document_attrs = document_type&.downcase == 'passport' ? | ||
| DocPiiPassport.pii_like_keypaths : | ||
| DocPiiStateId.pii_like_keypaths | ||
|
|
||
| attrs = %i[name dob dob_min_age] + document_attrs | ||
|
|
||
| attrs.each do |k| | ||
| keypaths << [:errors, k] | ||
| keypaths << [:error_details, k] | ||
|
|
@@ -84,6 +74,7 @@ def self.present_error(existing_errors) | |
|
|
||
| PII_ERROR_KEYS = %i[name dob address1 state zipcode jurisdiction state_id_number | ||
| dob_min_age].freeze | ||
| STATE_ID_TYPES = ['drivers_license', 'state_id_card', 'identification_card'].freeze | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was in a test suite causing this to fail. I see it may have been taken out on the latest branch so I will remove it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was spec where not having 'identification_card' in STATE_ID_TYPES was causing it to fail https://github.com/18F/identity-idp/blob/main/spec/jobs/socure_docv_results_job_spec.rb#L120
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @theabrad ... i don't think you should remove
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see you had to add |
||
|
|
||
| attr_reader :pii_from_doc | ||
|
|
||
|
|
@@ -108,22 +99,19 @@ def dob_valid? | |
| end | ||
| end | ||
|
|
||
| def state_id_expired? | ||
| # temporary fix, tracked for removal in LG-15600 | ||
| return if IdentityConfig.store.socure_docv_verification_data_test_mode && | ||
| DateParser.parse_legacy(state_id_expiration) == Date.parse('2020-01-01') | ||
|
|
||
| if state_id_expiration && DateParser.parse_legacy(state_id_expiration).past? | ||
| errors.add(:state_id_expiration, generic_error, type: :state_id_expiration) | ||
| def state_id_or_passport | ||
| case state_id_type | ||
| when *STATE_ID_TYPES | ||
| state_id_validation = DocPiiStateId.new(pii: pii_from_doc) | ||
| state_id_validation.valid? || errors.merge!(state_id_validation.errors) | ||
| when 'passport' | ||
| passport_validation = DocPiiPassport.new(pii: pii_from_doc) | ||
| passport_validation.valid? || errors.merge!(passport_validation.errors) | ||
| else | ||
| errors.add(:no_document, generic_error, type: :no_document) | ||
| end | ||
| end | ||
|
|
||
| def zipcode_valid? | ||
| return if zipcode.is_a?(String) && zipcode.present? | ||
|
|
||
| errors.add(:zipcode, generic_error, type: :zipcode) | ||
| end | ||
|
|
||
| def generic_error | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Idv | ||
| class DocPiiPassport | ||
| include ActiveModel::Model | ||
|
|
||
| validates :birth_place, | ||
| :passport_issued, | ||
| :nationality_code, | ||
| :mrz, | ||
| presence: { message: proc { I18n.t('doc_auth.errors.general.no_liveness') } } | ||
|
|
||
| validates :issuing_country_code, | ||
| :nationality_code, | ||
| inclusion: { | ||
| in: 'USA', message: proc { I18n.t('doc_auth.errors.general.no_liveness') } | ||
| } | ||
|
|
||
| validate :passport_expired? | ||
|
|
||
| attr_reader :birth_place, :passport_expiration, :passport_issued, :state_id_type, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like we should move away from this state_id_type naming ... created a ticket LG-16007 |
||
| :issuing_country_code, :nationality_code, :mrz | ||
|
|
||
| def initialize(pii:) | ||
| @pii_from_doc = pii | ||
| @birth_place = pii[:birth_place] | ||
| @passport_expiration = pii[:passport_expiration] | ||
| @passport_issued = pii[:passport_issued] | ||
| @issuing_country_code = pii[:issuing_country_code] | ||
| @nationality_code = pii[:nationality_code] | ||
| @mrz = pii[:mrz] | ||
| end | ||
|
|
||
| def self.pii_like_keypaths | ||
| %i[birth_place passport_issued issuing_country_code nationality_code mrz] | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :pii_from_doc | ||
|
|
||
| def generic_error | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| end | ||
|
|
||
| def passport_expired? | ||
| if passport_expiration && DateParser.parse_legacy(passport_expiration).past? | ||
| errors.add(:passport_expiration, generic_error, type: :passport_expiration) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Idv | ||
| class DocPiiStateId | ||
| include ActiveModel::Model | ||
|
|
||
| validates_presence_of :address1, { message: proc { | ||
| I18n.t('doc_auth.errors.alerts.address_check') | ||
| } } | ||
|
|
||
| validate :zipcode_valid? | ||
| validates :jurisdiction, :state, inclusion: { in: Idp::Constants::STATE_AND_TERRITORY_CODES, | ||
| message: proc { | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| } } | ||
|
|
||
| validates_presence_of :state_id_number, { message: proc { | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| } } | ||
| validate :state_id_expired? | ||
|
|
||
| attr_reader :address1, :state, :zipcode, :attention_with_barcode, :jurisdiction, | ||
| :state_id_number, :state_id_expiration | ||
| alias_method :attention_with_barcode?, :attention_with_barcode | ||
|
|
||
| def initialize(pii:) | ||
| @pii_from_doc = pii | ||
| @address1 = pii[:address1] | ||
| @state = pii[:state] | ||
| @zipcode = pii[:zipcode] | ||
| @jurisdiction = pii[:state_id_jurisdiction] | ||
| @state_id_number = pii[:state_id_number] | ||
| @state_id_expiration = pii[:state_id_expiration] | ||
| @attention_with_barcode = attention_with_barcode | ||
| end | ||
|
|
||
| def self.pii_like_keypaths | ||
| %i[address1 state zipcode jurisdiction state_id_number] | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :pii_from_doc | ||
|
|
||
| def generic_error | ||
| I18n.t('doc_auth.errors.general.no_liveness') | ||
| end | ||
|
|
||
| def state_id_expired? | ||
| # temporary fix, tracked for removal in LG-15600 | ||
| return if IdentityConfig.store.socure_docv_verification_data_test_mode && | ||
| DateParser.parse_legacy(state_id_expiration) == Date.parse('2020-01-01') | ||
|
|
||
| if state_id_expiration && DateParser.parse_legacy(state_id_expiration).past? | ||
| errors.add(:state_id_expiration, generic_error, type: :state_id_expiration) | ||
| end | ||
| end | ||
|
|
||
| def zipcode_valid? | ||
| return if zipcode.is_a?(String) && zipcode.present? | ||
|
|
||
| errors.add(:zipcode, generic_error, type: :zipcode) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking would we want something like "N/A" here if we are checking passports rather than state id?