Passport migrations ignore --database option

[magoun]

• Passport Version: 9.3.2
• Laravel Version: 7.28.3
• PHP Version: 7.4.11
• Database Driver & Version: MySQL 5.7.31

Description:

The included passport migrations no longer consider the --database flag passed to migrate on the command line, instead reading the default database from the connection specified in config/passport.

Steps To Reproduce:

On a new Laravel project with at two database connections in config/database (mysql and testing), run php artisan migrate:fresh then run php artisan migrate:fresh --database testing.

The second migration will fail, reporting that the oauth_auth_codes_table already exists, as it is accessing the mysql connection instead of the testing connection.

I believe this behavior was introduced in 555e02c on May9th.

[driesvints]

I'm investigating this one with the team. Thanks for reporting 👍

[driesvints]

We're gonna look at how we're going let the --database flag take over precedence here.

[tecbeast42]

I can reproduce this problem in laravel 8.

[driesvints]

Yeah still haven't gotten to this I'm afraid. Have been knee deep into PHP 8 updates. Appreciating help with it.

[CaptainBart]

Also running into this issue. Is there a workaround (except running the tests on the default database) available to avoid this issue?

[driesvints]

Not atm.

[magoun]

One workaround is to manually change the database config item to your testing database before running tests. If you have a database connection set up in Laravel called 'testing', edit the below section in config/passport.php:

After running tests, be sure to uncomment the production database setting and comment out the 'testing' line.

/*
    |--------------------------------------------------------------------------
    | Passport Storage Driver
    |--------------------------------------------------------------------------
    |
    | This configuration value allows you to customize the storage options
    | for Passport, such as the database connection that should be used
    | by Passport's internal database models which store tokens, etc.
    |
    */

    'storage' => [
        'database' => [
            'connection' => 'testing',
            // 'connection' => env('DB_CONNECTION', 'mysql'),
        ],
    ],

[CaptainBart]

I do something similar, but I prefer to change my .env file, since it won't be comitted to git. Locally I change the .env DB_DATABASE setting to the test database when I want to create/run unit tests and change it back when I'm done testing. For the build pipeline I've created a dedicated test .env file that is copied/renamed before the tests are run.

[micc83]

You can fix the issue publishing the config/passport.php file and setting storage.database.connection to null so that Schema::connection($name = null) will return the default db set by the --database option.

[derekmd]

@driesvints: A sensible fix might be to change the out-of-the-box connection value to null to make apps fallback to config('database.default') unless configured otherwise?

passport/config/passport.php

Line 62 in 7ab3bdb

'connection' => env('DB_CONNECTION', 'mysql'),

The above copy and pasted env('DB_CONNECTION', 'mysql') string from file config/database.php just causes the MigrateCommand option --database to be ignored.

An update to https://laravel.com/docs/passport can be done for the second case below since a userland config-based fix exists. #1255 essentially moved Passport database configuration into the migration files which overrides the CLI --database argument when a config value exist. But now apps can control the Passport models' destination database for the two use cases:

1. Passport models use the application's default database connection

   When the passport models share the same database as the rest of the app, a null value in config('passport.storage.database.connection') allows the command php artisan migrate --database=... to behave as before.

   config/passport.php

   return [
       // ...
       'storage' => [
           'database' => [
               'connection' => null, // this defaults to config('database.default')
           ],
       ],
   ];

2. Passport models are stored in a database different from the application's default connection

   Instead of calling php artisan migrate --database=..., configure a new environment variable and add it to each of the app's .env.* files. So in test environments they can share the same database, but staging/production, etc. can have split databases.

   config/passport.php

   return [
       // ...
       'storage' => [
           'database' => [
               'connection' => env('PASSPORT_DB_CONNECTION'),
           ],
       ],
   ];

   .env

   PASSPORT_DB_CONNECTION=prod_oauth
   

   .env.dusk

   PASSPORT_DB_CONNECTION=dusk_testing
   

   .env.testing

   PASSPORT_DB_CONNECTION=testing
   

[driesvints]

Thanks @derekmd for those insights.

I've decided to revert #1255 to properly fix this. Please see my reasoning on the PR: #1412

[driesvints]

We've reverted those changes for the next major release of Passport. At the moment we don't have a date yet for when that'll be released.

[Tylerian]

The revert doesn't help for cases where the passport config is exported. Replacing the passport database connection defaults to null on the config file fixes it tho.

[hasnain-nisan]

You can fix the issue publishing the config/passport.php file and setting storage.database.connection to null so that Schema::connection($name = null) will return the default db set by the --database option.

Helped me a lot