-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
API / Auth: Require OTP if Enforce TFA is enabled #22190
base: main
Are you sure you want to change the base?
API / Auth: Require OTP if Enforce TFA is enabled #22190
Conversation
|
@joselcvarela Hmm, it seems like the previous check was designed that way in order to still allow users to login and thus being able to set-up 2FA after role setting has been changed to "Require 2FA". Such users are currently redirected by the App: Lines 140 to 142 in dfe6cca
So maybe instead of adjusting the login method, we can block all routes except for /users/me/tfa/* via a middleware, as long as a user hasn't set-up 2FA yet?
|
As Pascal mentioned the user still working after toggling the "require 2fa" options is very much intentional until the first time the user logs into the App to actually configure 2fa since we cannot ask for otp codes before they have been set up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking back on this, the current solution blocks all logins when the "require TFA" is enabled for a role preventing users from logging into the app and setting up TFA which isnt ideal. The desired flow here is:
- when logging in using the App you get redirected to the TFA setup
- when logging in using the Api you get an error instructing you to set up TFA by logging into the App (as blindly redirecting may break server api interaction in unforeseen ways)
- We should probably document the API only flow for configuring TFA for "No App Access" + "Require TFA" roles
While doing some tests I have enabled "Require 2FA" under the role page.
Then I have tried to login with a user via API under this role and it worked.
I was expecting that to not work, as I have enabled the Require 2FA.
Although the login worked expected.
In this PR we fix this behaviour so if Required 2FA is enabled on role, then an
otp
will be required onlogin
.Scope
What's changed:
Potential Risks / Drawbacks
otp
field is now required per role conditions.Review Notes / Questions