laravel / jetstream

Tailwind scaffolding for the Laravel framework.
https://jetstream.laravel.com
MIT License
3.95k stars 808 forks source link

Migration error when enable/disable Features::twoFactorAuthentication #1207

Closed macklus closed 1 year ago

macklus commented 1 year ago

Description:

I found a migration problem related to our use case and Features::twoFactorAuthentication. We want to use 2FA but not yet, so we install jetpack with livewire support and no teams, add the migrations to our git repository, and disallow 2FA at fortify.php, that's required because we don't want our users to use 2FA yet, then we create a new migration to add our admin users.

Here, we have:

Now, when other colleague try to start working on their computer, founds an error like:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'two_factor_confirmed_at' in 'field list' (SQL: insert into `users` (`id`, `name`, `email`, `email_verified_at`, `password`, `two_factor_secret`, `two_factor_recovery_codes`, `two_factor_confirmed_at`, `remember_token`, `current_team_id`, `profile_photo_path`, `created_at`, `updated_at`) values (1, aaa, aaa@aaa.com, ?, aaa, ?, ?, ?, aaa, ?, ?, 2022-12-21 17:59:06, 2022-12-21 17:59:06))

That's because on 2014_10_12_200000_add_two_factor_columns_to_users_table.php migration class we have:

if (Fortify::confirmsTwoFactorAuthentication()) {
    $table->timestamp('two_factor_confirmed_at')
            ->after('two_factor_recovery_codes')
            ->nullable();
}

and, as 2FA is off, this field are not create. I think that migrations should not have conditionals on it, specially when it's related to features that you can activate or deactivate on demand.

Simple solution will be remove that conditional check on jetpack migration

Steps To Reproduce:

  1. Install laravel and jetpack
  2. Comment/disallow Features::twoFactorAuthentication on fortify.php
  3. Add a new migration to add one user
  4. commit changes
  5. try to install from commited repo (as on a new PC)
driesvints commented 1 year ago

These migrations are published to your app. You can customize them how you like 👍