The reason is a valid form-control-success

<input name="secretkey" autocomplete="one-time-code" id="mod-login-secretkey" type="text" class="form-control valid form-control-success" aria-invalid="false">

instead of a invalid form-control-error on the input field.
I would expect that when 2FA-Check fails.

Try Login into backend with correct user and passwort but without 2FA-Code.

2FA is an OPTIONAL INPUT ALWAYS, even when "enforce 2fa" is enabled... This is by design.

Phil Taylor notifications@github.com schrieb am Sa., 24. Okt. 2020, 19:57:

Try Login into backend with correct user and passwort but without 2FA-Code.

2FA is an OPTIONAL INPUT ALWAYS, even when "enforce 2fa" is enabled...
This is by design.

It's only optional, when you don't enable it.
On user's perspective, when enabled it is required to be filled out, it
should act like one login form.

If something is missing but required for login, it should be signaling with
class invalid that there was nothing entered or that what was entered,
was invalid.

And good user guidance is, to lead the user to the missing field by setting
the cursor into that field.

That behaviour you have in every good form today.

But the least thing what should be changed, is the wrong class of valid
and form-control-success, which is plain wrong and contrary to the system
message.

Trust me... I have banged my head enough with this project about 2FA. Do your research in this issue tracker.

It's only optional, when you don't enable it.

The secret key field is optional ALWAYS. Fact. The site can have "Enforce 2FA" enabled, but STILL the secret key is optional, and login is allowed without providing the secret key - by design - to allow users to login and set up their 2fa.

If something is missing but required for login, it should be signaling

Except that the system doesn't know "its required" for that user, until that user is already authenticated...

And good user guidance is, to lead the user to the missing field by setting the cursor into that field.

That might be able to be done... but probably not with the existing form validation.

You're right that the field is optional, when not setup by the user.

Maybe my description was not clear:

I used a user with already setup 2FA.

I am talking about the login form -->after<--
an unsuccessful login attempt and redirect to the login form because of a missing/ invalid 2FA Code.

And I trust you, Phil. ?

In that case, the 2FA Code is required, otherwise the 2FA Code would make no sense, because he could always login in with username and Password.

Test case 1:
testing with the same user, entering username and password, no 2FA Code gives:

Result:
Warning Two Factor code is invalid (should be "was empty" or not "was not filled out")
No indication (red border) of the Secret Code Field, no cursor in that field

Test case 2:
testing with the same user, entering username and password, invalid 2FA Code gives:

Result:
Warning Two Factor code is invalid (correct)
No indication (red border) of the Secret Code Field, no cursor in that field

Test case 3:
testing with the same user, entering username and password, valid 2FA Code gives:

Result:
User logged in successfully.

And yes, the system knows, that the user I used to login has the 2FA enabled, because I used the correct username and the correct password,
otherwise it would not show me the "Two Factor code invalid" message after the redirect to the login form to provide the missing Two Factor code.
Also it set the class "valid" and "form-control-success" to both username and password field.
(And again: Also for the 2FA field, which is wrong)

And now you have just revealed that this user has 2fa

Sorry, but this current code is correct whilst it is possible to have 2fa enabled optionally per user

Sorry but I said it already in the testing instruction that the 2FA-plugin
has to be enabled and setup/ enable for the user
and "not just revealed".

When the system already can tell me that my 2FA-code is missing/ wrong/
invalid (because I was to slow)
after redirect me to the login form again,
where is the problem to set the focus to the 2FA-Field as also set this to
"invalid"
(red border) to make it easy as possible to enter the code?

And no the code is not correct for the case of a failed login-attempt for a
user which has 2FA-enabled.

Brian Teeman notifications@github.com schrieb am Di., 24. Nov. 2020,
23:40:

And now you have just revealed that this user has 2fa

Sorry, but this current code is correct whilst it is possible to have 2fa
enabled optionally per user

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#31204 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AQNZU5Z2LIGQXR2SOLFHRLLSRQY6LANCNFSM4S27MDVA
.

You're missing the point that both Phil and I have told you. It is possible to have users on your site both with and without TFA. What your proposing is a security issue as you are enumerating the existence of a user and if they are using tfa

And you're missing the point that I'm not talking about first login but
about behaviour after a failed login attempt of a user with enabled two
factor.
And obviously the system can put out a system message "invalid 2FA Code"
but can not set 2FA field to invalid and set focus there in that case?

That's bullshit, sorry.

And it seems you have not even checked, if the issue exist about I'm
talking about.

Or are you assuming no user with enabled 2FA do ever enter invalid/wrong
2FA Codes?

Brian Teeman notifications@github.com schrieb am Mi., 25. Nov. 2020,
00:54:

You're missing the point that both Phil and I have told you. It is
possible to have users on your site both with and without TFA. What your
proposing is a security issue as you are enumerating the existence of a
user and if they are using tfa

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#31204 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AQNZU5ZFDXK3O4VBITYWRUTSRRBUNANCNFSM4S27MDVA
.

Can't help you then. You clearly don't understand user enumeration

And no, it's not a security risk as the system already tell it with a
system Message when a 2FA code is invalid.
And invalidating the field plus focus is nothing else!

It would be a security risk when a hacker have already a correct username
and password and can simply bruce force the 2FA-Code. I really hope there
are already mechanisms implemented to prevent this case

Brian Teeman notifications@github.com schrieb am Mi., 25. Nov. 2020,
00:54:

You're missing the point that both Phil and I have told you. It is
possible to have users on your site both with and without TFA. What your
proposing is a security issue as you are enumerating the existence of a
user and if they are using tfa

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#31204 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AQNZU5ZFDXK3O4VBITYWRUTSRRBUNANCNFSM4S27MDVA
.

You state something with false claims of a "security risk" which is simply
not true.
When you're such an expert in security, then tell me why it should be less
risky to print out a system message of invalid 2FA-Code but it is more
risky to set the involved field to invalid?

On that explanation I'm really curious.

Brian Teeman notifications@github.com schrieb am Mi., 25. Nov. 2020,
01:14:

Can't help you then. You clearly don't understand user enumeration

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#31204 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AQNZU556B35YJ6KREXGX5HTSRRD4TANCNFSM4S27MDVA
.