I can't really make sense of the request sequence, but in general the fact that every login session has a different verification code is an invitation for trouble if the user restarts login, has multiple tabs etc. We should use a single session key, like with CSRF.

The other thing that comes to mind is that maybe we are not trimming newlines etc.

In T395353#10861402, @Tgr wrote:

The other thing that comes to mind is that maybe we are not trimming newlines etc.

EmailAuthAuthenticationRequest uses the string field type, which is translated to a HTMLTextField, which will use WebRequest::getText(). So indeed no trimming.

Change #1167218 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/EmailAuth@master] Trim verification code input

https://gerrit.wikimedia.org/r/1167218

The input form field has inputmode="numeric" pattern="[0-9]*" so in theory it's impossible to paste whitespace into it. (Maybe the apps have a more permissive input field?) Anyway it's trivial to add trimming.

Change #1167218 merged by jenkins-bot:

[mediawiki/extensions/EmailAuth@master] Trim verification code input

https://gerrit.wikimedia.org/r/1167218

In T395353#10983711, @Tgr wrote:

The input form field has inputmode="numeric" pattern="[0-9]*" so in theory it's impossible to paste whitespace into it.

Or rather, possible to paste it but it cannot be submitted (the browser will show a validation error popup).

We should probably have some JS to strip whitespace on paste, it's nicer UX-wise.

Per T398902#10997800, I suspect there is also a third issue that's not whitespace or issuing new codes that override the previous ones.

Change #1168620 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/EmailAuth@master] Filter out non-digit characters when pasting into the form

https://gerrit.wikimedia.org/r/1168620

Change #1168625 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/EmailAuth@master] Log the first two characters of the token

https://gerrit.wikimedia.org/r/1168625

Change #1168646 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/EmailAuth@master] [WIP] Store the code in the normal session, not the auth session

https://gerrit.wikimedia.org/r/1168646

Change #1168625 merged by jenkins-bot:

[mediawiki/extensions/EmailAuth@master] Log the first two characters of the token

https://gerrit.wikimedia.org/r/1168625

In T395353#10997855, @Tgr wrote:

Per T398902#10997800, I suspect there is also a third issue that's not whitespace or issuing new codes that override the previous ones.

The logs confirm that. We have 3K failures for the last 7 days, and for 300 of them the expected and submitted code match. We only log the first two characters of the code for security reasons, so an 1% bogus log volume is expected, but this is 10%.

No idea how this is possible. The relevant code is

if ( $req && hash_equals( $token, $req->token ) ) {
    // ...
    return AuthenticationResponse::newPass();
}

if ( $req && $req->token ) {
    $logger->info( 'Failed verification for {user}', ... );
}

so the logging code is clearly unreachable when the expected and actual token are the same. Some kind of trailing character? We are trimming whitespace, but maybe users are accidentally copying the next few characters from the email? But Chrome at least prevents submitting non-numeric input, and there are plenty of Chrome UAs there.

Change #1168620 merged by jenkins-bot:

[mediawiki/extensions/EmailAuth@master] Filter out non-digit characters when pasting into the form

https://gerrit.wikimedia.org/r/1168620