LG-8432: SAML - Update ForceAuthn check#7514
Merged
julialeague merged 5 commits intomainfrom Dec 20, 2022
Merged
Conversation
orenyk
reviewed
Dec 20, 2022
Contributor
orenyk
left a comment
orenyk
left a comment
There was a problem hiding this comment.
Looks good overall, I left a couple of questions and it might be good to get a feature spec as well to make sure we're getting the redirect to the sign in page after a request with ForceAuthn=true. Nice work!
julialeague
changed the title
Contributor
Author
|
Logged https://cm-jira.usa.gov/browse/LG-8452 to add a feature spec around ForceAuthn. |
julialeague
force-pushed
the
jlallen/update-forceauthn-enforcement
branch
from
415e2eb to
6f68f38
Compare
orenyk
added a commit
that referenced
this pull request
Dec 20, 2022
This reverts commit a7577b6. [skip changelog]
orenyk
added a commit
that referenced
this pull request
Dec 20, 2022
Merged
mdiarra3
pushed a commit
that referenced
this pull request
Dec 22, 2022
mdiarra3
added a commit
that referenced
this pull request
Dec 23, 2022
…7524) * changelog: Internal, Authentication, Remove show_account_recovery_recovery_options feature flag since feature is permanently turned on * LG-7489: rspec fixes * remove unneeded spec * LG-8089: Update to support notifying user of OTP format in message (#7486) * Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect (LG-8089) * update voice * translation update * add check to otp_sender * changelog: Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect * fix spec * Revert "LG-8432: SAML - Update ForceAuthn check (#7514)" (#7520) This reverts commit a7577b6. [skip changelog] * LG-8388: Confirm ThreatMetrix doesn't block IPP (#7521) * LG-8388: Confirm ThreatMetrix doesn't block IPP Spec to confirm that IPP is not blocked by a ThreatMetrix result of 'Reject' [skip changelog] * ensure the threatmetrix profiling is enabled * lints * Update spec/features/idv/in_person_spec.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Switching newer Acuant SDK to be the default version (#7513) * Switching newer Acuant SDK to be the default version -- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade * Specifying acuant versions in configs and updating names Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade * Acuant context should use config's acuant version by default changelog: Internal, Upgrades, Acuant SDK Upgrade * Fixing lints [skip changelog] * Revert "Acuant context should use config's acuant version by default" This reverts commit b420998. [skip changelog] * Removing redundant condition (via @aduth) [skip changelog] * Add logging criteria, abstract logging decision into method (#7516) * Abstract logging decision into method * [skip changelog] * lintfix * Test logic in should_log? method * lintfix * Add filename fingerprint to JavaScript locale data script (#7527) * Add filename fingerprint to JavaScript locale data script changelog: Internal, Build Tooling, Invalidate cache for content-only revisions to JavaScript bundles * Update specs * LG-8438: Update language on pages related to OTP (#7515) Update OTP language changelog: Improvements, Multi-Factor Authentication, Update OTP language * Doc auth retry timer should decrease (#7498) * Move timeout calculation to helper * Fix calculation of attempted_at and expires_at * changelog: Bug Fixes, DocAuth, Modify retry timer to decrease * Remove instance variable from helper * Revert helper * Indent Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
mitchellhenke
pushed a commit
that referenced
this pull request
Dec 27, 2022
* build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths * Updating ThreatMetrix failure (sad face) screen content (#7529) * Updating ThreatMetrix failure (sad face) screen content changelog: User Facing Improvements, UX, ThreatMetrix failure screen content * Lints [skipe changelog] * Adding contact center number to config and setting dummy default [skip changelog] * Ignore non-yaml files when normalizing yamls (#7531) [skip changelog] * Lg-8264 po search frontend updates (#7519) * show updated ui for location page when arcgis is enabled * add mile(s) away to distance * changelog: Improvements, Location page, update ui on location pg when arcgis is enabled * normalize yaml * fix missing translation test failure * update strings to include count to handle pluralization * update collection item test * add es translation for tty to untranslated keys * refactor * Fix flakey in-person feature test (#7533) * make it okay for the interstitial to not show it's possible for the interstitial to disappear before the matcher can catch it. changelog: Internal, Automated Testing, Improve reliability of successful automated tests * Make `lintfix` fix erb, javascript, and CSS files too (#7528) * Make lintfix fix erb files too * Autofix javascript and CSS files * [skip changelog] * Mention that we're fixing things in status updates * Fix untranslated email input in password reset (#7537) * Fix untranslated email input in password reset changelog: Bug Fixes, Internationalization, Fix untranslated email input in password reset * fix specs * Update app/views/devise/passwords/new.html.erb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Move favicons to asset pipeline (#7532) * Move favicons to asset pipeline changelog: Internal, Assets, Move favicons to asset pipeline * use existing design system assets * LG-7489: Remove show_account_recovery_recovery_options Feature flag (#7524) * changelog: Internal, Authentication, Remove show_account_recovery_recovery_options feature flag since feature is permanently turned on * LG-7489: rspec fixes * remove unneeded spec * LG-8089: Update to support notifying user of OTP format in message (#7486) * Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect (LG-8089) * update voice * translation update * add check to otp_sender * changelog: Improvements, Voice messaging, Notify voice OTP users how long and what type of code to expect * fix spec * Revert "LG-8432: SAML - Update ForceAuthn check (#7514)" (#7520) This reverts commit a7577b6. [skip changelog] * LG-8388: Confirm ThreatMetrix doesn't block IPP (#7521) * LG-8388: Confirm ThreatMetrix doesn't block IPP Spec to confirm that IPP is not blocked by a ThreatMetrix result of 'Reject' [skip changelog] * ensure the threatmetrix profiling is enabled * lints * Update spec/features/idv/in_person_spec.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Switching newer Acuant SDK to be the default version (#7513) * Switching newer Acuant SDK to be the default version -- What The introduction of the AB Testing framework for upgrading the Acuant SDK assumes that the "newer" version is the one being tested, and that the older is the "default". This change updates the step doc capture step to assume that version 11.7.1 should be used in all cases when the ab test is switched off. changelog: Internal, Upgrades, Acuant SDK Upgrade * Specifying acuant versions in configs and updating names Specifically, we are switching from use_newer_sdk to use_alternate_sdk changelog: Internal, Upgrades, Acuant SDK Upgrade * Acuant context should use config's acuant version by default changelog: Internal, Upgrades, Acuant SDK Upgrade * Fixing lints [skip changelog] * Revert "Acuant context should use config's acuant version by default" This reverts commit b420998. [skip changelog] * Removing redundant condition (via @aduth) [skip changelog] * Add logging criteria, abstract logging decision into method (#7516) * Abstract logging decision into method * [skip changelog] * lintfix * Test logic in should_log? method * lintfix * Add filename fingerprint to JavaScript locale data script (#7527) * Add filename fingerprint to JavaScript locale data script changelog: Internal, Build Tooling, Invalidate cache for content-only revisions to JavaScript bundles * Update specs * LG-8438: Update language on pages related to OTP (#7515) Update OTP language changelog: Improvements, Multi-Factor Authentication, Update OTP language * Doc auth retry timer should decrease (#7498) * Move timeout calculation to helper * Fix calculation of attempted_at and expires_at * changelog: Bug Fixes, DocAuth, Modify retry timer to decrease * Remove instance variable from helper * Revert helper * Indent Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * build-sass: Add CLI flag for additional load paths (#7525) changelog: Internal, Build Tooling, Improve Sass compilation tool to support additional load paths Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * LG-8414 Remove flag from batch job (#7534) * LG-8414 Remove flag from batch job changelog: Internal, Attempts API, Events Batch work * LG-8390 Write rake tasks to pass or reject users (#7536) * initial steps for rake script to review profile * rewrite tasks for passing and rejecting * add tests for review_profile rake task * add changelog changelog: Internal, IdV, Make script to pass or reject TMX review * activate profile upon passing review also changed the deactivation reason upon rejection to :threatmetrix_review_rejected * check for deactivation reason in passing Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov> Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com> Co-authored-by: Tomas Apodaca <thomas.apodaca@gsa.gov> Co-authored-by: Malick Diarra <malick.diarra@gsa.gov> Co-authored-by: Oren Kanner <oren.kanner@gsa.gov> Co-authored-by: Doug Price <douglas.price@gsa.gov> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: Luis H. Matos <ThatSpaceGuy@users.noreply.github.com> Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎫 Ticket
https://cm-jira.usa.gov/browse/LG-8432
🛠 Summary of changes
Our recent updates to the SAML internal flow (see #6922) introduced an issue with our enforcement of the ForceAuthn attribute, where users would be stuck in an indefinite sign-out/sign-in loop for service providers passing ForceAuthn=true in their SAMLRequest. This update fixes that by checking whether we're at the final request which will redirect back to the service provider, and skips signing out if so. The step which the sign-out should be forced precedes that second step. This is also currently behind a feature flag that will eventually be removed once we confirm everything works as expected (covered in another JIRA).
This issue was identified as a part of the rollout of the feature flag
saml_internal_post, first introduced in #6922 . We have a related comms plan where we also include the dates for the feature rollout: https://docs.google.com/document/d/159Mjrvmll-4uIKhzSoFEw_3pp20TWSI1f0ZH1Kvtdfc/edit#heading=h.hptnw14jf34z