Page MenuHomePhabricator

Users should be notified when only two recovery codes are left
Open, Needs TriagePublic

Description

Probably should be done using Echo

Event Timeline

In OATHAuthKey.php...

		// See if the user is using a scratch token
		if ( !$retval ) {
			$length = count( $this->scratchTokens );
			// Detect condition where all scratch tokens have been used
			if ( $length == 1 && "" === $this->scratchTokens[0] ) {
				$retval = false;
			} else {
				for ( $i = 0; $i < $length; $i++ ) {
					if ( $token === $this->scratchTokens[$i] ) {
						// If there is a scratch token, remove it from the scratch token list
						unset( $this->scratchTokens[$i] );
						$oathrepo = OATHAuthHooks::getOATHUserRepository();
						$user->setKey( $this );
						$oathrepo->persist( $user );
						// Only return true if we removed it from the database
						$retval = self::SCRATCH_TOKEN;
						break;
					}
				}
			}
		}

It should just need the notification stuff plumbing in (after being designed?) somewhere around there

Should there also be a way to generate a new set of recovery tokens, or is the "fix" for that to disable and then re-enable OATH?

From a user experience POV it would be nice to be able to list the tokens at any time as well. Google allows this on https://myaccount.google.com/security/signinoptions/two-step-verification and it has been handy in the past for me.

Should there also be a way to generate a new set of recovery tokens, or is the "fix" for that to disable and then re-enable OATH?

Yeah. Having to disable and re-enable is a bad user experience for sure.

From a user experience POV it would be nice to be able to list the tokens at any time as well. Google allows this on https://myaccount.google.com/security/signinoptions/two-step-verification and it has been handy in the past for me.

@csteipp can maybe comment more, but I don't know if I agree with this. The backup codes are a shared secret between the server and user, and we should avoid communicating shared secrets as much as possible. It'd be better to only show the codes once, and if the user wants to see them again, just generate brand new codes.

It'd be better to only show the codes once, and if the user wants to see them again, just generate brand new codes.

I'd be fine with that solution.

A better solution is for MW to just list out the remaining scratch codes left.

A better solution is for MW to just list out the remaining scratch codes left.

I'm not sure how a list somewhere a user isn't going to probably look is a better solution than a notification that they are much more likely to see

Reedy renamed this task from Users should be notified when only two scratch tokens are left to Users should be notified when only two recovery codes are left.Jan 1 2024, 8:49 PM

Change 990132 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/OATHAuth@master] Add notification when user is running out of recovery codes

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

This message makes more sense in the context of using a token, so ideally IMO it shouldn't be a notification but an interstitial shown after using up the third-to-last token. If a notification is much easier to add, that makes sense, but not sure that is the case?

Maybe both make sense?

An interstitial is dismissed and it’s gone.

A notification (depending on how you deal with it), can still be there for referencing.

They can result in emails (but looks like we might need to wire that in a little differently; the 2FA ones don’t show in the Notifications tab in preferences. I don’t know if this is because they’re marked system, and actually trigger emails anyway) or via Apps too.

They therefore can work without JS.

And they also result in the user getting some notification such as if they log in via apps (rather than needing to do a separate implementation of the interstitial because they reimplement the login screen).

And also, work for our use cases like Striker (combined with the email notifications), where you can use a recovery code on a “third party” site, so wouldn’t see an interstitial inside MW either…