Authenticator App (TOTP 2FA) for Enterprise edition dashboard #14584
Conversation
17ba842 to
9a334c0
Compare
| %% try to login with good TOTP but bad password | ||
| %% this should mark the TOTP setup complete, |
There was a problem hiding this comment.
Should there be side-effects if the provided password is wrong? 🤔
There was a problem hiding this comment.
TOTP secret is only provided when password was once correct.
The second login attempt providing a valid TOTP code indicates that the authenticator app is set up.
Otherwise the user will get presented with the QR code again.
it is not needed by dashboard
so the user is forced to login again
| IsPwdOk = (ok =:= verify_hash(Password, PwdHash)), | ||
| MfaVerifyResult = verify_mfa_token(Username, MfaToken, IsPwdOk), | ||
| check_mfa_pwd(MfaVerifyResult, IsPwdOk, User); |
There was a problem hiding this comment.
I think that we shouldn't even do anything further once we establish that the password is incorrect.
i.e.:
| IsPwdOk = (ok =:= verify_hash(Password, PwdHash)), | |
| MfaVerifyResult = verify_mfa_token(Username, MfaToken, IsPwdOk), | |
| check_mfa_pwd(MfaVerifyResult, IsPwdOk, User); | |
| maybe | |
| true ?= (ok =:= verify_hash(Password, PwdHash)) orelse {error, <<"password_error">>}, | |
| MfaVerifyResult = verify_mfa_token(Username, MfaToken), | |
| check_mfa_pwd(MfaVerifyResult, User) | |
| end; |
There was a problem hiding this comment.
For TOTP, we can (and should) return token error over password error.
Otherwise a brute-force attacker can send any token to guess the password like without MFA.
For other mechanisms which costs to send the token (e.g. email / SMS, if we will need to implement), password needs to be checked before token.
Fixes EMQX-13509
Release version: v/e5.9.0
Summary
See emqx_dashboard/README.md
PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.mdfiles