Skip to content

LG-11962 return 406 if biometric selected in production#9837

Merged
jmax-gsa merged 14 commits intomainfrom
jmax/LG-11962-return-404-if-biometric-selected-in-production
Jan 4, 2024
Merged

LG-11962 return 406 if biometric selected in production#9837
jmax-gsa merged 14 commits intomainfrom
jmax/LG-11962-return-404-if-biometric-selected-in-production

Conversation

@jmax-gsa
Copy link
Contributor

@jmax-gsa jmax-gsa commented Dec 29, 2023

🎫 Ticket

LG-11962

🛠 Summary of changes

Added a check to ensure that SP requests for selfies are not permitted in production. This is a temporary measure, until we are actually ready to handle them properly.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Start the Sinatra sample SP and the IDP server.
  • From the Sinatra sample SP, request authentication with biometrics. Verify that you end up on the login.gov start page.
  • Edit feature_management to simulate a production environment (see below).
  • From the Sinatra sample SP, request authentication without biometrics. Verify that you end up on the login.gov start page.
  • From the Sinatra sample SP, request authentication with biometrics. Verify that you end up on the 406 page.

Simulating a production environment on a dev box turned out to be a time sink. After spending Friday afternoon shaving yaks, re-assessed and simulated a production environment via a code change. If desired, we could spin up a sandbox for closer simulation of production. I don't feel that's necessary for code review, but perhaps for acceptance.

The code change is in lib/feature_management.rb, lines 167-168:

  def self.idv_block_biometrics_requests?
    # (Identity::Hostdata.in_datacenter? && Identity::Hostdata.env == 'prod') ||
    #   !IdentityConfig.store.doc_auth_selfie_capture_enabled
    true
  end

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Development request for biometrics:

Screenshot 2024-01-03 at 10 47 56 AM

Screenshot 2024-01-03 at 10 49 16 AM

Production: no request for biometrics

Screenshot 2024-01-03 at 10 51 20 AM

Screenshot 2024-01-03 at 10 55 57 AM

Production: request for biometrics

Screenshot 2024-01-03 at 10 56 49 AM

Screenshot 2024-01-03 at 10 57 45 AM

Details

@jmax-gsa jmax-gsa changed the title lg 11962 return 404 if biometric selected in production LG-11962 return 404 if biometric selected in production Dec 29, 2023
@jmax-gsa
Copy link
Contributor Author

As an alternative, I could've re-used the existing 404 content and rendered it if necessary, but adding an explicit route to the existing controller and redirecting to it seemed like the better alternative to me. Arguments to the contrary invited.

@jmax-gsa jmax-gsa force-pushed the jmax/LG-11962-return-404-if-biometric-selected-in-production branch from 8771acd to b97507c Compare December 29, 2023 14:55
@jmax-gsa jmax-gsa force-pushed the jmax/LG-11962-return-404-if-biometric-selected-in-production branch 2 times, most recently from a1c0fcc to e721140 Compare January 2, 2024 17:23
@jmax-gsa jmax-gsa force-pushed the jmax/LG-11962-return-404-if-biometric-selected-in-production branch from b65b619 to bdc159c Compare January 3, 2024 15:40
The code no longer does a redirect, so the before action name needed
to change.
@jmax-gsa jmax-gsa marked this pull request as ready for review January 3, 2024 17:07
@jmax-gsa jmax-gsa requested a review from a team January 3, 2024 17:08
@jmax-gsa jmax-gsa changed the title LG-11962 return 404 if biometric selected in production LG-11962 return 406 if biometric selected in production Jan 3, 2024
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM. I like the way the check for prod is set up now. We might want to add the datacenter check to decorated_session.selfie_required? (but not in this PR).

@jmax-gsa jmax-gsa merged commit 427ba61 into main Jan 4, 2024
@jmax-gsa jmax-gsa deleted the jmax/LG-11962-return-404-if-biometric-selected-in-production branch January 4, 2024 19:41
@amirbey amirbey mentioned this pull request Jan 9, 2024
jmhooper added a commit that referenced this pull request Feb 28, 2024
In #9837 we modified the IdP to render a 406 if biometric comparison was requested but not allowed for the current environment.

This change applied to the query parameter method for requesting biometric comparison which was the only way to request biometric comparison at the time. Since then we have enabled biometric comparison requests using vectors of trust but did not port over the feature for blocking biometric comparison requests in environments where they are not allowed.

This commit applies the feature to requests that use a vector of trust to request biometric comparison.

[skip changelog]
jmhooper added a commit that referenced this pull request Feb 29, 2024
…10180)

In #9837 we modified the IdP to render a 406 if biometric comparison was requested but not allowed for the current environment.

This change applied to the query parameter method for requesting biometric comparison which was the only way to request biometric comparison at the time. Since then we have enabled biometric comparison requests using vectors of trust but did not port over the feature for blocking biometric comparison requests in environments where they are not allowed.

This commit applies the feature to requests that use a vector of trust to request biometric comparison.

[skip changelog]
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.

5 participants