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

[13.x] Fix issue with ClientRepository::handlesGrant method #1747

Closed
wants to merge 1 commit into from
Closed

[13.x] Fix issue with ClientRepository::handlesGrant method #1747

wants to merge 1 commit into from

Conversation

AsmaShayea
Copy link

@AsmaShayea AsmaShayea commented May 27, 2024

This pull request resolves an issue where calling createToken('admin_token')->accessToken resulted in a "Call to undefined method" error due to the hasGrantType() method being incorrectly referenced in the ClientRepository. The proposed fix updates the handlesGrant method in the ClientRepository to address this error by replacing the erroneous method call with the correct logic to handle grant types.

Changes:

  • Updated the handlesGrant method in the ClientRepository to resolve the "Call to undefined method" error.
  • Replaced the incorrect hasGrantType() method call with logic to check grant types using is_array() and in_array().

Replaced

if (! $record->hasGrantType($grantType)) {
return false;
}

with

if (is_array($record->grant_types) && ! in_array($grantType, $record->grant_types)) {
return false;
}

Testing:

  • Tested and verified that calling createToken('admin_token')->accessToken produces the expected result without errors.

…ned method App\Models\Passport\Client::hasGrantType()
@AsmaShayea AsmaShayea changed the base branch from 12.x to 13.x May 27, 2024 19:35
@AsmaShayea AsmaShayea changed the title Fix issue with ClientRepository::handlesGrant method [13.x] Fix issue with ClientRepository::handlesGrant method May 27, 2024
@driesvints
Copy link
Member

Thank you but this doesn't really explains the issue. Could you offer a concrete example? Also if this is a bug fix it can go to 12.x

@hafezdivandari
Copy link
Contributor

@AsmaShayea Are you overriding the default Client model using Passport::useClientModel()? If yes, your custom Client model class should extend the Passport Client model according to docs:

use Laravel\Passport\Client as PassportClient;
 
class Client extends PassportClient
{
    // ...
}

@taylorotwell
Copy link
Member

I am closing this pull request because it lacks sufficient explanation, tests, or both. It is difficult for us to merge pull requests without these things because the change may introduce breaking changes to the framework.

Feel free to re-submit your change with a thorough explanation of the feature and tests - integration tests are preferred over unit tests. Please include it's benefit to end users; the reasons it does not break any existing features; how it makes building web applications easier, etc.

Thanks!

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.

4 participants