Bug 1871065 - [DUO] Update BMO to move away from the deprecated iframe Duo prompt to the suggested Duo Universal Prompt #2202
Conversation
…e Duo prompt to the suggested Duo Universal Prompt
…ict with the endpoints needed by Duo Security. Duo takes precedence since we do not use OAuth2 logins yet and we do use Duo.
| # If we got this far and MFA is Duo, we should be verified | ||
| if ($user->mfa eq 'Duo' && !$event->{duo_verified}) { | ||
| ThrowUserError('duo_user_error', {reason => 'Invalid Duo Security MFA Code'}); | ||
| } | ||
|
|
There was a problem hiding this comment.
Let's make a function for this and re-use it in userprefs.cgi.
There was a problem hiding this comment.
I am not sure that creating a function for this will gain much as it would be a very small check and is only in two places currently. Would the utility function throw the error or would it just return true or false. Would it do the $mfa eq 'Duo' check or would we do that before calling it. Just not sure it is worth the code line savings yet.
There was a problem hiding this comment.
The whole if block should be in one function like assert_duo_verified($user->mfa, $event), since we're checking the exact same condition and throwing the exact same error in both places. It's mostly a maintainability suggestion, so if you're fine with maintaining the same logic in multiple places, it's no problem.
Connor and I will be doing a in-person code review for this due to the size and timing of it but I will add some details here as well.