passport:client ignores --provider for personal access clients

[amcsi]

• Passport Version: 10.1.2
• Laravel Version: 8.17.1
• PHP Version: 7.4.15
• Database Driver & Version: PostgreSQL(libpq) Version => 12.6 (Ubuntu 12.6-0ubuntu0.20.04.1)

Description:

When running php artisan passport:client --personal --provider=whatever, the provider does not get set in the created client.
With the password type client it does work: php artisan passport:client --password --provider=whatever.

I've already checked, and if I create a personal client, then manually set the provider to whatever in the DB, then that works as expected, and Laravel will only allow for that provider to be used with the tokens. If I leave the field as NULL, I can still authenticate in my routes using that provider, however I can also authenticate with that token for routes guarded by other guards using a different provider.

This issue has been mentioned in another ticket, but was likely ignored, because that wasn't what that ticket was about. ( #982 (comment) )

Steps To Reproduce:

On any passport project, run php artisan passport:client --personal --provider=whatever.

Expected:
The created client should have "whatever" in the provider column.

Actual:
The client is created, but its provider is NULL.

[driesvints]

There's no effect for declaring this on the client itself as the personal access client type doesn't direct interacts with the user provider. The token model will already use the correct user provider defined by the API guard.

[amcsi]

@driesvints That's not true. Please reconsider for the sake of security.

When a passport guard (for a provider) checks the token of a requests, the client of the token is looked up to check to see if the client's provider matches the auth guard's provider if the client has a provider set.

So here's a concerning scenario:

config/auth.php

    'guards' => [
        'users' => [
            'driver' => 'passport',
            'provider' => 'users',
        ],

        'adminUsers' => [
            'driver' => 'passport',
            'provider' => 'adminUsers',
        ],
    ]

routes/api.php

Route::middleware(['auth:users'])->group(function () {
    Route::get('somethingNormal', ...);
});

Route::middleware(['auth:adminUsers'])->group(function () {
    Route::delete('users', ...);
});

The users and adminUsers are from the users and adminUsers DB tables respectfully, both using auto-incrementing IDs.

If there's a Personal Access Client, and a regular has a token created for them with that client, they will have a token that - if the Personal client does not have the provider set - will let that regular user access the adminUsers endpoints (to delete users) even though they shouldn't have access. Assuming e.g. the regular user has an ID of 1, and there exists an adminUser also with an ID of 1.

If you manually edit the provider field in the DB and just put users as the provider for that Personal client, then that token won't have access to the adminUsers guarded endpoints, only the users ones.

That's why the --provider=something should be respected when calling passport:client --personal --provider=something.

[driesvints]

I'll check into this again when I find some time

[driesvints]

I now see what you actually mean. At the moment multiple guards simply isn't supported for any other grants than the Password Grant Tokens. Taylor made it clear here that we won't be implementing support for other grants atm: #1353

Sorry about that.

[amcsi]

Thank you for looking into it.

For the record (looking at the ticket you linked) I forgot about the fact that HasApiTokens is also agnostic about providers. In my local project when I was testing personal clients respecting provider, I only actually had one client, so had I two, I wouldn't have been able to specify which provider to use anyway. Though with a little bit of plumbing and invoking League OAuth directly, it's possible to get it to work. But of course this means it's really not that simple to implement proper support for this as I first thought.

[driesvints]

Yeah while looking at it in the core and at the PR it's definitely a lot more complicated than it seems.