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

Should MFA / OTP be required for credential reset #117

Open
UnicornChance opened this issue Jun 26, 2024 · 4 comments
Open

Should MFA / OTP be required for credential reset #117

UnicornChance opened this issue Jun 26, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@UnicornChance
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Follow on discussion to this PR, at the moment the MFA is not required when a user resets credentials which opens a backdoor for them to access their account and reset the MFA as well.

@UnicornChance UnicornChance added the enhancement New feature or request label Jun 26, 2024
@bburky
Copy link
Member

bburky commented Jul 2, 2024

This is maybe ok, but we should think through it.

  • Do we assume the user has MFA on their email, so we don't need to validate MFA here?
  • Is this an intentional MFA reset method? password reset (which requires email re-verification) → log in to account → delete MFA?

If we don't think the answers to those are "yes", then we should require MFA during the reset flow.

@rjferguson21
Copy link
Contributor

I think to be safe we should update this flow to include MFA at the end of the reset flow.

@UnicornChance UnicornChance self-assigned this Jul 16, 2024
@bburky
Copy link
Member

bburky commented Jul 16, 2024

I picked github.com as a quick test, it seems they do MFA validation after clicking the email verification link. I think that makes sense if we do it.

However, the logic gets a little bit weird. We have both MFA and x509 and they're kind of equivalent.

Currently, I think you can set an initial password on an x509 user with no password via password reset. It seems odd to enforce MFA on MFA users but not on x509 users.

Arguably we could just not allow password resets of users without passwords, but there are some legitimate use cases where maybe want to allow that? (I think civilian and contractors have different UPNs, and the new CAC won't be valid for existing accounts, you may want to allow password resets to allow these users to get into their old account with a different CAC).

Alternately, if we assume the user has MFA on their email, arguably we don't need MFA on password reset. That may not be a good assumption though.

I really don't have a great answer for this issue. I don't want to add a feature that protects password+MFA users more than x509 or SAML users, so doing it on them only seems like the wrong answer.

@UnicornChance
Copy link
Contributor Author

UnicornChance commented Jul 19, 2024

Would it make sense to require MFA for all registration types (x509, SSO, user/pass) regardless if there is a password or not?

I've set up a POC and have a list of steps that i can share for each of the identity types if interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants