Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Endpoint to Verify User Identity [#1062] #1111

Merged
merged 6 commits into from
Aug 19, 2022

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Aug 18, 2022

👉 Note: Docs will be added in follow-up #1010

Purpose

Add an endpoint to verify the identification code supplied by the user and then queue the privacy request for execution.

#1010 (not implemented yet) will email the user the generated identification code if identity verification is turned on. The user will take that code and submit it to the endpoint added here.

Changes

Add an endpoint to verify a user's identity before queuing the privacy request provided it doesn't need separate manual approval by a system admin.

  • Add a new PrivacyRequest.identity_verified_at timestamp
  • Add a new PrivacyRequestStatus - identity_unverified - if identity verification is on, and we go to create a privacy request it will be given this status first, instead of pending (to be completed in Backend - generate ID verification code #1010)
  • Add methods to cache the verification code in Redis for comparison with a default ttl of 10 minutes

POST {{host}}/privacy-request/{{privacy_request_id}}/verify

{"code": "123456"}

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section 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.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #1062

…y request provided it doesn't need separate manual approval by a system admin.

- Add a new PrivacyRequest.identity_verified_at timestamp
- Add a new PrivacyRequestStatus - "identity_unverified".
- Add methods to cache the verification code in Redis for comparison with a default ttl of 10 minutes
- Update changelog.
- Add endpoint to postman
@pattisdr pattisdr marked this pull request as ready for review August 18, 2022 23:31
…tyVerificationBodyParams that already exists.
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

This is looking good @pattisdr ! No blockers aside from using a diff class than SubjectIdentityVerificationBodyParams for the new endpoint req body.

Other than that, I was having trouble pinging the new endpoint in postman due to this error:

{
    "detail": "Method Not Allowed"
}

Maybe I have something misconfigured?

src/fidesops/ops/models/privacy_request.py Show resolved Hide resolved
"""
Generate one-time identity verification code
"""
return str(random.choice(range(100000, 999999)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the email mocks use a 4-digit code, but I suppose that's more prone to collision / overlap. We should confirm with design that 6 digits will work in the design, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

happy to change if needed, I usually see 6 on other applications

@@ -104,6 +107,7 @@ class Config:
class PrivacyRequestStatus(str, EnumType):
"""Enum for privacy request statuses, reflecting where they are in the Privacy Request Lifecycle"""

identity_unverified = "identity_unverified"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to think whether identity_unverified will always be set or only be set for requests where identity verification is turned on. This is more of a future convo, since I was doing some thinking here around edge cases- #1009 (comment)

Copy link
Contributor Author

@pattisdr pattisdr Aug 19, 2022

Choose a reason for hiding this comment

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

Yeah, different ways to organize. I was viewing it as a state that was only valid when we had identity verification on.

My work in #1010 (no PR yet) has a workflow that sets initial status as identity_unverified when identify verification is "on", otherwise, it sets the initial status to pending. If the identity is later verified, then I change the status from identity_unverified to pending, and also set a field to identity when it was verified to retain that it happened.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool, so depending on confirmation from product on #1009 (comment), your workflow sounds good to me!

privacy_request_id: str,
*,
db: Session = Depends(deps.get_db),
provided_code: SubjectIdentityVerificationBodyParams,
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I now realize SubjectIdentityVerificationBodyParams was badly named. That was anticipated to only account for email body params, not a request for subject identity verification. Could we make a new class to account for the request body here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure can, an earlier commit already has it!
40fcf6d

…ctIdentityVerificationBodyParams that already exists."

This reverts commit 40fcf6d.
@pattisdr
Copy link
Contributor Author

Reverted the commit @eastandwestwind, back to you!

@pattisdr
Copy link
Contributor Author

@eastandwestwind for the "Method Not Allowed" Error, you might just need to restart your webserver.

@eastandwestwind eastandwestwind merged commit 9dbe9ef into main Aug 19, 2022
@eastandwestwind eastandwestwind deleted the fidesops_1062_verification_endpoint branch August 19, 2022 21:11
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
* Add an endpoint to verify a user's identity before queuing the privacy request provided it doesn't need separate manual approval by a system admin.

- Add a new PrivacyRequest.identity_verified_at timestamp
- Add a new PrivacyRequestStatus - "identity_unverified".
- Add methods to cache the verification code in Redis for comparison with a default ttl of 10 minutes

* - Fix linting/copy-paste errors.
- Update changelog.
- Add endpoint to postman

* Add new keys to response bodies.

* Instead of using a new VerificationCode schema, use the SubjectIdentityVerificationBodyParams that already exists.

* Revert "Instead of using a new VerificationCode schema, use the SubjectIdentityVerificationBodyParams that already exists."

This reverts commit 40fcf6d.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backend - expose a verification endpoint
2 participants