Fix ephemeral webauthn_id for existing users

[santiagorodriguez96]

Fixes #120.

Details:

• Do a data migration as part of the migration to create the webauthn_id in the resource table that backfills webauthn_id for existing records.
• Change webauthn_id generation from after_initialize to before_validation, so it only runs once at record validation time. That way, when updating an existing user that doesn't have a webauthn_id, the callback will set it.

[RenzoMinelli]

Should we have the generated migration on the internal app?

[santiagorodriguez96]

We don't have the migrations in our internal app 😕

[santiagorodriguez96]

@RenzoMinelli I'm hesitant over mentioning in the code the option of setting the default in the database – as the value that we are trying to set is not static, I think it will trigger a full rewrite of the database which would lock the table on most database engines 😕

[RenzoMinelli]

yeah looking into this it would be basically the same as what's presented here, needs to update each row individually. Okay sounds good

[santiagorodriguez96]

Having considered some options, we decided that adding some sort of “placeholder” data migration to let users know that they should do that and perhaps find their own way of solving it. We know this solution is not ideal, but we think it’s the best we can do at least for now – feedback and alternative solutions are more than welcome 🙂

A couple of the solutions considered:

• First of all, dropping the webauthn_id altogether since right now it’s not being validated and we are even sending random values to the authenticator. We wanted to avoid this alternative since the WebAuthn specification recommends it to be 64 random bytes. There’s also a really interesting discussion about the reasoning of the WebAuthn group behind adding it.
• Setting thewebauthn_id for the users that have it blank in our controllers for fetching the registration options. We decided against it because it felt obscure to update a user in a GET request in a controller that lives in the engine – what’s more, some applications may use database replication with a strategy that uses replicas in GET request (this is even how ActiveRecord’s automatic role switching middleware works).
• This left us pretty much with only the option of adding the backfill to the migration. Now a couple of questions raised:
  • Should we add a NOT NULL constraint to the webauthn_id after the backfill is done? Without it and with the model validating for the uniqueness of it but allowing for nil values, we cannot ensure that the webauthn_id is not nil for some users (perhaps because insert_all or update_all were used, or because the user was created in the tiny window were the backfill was finished and the server is not up), which means that we could still have users that don’t have a webauthn_id. If we plan to validate the webauthn_id that comes from the client against the one of the user, we should handle this. We discussed the possibility of adding a webauthn_id presence check in our views to handle this but decided to tackle this as part of a separate effort.
  • We also talked about adding a default value to the column if we are adding the NOT NULL constraint but, as the default that we will be adding is non-deterministic, the migration will lock the entire table and take a lot of time for large tables. Given so, we decided against it. Perhaps it could be an option to set the default as part of a separate migration along with the NOT NULL constraint? Maybe we can add some sort of generator that could be run by users in order to generate this follow up migration?