The current implementation of OATHUserRepository::persist() causes every
key to get a new ID when it's saved. This, combined with ::removeKey()
which compares keys by ID, means that using recovery codes to disable
TOTP is broken since TOTPKey calls persist() to mark the code as saved
just before the key is deleted.
In this patch I've chosen to add a new ::updateKey() method instead of
fixing ::persist(). This is more in line with the other new APIs in
OATHUserRepository (namely ::createKey() and ::removeKey()), and is
something I've been planning to do eventually - this bug just made that
a bit more urgent. ::persist() should be dropped once WebAuthn has been
updated too.
Tests are also updated - OATHUserRepositoryTest now updates the key
before deleting it and there's a new TOTPDisableFormTest to test the
entire disabling process.
Bug: T363548
Change-Id: I86ddc8e5bfc9cf74c587ffdff523f559c5a3c08c
(cherry picked from commit 0dad2c7031)
This adds new database tables to support storing multiple authentication
factors for a single user. The current approach taken is to use a single
database row per 2fa method and key. The current module/key abstraction
will have to be updated to support having multiple module types for a
single user (for example for having a separate module for recovery
codes), but this patch does not address that and instead keeps the
existing limitations, however the needed updates for that should be
doable with this database schema.
I've decided to add a new table instead of modifying the existing
oathauth_users table. This is mainly because adding an auto_increment
column to the existing table would be difficult, but also allows us to
update the table definition to follow MW conventions (namely the column
name prefixes). I've also used the opportunity to normalize the device
types onto a separate table.
The migration stage variable is set to SCHEMA_COMPAT_NEW so that
third-party wikis can use update.php normally and don't have to adjust
anything. This means that it needs to be manually set to _OLD on
wmf-config before merging this patch.
Since we're already working with the database schema, this add a new,
currently unused column for the creation data, so that T242847 will not
require a new schema change.
Bug: T242031
Bug: T242847
Change-Id: I6aa69c089340434737b55201b80398708a70c355
Mostly I wanted to add tests for verifying serialization and
deserialization, since that's what I modify in my next commit.
Change-Id: I8223f2e3e1b3ce79afc8c5cd9ca4afe6d418abf9
