laravel-permission icon indicating copy to clipboard operation
laravel-permission copied to clipboard

Problems testing Passport with actingAsClient

Open SuperDJ opened this issue 2 years ago • 7 comments

While testing locally I noticed that I got an 403 error even though I was using Passport::actingAsClient(). That error occurs because there is no $user and no bearerToken() provided. But I did notice that auth()?->client() returned the Passport::actingAsClient() Client instance.

So I'm hoping that with this change testing works as expected. It should now check for either the bearerToken() or the loggedin Client provided by Passport::actingAsClient().

SuperDJ avatar Nov 15 '23 15:11 SuperDJ

While my experience with Passport is limited, the changes you are proposing here so far make sense. (Edit: except that, as parallels999 has said, the proposed checks are already in the code, elsewhere)

Are you satisfied that these changes have resolved the testing problem you were encountering?

drbyte avatar Nov 15 '23 22:11 drbyte

@drbyte I reset the file to the current situation and run the tests, they failed. Than changed it with this change and they passed.

I did wonder if for the sake of readability the if statement should become a method?

SuperDJ avatar Nov 16 '23 08:11 SuperDJ

getPassportClient already checks client(), Maybe you are doing something wrong in your tests, it would be great if you published an example, maybe your problem can be fixed without changing the library https://github.com/spatie/laravel-permission/blob/c66c0de99a95a88288507fa96676b89625f54488/src/Guard.php#L91-L93 https://github.com/spatie/laravel-permission/blob/c66c0de99a95a88288507fa96676b89625f54488/src/Guard.php#L95 https://github.com/spatie/laravel-permission/blob/c66c0de99a95a88288507fa96676b89625f54488/src/Guard.php#L81-L106

parallels999 avatar Nov 16 '23 13:11 parallels999

It seems that this was expected, they added a dump bearer for testing There is always supposed to be a passport bearer header, is that correct? laravel/passport#passing-the-access-token https://github.com/spatie/laravel-permission/blob/c66c0de99a95a88288507fa96676b89625f54488/tests/TestCase.php#L275-L277 Also passport does the same on their tests, look their code laravel/passport/CheckClientCredentialsTest.php#L44-L49 laravel/passport/search?q=Bearer%20token

parallels999 avatar Nov 16 '23 13:11 parallels999

auth()?->client() returned the Passport::actingAsClient() Client instance.

not always, depends on default guard, maybe you forget the guard

Passport::actingAsClient($YOUR_CLIENT, [], 'guard_name_here');

Also add guard on middleware Source: laravel/passport/Passport.php#L385

parallels999 avatar Nov 16 '23 14:11 parallels999

@parallels999 the way I handle Passport::actingAsClient is like the following:

// Some test
public function test()
{
   $this->actingAsClient();
}

// Passport actingAsClient wrapper method
protected function actingAsClient( client = null, $permissions = null )
    {
        if( !$client )
        {
            $client = Client::factory()->create();
        }
        Passport::actingAsClient(
            $client,
            [ '*' ]
        );

        if( $permissions )
        {
            $this->assignRole(<some params>);
        }

        return $this;
    }

From what I understand is that Passport::actingAsClient simulates a client being loggedin. So to me that is assuming that a bearer token is being passed. Unfortunately that appears not to be the case.

SuperDJ avatar Nov 17 '23 07:11 SuperDJ

That example doesn't help much. I can't see how you call the middlewares, I can't guess what code you used

You could try to make a PR to Passport so that it adds the dummy bearer token automatically

parallels999 avatar Nov 17 '23 13:11 parallels999

Dear contributor,

because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it.

spatie-bot avatar Apr 08 '24 10:04 spatie-bot