Page MenuHomePhabricator

Ability to overwrite 2FA method without providing/re-auth using existing secondary authentication method
Closed, ResolvedPublic

Description

Copying ou

Don't know the best place to leave this....

So if someone logs in with one 2FA, they can override and replace it without any warning/message. And without any re-auth... I'm guessing this is mostly due to me doing it as serial actions one after another, rather than letting some time expire (IIRC there's a 15 minute "no reauth window" or similar?)

However, it doesn't feel a good workflow that you can just override one 2FA with another, without providing input from the other device to disable it....

I do think the patch is in reasonable shape though that we can probably merge it, and make any changes ontop of it, rather than (many) more amendments
A great use case is if I logged in with one, left my machine unattended, someone could immediately just replace my 2FA, and if I didn't do anything about it very soon after... They'd have control of the 2FA on my device, and I'd be confused as to why mine apparently wasn't working

Event Timeline

Reedy created this task.Sep 4 2019, 4:40 PM
Reedy renamed this task from Ablity to overwrite 2FA method without providing secondary authentication method to Ability to overwrite 2FA method without providing secondary authentication method.Sep 4 2019, 4:42 PM
CCicalese_WMF triaged this task as Medium priority.Sep 4 2019, 5:21 PM
Reedy renamed this task from Ability to overwrite 2FA method without providing secondary authentication method to Ability to overwrite 2FA method without providing/re-auth using existing secondary authentication method.Sep 5 2019, 3:17 PM
Reedy added a comment.Sep 11 2019, 1:27 PM

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/535793/ is to address this, at least in part... The other thing is the overriding of another 2FA method without any confirmation/notification

Plus things like wiping out recovery tokens when disabling TOTP etc

Should we provide a warning when enabling a 2FA method, in case you already have one enabled?

I dont understand your point about wiping out recovery tokens

Reedy added a comment.Sep 11 2019, 2:05 PM

Should we provide a warning when enabling a 2FA method, in case you already have one enabled?

Yes, exactly! A warning/confirmation to continue

I dont understand your point about wiping out recovery tokens

Say I have TOTP 2FA enabled. I record my recovery tokens for later usage. I replace TOTP with WebAuthn... I'm not told my previous recovery tokens are now useless, as they've been removed from the database. And obviously, WebAuthn doesn't provide any new recovery tokens

This is kinda linked to the above, that we don't give any sort of warning to the changeover in devices

Change 536013 had a related patch set uploaded (by ItSpiderman; owner: ItSpiderman):
[mediawiki/extensions/WebAuthn@master] Update the module to implement new interface methods

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

Change 536014 had a related patch set uploaded (by ItSpiderman; owner: ItSpiderman):
[mediawiki/extensions/OATHAuth@master] Add warning page before method gets disabled

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

Change 536013 merged by jenkins-bot:
[mediawiki/extensions/WebAuthn@master] Update the module to implement new interface methods

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

Change 536014 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@master] Add warning page before method gets disabled

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

Is this task ready to be marked Resolved?

Reedy added a comment.Oct 7 2019, 3:46 PM

Is this task ready to be marked Resolved?

Not quite, the workflow is now mostly in place to warn users of the change. Yay!

What was blocking this, was that I was having an issue with https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/535793/ as it didn't seem to make any difference for me. However it looks like Dejan has found the issue and uploaded a new patch a few days ago. Will get it reviewed/re-tested later this week and we should be good to close this task off then

Reedy closed this task as Resolved.Oct 9 2019, 9:25 PM
Reedy assigned this task to ItSpiderman.