Page MenuHomePhabricator
Create Task
Maniphest T242031

Allow multiple different 2FA devices
Open, Needs TriagePublic
Actions

Description

Creating as an umbrella for T232336: Separate recovery codes into a separate MFA method and T230042: Allow multiple totp devices

It should be possible to have TOTP and WebAuthn enabled

[21:28:13] <AntiComposite> I know it was just deployed, but is there a reason that OATHAuth only supports TOTP _or_ WebAuthn (U2F)?
[21:29:40] <AntiComposite> Every other service I use that supports WebAuthn supports both at the same time, which means I can use my hardware key on my laptop but fall back to my TOTP generator for my phone, where WebAuthn isn't supported in the browser.

Details

ProjectBranchLines +/-Subject
operations/mediawiki-configmaster+1 -1[beta] Read new for OATHAuthMultipleDevicesMigrationStage
operations/mediawiki-configmaster+3 -0[beta] Write both for OATHAuthMultipleDevicesMigrationStage
mediawiki/extensions/OATHAuthmaster+575 -131Database-level support for multiple auth devices
operations/mediawiki-configmaster+2 -0Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD
mediawiki/extensions/OATHAuthmaster+0 -2API: Do not expose the module name in the output
Customize query in gerrit

Related Objects
Search...

Event Timeline

Reedy created this task.Jan 6 2020, 9:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 6 2020, 9:47 PM
Paladox added a subscriber: Paladox.Jan 6 2020, 9:48 PM
AntiCompositeNumber added a subscriber: AntiCompositeNumber.Jan 6 2020, 9:54 PM
revi added a subscriber: revi.Jan 14 2020, 3:32 PM
Reedy mentioned this in T244348: Recovery option for Webauthn.Feb 5 2020, 6:59 PM
RhinosF1 added a subscriber: RhinosF1.Aug 3 2020, 4:12 PM
Proc added a subscriber: Proc.Aug 3 2020, 4:12 PM
• taavi added a subscriber: • taavi.Aug 3 2020, 4:13 PM
mdaniels5757 added a subscriber: mdaniels5757.Aug 9 2020, 5:56 PM
Reedy moved this task from Backlog to User Experience on the MediaWiki-extensions-OATHAuth board.Feb 17 2022, 3:35 PM
Legoktm mentioned this in T230042: Allow multiple totp devices.Feb 18 2022, 9:53 AM
Legoktm merged a task: T230042: Allow multiple totp devices.
Legoktm added a subtask: T268564: Convert OATHAuth to AbstractSchema.
Legoktm added a parent task: T232336: Separate recovery codes into a separate MFA method.
Legoktm added subscribers: Legoktm, jeblad.
deryckchan mentioned this in T172079: Allow OATHAuth users with 2FA already enabled to add / switch devices without disabling.Feb 18 2022, 11:10 AM
Reedy closed subtask T268564: Convert OATHAuth to AbstractSchema as Resolved.Feb 19 2022, 8:01 PM
Legoktm claimed this task.Feb 22 2022, 6:13 AM

The current db schema looks like:

CREATE TABLE oathauth_users (
 id INTEGER not null primary key, -- fk to user/central ID
 module TEXT not null, -- varchar(255) in non-sqlite
 data BLOB null -- JSON blob
 );

I would like to introduce a new column with autoincrement IDs to be the primary key, and allow id to be just a reference to the central ID but non-unique. I hope schema change wise it'll be simple, the code changes will require more work.

Legoktm mentioned this in T150898: Force OATHAuth (2FA) for certain user groups in Wikimedia production.Mar 3 2022, 2:11 AM
Stang added a subscriber: Stang.Mar 3 2022, 5:52 AM
Ladsgroup added a subscriber: Ladsgroup.Mar 7 2022, 9:50 AM
SQL added a subscriber: SQL.Jun 11 2022, 5:27 PM
TheresNoTime removed a subscriber: RhinosF1.Dec 15 2022, 11:36 PM
gerritbot added a comment.Jan 1 2023, 12:00 AM

Change 873892 had a related patch set uploaded (by Majavah; author: Majavah):

[mediawiki/extensions/OATHAuth@master] Database-level support for multiple auth devices

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

gerritbot added a project: Patch-For-Review.Jan 1 2023, 12:00 AM
Octfx added a subscriber: Octfx.Jan 13 2023, 7:36 PM
Ahm_masum added a subscriber: Ahm_masum.Jan 30 2023, 7:11 PM
gerritbot added a comment.Jan 31 2023, 9:59 AM

Change 885284 had a related patch set uploaded (by Majavah; author: Majavah):

[mediawiki/extensions/OATHAuth@master] Api: Do not expose the module name in the output

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

Legoktm reassigned this task from Legoktm to • taavi.Jan 31 2023, 3:34 PM
gerritbot added a comment.Jan 31 2023, 3:39 PM

Change 885284 merged by jenkins-bot:

[mediawiki/extensions/OATHAuth@master] API: Do not expose the module name in the output

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.22; 2023-02-06).Jan 31 2023, 4:00 PM
Reedy added a comment.Feb 1 2023, 4:48 PM

Would this schema change be a good time to try and do/fix T242847: Include enrollment timestamp in oathauth_users.data?

It might make more sense to use/add a column rather than just bundling it into a json blob (as I originally suggested with the code as currently is)?

• taavi added a comment.Feb 1 2023, 5:30 PM
In T242031#8579072, @Reedy wrote:

Would this schema change be a good time to try and do/fix T242847: Include enrollment timestamp in oathauth_users.data?

It might make more sense to use/add a column rather than just bundling it into a json blob (as I originally suggested with the code as currently is)?

This is a great opportunity to add the a column for that, but I think the actual implementation is best left for later. (I have a few local patches that still need some polishing but would make this somewhat easier.) Updated the patch for that.

Reedy added a comment.Feb 1 2023, 5:33 PM
In T242031#8579220, @taavi wrote:
In T242031#8579072, @Reedy wrote:

Would this schema change be a good time to try and do/fix T242847: Include enrollment timestamp in oathauth_users.data?

It might make more sense to use/add a column rather than just bundling it into a json blob (as I originally suggested with the code as currently is)?

This is a great opportunity to add the a column for that, but I think the actual implementation is best left for later. (I have a few local patches that still need some polishing but would make this somewhat easier.) Updated the patch for that.

Oh yeah, I'm not bothered about getting it implemented (though if you were willing to later...). It's always going to have to have a "null" value for "we don't know when this person enrolled".

But yeah, including a useful schema change while is still cheap/easy (not that the DB tables are that large, but still), is a win all around.

Thanks!

Reedy mentioned this in T242847: Include enrollment timestamp in oathauth_users.data.Feb 1 2023, 5:36 PM
Yahya added a subscriber: Yahya.Feb 11 2023, 1:08 PM
waldyrious awarded a token.Feb 11 2023, 10:30 PM
waldyrious added a subscriber: waldyrious.
gerritbot added a comment.Feb 24 2023, 1:58 PM

Change 891833 had a related patch set uploaded (by Majavah; author: Majavah):

[operations/mediawiki-config@master] Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD

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

gerritbot added a comment.Wed, Mar 1, 4:17 PM

Change 891833 merged by jenkins-bot:

[operations/mediawiki-config@master] Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD

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

Stashbot added a comment.Wed, Mar 1, 4:17 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-01T16:17:58Z] <taavi@deploy2002> Started scap: Backport for [[gerrit:891833|Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD (T242031)]]

Stashbot added a comment.Wed, Mar 1, 4:20 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-01T16:20:00Z] <taavi@deploy2002> taavi: Backport for [[gerrit:891833|Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD (T242031)]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet

TheresNoTime added a subscriber: TheresNoTime.Wed, Mar 1, 4:24 PM
Stashbot added a comment.Wed, Mar 1, 4:26 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-01T16:26:21Z] <taavi@deploy2002> Finished scap: Backport for [[gerrit:891833|Set OATHAuthMultipleDevicesMigrationStage to MIGRATION_OLD (T242031)]] (duration: 08m 23s)

Stashbot added a comment.Wed, Mar 1, 4:32 PM

Mentioned in SAL (#wikimedia-releng) [2023-03-01T16:32:51Z] <taavi> deployment-prep: block automatic/update.php execution of UpdateForMultipleDevicesSupport maintenance script in OATHAuth by pre-inserting rows in updatelog tables, will be run manually instead. T242031

TheresNoTime added a project: Community-Wishlist-Survey-2023.Tue, Mar 14, 5:18 PM
TheDJ added a subscriber: TheDJ.Wed, Mar 15, 2:23 PM
gerritbot added a comment.Wed, Mar 22, 3:13 PM

Change 873892 merged by jenkins-bot:

[mediawiki/extensions/OATHAuth@master] Database-level support for multiple auth devices

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

Maintenance_bot removed a project: Patch-For-Review.Wed, Mar 22, 3:30 PM
• taavi closed subtask T330502: Create oathauth_types and oathauth_devices tables as Resolved.EditedWed, Mar 22, 8:08 PM
• taavi mentioned this in T330502: Create oathauth_types and oathauth_devices tables.

Migration plan:

  • Create the new tables on beta
  • Write both for beta
  • Run the migration script on beta
  • Read new for beta
  • Write new for beta
  • Rename/drop old table on beta
  • Wait for the train to deploy new code
  • Create the new tables on production
  • Write both for production
  • Run the migration script on production
  • Read new for production
  • Write new for production
  • Rename/drop old table in production
Stashbot added a comment.Wed, Mar 22, 8:34 PM

Mentioned in SAL (#wikimedia-releng) [2023-03-22T20:34:32Z] <taavi> cat /srv/mediawiki/php-master/extensions/OATHAuth/sql/mysql/tables-generated.sql | sql centralauth --write # T242031

gerritbot added a comment.Wed, Mar 22, 8:39 PM

Change 902189 had a related patch set uploaded (by Majavah; author: Majavah):

[operations/mediawiki-config@master] [beta] Write both for OATHAuthMultipleDevicesMigrationStage

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

gerritbot added a project: Patch-For-Review.Wed, Mar 22, 8:39 PM
gerritbot added a comment.Wed, Mar 22, 9:01 PM

Change 902189 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] Write both for OATHAuthMultipleDevicesMigrationStage

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

Stashbot added a comment.Wed, Mar 22, 9:06 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-22T21:06:26Z] <taavi@deploy2002> Started scap: Backport for [[gerrit:902188|Remove OATHAuthMultipleDevicesMigrationStage from CS]], [[gerrit:902189|[beta] Write both for OATHAuthMultipleDevicesMigrationStage (T242031)]]

Stashbot added a comment.Wed, Mar 22, 9:08 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-22T21:08:09Z] <taavi@deploy2002> taavi: Backport for [[gerrit:902188|Remove OATHAuthMultipleDevicesMigrationStage from CS]], [[gerrit:902189|[beta] Write both for OATHAuthMultipleDevicesMigrationStage (T242031)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet

Maintenance_bot removed a project: Patch-For-Review.Wed, Mar 22, 9:10 PM
Stashbot added a comment.Wed, Mar 22, 9:13 PM

Mentioned in SAL (#wikimedia-operations) [2023-03-22T21:13:55Z] <taavi@deploy2002> Finished scap: Backport for [[gerrit:902188|Remove OATHAuthMultipleDevicesMigrationStage from CS]], [[gerrit:902189|[beta] Write both for OATHAuthMultipleDevicesMigrationStage (T242031)]] (duration: 07m 29s)

Stashbot added a comment.Wed, Mar 22, 9:14 PM

Mentioned in SAL (#wikimedia-releng) [2023-03-22T21:14:15Z] <taavi> taavi@deployment-mwmaint02:~$ mwscript extensions/OATHAuth/maintenance/UpdateForMultipleDevicesSupport.php metawiki --force # T242031

gerritbot added a comment.Wed, Mar 22, 9:17 PM

Change 902198 had a related patch set uploaded (by Majavah; author: Majavah):

[operations/mediawiki-config@master] [beta] Read new for OATHAuthMultipleDevicesMigrationStage

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

gerritbot added a project: Patch-For-Review.Wed, Mar 22, 9:17 PM

Change 902198 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] Read new for OATHAuthMultipleDevicesMigrationStage

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

• taavi added a comment.Wed, Mar 22, 9:22 PM

Migrating beta uncovered a couple of issues with the migration process that I somehow did not think about before. Essentially:

  • If someone enables 2fa during the "write both" phase, the migration script will create a duplicate row in the new table.
  • Since the migration script deletes the rows from the old table, two-factor authentication will not work properly once the migration script has not been ran but write mode is still set to old.

So I need to re-think the migration strategy here. :( I'll think about it and come back with an updated proposal/patch.

Maintenance_bot removed a project: Patch-For-Review.Wed, Mar 22, 9:30 PM
Ladsgroup added a comment.Fri, Mar 24, 11:46 PM

The way we do other data migrations is that we keep the duplicate data around and remove it with the drop of the old tables/fields. Basically my suggestion is to simply avoid deleting the rows in the old table. Would that fix your issue?

mdaniels5757 removed a subscriber: mdaniels5757.Sat, Mar 25, 5:26 PM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL