#1008 User identity verification flow #1231
Conversation
TheAndrewJackson
changed the title
TheAndrewJackson
changed the title
There was a problem hiding this comment.
Nice work @TheAndrewJackson ! Just a couple requests, but nothing blocking if you wanted to tackle anything in a follow-up ticket or as part of future work.
Also confirmed e2e testing for the following scenarios:
subject_identity_verification_requiredset tofalsecreated a privacy request with no verification requirementssubject_identity_verification_requiredset totruewith no email config resulted in error message (in future we can deliver a more specific err message when no email config is found, within admin-ui)subject_identity_verification_requiredset totruewith email config produced an email with verification code. Entering the wrong code resulted in an error message. Entering the correct code proceeded to success state as expected- Confirmed that "try again" sent another code to email
- Confirmed success flow for a deletion request
| const server = setupServer(); | ||
| const server = setupServer( | ||
| rest.get( | ||
| "http://localhost:8080/api/v1/id-verification/config", |
There was a problem hiding this comment.
Can we use our ${hostUrl} const in tests too? Or is there a specific reason not to?
There was a problem hiding this comment.
I think we can. There's no particular reason. I'll update it now.
| headers: { | ||
| Accept: "application/json", | ||
| "Content-Type": "application/json", | ||
| "X-Fides-Source": "fidesops-privacy-center", |
There was a problem hiding this comment.
thanks for adding the source header! since this is duplicated across a couple endpoints, I'm curious whether we can do something similar to the admin-ui, where we have a method to add common headers to all endpoints?
There was a problem hiding this comment.
We can. I just added it in. It would be nice to keep things consistent between the projects. eslint complained and said the headers-polyfill package needed to be installed. They'll be one more small dependency added to the project. No big deal
| } else if ( | ||
| isVerificationRequired && | ||
| data.succeeded.length && | ||
| data.succeeded[0].status === "identity_unverified" |
There was a problem hiding this comment.
Can we add an enum/type for possible privacy request statuses, and use it here? Something similar to https://github.com/ethyca/fidesops/blob/main/clients/ops/admin-ui/src/features/privacy-requests/types.ts#L1-L10
There was a problem hiding this comment.
Good idea. I'll add that in. Once everything is unified we may be able to share a lot of these types and util functions between the 2 projects.
There was a problem hiding this comment.
I had to make it an enum instead of a type. Apparently you can't use types in value comparisons.
|
Thanks for the updates! Manually e2e testing the happy path now... |
* Update lint commands and eslint ignore * Run lints and refactor privacy cards * Refactor modal * Finish verification flow * get config from server * Update changelong * Fix test failures * Format file * Mock out route * Format file * Add code resending * Update test to use hostUrl * Add headers util function and PrivacyRequestStatus status enum
Purpose
Update the privacy center to utilize #1115 verify a users identity before fulfilling a privacy request.
Changes
Checklist
CHANGELOG.mdfileCHANGELOG.mdfile is being appended toUnreleasedsection in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.Run Unsafe PR Checkslabel has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #1008
User_Identity_verification.mov