Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the redis.identity_verification_code_ttl_seconds config property is not being respected #2104

Closed
adamsachs opened this issue Dec 22, 2022 · 0 comments · Fixed by #2105
Closed
Assignees
Labels
bug Something isn't working

Comments

@adamsachs
Copy link
Contributor

Bug Description

We have a redis.identity_verification_code_ttl_seconds config property that's used when generating the verification code message sent to an end user. But we don't use that property when actually setting the verification code in the redis cache, meaning that the verification code effectively expires based on the default_ttl_seconds property, and is therefore not in line with what's communicated to the end user (or with what the admin who set the property would expect)

Steps to Reproduce

  • set redis.identity_verification_code_ttl_seconds to a very low value (e.g. 1) in your fides.toml
  • set subject_identity_verification_required = true
  • set up email integration with mailgun as described in our docs -- you can use our testing mailgun account with details found in our Fides .env 1pw entry
  • run nox -s dev -- pc to run your dev server with the privacy center UI
  • submit an access data request with your email as the email
  • get the verification code from your email, wait a few seconds and enter it in the privacy center
  • it will work, which it shouldn't, given that you'd set the expiration time to 1 second in the first step

Expected behavior

Verification code shouldn't work after expiration time set in identity_verification_code_ttl_seconds

Screenshots

If applicable, add screenshots to help explain your problem.

Environment

  • Version:
  • OS:
  • Python Version:
  • Docker Version:

Additional context

Found this while looking through code for review of #2094, not directly related though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant