Skip to content

Use stubbed profile for authorization_count_spec#6255

Merged
aduth merged 8 commits intomainfrom
aduth-auth-count-spec-stubbed-profile
May 4, 2022
Merged

Use stubbed profile for authorization_count_spec#6255
aduth merged 8 commits intomainfrom
aduth-auth-count-spec-stubbed-profile

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Apr 26, 2022

Blocked by: #6253
Extracted from #6229

Why:

  • To unblock and limit the scope of LG-6193: Enable personal key step in development environment #6229
  • For improved compatibility with JS-enabled proofing, where authorization counts rely on an "Agree and continue" redirect back to the SP. With the JavaScript browser, there is no server to redirect to, resulting in an error.
  • Improved performance, since proofing involves many steps
  • To limit the concern of the specs to authorization counts, not to the ability to successfully proof

@aduth aduth force-pushed the aduth-saml-auth-ial2-strict branch 2 times, most recently from 3cd0288 to 42fa86f Compare April 27, 2022 15:55
@aduth aduth force-pushed the aduth-auth-count-spec-stubbed-profile branch from 8ed984f to 1cd4f79 Compare April 28, 2022 13:13
@aduth aduth changed the base branch from aduth-saml-auth-ial2-strict to main April 28, 2022 14:56
aduth added 3 commits April 28, 2022 13:41
**Why:**

- For improved compatibility with JS-enabled proofing, where authorization counts rely on an "Agree and continue" redirect back to the SP. With the JavaScript browser, there is no server to redirect to, resulting in an error.
- Improved performance, since proofing involves many steps
- To limit the concern of the specs to authorization counts, not to the ability to successfully proof

changelog: Internal, Automated Testing, Improve performance of automated tests
too many tests assume it won't be there (probably a problem worth resolving)
@aduth aduth force-pushed the aduth-auth-count-spec-stubbed-profile branch from 1cd4f79 to 9782f48 Compare April 28, 2022 17:41
aduth added 3 commits April 28, 2022 15:21
As of #6262, we now check component as "blank?". In the real world, the value would be the vendor name, so add a placeholder value for tests.
shouldn't have been here - bad cherry-pick?
@aduth aduth requested review from nprimak, peggles2 and solipet May 4, 2022 14:28
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@aduth aduth merged commit ab29c7d into main May 4, 2022
@aduth aduth deleted the aduth-auth-count-spec-stubbed-profile branch May 4, 2022 16:37
peggles2 pushed a commit that referenced this pull request May 5, 2022
* Use stubbed profile for authorization_count_spec

**Why:**

- For improved compatibility with JS-enabled proofing, where authorization counts rely on an "Agree and continue" redirect back to the SP. With the JavaScript browser, there is no server to redirect to, resulting in an error.
- Improved performance, since proofing involves many steps
- To limit the concern of the specs to authorization counts, not to the ability to successfully proof

changelog: Internal, Automated Testing, Improve performance of automated tests

* Only set PII for verified profile mocks

* Require PII opt-in for profile stubs

too many tests assume it won't be there (probably a problem worth resolving)

* Add non-empty vendor for liveness check component

As of #6262, we now check component as "blank?". In the real world, the value would be the vendor name, so add a placeholder value for tests.

* Update authorization_count_spec.rb

* Remove default PII

shouldn't have been here - bad cherry-pick?

* Avoid concat for user profile creation

See: https://github.com/18F/identity-idp/pull/6255/files#r863108404
Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>

* Remove unnecessary user save

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

Co-authored-by: Zach Margolis <zbmargolis@gmail.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
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.

2 participants