Lg-6202 - create accordion by peggles2 · Pull Request #6306 · 18F/identity-idp

peggles2 · 2022-05-04T18:24:04Z

This pull request, refactors the password confirmation page to be in react. This particular code only focuses on the accordion that is on the password confirmation page.

**Why**: Security release. Reference: https://rubyonrails.org/2022/4/26/Rails-7-0-2-4-6-1-5-1-6-0-4-8-and-5-2-7-1-have-been-released changelog: Internal, Dependencies, Update dependencies to resolve security advisories

* add failing spec * Ensure sp redirect_url exists before sending to completions page changelog: Bug Fixes, Authentication, Avoid 500 error by not incorrectly redirecting to service provider consent page * Update app/controllers/concerns/verify_sp_attributes_concern.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

[skip changelog]

* Add missing costs tracking for async doc auth **Why**: So that we are consistently tracking costs between sync and async implementations * Add changelog changelog: Upcoming Features, Async Document Authentication, Consistently log costs for document capture * Improve specs by filtering costs by expected issuer

Updates were made to require AAL2 if partners request it. changelog: Single SIgn On, Authentication, support requested aal timeout,

…a user reproofs without liveness (#6260) We use the proofing components on a profile to ensure a user's profile has a liveness check when a liveness check is required. This commit adds a spec to ensure that a user is not marked as having a liveness check present if they reproof without liveness. changelog: Internal, Testing, A test was added to cover an edge case when re-proofing without facial matching

[skip changelog]

changelog: Internal, Optimization, Do not create sp_costs for unused cost types

Liveness checking specific functionality is being discarded in favor of a full IAL2 compliant flow which may include additional deltas. That makes this flag confusing and hopefully unnecessary so this commit removes it. A follow on commit will be required to drop the column. * changelog: Internal, Service provider management, An unnecessary flag for setting liveness checking requirements on an SP was removed

[skip changelog]

* Add and fix spec coverage for JS-enabled, IAL2 strict doc auth **Why**: Because it appears we did not have coverage for this flow previously, evidenced by the fact that the current test helpers are currently broken. changelog: Internal, Automated Testing, Improve test coverage for proofing flow * Use translated labels for field association

* Add form validation for present TOTP name **Why**: For consistency with other MFA setup, and for consistency with the JS-enabled user experience, where the input is rendered as required. changelog: Bug Fixes, MFA Setup, Consistently validate TOTP name * Pass name parameter for controller specs * Improve specs for TOTP setup controller * Fill in name * More forms! * Try non-unique app names in specs should be unique user / MFA setup between specs

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

* Fix SAML auth context handling for IAL2 strict request **Why**: So that an IAL2 strict request behaves similar to a standard IAL2 request, particularly in how a user should be directed to enter their password to decrypt PII if encrypted. [skip changelog] * Check explicitly for ial2_strict in SAML requested_attributes Avoid disambiguating IAL2 as meaning either "IAL2" or "IAL2 strict", since we don't seem to have a tendency to do that

…ure-flagged) (#6269) * LG-6193: Integrate IdV app personal key step into proofing flow **Why**: So that a user can functionally complete the proofing process using the new IdV app. changelog: Upcoming Features, Identity Verification, Add personal key step screen * Move completion behavior to pack * Add spec for VerifyFlow * Revert feature specs covering personal key page deal with it later * Fix VerifyFlow spec Maybe bug with testing-library? Seems to struggle querying by role "TypeError: style.getPropertyValue is not a function" * Stub applicant for VerifyController spec copied from personal_key_controller_spec.rb

**Why**: Because `encrypt_recovery_pii` is already called as part of the preceding `encrypt_pii`. changelog: Internal, Automated Testing, Reduce redundancy in test data setup

* personal-key repo * update route to verify/complete/personal_key * get request working with postman to return empty hash for now * add_proofing_component to get_personal_key * add analytics for personal key * update complete_controller * update error messages for the 2 factor auth so api can get proper json response * cleanup code * update the application controller * changes made to return proper json error responses * latest changes * update code * create the profile and cache the pii * create the profile creation form correctly * clean up the JWT code, use idv certificate pair * update code * changes made to cleanup code * update FormResponse to return a {} if extra_attributes is nil * changes made to fix the correct jwt to return user key * specs for Api::ProfileCreationForm * specs for Api::ProfileCreationForm (for reals) * add rspec tests * cleanup test * cleanup code * fix profile creation form spec on recovery key * changes made to make it a post instead of a get * cleanup lint * fix some more linter errors * Include "personal_key" as alertable key in analytics PiiDetector **Why**: Since we don't want to be including this detail in any logs, as it is password-like. * Revert "Include "personal_key" as alertable key in analytics PiiDetector" This reverts commit 3f00de9. * implement/test complete_session * changes made to fix the code review feedacks * get rid of aliased methods * lints * fix linter error * remove parenthesis * code review feedback * changelog: Upcoming Features, Identity Verification, API endpoint for IdV completion * fix line space * code review feedback * fix lint error * add feature flagging * move the personal_key to a dedicated method, encapsulate the JWT in a decorator * lints * convert profile_completion_form to return the personal_key separately from the response. * remove unused custom form response class * Update config/routes.rb Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Update app/forms/api/profile_creation_form.rb to fetch pii from the PII::Cacher Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> * remove `gpo_otp` as a method on the form * move the feature flag check from routes.rb to the controller * remove unnecessary session usage * default keys for IdV JWTs * guard against small IdV JWT keys in production envs Co-authored-by: Douglas Price <douglas.price@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>

…6272) changelog: Internal, Continuous Integration, Fix issue with validating the changelog on long branch histories in GitLab

changelog: Internal, Maintenance, Drop unused columns on users table

* Upload artifact and sync static assets in GitLab CI changelog: Internal, Continuous Deployment, Enable uploading application artifacts and static assets from GitLab CI * add execute permissions to script * fix sources for running pipeline * add ruby cache to coverage step * return early if current branch is not configured

changelog: Bug Fixes, Logging, Fix error during SAML authentication when receiving invalid referer

[skip changelog]

…proofing (#6270) We may need to make a change in the future to require strict IAL2 users to proof with phone. This commit does some work towards making that possible. It makes changes to the authorization controller to prevent a user from being redirected at strict IAL2 unless they have proofed with phone. This change is not totally complete, there are changes that will need to go in place to remove the option during proofing. changelog: Upcoming feature, Proofing, A user will need to proof with a phone before being sent back to the IdP for strict IAL2 proofing

) changelog: Internal, Security, Improve consistency in storing and fetching PII bundle from user session

* Implement client session secret store **Why**: As a demonstration of secure client-side storage decrypted with key provided by server per session. changelog: Upcoming Features, Identity Proofing, Add client-side encrypted storage * Collapse readStorage try blocks * Clarify AES cipher key/iv generation To avoid magic number and make it clearer what's happening #6183 (comment) * Simplify cipher assignment logic * Refactor SecretsContextProvider as observable initializer - Avoid waiting to render the app - Manage subscribers automatically via context value change * Rename encode as s2ab Consistency #6183 (comment) * iv per encrypt #6183 (comment) * Make setItem await-able * Reference crypto consistently from window object * Add SecretSessionStorage inline comment docs * Add SecretSessionStorage specs * Merge useSecretValue to context implementation * Remove demo value from SecretValues * Add docs for SecretsContext * Use flow values as secrets * Split VerifyFlow from index Avoid dependency cycle, make room for more index-exported * Destructure storeKey in same way as other data attributes * Use user_session instead of session Route now authenticated * Inline encryption cipher initialization to memoized session assignment See: https://github.com/18F/identity-idp/pull/6183/files#r865355166 Co-Authored-By: Zach Margolis <zbmargolis@gmail.com> Co-authored-by: Zach Margolis <zbmargolis@gmail.com>

* TypeScript-ify Alert component * Components: Assign Alert role by type **Why**: - Alignment to AlertComponent Rails ViewComponent implementation - To avoid assertively announcing alert text for non-urgent alerts changelog: Improvements, Accessibility, Use status role for non-urgent alert content

[skip changelog]

* LG-5929-document-analytics-11 * Patch: analytics events 11 (#6303) * Remove @identity.idp.event_name - As of #6294, having it will cause build breakage since it's an unknown tag * Remove blank lines * Add clearer comments for each event Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

There is a chance we won't be able to roll out a compliant IAL2 flow with the USPS letter flow in place. This commit adds the ability to remove the letter flow option from strict IAL2 if that does become the case. * changelog: Upcoming Features, Proofing, The ability to disable the option to proof with a letter during IAL2 strict was added

changelog: Analytics, Document authorization, updates

* Translate labels for IdV app step indicator **Why**: So that labels are shown in the user's preferred language. changelog: Upcoming Features, Identity Verification, Add personal key step screen * Create type for step indicator steps See: https://github.com/18F/identity-idp/pull/6310/files#r866030157 Co-Authored-By: Zach Margolis <zbmargolis@gmail.com> Co-authored-by: Zach Margolis <zbmargolis@gmail.com>

…p into lg-6202-create-accordion

…e-accordion

aduth · 2022-05-11T12:28:29Z

app/controllers/verify_controller.rb

    redirect_to account_url if idv_session.profile.blank?
  end

-  def personal_key


I wouldn't expect this change to be here. Is it from a merge conflict?

It was there before. But now not needed.

It's still called here:

identity-idp/app/controllers/verify_controller.rb

Line 45 in b0c1f93

{ 'personalKey' => personal_key }

aduth · 2022-05-11T12:28:40Z

app/javascript/packages/form-steps/form-steps.spec.tsx

  it('renders the active step', async () => {
    const { getByText } = render(<FormSteps steps={STEPS} />);

-    await userEvent.click(getByText('forms.buttons.continue'));


I wouldn't expect this change to be here. Is it from a merge conflict?

aduth · 2022-05-11T12:29:22Z

app/javascript/packages/verify-flow/steps/password-confirm/index.ts

+
+export default {
+  name: 'password_confirm',
+  title: t('titles.idv.session.review'),


Via build failure, this locale key doesn't exist. I think it should be....

Suggested change

title: t('titles.idv.session.review'),

title: t('idv.titles.session.review'),

aduth · 2022-05-11T12:30:24Z

app/javascript/packages/verify-flow/steps/password-confirm/password-confirm-step.tsx

+import type { FormStepComponentProps } from '@18f/identity-form-steps';
+import type { VerifyFlowValues } from '../..';
+import { Accordion } from '@18f/identity-components';
+import parsePhoneNumber from 'libphonenumber-js';


Earlier review comment still stands.

aduth · 2022-05-11T12:32:29Z

app/javascript/packages/verify-flow/steps/password-confirm/password-confirm-step.tsx

+    <>
+      <PageHeading>{t('idv.titles.session.review', { app_name: 'Login.gov' })}</PageHeading>
+      <p>{t('idv.messages.sessions.review_message', { app_name: 'Login.gov' })}</p>
+      <Accordion header={t('idv.messages.review.intro')}>{PersonalInfoSummary(value)}</Accordion>


If we're treating PersonalInfoSummary as a component, we should use JSX, which will also require updating the parameters to accept a props object.

Suggested change

<Accordion header={t('idv.messages.review.intro')}>{PersonalInfoSummary(value)}</Accordion>

<Accordion header={t('idv.messages.review.intro')}><PersonalInfoSummary pii={value} /></Accordion>

aduth · 2022-05-11T12:33:08Z

package.json

    "webpack": "^5.65.0",
    "webpack-assets-manifest": "^5.0.6",
    "webpack-cli": "^4.9.1",
+    "yarn-deduplicate": "^5.0.0",


I wouldn't expect this to be installed to the project, and instead should be installed as a global package on a developer's machine.

Suggested change

"yarn-deduplicate": "^5.0.0",

aduth · 2022-05-11T12:33:22Z

scripts/artifact-upload

 branch_environments.map do |upload_environment|
  static_bucket = ENV["#{upload_environment.upcase}_STATIC_BUCKET"]
  if static_bucket.nil?
-    puts "skipping #{upload_environment} because it is missing static bucket config!"


I wouldn't expect this change to be here. Is it from a merge conflict?

peggles2 · 2022-05-11T21:27:12Z

cleaned up branch and moved here. #6338

peggles2 and others added 30 commits May 2, 2022 15:38

update using accordion component

a6fbff0

format the phone number

3f91225

code cleanup

a97602e

flatten out the pii

6b2f461

merge

6cc9f16

merge main

8b71b96

Update Rails to 6.1.5.1 (#6258)

c0e11b5

**Why**: Security release. Reference: https://rubyonrails.org/2022/4/26/Rails-7-0-2-4-6-1-5-1-6-0-4-8-and-5-2-7-1-have-been-released changelog: Internal, Dependencies, Update dependencies to resolve security advisories

Do not create double email addresses for user in tests (#6259)

c00587d

[skip changelog]

Fix tests for aal2 (#6252)

3ecdbc5

Updates were made to require AAL2 if partners request it. changelog: Single SIgn On, Authentication, support requested aal timeout,

Update idp/ci image to one with healthcheck (#6256)

a7e90cb

[skip changelog]

Drop unused types for sp_costs (#6264)

de26879

changelog: Internal, Optimization, Do not create sp_costs for unused cost types

Remove unused test helper (#6266)

875b332

[skip changelog]

Avoid redundant recovery PII encrypt in profile spec factory (#6271)

ffd9f9c

**Why**: Because `encrypt_recovery_pii` is already called as part of the preceding `encrypt_pii`. changelog: Internal, Automated Testing, Reduce redundancy in test data setup

Ensure changelog_check script has the full branch history in GitLab (#…

923978d

…6272) changelog: Internal, Continuous Integration, Fix issue with validating the changelog on long branch histories in GitLab

Drop unused user columns (#6246)

de698a3

changelog: Internal, Maintenance, Drop unused columns on users table

Fix 500 error when parsing malformed referer header (#6275)

ce0f115

changelog: Bug Fixes, Logging, Fix error during SAML authentication when receiving invalid referer

fix bug in artifact upload script (#6274)

313b3e4

[skip changelog]

Move more PII bundle usage to the common interface in Pii::Cacher (#6273

7d892ff

) changelog: Internal, Security, Improve consistency in storing and fetching PII bundle from user session

aduth and others added 18 commits May 5, 2022 15:08

Revert dropping some sp_costs (#6313)

9d80858

[skip changelog]

document-analytics-11-updates (#6314)

207b88a

changelog: Analytics, Document authorization, updates

Merge branch 'lg-6202-create-accordion' of github.com:18F/identity-id…

543550f

…p into lg-6202-create-accordion

merge main

dd0c10f

cleanup some conflicts

fe925a9

Merge branch 'main' of github.com:18F/identity-idp into lg-6202-creat…

bcfadfd

…e-accordion

Merge branch 'main' of github.com:18F/identity-idp into lg-6202-creat…

6461d19

…e-accordion

mocha tests

9f2c28d

test there is some content

6a61049

remove debugger

8fc061a

merge main

cea8386

fix spacing

fda6ef9

remove components not used

47eb9b3

peggles2 removed the status - work in progress label May 10, 2022

peggles2 requested review from aduth and zachmargolis May 10, 2022 20:58

fix typo

b0c1f93

aduth reviewed May 11, 2022

View reviewed changes

peggles2 added 4 commits May 11, 2022 13:25

code review feedbacks

ac7e744

update verify_controller

df5219b

merge main

c24b346

rebase main

2cb4bce

peggles2 closed this May 11, 2022

aduth mentioned this pull request May 12, 2022

Lg 6202 add accordion #6338

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lg-6202 - create accordion#6306

Lg-6202 - create accordion#6306
peggles2 wants to merge 112 commits intomainfrom
lg-6202-create-accordion

peggles2 commented May 4, 2022 •

edited

Loading

Uh oh!

aduth May 11, 2022

Uh oh!

peggles2 May 11, 2022

Uh oh!

aduth May 11, 2022 •

edited

Loading

Uh oh!

aduth May 11, 2022

Uh oh!

aduth May 11, 2022

Uh oh!

aduth May 11, 2022

Uh oh!

aduth May 11, 2022

Uh oh!

aduth May 11, 2022

Uh oh!

aduth May 11, 2022

Uh oh!

peggles2 May 11, 2022

Uh oh!

peggles2 commented May 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

	title: t('titles.idv.session.review'),
	title: t('idv.titles.session.review'),

	<Accordion header={t('idv.messages.review.intro')}>{PersonalInfoSummary(value)}</Accordion>
	<Accordion header={t('idv.messages.review.intro')}><PersonalInfoSummary pii={value} /></Accordion>

Conversation

peggles2 commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduth May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peggles2 commented May 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

peggles2 commented May 4, 2022 •

edited

Loading

aduth May 11, 2022 •

edited

Loading