Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems testing Passport with actingAsClient #2556

Closed
wants to merge 2 commits into from

Conversation

SuperDJ
Copy link
Contributor

@SuperDJ SuperDJ commented Nov 15, 2023

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().

@drbyte
Copy link
Collaborator

drbyte commented Nov 15, 2023

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?

@SuperDJ
Copy link
Contributor Author

SuperDJ commented Nov 16, 2023

@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?

@parallels999
Copy link
Contributor

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

if (! \method_exists($authGuard, 'client')) {
return null;
}

$client = $authGuard->client();

public static function getPassportClient($guard): ?Authorizable
{
$guards = collect(config('auth.guards'))->where('driver', 'passport');
if (! $guards->count()) {
return null;
}
$authGuard = Auth::guard($guards->keys()[0]);
if (! \method_exists($authGuard, 'client')) {
return null;
}
$client = $authGuard->client();
if (! $guard || ! $client) {
return $client;
}
if (self::getNames($client)->contains($guard)) {
return $client;
}
return null;
}

@parallels999
Copy link
Contributor

parallels999 commented Nov 16, 2023

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

if ($client) {
$request->headers->set('Authorization', 'Bearer '.str()->random(30));
}

Also passport does the same on their tests, look their code
laravel/passport/CheckClientCredentialsTest.php#L44-L49
laravel/passport/search?q=Bearer%20token

@parallels999
Copy link
Contributor

parallels999 commented Nov 16, 2023

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

@SuperDJ
Copy link
Contributor Author

SuperDJ commented Nov 17, 2023

@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.

@parallels999
Copy link
Contributor

parallels999 commented Nov 17, 2023

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

@drbyte drbyte changed the title Hopefully fix issue for testing locally Problems testing Passport with actingAsClient Nov 19, 2023
@spatie-bot
Copy link

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 spatie-bot closed this Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants