Skip to content

LG-13216: AAMVA issue date + expiration validation (log only)#10653

Merged
matthinz merged 15 commits intomainfrom
matthinz/13216-aamva-issue-and-expiry-validation
May 21, 2024
Merged

LG-13216: AAMVA issue date + expiration validation (log only)#10653
matthinz merged 15 commits intomainfrom
matthinz/13216-aamva-issue-and-expiry-validation

Conversation

@matthinz
Copy link
Contributor

@matthinz matthinz commented May 17, 2024

🎫 Ticket

Link to the relevant ticket:
LG-13216

🛠 Summary of changes

This PR updates the AAMVA proofer to send issue date + expiration date (if available). These new attributes are not currently required to validate to pass.

It also adds a new key to AAMVA proofer results, requested_attributes that lists the attributes that were originally sent to AAMVA for verification. It's the same format as verified_attributes (an array). The intention is that this will make it possible to do analytics on requests with issue + expiration date.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Run through IdV, uploading a document proceeding at least through "Verify your information"
  • Check analytics logs and verify that the requested_attributes includes state_id_expiration and state_id_issued (and hopefully the verified_attributes too!)

This will likely require testing on staging before being promoted to production.

@matthinz matthinz requested a review from a team May 17, 2024 23:00
@matthinz matthinz force-pushed the matthinz/13216-aamva-issue-and-expiry-validation branch from 797b058 to 48c50af Compare May 20, 2024 17:39
@matthinz matthinz requested a review from jmhooper May 20, 2024 18:26
attributes.add :address if address_verified?(results)
def requested_attributes(verification_response)
attributes = verification_response.
verification_results.filter { |_, verified| !verified.nil? }.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended to reflect the requested attributes based on what we see from the response object? The response object iterates over all possible attributes so it will include attributes even if we did not request them:

VERIFICATION_ATTRIBUTES_MAP.each_pair do |match_indicator_name, attribute_name|
attribute_node = node_for_match_indicator(match_indicator_name)
if attribute_node.nil?
handle_missing_attribute(attribute_name)
elsif attribute_node.text == 'true'
verification_results[attribute_name] = true
else
verification_results[attribute_name] = false
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what the filter is about -- verification_results will have all attributes present, but any missing attributes will have nil values

Copy link
Contributor

Choose a reason for hiding this comment

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

That should work for this issue. 2 things to keep in mind:

  1. When AAMVA fails to match a DLN it returns a false match indicator for DLN and no match indicator for other attributes. In that case it will appear that we only requested DLN even when we sent a request for all of the PII including issue and expiration date
  2. Cloudwatch has a tough time handling array parameters and parsing them into something useful. If we could find a way to return keys/values here that may be easier to query when we are trying to visualize this. The verified_attributes value is an array so we can use it in get-to-yes IIRC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, I did not know that about DLN. It would be a little more work to look at the request itself (currently the proofer just does client.send_verification_request, and doesn't actually get a copy of the request data).

I can change requested_attributes to a hash no prob, I might poke around and see if it is worth doing verified_attributes as well

@solipet
Copy link
Contributor

solipet commented May 20, 2024

For the Test Plan, why would the attributes be missing the second time through?

@matthinz
Copy link
Contributor Author

For the Test Plan, why would the attributes be missing the second time through?

Doh, good catch, I removed the feature flag this would've been testing

Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

One nit, one curiosity, but LGTM.

state_id_type: 'drivers_license',
state_id_issued: '2023-04-05',
state_id_expiration: '2030-01-02',

Copy link
Contributor

Choose a reason for hiding this comment

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

extra blank line?

add_street_address_line_2_to_rexml_document(document) if applicant.address2.present?

add_optional_element(
'ns2:AddressDeliveryPointText',
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, is ns2 for optional attributes, while ns1 is for required? or...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The AAMVA SOAP request template we use includes 4 namespaces:

Abbrev URL
xmlns:soap http://www.w3.org/2003/05/soap-envelope
xmlns:ns http://aamva.org/dldv/wsdl/2.1
xmlns:ns1 http://aamva.org/niem/extensions/1.0
xmlns:ns2 http://niem.gov/niem/niem-core/2.0

we could probably clean up those ns* abbreviations to be more like what they are.

Co-authored-by: Doug Price <douglas.price@gsa.gov>
@matthinz matthinz merged commit ca1ce25 into main May 21, 2024
@matthinz matthinz deleted the matthinz/13216-aamva-issue-and-expiry-validation branch May 21, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants