Skip to content

Add a includes_phone_check? method to Profile#6262

Merged
jmhooper merged 2 commits intomainfrom
jmhooper-add-phone-check-present-method
Apr 28, 2022
Merged

Add a includes_phone_check? method to Profile#6262
jmhooper merged 2 commits intomainfrom
jmhooper-add-phone-check-present-method

Conversation

@jmhooper
Copy link
Contributor

The letter flow as it is implemented may not meet the bar for strict IAL2. This commit adds a method for checking whether a letter was used to verify a user to enable building conditionals around that in a future change.

The letter flow as it is implemented may not meet the bar for strict IAL2. This commit adds a method for checking whether a letter was used to verify a user to enable building conditionals around that in a future change.
expect(profile.proofing_components).to eq('')
end

it 'does not blow up in #includes_liveness_check?' do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this into a dedicated test around #includes_liveness_check?

…l to determine if a user verified their address with a letter or by phone
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jmhooper jmhooper merged commit 3823df0 into main Apr 28, 2022
@jmhooper jmhooper deleted the jmhooper-add-phone-check-present-method branch April 28, 2022 16:01
aduth added a commit that referenced this pull request Apr 28, 2022
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.
peggles2 pushed a commit that referenced this pull request May 3, 2022
The letter flow as it is implemented may not meet the bar for strict IAL2. This commit adds a method for checking whether a letter was used to verify a user to enable building conditionals around that in a future change.

* changelog: Internal, Proofing, A helper was added to the Profile model to determine if a user verified their address with a letter or by phone
aduth added a commit that referenced this pull request May 4, 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>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants