Skip to content

Conversation

@maximepvrt
Copy link
Contributor

@maximepvrt maximepvrt commented Oct 24, 2025

Currently, Laravel Passport uses the findForPassport(string $username) method to retrieve the user during password grant authentication. This method only receives the username, which limits cases where we might want to filter users based on the OAuth client.

This PR introduces the possibility to define findForPassport with a second optional parameter, a ClientEntityInterface $client, allowing:

  • Filtering users based on the OAuth client if needed.
  • Maintaining compatibility with existing methods that only accept one parameter.

Example usage:

use Laravel\Passport\Bridge\Client;

public function findForPassport(string $username, Client $client)
{
    return $this->where('email', $username)
        ->whereHas('clients', function ($q) use ($client) {
            $q->where('id', $client->getIdentifier());
        })->first();
}

This is not a breaking change: existing methods with a single parameter will continue to work.

📖 I also prepared a PR for the documentation to explain this new possibility: laravel/docs#10880

Refactor user retrieval logic to use ReflectionMethod for dynamic argument handling.
@maximepvrt maximepvrt changed the title Patch 2 feat: Allow findForPassport to optionally receive the OAuth client Oct 24, 2025
@maximepvrt maximepvrt marked this pull request as draft October 24, 2025 17:16
@maximepvrt maximepvrt marked this pull request as ready for review October 24, 2025 17:29
Copy link
Contributor

@hafezdivandari hafezdivandari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to use reflection, just pass the $clientEntity as argument. If you want to make this change, please apply the same to line 38:

- $user = (new $model)->findAndValidateForPassport($username, $password);
+ $user = (new $model)->findAndValidateForPassport($username, $password, $clientEntity);

@maximepvrt
Copy link
Contributor Author

@hafezdivandari updated ;-)

@taylorotwell taylorotwell merged commit aee8af1 into laravel:13.x Oct 26, 2025
8 checks passed
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.

3 participants