From 3563bc2a3a6204134ed25bde8ee3c81846e36792 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 22:51:58 +0330 Subject: [PATCH 01/26] make client RFC compatible --- config/passport.php | 13 --- database/factories/ClientFactory.php | 34 ++---- ...1_000001_create_oauth_auth_codes_table.php | 18 ++- ...00002_create_oauth_access_tokens_table.php | 18 ++- ...0003_create_oauth_refresh_tokens_table.php | 14 ++- ...6_01_000004_create_oauth_clients_table.php | 22 +++- ...te_oauth_personal_access_clients_table.php | 28 ----- src/Bridge/Client.php | 4 +- src/Bridge/ClientRepository.php | 29 ++--- src/Bridge/PersonalAccessGrant.php | 2 +- src/Client.php | 59 ++-------- src/ClientRepository.php | 105 ++++++++++-------- src/Console/ClientCommand.php | 69 ++++++++---- src/Console/HashCommand.php | 9 +- src/Console/InstallCommand.php | 48 +------- src/Http/Controllers/ClientController.php | 43 +++---- .../PersonalAccessTokenController.php | 4 +- src/Http/Rules/RedirectRule.php | 51 --------- src/Http/Rules/UriRule.php | 21 +--- src/Passport.php | 85 -------------- src/PassportServiceProvider.php | 2 - src/PersonalAccessClient.php | 42 ------- src/PersonalAccessTokenFactory.php | 4 +- tests/Feature/AccessTokenControllerTest.php | 12 +- tests/Feature/ClientControllerTest.php | 73 ++++++++++++ tests/Feature/ClientTest.php | 28 +---- tests/Feature/Console/HashCommand.php | 4 - .../AuthorizedAccessTokenControllerTest.php | 4 +- ...ridgeClientRepositoryHashedSecretsTest.php | 29 ----- tests/Unit/BridgeClientRepositoryTest.php | 53 +++------ tests/Unit/BridgeScopeRepositoryTest.php | 33 ++---- tests/Unit/ClientControllerTest.php | 64 +++++------ tests/Unit/PassportTest.php | 11 -- .../PersonalAccessTokenControllerTest.php | 7 +- tests/Unit/PersonalAccessTokenFactoryTest.php | 5 +- tests/Unit/RedirectRuleTest.php | 49 -------- 36 files changed, 361 insertions(+), 735 deletions(-) delete mode 100644 database/migrations/2016_06_01_000005_create_oauth_personal_access_clients_table.php delete mode 100644 src/Http/Rules/RedirectRule.php delete mode 100644 src/PersonalAccessClient.php create mode 100644 tests/Feature/ClientControllerTest.php delete mode 100644 tests/Unit/BridgeClientRepositoryHashedSecretsTest.php delete mode 100644 tests/Unit/RedirectRuleTest.php diff --git a/config/passport.php b/config/passport.php index ae902d80c..dbdbfed1f 100644 --- a/config/passport.php +++ b/config/passport.php @@ -43,19 +43,6 @@ 'connection' => env('PASSPORT_CONNECTION'), - /* - |-------------------------------------------------------------------------- - | Client UUIDs - |-------------------------------------------------------------------------- - | - | By default, Passport uses auto-incrementing primary keys when assigning - | IDs to clients. However, if Passport is installed using the provided - | --uuids switch, this will be set to "true" and UUIDs will be used. - | - */ - - 'client_uuids' => false, - /* |-------------------------------------------------------------------------- | Personal Access Client diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index 5089b748f..ce049975f 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -28,32 +28,16 @@ public function modelName() */ public function definition() { - return $this->ensurePrimaryKeyIsSet([ + return [ 'user_id' => null, 'name' => $this->faker->company(), + 'grant_types' => [], + 'scopes' => [], + 'redirect_uris' => [$this->faker->url()], + 'provider' => null, 'secret' => Str::random(40), - 'redirect' => $this->faker->url(), - 'personal_access_client' => false, - 'password_client' => false, 'revoked' => false, - ]); - } - - /** - * Ensure the primary key is set on the model when using UUIDs. - * - * @param array $data - * @return array - */ - protected function ensurePrimaryKeyIsSet(array $data) - { - if (Passport::clientUuids()) { - $keyName = (new ($this->modelName()))->getKeyName(); - - $data[$keyName] = (string) Str::orderedUuid(); - } - - return $data; + ]; } /** @@ -64,8 +48,7 @@ protected function ensurePrimaryKeyIsSet(array $data) public function asPasswordClient() { return $this->state([ - 'personal_access_client' => false, - 'password_client' => true, + 'grant_types' => ['password', 'refresh_token'], ]); } @@ -77,8 +60,7 @@ public function asPasswordClient() public function asClientCredentials() { return $this->state([ - 'personal_access_client' => false, - 'password_client' => false, + 'grant_types' => ['client_credentials'], ]); } } diff --git a/database/migrations/2016_06_01_000001_create_oauth_auth_codes_table.php b/database/migrations/2016_06_01_000001_create_oauth_auth_codes_table.php index 7b93b406a..445a2ff5d 100644 --- a/database/migrations/2016_06_01_000001_create_oauth_auth_codes_table.php +++ b/database/migrations/2016_06_01_000001_create_oauth_auth_codes_table.php @@ -12,10 +12,10 @@ public function up(): void { Schema::create('oauth_auth_codes', function (Blueprint $table) { - $table->string('id', 100)->primary(); - $table->unsignedBigInteger('user_id')->index(); - $table->unsignedBigInteger('client_id'); - $table->text('scopes')->nullable(); + $table->char('id', 80)->primary(); + $table->foreignId('user_id')->index(); + $table->foreignUuid('client_id')->index(); + $table->text('scopes'); $table->boolean('revoked'); $table->dateTime('expires_at')->nullable(); }); @@ -28,4 +28,14 @@ public function down(): void { Schema::dropIfExists('oauth_auth_codes'); } + + /** + * Get the migration connection name. + * + * @return string|null + */ + public function getConnection() + { + return $this->connection ?? config('passport.connection'); + } }; diff --git a/database/migrations/2016_06_01_000002_create_oauth_access_tokens_table.php b/database/migrations/2016_06_01_000002_create_oauth_access_tokens_table.php index 598798eef..52cb80ef3 100644 --- a/database/migrations/2016_06_01_000002_create_oauth_access_tokens_table.php +++ b/database/migrations/2016_06_01_000002_create_oauth_access_tokens_table.php @@ -12,11 +12,11 @@ public function up(): void { Schema::create('oauth_access_tokens', function (Blueprint $table) { - $table->string('id', 100)->primary(); - $table->unsignedBigInteger('user_id')->nullable()->index(); - $table->unsignedBigInteger('client_id'); + $table->char('id', 80)->primary(); + $table->foreignId('user_id')->nullable()->index(); + $table->foreignUuid('client_id')->index(); $table->string('name')->nullable(); - $table->text('scopes')->nullable(); + $table->text('scopes'); $table->boolean('revoked'); $table->timestamps(); $table->dateTime('expires_at')->nullable(); @@ -30,4 +30,14 @@ public function down(): void { Schema::dropIfExists('oauth_access_tokens'); } + + /** + * Get the migration connection name. + * + * @return string|null + */ + public function getConnection() + { + return $this->connection ?? config('passport.connection'); + } }; diff --git a/database/migrations/2016_06_01_000003_create_oauth_refresh_tokens_table.php b/database/migrations/2016_06_01_000003_create_oauth_refresh_tokens_table.php index b007904ce..c94c91abd 100644 --- a/database/migrations/2016_06_01_000003_create_oauth_refresh_tokens_table.php +++ b/database/migrations/2016_06_01_000003_create_oauth_refresh_tokens_table.php @@ -12,8 +12,8 @@ public function up(): void { Schema::create('oauth_refresh_tokens', function (Blueprint $table) { - $table->string('id', 100)->primary(); - $table->string('access_token_id', 100)->index(); + $table->char('id', 80)->primary(); + $table->char('access_token_id', 80)->index(); $table->boolean('revoked'); $table->dateTime('expires_at')->nullable(); }); @@ -26,4 +26,14 @@ public function down(): void { Schema::dropIfExists('oauth_refresh_tokens'); } + + /** + * Get the migration connection name. + * + * @return string|null + */ + public function getConnection() + { + return $this->connection ?? config('passport.connection'); + } }; diff --git a/database/migrations/2016_06_01_000004_create_oauth_clients_table.php b/database/migrations/2016_06_01_000004_create_oauth_clients_table.php index 776ccfab2..011b17d64 100644 --- a/database/migrations/2016_06_01_000004_create_oauth_clients_table.php +++ b/database/migrations/2016_06_01_000004_create_oauth_clients_table.php @@ -12,14 +12,14 @@ public function up(): void { Schema::create('oauth_clients', function (Blueprint $table) { - $table->bigIncrements('id'); - $table->unsignedBigInteger('user_id')->nullable()->index(); + $table->uuid('id')->primary(); + $table->foreignId('user_id')->nullable()->index(); $table->string('name'); - $table->string('secret', 100)->nullable(); + $table->text('grant_types'); + $table->text('scopes'); + $table->text('redirect_uris'); $table->string('provider')->nullable(); - $table->text('redirect'); - $table->boolean('personal_access_client'); - $table->boolean('password_client'); + $table->string('secret')->nullable(); $table->boolean('revoked'); $table->timestamps(); }); @@ -32,4 +32,14 @@ public function down(): void { Schema::dropIfExists('oauth_clients'); } + + /** + * Get the migration connection name. + * + * @return string|null + */ + public function getConnection() + { + return $this->connection ?? config('passport.connection'); + } }; diff --git a/database/migrations/2016_06_01_000005_create_oauth_personal_access_clients_table.php b/database/migrations/2016_06_01_000005_create_oauth_personal_access_clients_table.php deleted file mode 100644 index 7c9d1e8f1..000000000 --- a/database/migrations/2016_06_01_000005_create_oauth_personal_access_clients_table.php +++ /dev/null @@ -1,28 +0,0 @@ -bigIncrements('id'); - $table->unsignedBigInteger('client_id'); - $table->timestamps(); - }); - } - - /** - * Reverse the migrations. - */ - public function down(): void - { - Schema::dropIfExists('oauth_personal_access_clients'); - } -}; diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 50b129c90..51ac50d2b 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -21,7 +21,7 @@ class Client implements ClientEntityInterface public function __construct( string $identifier, string $name, - string $redirectUri, + string|array $redirectUri, bool $isConfidential = false, ?string $provider = null ) { @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = explode(',', $redirectUri); + $this->redirectUri = is_array($redirectUri) ? $redirectUri : [$redirectUri]; $this->provider = $provider; } } diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index 1c21ce623..3aceef2ef 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -2,9 +2,9 @@ namespace Laravel\Passport\Bridge; +use Illuminate\Contracts\Hashing\Hasher; use Laravel\Passport\Client as ClientModel; use Laravel\Passport\ClientRepository as ClientModelRepository; -use Laravel\Passport\Passport; use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; @@ -15,12 +15,18 @@ class ClientRepository implements ClientRepositoryInterface */ protected ClientModelRepository $clients; + /** + * The hasher implementation. + */ + protected Hasher $hasher; + /** * Create a new repository instance. */ - public function __construct(ClientModelRepository $clients) + public function __construct(ClientModelRepository $clients, Hasher $hasher) { $this->clients = $clients; + $this->hasher = $hasher; } /** @@ -37,7 +43,7 @@ public function getClientEntity(string $clientIdentifier): ?ClientEntityInterfac return new Client( $clientIdentifier, $record->name, - $record->redirect, + $record->redirect_uris, $record->confidential(), $record->provider ); @@ -65,17 +71,8 @@ public function validateClient(string $clientIdentifier, ?string $clientSecret, */ protected function handlesGrant(ClientModel $record, string $grantType): bool { - if (! $record->hasGrantType($grantType)) { - return false; - } - - return match ($grantType) { - 'authorization_code' => ! $record->firstParty(), - 'personal_access' => $record->personal_access_client && $record->confidential(), - 'password' => $record->password_client, - 'client_credentials' => $record->confidential(), - default => true, - }; + return $record->hasGrantType($grantType) && + (! in_array($grantType, ['personal_access', 'client_credentials']) || $record->confidential()); } /** @@ -83,8 +80,6 @@ protected function handlesGrant(ClientModel $record, string $grantType): bool */ protected function verifySecret(string $clientSecret, string $storedHash): bool { - return Passport::$hashesClientSecrets - ? password_verify($clientSecret, $storedHash) - : hash_equals($storedHash, $clientSecret); + return $this->hasher->check($clientSecret, $storedHash); } } diff --git a/src/Bridge/PersonalAccessGrant.php b/src/Bridge/PersonalAccessGrant.php index fd2d4aecb..58d6da24c 100644 --- a/src/Bridge/PersonalAccessGrant.php +++ b/src/Bridge/PersonalAccessGrant.php @@ -19,7 +19,7 @@ public function respondToAccessTokenRequest( ): ResponseTypeInterface { // Validate request $client = $this->validateClient($request); - $scopes = $this->validateScopes($this->getRequestParameter('scope', $request)); + $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope)); $userIdentifier = $this->getRequestParameter('user_id', $request); // Finalize the requested scopes diff --git a/src/Client.php b/src/Client.php index 16e9dfff4..4862e8d1d 100644 --- a/src/Client.php +++ b/src/Client.php @@ -2,14 +2,16 @@ namespace Laravel\Passport; +use Illuminate\Database\Eloquent\Concerns\HasUuids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Str; +use Illuminate\Support\Facades\Hash; use Laravel\Passport\Database\Factories\ClientFactory; class Client extends Model { use HasFactory; + use HasUuids; use ResolvesInheritedScopes; /** @@ -43,8 +45,7 @@ class Client extends Model protected $casts = [ 'grant_types' => 'array', 'scopes' => 'array', - 'personal_access_client' => 'bool', - 'password_client' => 'bool', + 'redirect_uris' => 'array', 'revoked' => 'bool', ]; @@ -57,22 +58,6 @@ class Client extends Model */ public $plainSecret; - /** - * Bootstrap the model and its traits. - * - * @return void - */ - public static function boot() - { - parent::boot(); - - static::creating(function ($model) { - if (Passport::clientUuids()) { - $model->{$model->getKeyName()} = $model->{$model->getKeyName()} ?: (string) Str::orderedUuid(); - } - }); - } - /** * Get the user that the client belongs to. * @@ -129,13 +114,7 @@ public function setSecretAttribute($value) { $this->plainSecret = $value; - if (is_null($value) || ! Passport::$hashesClientSecrets) { - $this->attributes['secret'] = $value; - - return; - } - - $this->attributes['secret'] = password_hash($value, PASSWORD_BCRYPT); + $this->attributes['secret'] = is_null($value) ? $value : Hash::make($value); } /** @@ -145,7 +124,7 @@ public function setSecretAttribute($value) */ public function firstParty() { - return $this->personal_access_client || $this->password_client; + return $this->hasGrantType('personal_access') || $this->hasGrantType('password'); } /** @@ -166,10 +145,6 @@ public function skipsAuthorization() */ public function hasGrantType($grantType) { - if (! isset($this->attributes['grant_types']) || ! is_array($this->grant_types)) { - return true; - } - return in_array($grantType, $this->grant_types); } @@ -181,7 +156,7 @@ public function hasGrantType($grantType) */ public function hasScope($scope) { - if (! isset($this->attributes['scopes']) || ! is_array($this->scopes)) { + if (in_array('*', $this->scopes)) { return true; } @@ -208,26 +183,6 @@ public function confidential() return ! empty($this->secret); } - /** - * Get the auto-incrementing key type. - * - * @return string - */ - public function getKeyType() - { - return Passport::clientUuids() ? 'string' : $this->keyType; - } - - /** - * Get the value indicating whether the IDs are incrementing. - * - * @return bool - */ - public function getIncrementing() - { - return Passport::clientUuids() ? false : $this->incrementing; - } - /** * Get the current connection name for the model. * diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 2809b3ae1..f04fa0290 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -112,41 +112,38 @@ public function activeForUser($userId) */ public function personalAccessClient() { - if ($this->personalAccessClientId) { - return $this->find($this->personalAccessClientId); - } + $client = $this->personalAccessClientId ? $this->find($this->personalAccessClientId) : null; - $client = Passport::personalAccessClient(); - - if (! $client->exists()) { - throw new RuntimeException('Personal access client not found. Please create one.'); - } - - return $client->orderBy($client->getKeyName(), 'desc')->first()->client; + return $client ?? throw new RuntimeException( + 'Personal access client not found. Please create one and set `PASSPORT_PERSONAL_ACCESS_CLIENT_ID` and `PASSPORT_PERSONAL_ACCESS_CLIENT_SECRET` environment variables.' + ); } /** * Store a new client. * - * @param int|null $userId - * @param string $name - * @param string $redirect - * @param string|null $provider - * @param bool $personalAccess - * @param bool $password - * @param bool $confidential - * @return \Laravel\Passport\Client + * @param string[] $redirectUris + * @param string[] $grantTypes + * @param string[] $scopes */ - public function create($userId, $name, $redirect, $provider = null, $personalAccess = false, $password = false, $confidential = true) + protected function create( + string $name, + array $grantTypes, + array $redirectUris = [], + ?string $provider = null, + ?string $userId = null, + bool $confidential = true, + array $scopes = ['*'] + ): Client { $client = Passport::client()->forceFill([ 'user_id' => $userId, 'name' => $name, - 'secret' => ($confidential || $personalAccess) ? Str::random(40) : null, + 'grant_types' => $grantTypes, + 'scopes' => $scopes, + 'redirect_uris' => $redirectUris, 'provider' => $provider, - 'redirect' => $redirect, - 'personal_access_client' => $personalAccess, - 'password_client' => $password, + 'secret' => $confidential ? Str::random(40) : null, 'revoked' => false, ]); @@ -157,33 +154,53 @@ public function create($userId, $name, $redirect, $provider = null, $personalAcc /** * Store a new personal access token client. - * - * @param int|null $userId - * @param string $name - * @param string $redirect - * @return \Laravel\Passport\Client */ - public function createPersonalAccessClient($userId, $name, $redirect) + public function createPersonalAccessGrantClient(string $name): Client { - return tap($this->create($userId, $name, $redirect, null, true), function ($client) { - $accessClient = Passport::personalAccessClient(); - $accessClient->client_id = $client->getKey(); - $accessClient->save(); - }); + return $this->create($name, ['personal_access']); } /** * Store a new password grant client. + */ + public function createPasswordGrantClient(string $name, ?string $provider = null): Client + { + return $this->create($name, ['password', 'refresh_token'], [], $provider); + } + + /** + * Store a new client credentials grant client. + */ + public function createClientCredentialsGrantClient(string $name): Client + { + return $this->create($name, ['client_credentials']); + } + + /** + * Store a new implicit grant client. * - * @param int|null $userId - * @param string $name - * @param string $redirect - * @param string|null $provider - * @return \Laravel\Passport\Client + * @param string[] $redirectUris + */ + public function createImplicitGrantClient(string $name, array $redirectUris): Client + { + return $this->create($name, ['implicit'], $redirectUris); + } + + /** + * Store a new authorization code grant client. + * + * @param string[] $redirectUris */ - public function createPasswordGrantClient($userId, $name, $redirect, $provider = null) + public function createAuthorizationCodeGrantClient( + string $name, + array $redirectUris, + bool $confidential = true, + ?string $userId = null + ): Client { - return $this->create($userId, $name, $redirect, $provider, false, true); + return $this->create( + $name, ['authorization_code', 'refresh_token'], $redirectUris, null, $userId, $confidential + ); } /** @@ -191,13 +208,13 @@ public function createPasswordGrantClient($userId, $name, $redirect, $provider = * * @param \Laravel\Passport\Client $client * @param string $name - * @param string $redirect + * @param string[] $redirectUris * @return \Laravel\Passport\Client */ - public function update(Client $client, $name, $redirect) + public function update(Client $client, $name, $redirectUris) { $client->forceFill([ - 'name' => $name, 'redirect' => $redirect, + 'name' => $name, 'redirect_uris' => $redirectUris, ])->save(); return $client; diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index 58c52ed64..0e31e2cbf 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -5,7 +5,6 @@ use Illuminate\Console\Command; use Laravel\Passport\Client; use Laravel\Passport\ClientRepository; -use Laravel\Passport\Passport; use Symfony\Component\Console\Attribute\AsCommand; #[AsCommand(name: 'passport:client')] @@ -20,11 +19,12 @@ class ClientCommand extends Command {--personal : Create a personal access token client} {--password : Create a password grant client} {--client : Create a client credentials grant client} + {--implicit : Create a implicit grant client} + {--public : Create a public client (Auth code grant type only) } {--name= : The name of the client} {--provider= : The name of the user provider} {--redirect_uri= : The URI to redirect to after authorization } - {--user_id= : The user ID the client should be assigned to } - {--public : Create a public client (Auth code grant type only) }'; + {--user_id= : The user ID the client should be assigned to }'; /** * The console command description. @@ -42,11 +42,13 @@ class ClientCommand extends Command public function handle(ClientRepository $clients) { if ($this->option('personal')) { - $this->createPersonalClient($clients); + $this->createPersonalAccessClient($clients); } elseif ($this->option('password')) { $this->createPasswordClient($clients); } elseif ($this->option('client')) { $this->createClientCredentialsClient($clients); + } elseif ($this->option('implicit')) { + $this->createImplicitClient($clients); } else { $this->createAuthCodeClient($clients); } @@ -58,19 +60,21 @@ public function handle(ClientRepository $clients) * @param \Laravel\Passport\ClientRepository $clients * @return void */ - protected function createPersonalClient(ClientRepository $clients) + protected function createPersonalAccessClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( 'What should we name the personal access client?', config('app.name').' Personal Access Client' ); - $client = $clients->createPersonalAccessClient( - null, $name, 'http://localhost' - ); + $client = $clients->createPersonalAccessGrantClient($name); $this->components->info('Personal access client created successfully.'); + if (! config('passport.personal_access_client')) { + $this->components->info('You may set `PASSPORT_PERSONAL_ACCESS_CLIENT_ID` and `PASSPORT_PERSONAL_ACCESS_CLIENT_SECRET` environment variables.'); + } + $this->outputClientDetails($client); } @@ -92,12 +96,9 @@ protected function createPasswordClient(ClientRepository $clients) $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', $providers, - in_array('users', $providers) ? 'users' : null ); - $client = $clients->createPasswordGrantClient( - null, $name, 'http://localhost', $provider - ); + $client = $clients->createPasswordGrantClient($name, $provider); $this->components->info('Password grant client created successfully.'); @@ -114,13 +115,35 @@ protected function createClientCredentialsClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( 'What should we name the client?', - config('app.name').' ClientCredentials Grant Client' + config('app.name').' Client Credentials Grant Client' ); - $client = $clients->create( - null, $name, '' + $client = $clients->createClientCredentialsGrantClient($name); + + $this->components->info('New client created successfully.'); + + $this->outputClientDetails($client); + } + + /** + * Create an implicit grant client. + * + * @param \Laravel\Passport\ClientRepository $clients + * @return void + */ + protected function createImplicitClient(ClientRepository $clients) + { + $name = $this->option('name') ?: $this->ask( + 'What should we name the client?' + ); + + $redirect = $this->option('redirect_uri') ?: $this->ask( + 'Where should we redirect the request after authorization?', + url('/auth/callback') ); + $client = $clients->createImplicitGrantClient($name, explode(',', $redirect)); + $this->components->info('New client created successfully.'); $this->outputClientDetails($client); @@ -147,8 +170,11 @@ protected function createAuthCodeClient(ClientRepository $clients) url('/auth/callback') ); - $client = $clients->create( - $userId, $name, $redirect, null, false, false, ! $this->option('public') + $client = $clients->createAuthorizationCodeGrantClient( + $name, + explode(',', $redirect), + ! $this->option('public'), + $userId ); $this->components->info('New client created successfully.'); @@ -164,12 +190,9 @@ protected function createAuthCodeClient(ClientRepository $clients) */ protected function outputClientDetails(Client $client) { - if (Passport::$hashesClientSecrets) { - $this->line('Here is your new client secret. This is the only time it will be shown so don\'t lose it!'); - $this->line(''); - } + $this->components->warn('Here is your new client secret. This is the only time it will be shown so don\'t lose it!'); - $this->components->twoColumnDetail('Client ID', $client->getKey()); - $this->components->twoColumnDetail('Client secret', $client->plainSecret); + $this->components->twoColumnDetail('Client ID', $client->getKey()); + $this->components->twoColumnDetail('Client Secret', $client->plainSecret); } } diff --git a/src/Console/HashCommand.php b/src/Console/HashCommand.php index 4e55b1350..65cf62c32 100644 --- a/src/Console/HashCommand.php +++ b/src/Console/HashCommand.php @@ -3,6 +3,7 @@ namespace Laravel\Passport\Console; use Illuminate\Console\Command; +use Illuminate\Support\Facades\Hash; use Laravel\Passport\Passport; use Symfony\Component\Console\Attribute\AsCommand; @@ -30,17 +31,11 @@ class HashCommand extends Command */ public function handle() { - if (! Passport::$hashesClientSecrets) { - $this->components->warn('Please enable client hashing yet in your AppServiceProvider before continuing.'); - - return; - } - if ($this->option('force') || $this->confirm('Are you sure you want to hash all client secrets? This cannot be undone.')) { $model = Passport::clientModel(); foreach ((new $model)->whereNotNull('secret')->cursor() as $client) { - if (password_get_info($client->secret)['algo'] === PASSWORD_BCRYPT) { + if (Hash::isHashed($client->secret) && ! Hash::needsRehash($client->secret)) { continue; } diff --git a/src/Console/InstallCommand.php b/src/Console/InstallCommand.php index 2d84fc8df..7f91f18d5 100644 --- a/src/Console/InstallCommand.php +++ b/src/Console/InstallCommand.php @@ -3,7 +3,6 @@ namespace Laravel\Passport\Console; use Illuminate\Console\Command; -use Laravel\Passport\Passport; use Symfony\Component\Console\Attribute\AsCommand; #[AsCommand(name: 'passport:install')] @@ -15,7 +14,6 @@ class InstallCommand extends Command * @var string */ protected $signature = 'passport:install - {--uuids : Use UUIDs for all client IDs} {--force : Overwrite keys they already exist} {--length=4096 : The length of the private key}'; @@ -37,56 +35,12 @@ public function handle() $this->call('vendor:publish', ['--tag' => 'passport-migrations']); - if ($this->option('uuids')) { - $this->configureUuids(); - } - if ($this->confirm('Would you like to run all pending database migrations?', true)) { $this->call('migrate'); - if ($this->confirm('Would you like to create the "personal access" and "password grant" clients?', true)) { - $provider = in_array('users', array_keys(config('auth.providers'))) ? 'users' : null; - + if ($this->confirm('Would you like to create the "personal access token" grant client?', true)) { $this->call('passport:client', ['--personal' => true, '--name' => config('app.name').' Personal Access Client']); - $this->call('passport:client', ['--password' => true, '--name' => config('app.name').' Password Grant Client', '--provider' => $provider]); } } } - - /** - * Configure Passport for client UUIDs. - * - * @return void - */ - protected function configureUuids() - { - $this->call('vendor:publish', ['--tag' => 'passport-config']); - - config(['passport.client_uuids' => true]); - Passport::setClientUuids(true); - - $this->replaceInFile(config_path('passport.php'), '\'client_uuids\' => false', '\'client_uuids\' => true'); - $this->replaceInFile(database_path('migrations/****_**_**_******_create_oauth_auth_codes_table.php'), '$table->unsignedBigInteger(\'client_id\');', '$table->uuid(\'client_id\');'); - $this->replaceInFile(database_path('migrations/****_**_**_******_create_oauth_access_tokens_table.php'), '$table->unsignedBigInteger(\'client_id\');', '$table->uuid(\'client_id\');'); - $this->replaceInFile(database_path('migrations/****_**_**_******_create_oauth_clients_table.php'), '$table->bigIncrements(\'id\');', '$table->uuid(\'id\')->primary();'); - $this->replaceInFile(database_path('migrations/****_**_**_******_create_oauth_personal_access_clients_table.php'), '$table->unsignedBigInteger(\'client_id\');', '$table->uuid(\'client_id\');'); - } - - /** - * Replace a given string in a given file. - * - * @param string $path - * @param string $search - * @param string $replace - * @return void - */ - protected function replaceInFile($path, $search, $replace) - { - foreach (glob($path) as $file) { - file_put_contents( - $file, - str_replace($search, $replace, file_get_contents($file)) - ); - } - } } diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php index a36be44c1..ba5a50d0b 100644 --- a/src/Http/Controllers/ClientController.php +++ b/src/Http/Controllers/ClientController.php @@ -6,8 +6,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Laravel\Passport\ClientRepository; -use Laravel\Passport\Http\Rules\RedirectRule; -use Laravel\Passport\Passport; +use Laravel\Passport\Http\Rules\UriRule; class ClientController { @@ -25,29 +24,19 @@ class ClientController */ protected $validation; - /** - * The redirect validation rule. - * - * @var \Laravel\Passport\Http\Rules\RedirectRule - */ - protected $redirectRule; - /** * Create a client controller instance. * * @param \Laravel\Passport\ClientRepository $clients * @param \Illuminate\Contracts\Validation\Factory $validation - * @param \Laravel\Passport\Http\Rules\RedirectRule $redirectRule * @return void */ public function __construct( ClientRepository $clients, ValidationFactory $validation, - RedirectRule $redirectRule ) { $this->clients = $clients; $this->validation = $validation; - $this->redirectRule = $redirectRule; } /** @@ -62,11 +51,7 @@ public function forUser(Request $request) $clients = $this->clients->activeForUser($userId); - if (Passport::$hashesClientSecrets) { - return $clients; - } - - return $clients->makeVisible('secret'); + return $clients; } /** @@ -78,19 +63,20 @@ public function forUser(Request $request) public function store(Request $request) { $this->validation->make($request->all(), [ - 'name' => 'required|max:191', - 'redirect' => ['required', $this->redirectRule], + 'name' => 'required|max:255', + 'redirect_uris' => 'required|array|min:1', + 'redirect_uris.*' => ['required', new UriRule], 'confidential' => 'boolean', ])->validate(); - $client = $this->clients->create( - $request->user()->getAuthIdentifier(), $request->name, $request->redirect, - null, false, false, (bool) $request->input('confidential', true) + $client = $this->clients->createAuthorizationCodeGrantClient( + $request->name, + $request->redirect_uris, + (bool) $request->input('confidential', true), + $request->user()->getAuthIdentifier(), ); - if (Passport::$hashesClientSecrets) { - return ['plainSecret' => $client->plainSecret] + $client->toArray(); - } + $client->secret = $client->plainSecret; return $client->makeVisible('secret'); } @@ -111,12 +97,13 @@ public function update(Request $request, $clientId) } $this->validation->make($request->all(), [ - 'name' => 'required|max:191', - 'redirect' => ['required', $this->redirectRule], + 'name' => 'required|max:255', + 'redirect_uris' => 'required|array|min:1', + 'redirect_uris.*' => ['required', new UriRule], ])->validate(); return $this->clients->update( - $client, $request->name, $request->redirect + $client, $request->name, $request->redirect_uris ); } diff --git a/src/Http/Controllers/PersonalAccessTokenController.php b/src/Http/Controllers/PersonalAccessTokenController.php index 2fde42483..0fce3047c 100644 --- a/src/Http/Controllers/PersonalAccessTokenController.php +++ b/src/Http/Controllers/PersonalAccessTokenController.php @@ -48,7 +48,7 @@ public function forUser(Request $request) $tokens = $this->tokenRepository->forUser($request->user()->getAuthIdentifier()); return $tokens->load('client')->filter(function ($token) { - return $token->client->personal_access_client && ! $token->revoked; + return $token->client->hasGrantType('personal_access') && ! $token->revoked; })->values(); } @@ -61,7 +61,7 @@ public function forUser(Request $request) public function store(Request $request) { $this->validation->make($request->all(), [ - 'name' => 'required|max:191', + 'name' => 'required|max:255', 'scopes' => 'array|in:'.implode(',', Passport::scopeIds()), ])->validate(); diff --git a/src/Http/Rules/RedirectRule.php b/src/Http/Rules/RedirectRule.php deleted file mode 100644 index e14d11c3d..000000000 --- a/src/Http/Rules/RedirectRule.php +++ /dev/null @@ -1,51 +0,0 @@ -validator = $validator; - } - - /** - * {@inheritdoc} - */ - public function passes($attribute, $value) - { - foreach (explode(',', $value) as $redirect) { - $validator = $this->validator->make(['redirect' => $redirect], ['redirect' => new UriRule]); - - if ($validator->fails()) { - return false; - } - } - - return true; - } - - /** - * {@inheritdoc} - */ - public function message() - { - return 'One or more redirects have an invalid URI format.'; - } -} diff --git a/src/Http/Rules/UriRule.php b/src/Http/Rules/UriRule.php index f6d02c65e..8fea7e82a 100644 --- a/src/Http/Rules/UriRule.php +++ b/src/Http/Rules/UriRule.php @@ -2,27 +2,18 @@ namespace Laravel\Passport\Http\Rules; -use Illuminate\Contracts\Validation\Rule; +use Closure; +use Illuminate\Contracts\Validation\ValidationRule; -class UriRule implements Rule +class UriRule implements ValidationRule { /** * {@inheritdoc} */ - public function passes($attribute, $value): bool + public function validate(string $attribute, mixed $value, Closure $fail): void { - if (filter_var($value, FILTER_VALIDATE_URL)) { - return true; + if (! filter_var($value, FILTER_VALIDATE_URL)) { + $fail('The :attribute must be valid URI.'); } - - return false; - } - - /** - * {@inheritdoc} - */ - public function message(): string - { - return 'The :attribute must be valid URI.'; } } diff --git a/src/Passport.php b/src/Passport.php index 728cc20d9..d0e9a8738 100644 --- a/src/Passport.php +++ b/src/Passport.php @@ -114,20 +114,6 @@ class Passport */ public static $clientModel = 'Laravel\Passport\Client'; - /** - * Indicates if client's are identified by UUIDs. - * - * @var bool - */ - public static $clientUuids = false; - - /** - * The personal access client model class name. - * - * @var string - */ - public static $personalAccessClientModel = 'Laravel\Passport\PersonalAccessClient'; - /** * The token model class name. * @@ -156,13 +142,6 @@ class Passport */ public static $decryptsCookies = true; - /** - * Indicates if client secrets will be hashed. - * - * @var bool - */ - public static $hashesClientSecrets = false; - /** * The callback that should be used to generate JWT encryption keys. * @@ -537,58 +516,6 @@ public static function client() return new static::$clientModel; } - /** - * Determine if clients are identified using UUIDs. - * - * @return bool - */ - public static function clientUuids() - { - return static::$clientUuids; - } - - /** - * Specify if clients are identified using UUIDs. - * - * @param bool $value - * @return void - */ - public static function setClientUuids($value) - { - static::$clientUuids = $value; - } - - /** - * Set the personal access client model class name. - * - * @param string $clientModel - * @return void - */ - public static function usePersonalAccessClientModel($clientModel) - { - static::$personalAccessClientModel = $clientModel; - } - - /** - * Get the personal access client model class name. - * - * @return string - */ - public static function personalAccessClientModel() - { - return static::$personalAccessClientModel; - } - - /** - * Get a new personal access client model instance. - * - * @return \Laravel\Passport\PersonalAccessClient - */ - public static function personalAccessClient() - { - return new static::$personalAccessClientModel; - } - /** * Set the token model class name. * @@ -651,18 +578,6 @@ public static function refreshToken() return new static::$refreshTokenModel; } - /** - * Configure Passport to hash client credential secrets. - * - * @return static - */ - public static function hashClientSecrets() - { - static::$hashesClientSecrets = true; - - return new static; - } - /** * Specify the callback that should be invoked to generate encryption keys for encrypting JWT tokens. * diff --git a/src/PassportServiceProvider.php b/src/PassportServiceProvider.php index c3eb97a81..ce5925cfc 100644 --- a/src/PassportServiceProvider.php +++ b/src/PassportServiceProvider.php @@ -126,8 +126,6 @@ public function register() { $this->mergeConfigFrom(__DIR__.'/../config/passport.php', 'passport'); - Passport::setClientUuids($this->app->make(Config::class)->get('passport.client_uuids', false)); - $this->app->when(AuthorizationController::class) ->needs(StatefulGuard::class) ->give(fn () => Auth::guard(config('passport.guard', null))); diff --git a/src/PersonalAccessClient.php b/src/PersonalAccessClient.php deleted file mode 100644 index 2e009c2b3..000000000 --- a/src/PersonalAccessClient.php +++ /dev/null @@ -1,42 +0,0 @@ -belongsTo(Passport::clientModel()); - } - - /** - * Get the current connection name for the model. - * - * @return string|null - */ - public function getConnectionName() - { - return $this->connection ?? config('passport.connection'); - } -} diff --git a/src/PersonalAccessTokenFactory.php b/src/PersonalAccessTokenFactory.php index 15ac840ad..7983c09d0 100644 --- a/src/PersonalAccessTokenFactory.php +++ b/src/PersonalAccessTokenFactory.php @@ -94,12 +94,10 @@ public function make($userId, $name, array $scopes = []) */ protected function createRequest($client, $userId, array $scopes) { - $secret = Passport::$hashesClientSecrets ? $this->clients->getPersonalAccessClientSecret() : $client->secret; - return (new ServerRequest('POST', 'not-important'))->withParsedBody([ 'grant_type' => 'personal_access', 'client_id' => $client->getKey(), - 'client_secret' => $secret, + 'client_secret' => $this->clients->getPersonalAccessClientSecret(), 'user_id' => $userId, 'scope' => implode(' ', $scopes), ]); diff --git a/tests/Feature/AccessTokenControllerTest.php b/tests/Feature/AccessTokenControllerTest.php index 215558e9a..b39cf7cc2 100644 --- a/tests/Feature/AccessTokenControllerTest.php +++ b/tests/Feature/AccessTokenControllerTest.php @@ -33,7 +33,7 @@ public function testGettingAccessTokenWithClientCredentialsGrant() [ 'grant_type' => 'client_credentials', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret, + 'client_secret' => $client->plainSecret, ] ); @@ -76,7 +76,7 @@ public function testGettingAccessTokenWithClientCredentialsGrantInvalidClientSec [ 'grant_type' => 'client_credentials', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret.'foo', + 'client_secret' => $client->plainSecret.'foo', ] ); @@ -120,7 +120,7 @@ public function testGettingAccessTokenWithPasswordGrant() [ 'grant_type' => 'password', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret, + 'client_secret' => $client->plainSecret, 'username' => $user->email, 'password' => $password, ] @@ -169,7 +169,7 @@ public function testGettingAccessTokenWithPasswordGrantWithInvalidPassword() [ 'grant_type' => 'password', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret, + 'client_secret' => $client->plainSecret, 'username' => $user->email, 'password' => $password.'foo', ] @@ -213,7 +213,7 @@ public function testGettingAccessTokenWithPasswordGrantWithInvalidClientSecret() [ 'grant_type' => 'password', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret.'foo', + 'client_secret' => $client->plainSecret.'foo', 'username' => $user->email, 'password' => $password, ] @@ -258,7 +258,7 @@ public function testGettingCustomResponseType() [ 'grant_type' => 'client_credentials', 'client_id' => $client->getKey(), - 'client_secret' => $client->secret, + 'client_secret' => $client->plainSecret, ] ); diff --git a/tests/Feature/ClientControllerTest.php b/tests/Feature/ClientControllerTest.php new file mode 100644 index 000000000..52311e6ae --- /dev/null +++ b/tests/Feature/ClientControllerTest.php @@ -0,0 +1,73 @@ +create([ + 'email' => 'foo@gmail.com', + 'password' => $this->app->make(Hasher::class)->make('foobar123'), + ]); + + $this->actingAs($user); + + $response = $this->post( + '/oauth/clients', + [ + 'name' => 'new client', + 'redirect_uris' => ['https://localhost'], + ] + ); + + $response->assertSuccessful(); + + $decodedResponse = $response->json(); + + $this->assertEquals(1, $decodedResponse['user_id']); + $this->assertEquals('new client', $decodedResponse['name']); + $this->assertEquals(['https://localhost'], $decodedResponse['redirect_uris']); + $this->assertArrayHasKey('secret', $decodedResponse); + $this->assertArrayHasKey('provider', $decodedResponse); + $this->assertArrayHasKey('revoked', $decodedResponse); + } + + public function testUpdatingClient() + { + $user = UserFactory::new()->create([ + 'email' => 'foo@gmail.com', + 'password' => $this->app->make(Hasher::class)->make('foobar123'), + ]); + + $this->actingAs($user); + + $client = $this->post('/oauth/clients', [ + 'name' => 'new client', 'redirect_uris' => ['https://localhost'] + ])->json(); + + $response = $this->put( + '/oauth/clients/'.$client['id'], + [ + 'name' => 'updated client', + 'redirect_uris' => ['https://localhost'], + ] + ); + + $response->assertSuccessful(); + + $decodedResponse = $response->json(); + + $this->assertEquals(1, $decodedResponse['user_id']); + $this->assertEquals('updated client', $decodedResponse['name']); + $this->assertEquals(['https://localhost'], $decodedResponse['redirect_uris']); + $this->assertArrayHasKey('provider', $decodedResponse); + $this->assertArrayHasKey('revoked', $decodedResponse); + } +} diff --git a/tests/Feature/ClientTest.php b/tests/Feature/ClientTest.php index 38fb93e14..f96292ac1 100644 --- a/tests/Feature/ClientTest.php +++ b/tests/Feature/ClientTest.php @@ -38,17 +38,9 @@ public function testScopesWhenClientHasScope(): void $this->assertTrue($client->hasScope('foo')); } - public function testScopesWhenColumnDoesNotExist(): void + public function testScopesWhenClientHasAny(): void { - $client = new Client(); - $client->exists = true; - - $this->assertTrue($client->hasScope('foo')); - } - - public function testScopesWhenColumnIsNull(): void - { - $client = new Client(['scopes' => null]); + $client = new Client(['scopes' => ['*']]); $client->exists = true; $this->assertTrue($client->hasScope('foo')); @@ -69,20 +61,4 @@ public function testGrantTypesWhenClientHasGrantType(): void $this->assertTrue($client->hasGrantType('foo')); } - - public function testGrantTypesWhenColumnDoesNotExist(): void - { - $client = new Client(); - $client->exists = true; - - $this->assertTrue($client->hasGrantType('foo')); - } - - public function testGrantTypesWhenColumnIsNull(): void - { - $client = new Client(['scopes' => null]); - $client->exists = true; - - $this->assertTrue($client->hasGrantType('foo')); - } } diff --git a/tests/Feature/Console/HashCommand.php b/tests/Feature/Console/HashCommand.php index fae29cab1..89b18cd4a 100644 --- a/tests/Feature/Console/HashCommand.php +++ b/tests/Feature/Console/HashCommand.php @@ -15,12 +15,8 @@ public function test_it_can_properly_hash_client_secrets() $client = factory(Client::class)->create(['secret' => $secret = Str::random(40)]); $hasher = $this->app->make(Hasher::class); - Passport::hashClientSecrets(); - $this->artisan('passport:hash', ['--force' => true]); $this->assertTrue($hasher->check($secret, $client->refresh()->secret)); - - Passport::$hashesClientSecrets = false; } } diff --git a/tests/Unit/AuthorizedAccessTokenControllerTest.php b/tests/Unit/AuthorizedAccessTokenControllerTest.php index fdf04fa54..2846918bb 100644 --- a/tests/Unit/AuthorizedAccessTokenControllerTest.php +++ b/tests/Unit/AuthorizedAccessTokenControllerTest.php @@ -52,9 +52,9 @@ public function test_tokens_can_be_retrieved_for_users() $userTokens = m::mock(); $client1 = new Client; - $client1->personal_access_client = true; + $client1->grant_types = ['personal_access']; $client2 = new Client; - $client2->personal_access_client = false; + $client2->grant_types = []; $token1->client = $client1; $token2->client = $client2; $userTokens->shouldReceive('load')->with('client')->andReturn(collect([ diff --git a/tests/Unit/BridgeClientRepositoryHashedSecretsTest.php b/tests/Unit/BridgeClientRepositoryHashedSecretsTest.php deleted file mode 100644 index e94017977..000000000 --- a/tests/Unit/BridgeClientRepositoryHashedSecretsTest.php +++ /dev/null @@ -1,29 +0,0 @@ -shouldReceive('findActive') - ->with(1) - ->andReturn(new BridgeClientRepositoryHashedTestClientStub); - - $this->clientModelRepository = $clientModelRepository; - $this->repository = new BridgeClientRepository($clientModelRepository); - } -} - -class BridgeClientRepositoryHashedTestClientStub extends BridgeClientRepositoryTestClientStub -{ - public $secret = '$2y$10$WgqU4wQpfsARCIQk.nPSOOiNkrMpPVxQiLCFUt8comvQwh1z6WFMG'; -} diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index 4b8b678c5..fed5efbf5 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -2,10 +2,10 @@ namespace Laravel\Passport\Tests\Unit; +use Illuminate\Contracts\Hashing\Hasher; use Laravel\Passport\Bridge\Client; use Laravel\Passport\Bridge\ClientRepository as BridgeClientRepository; use Laravel\Passport\ClientRepository; -use Laravel\Passport\Passport; use Mockery as m; use PHPUnit\Framework\TestCase; @@ -23,15 +23,17 @@ class BridgeClientRepositoryTest extends TestCase protected function setUp(): void { - Passport::$hashesClientSecrets = false; - $clientModelRepository = m::mock(ClientRepository::class); $clientModelRepository->shouldReceive('findActive') ->with(1) - ->andReturn(new BridgeClientRepositoryTestClientStub); + ->andReturn($client = new BridgeClientRepositoryTestClientStub); + + $hasher = m::mock(Hasher::class); + $hasher->shouldReceive('check')->with('secret', $client->secret)->andReturn(true); + $hasher->shouldReceive('check')->withAnyArgs()->andReturn(false); $this->clientModelRepository = $clientModelRepository; - $this->repository = new BridgeClientRepository($clientModelRepository); + $this->repository = new BridgeClientRepository($clientModelRepository, $hasher); } protected function tearDown(): void @@ -62,17 +64,17 @@ public function test_can_validate_client_for_auth_code_grant() public function test_can_validate_client_for_client_credentials_grant() { $client = $this->clientModelRepository->findActive(1); - $client->personal_access_client = true; + $client->grant_types[] = 'personal_access'; $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'client_credentials')); - $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'authorization_code')); } public function test_password_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->password_client = true; + $client->grant_types[] = 'password'; $this->assertTrue($this->repository->validateClient(1, 'secret', 'password')); } @@ -80,7 +82,7 @@ public function test_password_grant_is_permitted() public function test_public_client_password_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->password_client = true; + $client->grant_types[] = 'password'; $client->secret = null; $this->assertTrue($this->repository->validateClient(1, null, 'password')); @@ -107,7 +109,7 @@ public function test_public_client_authorization_code_grant_is_permitted() public function test_authorization_code_grant_is_prevented() { $client = $this->clientModelRepository->findActive(1); - $client->password_client = true; + $client->grant_types = ['password']; $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); } @@ -115,7 +117,7 @@ public function test_authorization_code_grant_is_prevented() public function test_personal_access_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->personal_access_client = true; + $client->grant_types[] = 'personal_access'; $this->assertTrue($this->repository->validateClient(1, 'secret', 'personal_access')); } @@ -186,34 +188,11 @@ class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client { public $name = 'Client'; - public $redirect = 'http://localhost'; - - public $secret = 'secret'; + public $redirect_uris = ['http://localhost']; - public $personal_access_client = false; - - public $password_client = false; + public $secret = '$2y$10$JBJdbMNY0PlXZaUMShpkgO2yfKtSMVo.ZZOBOue49jatGL49jBkgu'; // 'secret' public $provider = null; - public $grant_types; - - public function firstParty() - { - return $this->personal_access_client || $this->password_client; - } - - public function confidential() - { - return ! empty($this->secret); - } - - public function hasGrantType($grantType) - { - if (! isset($this->grant_types) || ! is_array($this->grant_types)) { - return true; - } - - return in_array($grantType, $this->grant_types); - } + public $grant_types = ['authorization_code', 'client_credentials', 'refresh_token']; } diff --git a/tests/Unit/BridgeScopeRepositoryTest.php b/tests/Unit/BridgeScopeRepositoryTest.php index fb204ff53..fcb82c7ac 100644 --- a/tests/Unit/BridgeScopeRepositoryTest.php +++ b/tests/Unit/BridgeScopeRepositoryTest.php @@ -25,6 +25,7 @@ public function test_invalid_scopes_are_removed() ]); $client = Mockery::mock(ClientModel::class)->makePartial(); + $client->scopes = ['*']; $clients = Mockery::mock(ClientRepository::class); $clients->shouldReceive('findActive')->withAnyArgs()->andReturn($client); @@ -38,46 +39,26 @@ public function test_invalid_scopes_are_removed() $this->assertEquals([$scope1], $scopes); } - public function test_invalid_scopes_are_removed_without_a_client_repository() + public function test_invalid_scopes_are_removed_with_client_scopes() { Passport::tokensCan([ 'scope-1' => 'description', - ]); - - $clients = Mockery::mock(ClientRepository::class); - $clients->shouldReceive('findActive') - ->with('id') - ->andReturn(Mockery::mock(ClientModel::class)->makePartial()); - - $repository = new ScopeRepository($clients); - - $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 - ); - - $this->assertEquals([$scope1], $scopes); - } - - public function test_clients_do_not_restrict_scopes_by_default() - { - Passport::tokensCan([ - 'scope-1' => 'description', - 'scope-2' => 'description', + 'scope-2' => 'description-2', ]); $client = Mockery::mock(ClientModel::class)->makePartial(); - $client->scopes = null; + $client->scopes = ['scope-1']; $clients = Mockery::mock(ClientRepository::class); - $clients->shouldReceive('findActive')->withAnyArgs()->andReturn($client); + $clients->shouldReceive('findActive')->with('id')->andReturn($client); $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), $scope2 = new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 ); - $this->assertEquals([$scope1, $scope2], $scopes); + $this->assertEquals([$scope1], $scopes); } public function test_scopes_disallowed_for_client_are_removed() diff --git a/tests/Unit/ClientControllerTest.php b/tests/Unit/ClientControllerTest.php index 7da484042..6acfabe54 100644 --- a/tests/Unit/ClientControllerTest.php +++ b/tests/Unit/ClientControllerTest.php @@ -7,7 +7,7 @@ use Laravel\Passport\Client; use Laravel\Passport\ClientRepository; use Laravel\Passport\Http\Controllers\ClientController; -use Laravel\Passport\Http\Rules\RedirectRule; +use Laravel\Passport\Http\Rules\UriRule; use Mockery as m; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Response; @@ -31,7 +31,6 @@ public function test_all_the_clients_for_the_current_user_can_be_retrieved() $controller = new ClientController( $clients, m::mock(Factory::class), - m::mock(RedirectRule::class) ); $this->assertEquals($client, $controller->forUser($request)); @@ -41,31 +40,30 @@ public function test_clients_can_be_stored() { $clients = m::mock(ClientRepository::class); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); $request->setUserResolver(function () { return new ClientControllerFakeUser; }); - $clients->shouldReceive('create') + $clients->shouldReceive('createAuthorizationCodeGrantClient') ->once() - ->with(1, 'client name', 'http://localhost', null, false, false, true) + ->with('client name', ['http://localhost'], true, 1) ->andReturn($client = new Client); - $redirectRule = m::mock(RedirectRule::class); - $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect' => 'http://localhost', + 'redirect_uris' => ['http://localhost'], ], [ - 'name' => 'required|max:191', - 'redirect' => ['required', $redirectRule], + 'name' => 'required|max:255', + 'redirect_uris' => 'required|array|min:1', + 'redirect_uris.*' => ['required', new UriRule], 'confidential' => 'boolean', ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator, $redirectRule + $clients, $validator ); $this->assertEquals($client, $controller->store($request)); @@ -78,33 +76,32 @@ public function test_public_clients_can_be_stored() $request = Request::create( '/', 'GET', - ['name' => 'client name', 'redirect' => 'http://localhost', 'confidential' => false] + ['name' => 'client name', 'redirect_uris' => ['http://localhost'], 'confidential' => false] ); $request->setUserResolver(function () { return new ClientControllerFakeUser; }); - $clients->shouldReceive('create') + $clients->shouldReceive('createAuthorizationCodeGrantClient') ->once() - ->with(1, 'client name', 'http://localhost', null, false, false, false) + ->with('client name', ['http://localhost'], false, 1) ->andReturn($client = new Client); - $redirectRule = m::mock(RedirectRule::class); - $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect' => 'http://localhost', + 'redirect_uris' => ['http://localhost'], 'confidential' => false, ], [ - 'name' => 'required|max:191', - 'redirect' => ['required', $redirectRule], + 'name' => 'required|max:255', + 'redirect_uris' => 'required|array|min:1', + 'redirect_uris.*' => ['required', new UriRule], 'confidential' => 'boolean', ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator, $redirectRule + $clients, $validator ); $this->assertEquals($client, $controller->store($request)); @@ -116,7 +113,7 @@ public function test_clients_can_be_updated() $client = m::mock(Client::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturn($client); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); $request->setUserResolver(function () { $user = m::mock(); @@ -126,23 +123,22 @@ public function test_clients_can_be_updated() }); $clients->shouldReceive('update')->once()->with( - m::type(Client::class), 'client name', 'http://localhost' + m::type(Client::class), 'client name', ['http://localhost'] )->andReturn('response'); - $redirectRule = m::mock(RedirectRule::class); - $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect' => 'http://localhost', + 'redirect_uris' => ['http://localhost'], ], [ - 'name' => 'required|max:191', - 'redirect' => ['required', $redirectRule], + 'name' => 'required|max:255', + 'redirect_uris' => 'required|array|min:1', + 'redirect_uris.*' => ['required', new UriRule], ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator, $redirectRule + $clients, $validator ); $this->assertSame('response', $controller->update($request, 1)); @@ -153,7 +149,7 @@ public function test_404_response_if_client_doesnt_belong_to_user() $clients = m::mock(ClientRepository::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturnNull(); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); $request->setUserResolver(function () { $user = m::mock(); @@ -167,7 +163,7 @@ public function test_404_response_if_client_doesnt_belong_to_user() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator, m::mock(RedirectRule::class) + $clients, $validator ); $this->assertSame(404, $controller->update($request, 1)->status()); @@ -179,7 +175,7 @@ public function test_clients_can_be_deleted() $client = m::mock(Client::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturn($client); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); $request->setUserResolver(function () { $user = m::mock(); @@ -195,7 +191,7 @@ public function test_clients_can_be_deleted() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator, m::mock(RedirectRule::class) + $clients, $validator ); $response = $controller->destroy($request, 1); @@ -208,7 +204,7 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() $clients = m::mock(ClientRepository::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturnNull(); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); $request->setUserResolver(function () { $user = m::mock(); @@ -222,7 +218,7 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator, m::mock(RedirectRule::class) + $clients, $validator ); $this->assertSame(404, $controller->destroy($request, 1)->status()); diff --git a/tests/Unit/PassportTest.php b/tests/Unit/PassportTest.php index b6e69e2ad..d93e60f16 100644 --- a/tests/Unit/PassportTest.php +++ b/tests/Unit/PassportTest.php @@ -6,7 +6,6 @@ use Laravel\Passport\Client; use Laravel\Passport\ClientRepository; use Laravel\Passport\Passport; -use Laravel\Passport\PersonalAccessClient; use Laravel\Passport\RefreshToken; use Laravel\Passport\Token; use PHPUnit\Framework\TestCase; @@ -40,20 +39,10 @@ public function test_client_instance_can_be_created() $this->assertInstanceOf(Passport::clientModel(), $client); } - public function test_personal_access_client_instance_can_be_created() - { - $client = Passport::personalAccessClient(); - - $this->assertInstanceOf(PersonalAccessClient::class, $client); - $this->assertInstanceOf(Passport::personalAccessClientModel(), $client); - } - public function test_missing_personal_access_client_is_reported() { $this->expectException('RuntimeException'); - Passport::usePersonalAccessClientModel(PersonalAccessClientStub::class); - $clientRepository = new ClientRepository; $clientRepository->personalAccessClient(); } diff --git a/tests/Unit/PersonalAccessTokenControllerTest.php b/tests/Unit/PersonalAccessTokenControllerTest.php index 9e8b1ad50..4610b826d 100644 --- a/tests/Unit/PersonalAccessTokenControllerTest.php +++ b/tests/Unit/PersonalAccessTokenControllerTest.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\Validation\Factory; use Illuminate\Http\Request; +use Laravel\Passport\Client; use Laravel\Passport\Http\Controllers\PersonalAccessTokenController; use Laravel\Passport\Passport; use Laravel\Passport\Token; @@ -27,8 +28,8 @@ public function test_tokens_can_be_retrieved_for_users() $token2 = new Token; $userTokens = m::mock(); - $token1->client = (object) ['personal_access_client' => true]; - $token2->client = (object) ['personal_access_client' => false]; + $token1->client = new Client(['grant_types' => ['personal_access']]); + $token2->client = new Client(['grant_types' => []]); $userTokens->shouldReceive('load')->with('client')->andReturn(collect([ $token1, $token2, ])); @@ -74,7 +75,7 @@ public function test_tokens_can_be_updated() 'name' => 'token name', 'scopes' => ['user', 'user-admin'], ], [ - 'name' => 'required|max:191', + 'name' => 'required|max:255', 'scopes' => 'array|in:'.implode(',', Passport::scopeIds()), ])->andReturn($validator); $validator->shouldReceive('validate')->once(); diff --git a/tests/Unit/PersonalAccessTokenFactoryTest.php b/tests/Unit/PersonalAccessTokenFactoryTest.php index f20eaecef..01fb33edf 100644 --- a/tests/Unit/PersonalAccessTokenFactoryTest.php +++ b/tests/Unit/PersonalAccessTokenFactoryTest.php @@ -35,6 +35,7 @@ public function test_access_token_can_be_created() $factory = new PersonalAccessTokenFactory($server, $clients, $tokens, $jwt); $clients->shouldReceive('personalAccessClient')->andReturn($client = new PersonalAccessTokenFactoryTestClientStub); + $clients->shouldReceive('getPersonalAccessClientSecret')->andReturn('secret'); $server->shouldReceive('respondToAccessTokenRequest')->andReturn($response = m::mock(ResponseInterface::class)); $response->shouldReceive('getBody->__toString')->andReturn(json_encode([ 'access_token' => 'foo', @@ -61,13 +62,9 @@ public function test_access_token_can_be_created() class PersonalAccessTokenFactoryTestClientStub extends Client { public $id = 1; - - public $secret = 'something'; } class PersonalAccessTokenFactoryTestModelStub extends Token { public $id = 1; - - public $secret = 'something'; } diff --git a/tests/Unit/RedirectRuleTest.php b/tests/Unit/RedirectRuleTest.php deleted file mode 100644 index e4c226564..000000000 --- a/tests/Unit/RedirectRuleTest.php +++ /dev/null @@ -1,49 +0,0 @@ -rule($fails = false); - - $this->assertTrue($rule->passes('redirect', 'https://example.com')); - } - - public function test_it_passes_with_multiple_valid_urls() - { - $rule = $this->rule($fails = false); - - $this->assertTrue($rule->passes('redirect', 'https://example.com,https://example2.com')); - } - - public function test_it_fails_with_a_single_invalid_url() - { - $rule = $this->rule($fails = true); - - $this->assertFalse($rule->passes('redirect', 'https://example.com,invalid')); - } - - private function rule(bool $fails): RedirectRule - { - $validator = m::mock(Validator::class); - $validator->shouldReceive('fails')->andReturn($fails); - - $factory = m::mock(Factory::class); - $factory->shouldReceive('make')->andReturn($validator); - - return new RedirectRule($factory); - } -} From a521a78af36a279a2b738b004aa2d9bcaaa10fe4 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 22:55:55 +0330 Subject: [PATCH 02/26] formatting --- src/ClientRepository.php | 6 ++---- tests/Feature/ClientControllerTest.php | 2 +- tests/Feature/Console/HashCommand.php | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/ClientRepository.php b/src/ClientRepository.php index f04fa0290..0e52dc9bb 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -134,8 +134,7 @@ protected function create( ?string $userId = null, bool $confidential = true, array $scopes = ['*'] - ): Client - { + ): Client { $client = Passport::client()->forceFill([ 'user_id' => $userId, 'name' => $name, @@ -196,8 +195,7 @@ public function createAuthorizationCodeGrantClient( array $redirectUris, bool $confidential = true, ?string $userId = null - ): Client - { + ): Client { return $this->create( $name, ['authorization_code', 'refresh_token'], $redirectUris, null, $userId, $confidential ); diff --git a/tests/Feature/ClientControllerTest.php b/tests/Feature/ClientControllerTest.php index 52311e6ae..8ce57e4b6 100644 --- a/tests/Feature/ClientControllerTest.php +++ b/tests/Feature/ClientControllerTest.php @@ -49,7 +49,7 @@ public function testUpdatingClient() $this->actingAs($user); $client = $this->post('/oauth/clients', [ - 'name' => 'new client', 'redirect_uris' => ['https://localhost'] + 'name' => 'new client', 'redirect_uris' => ['https://localhost'], ])->json(); $response = $this->put( diff --git a/tests/Feature/Console/HashCommand.php b/tests/Feature/Console/HashCommand.php index 89b18cd4a..1794eeac1 100644 --- a/tests/Feature/Console/HashCommand.php +++ b/tests/Feature/Console/HashCommand.php @@ -5,7 +5,6 @@ use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Support\Str; use Laravel\Passport\Client; -use Laravel\Passport\Passport; use Laravel\Passport\Tests\Feature\PassportTestCase; class HashCommand extends PassportTestCase From e9068217d34401d3b34527e3cfac526786d67ff6 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 23:48:16 +0330 Subject: [PATCH 03/26] drop support for Laravel 9 --- .github/workflows/tests.yml | 4 +--- composer.json | 18 +++++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6292d4602..e83e99993 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,12 +17,10 @@ jobs: fail-fast: true matrix: php: [8.1, 8.2, 8.3] - laravel: [9, 10, 11] + laravel: [10, 11] exclude: - php: 8.1 laravel: 11 - - php: 8.3 - laravel: 9 name: PHP ${{ matrix.php }} - Laravel ${{ matrix.laravel }} diff --git a/composer.json b/composer.json index e8dad6784..b806d86a7 100644 --- a/composer.json +++ b/composer.json @@ -18,15 +18,15 @@ "ext-json": "*", "ext-openssl": "*", "firebase/php-jwt": "^6.4", - "illuminate/auth": "^9.21|^10.0|^11.0", - "illuminate/console": "^9.21|^10.0|^11.0", - "illuminate/container": "^9.21|^10.0|^11.0", - "illuminate/contracts": "^9.21|^10.0|^11.0", - "illuminate/cookie": "^9.21|^10.0|^11.0", - "illuminate/database": "^9.21|^10.0|^11.0", - "illuminate/encryption": "^9.21|^10.0|^11.0", - "illuminate/http": "^9.21|^10.0|^11.0", - "illuminate/support": "^9.21|^10.0|^11.0", + "illuminate/auth": "^10.0|^11.0", + "illuminate/console": "^10.0|^11.0", + "illuminate/container": "^10.0|^11.0", + "illuminate/contracts": "^10.0|^11.0", + "illuminate/cookie": "^10.0|^11.0", + "illuminate/database": "^10.0|^11.0", + "illuminate/encryption": "^10.0|^11.0", + "illuminate/http": "^10.0|^11.0", + "illuminate/support": "^10.0|^11.0", "lcobucci/jwt": "^5.0", "league/oauth2-server": "^9.0", "nyholm/psr7": "^1.5", From 7b33d5d214f3e4c8db6a7ef332bb40e8b81e7fec Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 19 May 2024 17:20:30 +0330 Subject: [PATCH 04/26] update upgrade guide --- UPGRADE.md | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index 6127c47ae..a66966777 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,12 +10,98 @@ PR: https://github.com/laravel/passport/pull/1734 PHP 8.1 is now the minimum required version. +### Minimum Laravel Version + +PR: https://github.com/laravel/passport/pull/1744 + +Laravel 10.0 is now the minimum required version. + ### OAuth2 Server PR: https://github.com/laravel/passport/pull/1734 The `league/oauth2-server` Composer package which is utilized internally by Passport has been updated to 9.0, which adds additional types to method signatures. To ensure your application is compatible, you should review this package's complete [changelog](https://github.com/thephpleague/oauth2-server/blob/master/CHANGELOG.md#900---released-2024-05-13). +### Client Secret + +PR: https://github.com/laravel/passport/pull/1744 + +Passport now always hashes clients' secret, you may call the following command if you need to hash your clients' secrets: + + php artisan passport:hash + +### Client ID + +PR: https://github.com/laravel/passport/pull/1744 + +Passport now uses UUID to identify clients. If you were already using client with UUIDs you don't have to make any changes, but if you were not using client with UUIDs you must change the type of client ID columns to `char(36)`. Your previous client IDs won't change and you will get UUID for newly created clients from now on: + +```php +Schema::table('oauth_clients', function (Blueprint $table) { + $table->char('id', 36)->change(); +}); + +Schema::table('oauth_auth_codes', function (Blueprint $table) { + $table->char('client_id', 36)->index()->change(); +}); + +Schema::table('oauth_access_tokens', function (Blueprint $table) { + $table->char('client_id', 36)->index()->change(); +}); +``` + +### Clients Table + +PR: https://github.com/laravel/passport/pull/1744 + +The `oauth_clients` table now requires `grant_types`, `scopes` and `redirect_uris` columns as JSON array and `personal_access_client` and `password_client` columns are removed: + +```php +Schema::table('oauth_clients', function (Blueprint $table) { + $table->after('name', function (Blueprint $table) { + $table->text('grant_types'); + $table->text('scopes'); + $table->text('redirect_uris'); + }); +}); + +foreach (Passport::client()->cursor() as $client) { + Model::withoutTimestamps(fn () => $client->forceFill([ + 'grant_types' => match (true) { + (bool) $client->personal_access_client => ['personal_access'], + (bool) $client->password_client => ['password', 'refresh_token'], + empty($client->secret) && ! empty($client->redirect) => ['authorization_code', 'implicit', 'refresh_token'], + ! empty($client->secret) && empty($client->redirect) => ['client_credentials'], + ! empty($client->secret) && ! empty($client->redirect) => ['authorization_code', 'implicit', 'refresh_token', 'client_credentials'], + default => [], + }, + 'scopes' => ['*'], + 'redirect_uris' => explode(',', $client->redirect), + ])->save()); +} + +Schema::table('oauth_clients', function (Blueprint $table) { + $table->dropColumn(['redirect', 'personal_access_client', 'password_client']); +}); +``` + +### Removed functionalities + +PR: https://github.com/laravel/passport/pull/1744 + +The following list of properties and methods have been removed: + +* `Passport::$clientUuids` property. +* `Passport::clientUuids()` method. +* `Passport::setClientUuids()` method. +* `'passport.client_uuids'` config property. +* `Passport::$hashesClientSecrets` property. +* `Passport::hashClientSecrets()` method. +* `Passport::$personalAccessClientModel` property. +* `Passport::usePersonalAccessClientModel()` method. +* `Passport::personalAccessClientModel()` method. +* `Passport::personalAccessClient()` method. + ## Upgrading To 12.0 From 11.x ### Migration Changes From 5bca85385f4440f3107bff77c36e5bbfbdd386d0 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 30 May 2024 22:51:48 +0330 Subject: [PATCH 05/26] wip --- UPGRADE.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index f1486a966..6e1ac1640 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,6 +10,12 @@ PR: https://github.com/laravel/passport/pull/1734 PHP 8.1 is now the minimum required version. +### Minimum Laravel Version + +PR: https://github.com/laravel/passport/pull/1744 + +Laravel 10.0 is now the minimum required version. + ### OAuth2 Server PR: https://github.com/laravel/passport/pull/1734 From da3edf0db405aa6367fb714fb167a8b5fbde43e0 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 27 Jun 2024 18:30:34 +0330 Subject: [PATCH 06/26] revert some changes --- config/passport.php | 13 +++++++++++++ src/Passport.php | 28 ++++++++++++++++++++++++++++ src/PassportServiceProvider.php | 2 ++ 3 files changed, 43 insertions(+) diff --git a/config/passport.php b/config/passport.php index dbdbfed1f..ae902d80c 100644 --- a/config/passport.php +++ b/config/passport.php @@ -43,6 +43,19 @@ 'connection' => env('PASSPORT_CONNECTION'), + /* + |-------------------------------------------------------------------------- + | Client UUIDs + |-------------------------------------------------------------------------- + | + | By default, Passport uses auto-incrementing primary keys when assigning + | IDs to clients. However, if Passport is installed using the provided + | --uuids switch, this will be set to "true" and UUIDs will be used. + | + */ + + 'client_uuids' => false, + /* |-------------------------------------------------------------------------- | Personal Access Client diff --git a/src/Passport.php b/src/Passport.php index bb6085b69..c9599ea62 100644 --- a/src/Passport.php +++ b/src/Passport.php @@ -114,6 +114,13 @@ class Passport */ public static $clientModel = 'Laravel\Passport\Client'; + /** + * Indicates if client's are identified by UUIDs. + * + * @var bool + */ + public static $clientUuids = false; + /** * The token model class name. * @@ -504,6 +511,27 @@ public static function client() return new static::$clientModel; } + /** + * Determine if clients are identified using UUIDs. + * + * @return bool + */ + public static function clientUuids() + { + return static::$clientUuids; + } + + /** + * Specify if clients are identified using UUIDs. + * + * @param bool $value + * @return void + */ + public static function setClientUuids($value) + { + static::$clientUuids = $value; + } + /** * Set the token model class name. * diff --git a/src/PassportServiceProvider.php b/src/PassportServiceProvider.php index b0b560c07..a22dc37cb 100644 --- a/src/PassportServiceProvider.php +++ b/src/PassportServiceProvider.php @@ -126,6 +126,8 @@ public function register() { $this->mergeConfigFrom(__DIR__.'/../config/passport.php', 'passport'); + Passport::setClientUuids($this->app->make(Config::class)->get('passport.client_uuids', false)); + $this->app->when(AuthorizationController::class) ->needs(StatefulGuard::class) ->give(fn () => Auth::guard(config('passport.guard', null))); From d8e89ca12ad2562bb9301e74d9ddcde47727a34d Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 29 Jul 2024 21:19:58 +0330 Subject: [PATCH 07/26] fix tests --- database/factories/ClientFactory.php | 3 +-- tests/Feature/PersonalAccessTokenFactoryTest.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index e026586bc..7efac38dd 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -60,8 +60,7 @@ public function asPasswordClient() public function asPersonalAccessTokenClient() { return $this->state([ - 'personal_access_client' => true, - 'password_client' => false, + 'grant_types' => ['personal_access'], ]); } diff --git a/tests/Feature/PersonalAccessTokenFactoryTest.php b/tests/Feature/PersonalAccessTokenFactoryTest.php index b23dfc2c6..1900f6c48 100644 --- a/tests/Feature/PersonalAccessTokenFactoryTest.php +++ b/tests/Feature/PersonalAccessTokenFactoryTest.php @@ -25,7 +25,7 @@ public function testIssueToken() ]); /** @var Client $client */ - $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(); + $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(['scopes' => ['foo', 'bar']]); config([ 'passport.personal_access_client.id' => $client->getKey(), From 3c6642364dc8d92ff9aa54251983e0fea5a4e0c5 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 29 Jul 2024 21:25:48 +0330 Subject: [PATCH 08/26] wip --- src/Client.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Client.php b/src/Client.php index 7e94b82f8..04504ffa5 100644 --- a/src/Client.php +++ b/src/Client.php @@ -170,10 +170,6 @@ public function hasGrantType($grantType) */ public function hasScope($scope) { - if (in_array('*', $this->scopes)) { - return true; - } - $scopes = Passport::$withInheritedScopes ? $this->resolveInheritedScopes($scope) : [$scope]; From e4931d5dcc084e3318de3f3de2b12df1a6607b0d Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 29 Jul 2024 21:46:54 +0330 Subject: [PATCH 09/26] fix tests --- tests/Feature/ClientTest.php | 8 -------- tests/Unit/BridgeScopeRepositoryTest.php | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/Feature/ClientTest.php b/tests/Feature/ClientTest.php index f96292ac1..b1fba3012 100644 --- a/tests/Feature/ClientTest.php +++ b/tests/Feature/ClientTest.php @@ -38,14 +38,6 @@ public function testScopesWhenClientHasScope(): void $this->assertTrue($client->hasScope('foo')); } - public function testScopesWhenClientHasAny(): void - { - $client = new Client(['scopes' => ['*']]); - $client->exists = true; - - $this->assertTrue($client->hasScope('foo')); - } - public function testGrantTypesWhenClientDoesNotHaveGrantType(): void { $client = new Client(['grant_types' => ['bar']]); diff --git a/tests/Unit/BridgeScopeRepositoryTest.php b/tests/Unit/BridgeScopeRepositoryTest.php index fcb82c7ac..5a727f7ca 100644 --- a/tests/Unit/BridgeScopeRepositoryTest.php +++ b/tests/Unit/BridgeScopeRepositoryTest.php @@ -25,7 +25,7 @@ public function test_invalid_scopes_are_removed() ]); $client = Mockery::mock(ClientModel::class)->makePartial(); - $client->scopes = ['*']; + $client->scopes = ['scope-1']; $clients = Mockery::mock(ClientRepository::class); $clients->shouldReceive('findActive')->withAnyArgs()->andReturn($client); From 2784abff6a2dcef36ed2bf7859b1222ae9ed6cff Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 00:23:19 +0330 Subject: [PATCH 10/26] wip --- UPGRADE.md | 12 +-- database/factories/ClientFactory.php | 5 +- ...6_01_000004_create_oauth_clients_table.php | 7 +- src/Bridge/Client.php | 2 +- src/Bridge/ClientRepository.php | 3 +- src/Client.php | 4 + src/ClientRepository.php | 12 ++- src/Console/ClientCommand.php | 24 +++--- src/Http/Controllers/ClientController.php | 26 ++++--- .../PersonalAccessTokenController.php | 2 +- src/Http/Rules/RedirectRule.php | 51 +++++++++++++ tests/Feature/ClientControllerTest.php | 73 ------------------- tests/Feature/ClientTest.php | 16 ++++ .../PersonalAccessTokenFactoryTest.php | 2 +- tests/Unit/BridgeClientRepositoryTest.php | 8 -- tests/Unit/BridgeScopeRepositoryTest.php | 33 +++++++-- tests/Unit/ClientControllerTest.php | 54 +++++++------- .../PersonalAccessTokenControllerTest.php | 2 +- tests/Unit/RedirectRuleTest.php | 49 +++++++++++++ 19 files changed, 222 insertions(+), 163 deletions(-) create mode 100644 src/Http/Rules/RedirectRule.php delete mode 100644 tests/Feature/ClientControllerTest.php create mode 100644 tests/Unit/RedirectRuleTest.php diff --git a/UPGRADE.md b/UPGRADE.md index 88a720264..c283763b6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -65,14 +65,13 @@ In addition, the `Laravel\Passport\PersonalAccessClient` model, `Passport::$pers PR: https://github.com/laravel/passport/pull/1744 -The `oauth_clients` table now requires `grant_types`, `scopes` and `redirect_uris` columns as JSON array and `personal_access_client` and `password_client` columns are removed: +The `oauth_clients` table now requires `grant_types` and `redirect_uris` columns as JSON array and `redirect`, `personal_access_client`, and `password_client` columns are removed: ```php Schema::table('oauth_clients', function (Blueprint $table) { - $table->after('name', function (Blueprint $table) { - $table->text('grant_types'); - $table->text('scopes'); + $table->after('provider', function (Blueprint $table) { $table->text('redirect_uris'); + $table->text('grant_types'); }); }); @@ -81,12 +80,9 @@ foreach (Passport::client()->cursor() as $client) { 'grant_types' => match (true) { (bool) $client->personal_access_client => ['personal_access'], (bool) $client->password_client => ['password', 'refresh_token'], - empty($client->secret) && ! empty($client->redirect) => ['authorization_code', 'implicit', 'refresh_token'], ! empty($client->secret) && empty($client->redirect) => ['client_credentials'], - ! empty($client->secret) && ! empty($client->redirect) => ['authorization_code', 'implicit', 'refresh_token', 'client_credentials'], - default => [], + default => ['authorization_code', 'implicit', 'refresh_token'], }, - 'scopes' => ['*'], 'redirect_uris' => explode(',', $client->redirect), ])->save()); } diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index 7efac38dd..87e8801cb 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -31,11 +31,10 @@ public function definition() return [ 'user_id' => null, 'name' => $this->faker->company(), - 'grant_types' => [], - 'scopes' => [], + 'secret' => Str::random(40), 'redirect_uris' => [$this->faker->url()], 'provider' => null, - 'secret' => Str::random(40), + 'grant_types' => [], 'revoked' => false, ]; } diff --git a/database/migrations/2016_06_01_000004_create_oauth_clients_table.php b/database/migrations/2016_06_01_000004_create_oauth_clients_table.php index 011b17d64..21ec74624 100644 --- a/database/migrations/2016_06_01_000004_create_oauth_clients_table.php +++ b/database/migrations/2016_06_01_000004_create_oauth_clients_table.php @@ -15,11 +15,10 @@ public function up(): void $table->uuid('id')->primary(); $table->foreignId('user_id')->nullable()->index(); $table->string('name'); - $table->text('grant_types'); - $table->text('scopes'); - $table->text('redirect_uris'); - $table->string('provider')->nullable(); $table->string('secret')->nullable(); + $table->string('provider')->nullable(); + $table->text('redirect_uris'); + $table->text('grant_types'); $table->boolean('revoked'); $table->timestamps(); }); diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 51ac50d2b..4a1df7d8f 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = is_array($redirectUri) ? $redirectUri : [$redirectUri]; + $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri);; $this->provider = $provider; } } diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index 3aceef2ef..2ed2b45d7 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -71,8 +71,7 @@ public function validateClient(string $clientIdentifier, ?string $clientSecret, */ protected function handlesGrant(ClientModel $record, string $grantType): bool { - return $record->hasGrantType($grantType) && - (! in_array($grantType, ['personal_access', 'client_credentials']) || $record->confidential()); + return $record->hasGrantType($grantType); } /** diff --git a/src/Client.php b/src/Client.php index 04504ffa5..9c80a4c37 100644 --- a/src/Client.php +++ b/src/Client.php @@ -170,6 +170,10 @@ public function hasGrantType($grantType) */ public function hasScope($scope) { + if (! isset($this->attributes['scopes']) || ! is_array($this->scopes)) { + return true; + } + $scopes = Passport::$withInheritedScopes ? $this->resolveInheritedScopes($scope) : [$scope]; diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 65622fb0e..785277a48 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -80,7 +80,6 @@ public function activeForUser($userId) * * @param string[] $redirectUris * @param string[] $grantTypes - * @param string[] $scopes */ protected function create( string $name, @@ -88,14 +87,12 @@ protected function create( array $redirectUris = [], ?string $provider = null, ?string $userId = null, - bool $confidential = true, - array $scopes = ['*'] + bool $confidential = true ): Client { $client = Passport::client()->forceFill([ 'user_id' => $userId, 'name' => $name, 'grant_types' => $grantTypes, - 'scopes' => $scopes, 'redirect_uris' => $redirectUris, 'provider' => $provider, 'secret' => $confidential ? Str::random(40) : null, @@ -110,9 +107,9 @@ protected function create( /** * Store a new personal access token client. */ - public function createPersonalAccessGrantClient(string $name): Client + public function createPersonalAccessGrantClient(string $name, ?string $provider = null): Client { - return $this->create($name, ['personal_access']); + return $this->create($name, ['personal_access'], [], $provider); } /** @@ -168,7 +165,8 @@ public function createAuthorizationCodeGrantClient( public function update(Client $client, $name, $redirectUris) { $client->forceFill([ - 'name' => $name, 'redirect_uris' => $redirectUris, + 'name' => $name, + 'redirect_uris' => $redirectUris, ])->save(); return $client; diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index 828a76017..44b06fe62 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -19,7 +19,7 @@ class ClientCommand extends Command {--personal : Create a personal access token client} {--password : Create a password grant client} {--client : Create a client credentials grant client} - {--implicit : Create a implicit grant client} + {--implicit : Create an implicit grant client} {--public : Create a public client (Auth code grant type only) } {--name= : The name of the client} {--provider= : The name of the user provider} @@ -63,11 +63,16 @@ public function handle(ClientRepository $clients) protected function createPersonalAccessClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( - 'What should we name the personal access client?', - config('app.name').' Personal Access Client' + 'What should we name the client?', + config('app.name').' Personal Access Grant Client' + ); + + $provider = $this->option('provider') ?: $this->choice( + 'Which user provider should this client use to retrieve users?', + array_keys(config('auth.providers')), ); - $client = $clients->createPersonalAccessGrantClient($name); + $client = $clients->createPersonalAccessGrantClient($name, $provider); $this->components->info('Personal access client created successfully.'); @@ -87,15 +92,13 @@ protected function createPersonalAccessClient(ClientRepository $clients) protected function createPasswordClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( - 'What should we name the password grant client?', + 'What should we name the client?', config('app.name').' Password Grant Client' ); - $providers = array_keys(config('auth.providers')); - $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', - $providers, + array_keys(config('auth.providers')), ); $client = $clients->createPasswordGrantClient($name, $provider); @@ -157,10 +160,6 @@ protected function createImplicitClient(ClientRepository $clients) */ protected function createAuthCodeClient(ClientRepository $clients) { - $userId = $this->option('user_id') ?: $this->ask( - 'Which user ID should the client be assigned to? (Optional)' - ); - $name = $this->option('name') ?: $this->ask( 'What should we name the client?' ); @@ -174,7 +173,6 @@ protected function createAuthCodeClient(ClientRepository $clients) $name, explode(',', $redirect), ! $this->option('public'), - $userId ); $this->components->info('New client created successfully.'); diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php index a724a8cd7..bf5ac8564 100644 --- a/src/Http/Controllers/ClientController.php +++ b/src/Http/Controllers/ClientController.php @@ -6,7 +6,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Laravel\Passport\ClientRepository; -use Laravel\Passport\Http\Rules\UriRule; +use Laravel\Passport\Http\Rules\RedirectRule; class ClientController { @@ -24,19 +24,29 @@ class ClientController */ protected $validation; + /** + * The redirect validation rule. + * + * @var \Laravel\Passport\Http\Rules\RedirectRule + */ + protected $redirectRule; + /** * Create a client controller instance. * * @param \Laravel\Passport\ClientRepository $clients * @param \Illuminate\Contracts\Validation\Factory $validation + * @param \Laravel\Passport\Http\Rules\RedirectRule $redirectRule * @return void */ public function __construct( ClientRepository $clients, ValidationFactory $validation, + RedirectRule $redirectRule ) { $this->clients = $clients; $this->validation = $validation; + $this->redirectRule = $redirectRule; } /** @@ -61,15 +71,14 @@ public function forUser(Request $request) public function store(Request $request) { $this->validation->make($request->all(), [ - 'name' => 'required|max:255', - 'redirect_uris' => 'required|array|min:1', - 'redirect_uris.*' => ['required', new UriRule], + 'name' => 'required|max:191', + 'redirect' => ['required', $this->redirectRule], 'confidential' => 'boolean', ])->validate(); $client = $this->clients->createAuthorizationCodeGrantClient( $request->name, - $request->redirect_uris, + explode(',', $request->redirect), (bool) $request->input('confidential', true), $request->user()->getAuthIdentifier(), ); @@ -95,13 +104,12 @@ public function update(Request $request, $clientId) } $this->validation->make($request->all(), [ - 'name' => 'required|max:255', - 'redirect_uris' => 'required|array|min:1', - 'redirect_uris.*' => ['required', new UriRule], + 'name' => 'required|max:191', + 'redirect' => ['required', $this->redirectRule], ])->validate(); return $this->clients->update( - $client, $request->name, $request->redirect_uris + $client, $request->name, explode(',', $request->redirect) ); } diff --git a/src/Http/Controllers/PersonalAccessTokenController.php b/src/Http/Controllers/PersonalAccessTokenController.php index 0fce3047c..565fd4fa9 100644 --- a/src/Http/Controllers/PersonalAccessTokenController.php +++ b/src/Http/Controllers/PersonalAccessTokenController.php @@ -61,7 +61,7 @@ public function forUser(Request $request) public function store(Request $request) { $this->validation->make($request->all(), [ - 'name' => 'required|max:255', + 'name' => 'required|max:191', 'scopes' => 'array|in:'.implode(',', Passport::scopeIds()), ])->validate(); diff --git a/src/Http/Rules/RedirectRule.php b/src/Http/Rules/RedirectRule.php new file mode 100644 index 000000000..e14d11c3d --- /dev/null +++ b/src/Http/Rules/RedirectRule.php @@ -0,0 +1,51 @@ +validator = $validator; + } + + /** + * {@inheritdoc} + */ + public function passes($attribute, $value) + { + foreach (explode(',', $value) as $redirect) { + $validator = $this->validator->make(['redirect' => $redirect], ['redirect' => new UriRule]); + + if ($validator->fails()) { + return false; + } + } + + return true; + } + + /** + * {@inheritdoc} + */ + public function message() + { + return 'One or more redirects have an invalid URI format.'; + } +} diff --git a/tests/Feature/ClientControllerTest.php b/tests/Feature/ClientControllerTest.php deleted file mode 100644 index 8ce57e4b6..000000000 --- a/tests/Feature/ClientControllerTest.php +++ /dev/null @@ -1,73 +0,0 @@ -create([ - 'email' => 'foo@gmail.com', - 'password' => $this->app->make(Hasher::class)->make('foobar123'), - ]); - - $this->actingAs($user); - - $response = $this->post( - '/oauth/clients', - [ - 'name' => 'new client', - 'redirect_uris' => ['https://localhost'], - ] - ); - - $response->assertSuccessful(); - - $decodedResponse = $response->json(); - - $this->assertEquals(1, $decodedResponse['user_id']); - $this->assertEquals('new client', $decodedResponse['name']); - $this->assertEquals(['https://localhost'], $decodedResponse['redirect_uris']); - $this->assertArrayHasKey('secret', $decodedResponse); - $this->assertArrayHasKey('provider', $decodedResponse); - $this->assertArrayHasKey('revoked', $decodedResponse); - } - - public function testUpdatingClient() - { - $user = UserFactory::new()->create([ - 'email' => 'foo@gmail.com', - 'password' => $this->app->make(Hasher::class)->make('foobar123'), - ]); - - $this->actingAs($user); - - $client = $this->post('/oauth/clients', [ - 'name' => 'new client', 'redirect_uris' => ['https://localhost'], - ])->json(); - - $response = $this->put( - '/oauth/clients/'.$client['id'], - [ - 'name' => 'updated client', - 'redirect_uris' => ['https://localhost'], - ] - ); - - $response->assertSuccessful(); - - $decodedResponse = $response->json(); - - $this->assertEquals(1, $decodedResponse['user_id']); - $this->assertEquals('updated client', $decodedResponse['name']); - $this->assertEquals(['https://localhost'], $decodedResponse['redirect_uris']); - $this->assertArrayHasKey('provider', $decodedResponse); - $this->assertArrayHasKey('revoked', $decodedResponse); - } -} diff --git a/tests/Feature/ClientTest.php b/tests/Feature/ClientTest.php index b1fba3012..e37635f17 100644 --- a/tests/Feature/ClientTest.php +++ b/tests/Feature/ClientTest.php @@ -38,6 +38,22 @@ public function testScopesWhenClientHasScope(): void $this->assertTrue($client->hasScope('foo')); } + public function testScopesWhenColumnDoesNotExist(): void + { + $client = new Client(); + $client->exists = true; + + $this->assertTrue($client->hasScope('foo')); + } + + public function testScopesWhenColumnIsNull(): void + { + $client = new Client(['scopes' => null]); + $client->exists = true; + + $this->assertTrue($client->hasScope('foo')); + } + public function testGrantTypesWhenClientDoesNotHaveGrantType(): void { $client = new Client(['grant_types' => ['bar']]); diff --git a/tests/Feature/PersonalAccessTokenFactoryTest.php b/tests/Feature/PersonalAccessTokenFactoryTest.php index 1900f6c48..b23dfc2c6 100644 --- a/tests/Feature/PersonalAccessTokenFactoryTest.php +++ b/tests/Feature/PersonalAccessTokenFactoryTest.php @@ -25,7 +25,7 @@ public function testIssueToken() ]); /** @var Client $client */ - $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(['scopes' => ['foo', 'bar']]); + $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(); config([ 'passport.personal_access_client.id' => $client->getKey(), diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index bd9aa50d5..1750a0835 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -141,14 +141,6 @@ public function test_client_credentials_grant_is_permitted() $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); } - public function test_client_credentials_grant_is_prevented() - { - $client = $this->clientModelRepository->findActive(1); - $client->secret = null; - - $this->assertFalse($this->repository->validateClient(1, 'secret', 'client_credentials')); - } - public function test_grant_types_allows_request() { $client = $this->clientModelRepository->findActive(1); diff --git a/tests/Unit/BridgeScopeRepositoryTest.php b/tests/Unit/BridgeScopeRepositoryTest.php index 5a727f7ca..fb204ff53 100644 --- a/tests/Unit/BridgeScopeRepositoryTest.php +++ b/tests/Unit/BridgeScopeRepositoryTest.php @@ -25,7 +25,6 @@ public function test_invalid_scopes_are_removed() ]); $client = Mockery::mock(ClientModel::class)->makePartial(); - $client->scopes = ['scope-1']; $clients = Mockery::mock(ClientRepository::class); $clients->shouldReceive('findActive')->withAnyArgs()->andReturn($client); @@ -39,18 +38,16 @@ public function test_invalid_scopes_are_removed() $this->assertEquals([$scope1], $scopes); } - public function test_invalid_scopes_are_removed_with_client_scopes() + public function test_invalid_scopes_are_removed_without_a_client_repository() { Passport::tokensCan([ 'scope-1' => 'description', - 'scope-2' => 'description-2', ]); - $client = Mockery::mock(ClientModel::class)->makePartial(); - $client->scopes = ['scope-1']; - $clients = Mockery::mock(ClientRepository::class); - $clients->shouldReceive('findActive')->with('id')->andReturn($client); + $clients->shouldReceive('findActive') + ->with('id') + ->andReturn(Mockery::mock(ClientModel::class)->makePartial()); $repository = new ScopeRepository($clients); @@ -61,6 +58,28 @@ public function test_invalid_scopes_are_removed_with_client_scopes() $this->assertEquals([$scope1], $scopes); } + public function test_clients_do_not_restrict_scopes_by_default() + { + Passport::tokensCan([ + 'scope-1' => 'description', + 'scope-2' => 'description', + ]); + + $client = Mockery::mock(ClientModel::class)->makePartial(); + $client->scopes = null; + + $clients = Mockery::mock(ClientRepository::class); + $clients->shouldReceive('findActive')->withAnyArgs()->andReturn($client); + + $repository = new ScopeRepository($clients); + + $scopes = $repository->finalizeScopes( + [$scope1 = new Scope('scope-1'), $scope2 = new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + ); + + $this->assertEquals([$scope1, $scope2], $scopes); + } + public function test_scopes_disallowed_for_client_are_removed() { Passport::tokensCan([ diff --git a/tests/Unit/ClientControllerTest.php b/tests/Unit/ClientControllerTest.php index 6acfabe54..e65934b98 100644 --- a/tests/Unit/ClientControllerTest.php +++ b/tests/Unit/ClientControllerTest.php @@ -7,7 +7,7 @@ use Laravel\Passport\Client; use Laravel\Passport\ClientRepository; use Laravel\Passport\Http\Controllers\ClientController; -use Laravel\Passport\Http\Rules\UriRule; +use Laravel\Passport\Http\Rules\RedirectRule; use Mockery as m; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Response; @@ -31,6 +31,7 @@ public function test_all_the_clients_for_the_current_user_can_be_retrieved() $controller = new ClientController( $clients, m::mock(Factory::class), + m::mock(RedirectRule::class) ); $this->assertEquals($client, $controller->forUser($request)); @@ -40,7 +41,7 @@ public function test_clients_can_be_stored() { $clients = m::mock(ClientRepository::class); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); $request->setUserResolver(function () { return new ClientControllerFakeUser; }); @@ -50,20 +51,21 @@ public function test_clients_can_be_stored() ->with('client name', ['http://localhost'], true, 1) ->andReturn($client = new Client); + $redirectRule = m::mock(RedirectRule::class); + $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect_uris' => ['http://localhost'], + 'redirect' => 'http://localhost', ], [ - 'name' => 'required|max:255', - 'redirect_uris' => 'required|array|min:1', - 'redirect_uris.*' => ['required', new UriRule], + 'name' => 'required|max:191', + 'redirect' => ['required', $redirectRule], 'confidential' => 'boolean', ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator + $clients, $validator, $redirectRule ); $this->assertEquals($client, $controller->store($request)); @@ -76,7 +78,7 @@ public function test_public_clients_can_be_stored() $request = Request::create( '/', 'GET', - ['name' => 'client name', 'redirect_uris' => ['http://localhost'], 'confidential' => false] + ['name' => 'client name', 'redirect' => 'http://localhost', 'confidential' => false] ); $request->setUserResolver(function () { return new ClientControllerFakeUser; @@ -87,21 +89,22 @@ public function test_public_clients_can_be_stored() ->with('client name', ['http://localhost'], false, 1) ->andReturn($client = new Client); + $redirectRule = m::mock(RedirectRule::class); + $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect_uris' => ['http://localhost'], + 'redirect' => 'http://localhost', 'confidential' => false, ], [ - 'name' => 'required|max:255', - 'redirect_uris' => 'required|array|min:1', - 'redirect_uris.*' => ['required', new UriRule], + 'name' => 'required|max:191', + 'redirect' => ['required', $redirectRule], 'confidential' => 'boolean', ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator + $clients, $validator, $redirectRule ); $this->assertEquals($client, $controller->store($request)); @@ -113,7 +116,7 @@ public function test_clients_can_be_updated() $client = m::mock(Client::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturn($client); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); $request->setUserResolver(function () { $user = m::mock(); @@ -126,19 +129,20 @@ public function test_clients_can_be_updated() m::type(Client::class), 'client name', ['http://localhost'] )->andReturn('response'); + $redirectRule = m::mock(RedirectRule::class); + $validator = m::mock(Factory::class); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', - 'redirect_uris' => ['http://localhost'], + 'redirect' => 'http://localhost', ], [ - 'name' => 'required|max:255', - 'redirect_uris' => 'required|array|min:1', - 'redirect_uris.*' => ['required', new UriRule], + 'name' => 'required|max:191', + 'redirect' => ['required', $redirectRule], ])->andReturn($validator); $validator->shouldReceive('validate')->once(); $controller = new ClientController( - $clients, $validator + $clients, $validator, $redirectRule ); $this->assertSame('response', $controller->update($request, 1)); @@ -149,7 +153,7 @@ public function test_404_response_if_client_doesnt_belong_to_user() $clients = m::mock(ClientRepository::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturnNull(); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); $request->setUserResolver(function () { $user = m::mock(); @@ -163,7 +167,7 @@ public function test_404_response_if_client_doesnt_belong_to_user() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator + $clients, $validator, m::mock(RedirectRule::class) ); $this->assertSame(404, $controller->update($request, 1)->status()); @@ -175,7 +179,7 @@ public function test_clients_can_be_deleted() $client = m::mock(Client::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturn($client); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); $request->setUserResolver(function () { $user = m::mock(); @@ -191,7 +195,7 @@ public function test_clients_can_be_deleted() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator + $clients, $validator, m::mock(RedirectRule::class) ); $response = $controller->destroy($request, 1); @@ -204,7 +208,7 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() $clients = m::mock(ClientRepository::class); $clients->shouldReceive('findForUser')->with(1, 1)->andReturnNull(); - $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect_uris' => ['http://localhost']]); + $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); $request->setUserResolver(function () { $user = m::mock(); @@ -218,7 +222,7 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() $validator = m::mock(Factory::class); $controller = new ClientController( - $clients, $validator + $clients, $validator, m::mock(RedirectRule::class) ); $this->assertSame(404, $controller->destroy($request, 1)->status()); diff --git a/tests/Unit/PersonalAccessTokenControllerTest.php b/tests/Unit/PersonalAccessTokenControllerTest.php index 4610b826d..0302c7d77 100644 --- a/tests/Unit/PersonalAccessTokenControllerTest.php +++ b/tests/Unit/PersonalAccessTokenControllerTest.php @@ -75,7 +75,7 @@ public function test_tokens_can_be_updated() 'name' => 'token name', 'scopes' => ['user', 'user-admin'], ], [ - 'name' => 'required|max:255', + 'name' => 'required|max:191', 'scopes' => 'array|in:'.implode(',', Passport::scopeIds()), ])->andReturn($validator); $validator->shouldReceive('validate')->once(); diff --git a/tests/Unit/RedirectRuleTest.php b/tests/Unit/RedirectRuleTest.php new file mode 100644 index 000000000..e4c226564 --- /dev/null +++ b/tests/Unit/RedirectRuleTest.php @@ -0,0 +1,49 @@ +rule($fails = false); + + $this->assertTrue($rule->passes('redirect', 'https://example.com')); + } + + public function test_it_passes_with_multiple_valid_urls() + { + $rule = $this->rule($fails = false); + + $this->assertTrue($rule->passes('redirect', 'https://example.com,https://example2.com')); + } + + public function test_it_fails_with_a_single_invalid_url() + { + $rule = $this->rule($fails = true); + + $this->assertFalse($rule->passes('redirect', 'https://example.com,invalid')); + } + + private function rule(bool $fails): RedirectRule + { + $validator = m::mock(Validator::class); + $validator->shouldReceive('fails')->andReturn($fails); + + $factory = m::mock(Factory::class); + $factory->shouldReceive('make')->andReturn($validator); + + return new RedirectRule($factory); + } +} From 7edb0ef2c78e3ad40e06cabd932a2206430f1c87 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 00:30:53 +0330 Subject: [PATCH 11/26] wip --- database/factories/ClientFactory.php | 1 - src/Bridge/Client.php | 2 +- src/ClientRepository.php | 11 +++-------- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index 87e8801cb..6398a48e0 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -33,7 +33,6 @@ public function definition() 'name' => $this->faker->company(), 'secret' => Str::random(40), 'redirect_uris' => [$this->faker->url()], - 'provider' => null, 'grant_types' => [], 'revoked' => false, ]; diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 4a1df7d8f..9a102ac8b 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri);; + $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri); $this->provider = $provider; } } diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 785277a48..6dd7cbea2 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -92,10 +92,10 @@ protected function create( $client = Passport::client()->forceFill([ 'user_id' => $userId, 'name' => $name, + 'secret' => $confidential ? Str::random(40) : null, 'grant_types' => $grantTypes, 'redirect_uris' => $redirectUris, 'provider' => $provider, - 'secret' => $confidential ? Str::random(40) : null, 'revoked' => false, ]); @@ -157,19 +157,14 @@ public function createAuthorizationCodeGrantClient( /** * Update the given client. * - * @param \Laravel\Passport\Client $client - * @param string $name * @param string[] $redirectUris - * @return \Laravel\Passport\Client */ - public function update(Client $client, $name, $redirectUris) + public function update(Client $client, string $name, array $redirectUris): bool { - $client->forceFill([ + return $client->forceFill([ 'name' => $name, 'redirect_uris' => $redirectUris, ])->save(); - - return $client; } /** From 14df618c8c6f47c71a373c3c097878782553ca1c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 00:38:40 +0330 Subject: [PATCH 12/26] wip --- src/Http/Controllers/ClientController.php | 4 +++- tests/Unit/ClientControllerTest.php | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php index bf5ac8564..6cc3f49b0 100644 --- a/src/Http/Controllers/ClientController.php +++ b/src/Http/Controllers/ClientController.php @@ -108,9 +108,11 @@ public function update(Request $request, $clientId) 'redirect' => ['required', $this->redirectRule], ])->validate(); - return $this->clients->update( + $this->clients->update( $client, $request->name, explode(',', $request->redirect) ); + + return $client; } /** diff --git a/tests/Unit/ClientControllerTest.php b/tests/Unit/ClientControllerTest.php index e65934b98..aeff10120 100644 --- a/tests/Unit/ClientControllerTest.php +++ b/tests/Unit/ClientControllerTest.php @@ -126,8 +126,8 @@ public function test_clients_can_be_updated() }); $clients->shouldReceive('update')->once()->with( - m::type(Client::class), 'client name', ['http://localhost'] - )->andReturn('response'); + $client, 'client name', ['http://localhost'] + )->andReturn(true); $redirectRule = m::mock(RedirectRule::class); @@ -145,7 +145,7 @@ public function test_clients_can_be_updated() $clients, $validator, $redirectRule ); - $this->assertSame('response', $controller->update($request, 1)); + $this->assertSame($client, $controller->update($request, 1)); } public function test_404_response_if_client_doesnt_belong_to_user() From 82bc8daf4f1a7a66f3fe6ea952e9a57da89e9669 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 00:51:09 +0330 Subject: [PATCH 13/26] wip --- src/ClientRepository.php | 4 ++-- src/Console/ClientCommand.php | 3 +-- tests/Unit/BridgeClientRepositoryTest.php | 9 --------- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 6dd7cbea2..afa8d678a 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -93,9 +93,9 @@ protected function create( 'user_id' => $userId, 'name' => $name, 'secret' => $confidential ? Str::random(40) : null, - 'grant_types' => $grantTypes, - 'redirect_uris' => $redirectUris, 'provider' => $provider, + 'redirect_uris' => $redirectUris, + 'grant_types' => $grantTypes, 'revoked' => false, ]); diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index 44b06fe62..be342e96a 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -23,8 +23,7 @@ class ClientCommand extends Command {--public : Create a public client (Auth code grant type only) } {--name= : The name of the client} {--provider= : The name of the user provider} - {--redirect_uri= : The URI to redirect to after authorization } - {--user_id= : The user ID the client should be assigned to }'; + {--redirect_uri= : The URI to redirect to after authorization }'; /** * The console command description. diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index 1750a0835..cccafcf4d 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -127,15 +127,6 @@ public function test_personal_access_grant_is_prevented() $this->assertFalse($this->repository->validateClient(1, 'secret', 'personal_access')); } - public function test_public_client_personal_access_grant_is_prevented() - { - $client = $this->clientModelRepository->findActive(1); - $client->personal_access_client = true; - $client->secret = null; - - $this->assertFalse($this->repository->validateClient(1, null, 'personal_access')); - } - public function test_client_credentials_grant_is_permitted() { $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); From baa6395c3ac24716d5a61551ecf0a21015ee1e00 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 01:22:50 +0330 Subject: [PATCH 14/26] wip --- database/factories/ClientFactory.php | 2 +- tests/Unit/BridgeClientRepositoryTest.php | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index 6398a48e0..6791b911e 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -33,7 +33,7 @@ public function definition() 'name' => $this->faker->company(), 'secret' => Str::random(40), 'redirect_uris' => [$this->faker->url()], - 'grant_types' => [], + 'grant_types' => ['authorization_code', 'refresh_token'], 'revoked' => false, ]; } diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index cccafcf4d..c8b599d6e 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -64,11 +64,11 @@ public function test_can_validate_client_for_auth_code_grant() public function test_can_validate_client_for_client_credentials_grant() { $client = $this->clientModelRepository->findActive(1); - $client->grant_types[] = 'personal_access'; + $client->grant_types = ['client_credentials']; $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'client_credentials')); - $this->assertTrue($this->repository->validateClient(1, 'secret', 'authorization_code')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); } public function test_password_grant_is_permitted() @@ -117,7 +117,7 @@ public function test_authorization_code_grant_is_prevented() public function test_personal_access_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->grant_types[] = 'personal_access'; + $client->grant_types = ['personal_access']; $this->assertTrue($this->repository->validateClient(1, 'secret', 'personal_access')); } @@ -127,9 +127,9 @@ public function test_personal_access_grant_is_prevented() $this->assertFalse($this->repository->validateClient(1, 'secret', 'personal_access')); } - public function test_client_credentials_grant_is_permitted() + public function test_client_credentials_grant_is_prevented() { - $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'client_credentials')); } public function test_grant_types_allows_request() @@ -175,7 +175,7 @@ class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client public $secret = '$2y$10$WgqU4wQpfsARCIQk.nPSOOiNkrMpPVxQiLCFUt8comvQwh1z6WFMG'; - public $provider = null; + public $grant_types = ['authorization_code', 'refresh_token']; - public $grant_types = ['authorization_code', 'client_credentials', 'refresh_token']; + public $provider = null; } From a579f4ff9591833025f39b1a63eb80012aeb90e7 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 19:07:23 +0330 Subject: [PATCH 15/26] wip --- src/Bridge/Client.php | 4 ++-- src/Console/ClientCommand.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 9a102ac8b..df4255957 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -21,7 +21,7 @@ class Client implements ClientEntityInterface public function __construct( string $identifier, string $name, - string|array $redirectUri, + array $redirectUri, bool $isConfidential = false, ?string $provider = null ) { @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri); + $this->redirectUri = $redirectUri; $this->provider = $provider; } } diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index be342e96a..a702e1e59 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -20,10 +20,10 @@ class ClientCommand extends Command {--password : Create a password grant client} {--client : Create a client credentials grant client} {--implicit : Create an implicit grant client} - {--public : Create a public client (Auth code grant type only) } {--name= : The name of the client} {--provider= : The name of the user provider} - {--redirect_uri= : The URI to redirect to after authorization }'; + {--redirect_uri= : The URI to redirect to after authorization } + {--public : Create a public client (Auth code grant type only) }'; /** * The console command description. From 5c7944a1545b2177183d9d28c5432aef62024ab5 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 19:22:53 +0330 Subject: [PATCH 16/26] wip --- UPGRADE.md | 10 +++++----- src/Bridge/Client.php | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index c283763b6..481320ce5 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -77,13 +77,13 @@ Schema::table('oauth_clients', function (Blueprint $table) { foreach (Passport::client()->cursor() as $client) { Model::withoutTimestamps(fn () => $client->forceFill([ + 'redirect_uris' => explode(',', $client->redirect), 'grant_types' => match (true) { - (bool) $client->personal_access_client => ['personal_access'], - (bool) $client->password_client => ['password', 'refresh_token'], - ! empty($client->secret) && empty($client->redirect) => ['client_credentials'], - default => ['authorization_code', 'implicit', 'refresh_token'], + $client->personal_access_client => ['personal_access'], + $client->password_client => ['password', 'refresh_token'], + $client->secret && ! $client->redirect => ['client_credentials'], + default => ['authorization_code', 'refresh_token', 'implicit'], }, - 'redirect_uris' => explode(',', $client->redirect), ])->save()); } diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index df4255957..9a102ac8b 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -21,7 +21,7 @@ class Client implements ClientEntityInterface public function __construct( string $identifier, string $name, - array $redirectUri, + string|array $redirectUri, bool $isConfidential = false, ?string $provider = null ) { @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = $redirectUri; + $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri); $this->provider = $provider; } } From 1149e9c53d9c831324edc1ee10cd3eda580fc042 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 19:31:31 +0330 Subject: [PATCH 17/26] wip --- src/Bridge/Client.php | 4 ++-- tests/Unit/BridgeAccessTokenRepositoryTest.php | 4 ++-- tests/Unit/BridgeScopeRepositoryTest.php | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 9a102ac8b..df4255957 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -21,7 +21,7 @@ class Client implements ClientEntityInterface public function __construct( string $identifier, string $name, - string|array $redirectUri, + array $redirectUri, bool $isConfidential = false, ?string $provider = null ) { @@ -29,7 +29,7 @@ public function __construct( $this->name = $name; $this->isConfidential = $isConfidential; - $this->redirectUri = is_array($redirectUri) ? $redirectUri : explode(',', $redirectUri); + $this->redirectUri = $redirectUri; $this->provider = $provider; } } diff --git a/tests/Unit/BridgeAccessTokenRepositoryTest.php b/tests/Unit/BridgeAccessTokenRepositoryTest.php index f8049d188..6f64b658a 100644 --- a/tests/Unit/BridgeAccessTokenRepositoryTest.php +++ b/tests/Unit/BridgeAccessTokenRepositoryTest.php @@ -40,7 +40,7 @@ public function test_access_tokens_can_be_persisted() $events->shouldReceive('dispatch')->once(); - $accessToken = new AccessToken(2, [new Scope('scopes')], new Client('client-id', 'name', 'redirect')); + $accessToken = new AccessToken(2, [new Scope('scopes')], new Client('client-id', 'name', ['redirect'])); $accessToken->setIdentifier(1); $accessToken->setExpiryDateTime($expiration); @@ -54,7 +54,7 @@ public function test_can_get_new_access_token() $tokenRepository = m::mock(TokenRepository::class); $events = m::mock(Dispatcher::class); $repository = new AccessTokenRepository($tokenRepository, $events); - $client = new Client('client-id', 'name', 'redirect'); + $client = new Client('client-id', 'name', ['redirect']); $scopes = [new Scope('place-orders'), new Scope('check-status')]; $userIdentifier = 123; diff --git a/tests/Unit/BridgeScopeRepositoryTest.php b/tests/Unit/BridgeScopeRepositoryTest.php index fb204ff53..8b204935d 100644 --- a/tests/Unit/BridgeScopeRepositoryTest.php +++ b/tests/Unit/BridgeScopeRepositoryTest.php @@ -32,7 +32,7 @@ public function test_invalid_scopes_are_removed() $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([$scope1], $scopes); @@ -52,7 +52,7 @@ public function test_invalid_scopes_are_removed_without_a_client_repository() $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([$scope1], $scopes); @@ -74,7 +74,7 @@ public function test_clients_do_not_restrict_scopes_by_default() $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), $scope2 = new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1'), $scope2 = new Scope('scope-2')], 'client_credentials', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([$scope1, $scope2], $scopes); @@ -96,7 +96,7 @@ public function test_scopes_disallowed_for_client_are_removed() $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([$scope1], $scopes); @@ -121,7 +121,7 @@ public function test_scopes_disallowed_for_client_are_removed_but_inherited_scop $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('scope-1:limited-access'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('scope-1:limited-access'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([$scope1], $scopes); @@ -141,7 +141,7 @@ public function test_superuser_scope_cant_be_applied_if_wrong_grant() $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('*')], 'refresh_token', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('*')], 'refresh_token', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([], $scopes); @@ -161,7 +161,7 @@ public function test_superuser_scope_cant_be_applied_if_wrong_grant_without_a_cl $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( - [$scope1 = new Scope('*')], 'refresh_token', new Client('id', 'name', 'http://localhost'), 1 + [$scope1 = new Scope('*')], 'refresh_token', new Client('id', 'name', ['http://localhost']), 1 ); $this->assertEquals([], $scopes); From 405d5596721f9e8287586e7cddd8c9ce7a4b3fd4 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 20:07:10 +0330 Subject: [PATCH 18/26] formatting --- src/Console/ClientCommand.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index a702e1e59..7a79f8383 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -169,9 +169,7 @@ protected function createAuthCodeClient(ClientRepository $clients) ); $client = $clients->createAuthorizationCodeGrantClient( - $name, - explode(',', $redirect), - ! $this->option('public'), + $name, explode(',', $redirect), ! $this->option('public'), ); $this->components->info('New client created successfully.'); From 1fc5d18d7a23813dcd5cf10e5e29426a9a1df6a3 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 20:13:50 +0330 Subject: [PATCH 19/26] formatting --- src/Console/ClientCommand.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index 7a79f8383..ef06d04b0 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -69,6 +69,7 @@ protected function createPersonalAccessClient(ClientRepository $clients) $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', array_keys(config('auth.providers')), + config('auth.guards.api.provider') ); $client = $clients->createPersonalAccessGrantClient($name, $provider); @@ -98,6 +99,7 @@ protected function createPasswordClient(ClientRepository $clients) $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', array_keys(config('auth.providers')), + config('auth.guards.api.provider') ); $client = $clients->createPasswordGrantClient($name, $provider); From 407082f0b1f25bec3e2cbbad7a6ef9cbf7290588 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 30 Jul 2024 20:17:14 +0330 Subject: [PATCH 20/26] formatting --- src/Console/ClientCommand.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index ef06d04b0..a783dedb3 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -138,7 +138,8 @@ protected function createClientCredentialsClient(ClientRepository $clients) protected function createImplicitClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( - 'What should we name the client?' + 'What should we name the client?', + config('app.name').' Implicit Grant Client' ); $redirect = $this->option('redirect_uri') ?: $this->ask( @@ -162,7 +163,8 @@ protected function createImplicitClient(ClientRepository $clients) protected function createAuthCodeClient(ClientRepository $clients) { $name = $this->option('name') ?: $this->ask( - 'What should we name the client?' + 'What should we name the client?', + config('app.name') ); $redirect = $this->option('redirect_uri') ?: $this->ask( From dfdec662d02067c185140cf49d81be50bbbbbf2d Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 2 Aug 2024 06:32:41 +0330 Subject: [PATCH 21/26] wip --- UPGRADE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.md b/UPGRADE.md index 481320ce5..6b89986ea 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -77,7 +77,7 @@ Schema::table('oauth_clients', function (Blueprint $table) { foreach (Passport::client()->cursor() as $client) { Model::withoutTimestamps(fn () => $client->forceFill([ - 'redirect_uris' => explode(',', $client->redirect), + 'redirect_uris' => $client->redirect ? explode(',', $client->redirect) : [], 'grant_types' => match (true) { $client->personal_access_client => ['personal_access'], $client->password_client => ['password', 'refresh_token'], From 72358906fae84be75f831cdd9b100de46701c60a Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 4 Aug 2024 11:17:01 +0330 Subject: [PATCH 22/26] revert unrelated changes --- src/Http/Rules/UriRule.php | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Http/Rules/UriRule.php b/src/Http/Rules/UriRule.php index 8fea7e82a..f6d02c65e 100644 --- a/src/Http/Rules/UriRule.php +++ b/src/Http/Rules/UriRule.php @@ -2,18 +2,27 @@ namespace Laravel\Passport\Http\Rules; -use Closure; -use Illuminate\Contracts\Validation\ValidationRule; +use Illuminate\Contracts\Validation\Rule; -class UriRule implements ValidationRule +class UriRule implements Rule { /** * {@inheritdoc} */ - public function validate(string $attribute, mixed $value, Closure $fail): void + public function passes($attribute, $value): bool { - if (! filter_var($value, FILTER_VALIDATE_URL)) { - $fail('The :attribute must be valid URI.'); + if (filter_var($value, FILTER_VALIDATE_URL)) { + return true; } + + return false; + } + + /** + * {@inheritdoc} + */ + public function message(): string + { + return 'The :attribute must be valid URI.'; } } From d3136c499144bacaa7f8e5c6bb40860c2ba472e7 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 4 Aug 2024 11:17:37 +0330 Subject: [PATCH 23/26] fix backward compatibility --- src/Client.php | 31 +++++++++++++++++- src/ClientRepository.php | 38 +++++++++++++++-------- src/Http/Controllers/ClientController.php | 2 +- tests/Unit/BridgeClientRepositoryTest.php | 19 +++++------- tests/Unit/ClientControllerTest.php | 16 +++++----- 5 files changed, 71 insertions(+), 35 deletions(-) diff --git a/src/Client.php b/src/Client.php index 9c80a4c37..ab2bc27dc 100644 --- a/src/Client.php +++ b/src/Client.php @@ -2,6 +2,7 @@ namespace Laravel\Passport; +use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Hash; @@ -45,6 +46,8 @@ class Client extends Model 'grant_types' => 'array', 'scopes' => 'array', 'redirect_uris' => 'array', + 'personal_access_client' => 'bool', + 'password_client' => 'bool', 'revoked' => 'bool', ]; @@ -131,6 +134,22 @@ public function setSecretAttribute($value) $this->attributes['secret'] = is_null($value) ? $value : Hash::make($value); } + /** + * Get the client's redirect URIs. + */ + protected function redirectUris(): Attribute + { + return Attribute::make( + get: function (?string $value, array $attributes) { + if (isset($value)) { + return $this->fromJson($value); + } + + return empty($attributes['redirect']) ? [] : explode(',', $attributes['redirect']); + }, + ); + } + /** * Determine if the client is a "first party" client. * @@ -159,7 +178,17 @@ public function skipsAuthorization() */ public function hasGrantType($grantType) { - return in_array($grantType, $this->grant_types); + if (isset($this->attributes['grant_types']) && is_array($this->grant_types)) { + return in_array($grantType, $this->grant_types); + } + + return match ($grantType) { + 'authorization_code' => ! $this->personal_access_client && ! $this->password_client, + 'personal_access' => $this->personal_access_client && $this->confidential(), + 'password' => $this->password_client, + 'client_credentials' => $this->confidential(), + default => true, + }; } /** diff --git a/src/ClientRepository.php b/src/ClientRepository.php index afa8d678a..e945bcc1d 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -2,6 +2,7 @@ namespace Laravel\Passport; +use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Support\Str; class ClientRepository @@ -86,22 +87,33 @@ protected function create( array $grantTypes, array $redirectUris = [], ?string $provider = null, - ?string $userId = null, - bool $confidential = true + bool $confidential = true, + ?Authenticatable $user = null ): Client { - $client = Passport::client()->forceFill([ - 'user_id' => $userId, + $client = Passport::client(); + $columns = $client->getConnection()->getSchemaBuilder()->getColumnListing($client->getTable()); + + $attributes = [ 'name' => $name, 'secret' => $confidential ? Str::random(40) : null, 'provider' => $provider, - 'redirect_uris' => $redirectUris, - 'grant_types' => $grantTypes, 'revoked' => false, - ]); - - $client->save(); - - return $client; + ...(in_array('redirect_uris', $columns) ? [ + 'redirect_uris' => $redirectUris + ] : [ + 'redirect' => implode(',', $redirectUris) + ]), + ...(in_array('grant_types', $columns) ? [ + 'grant_types' => $grantTypes + ] : [ + 'personal_access_client' => in_array('personal_access', $grantTypes), + 'password_client' => in_array('password', $grantTypes), + ]), + ]; + + return $user + ? $user->clients()->forceCreate($attributes) + : $client->forceCreate($attributes); } /** @@ -147,10 +159,10 @@ public function createAuthorizationCodeGrantClient( string $name, array $redirectUris, bool $confidential = true, - ?string $userId = null + ?Authenticatable $user = null ): Client { return $this->create( - $name, ['authorization_code', 'refresh_token'], $redirectUris, null, $userId, $confidential + $name, ['authorization_code', 'refresh_token'], $redirectUris, null, $confidential, $user ); } diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php index 6cc3f49b0..ca9117f2b 100644 --- a/src/Http/Controllers/ClientController.php +++ b/src/Http/Controllers/ClientController.php @@ -80,7 +80,7 @@ public function store(Request $request) $request->name, explode(',', $request->redirect), (bool) $request->input('confidential', true), - $request->user()->getAuthIdentifier(), + $request->user(), ); $client->secret = $client->plainSecret; diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index c8b599d6e..8d04017e3 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -74,7 +74,7 @@ public function test_can_validate_client_for_client_credentials_grant() public function test_password_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->grant_types[] = 'password'; + $client->grant_types = ['password']; $this->assertTrue($this->repository->validateClient(1, 'secret', 'password')); } @@ -82,7 +82,7 @@ public function test_password_grant_is_permitted() public function test_public_client_password_grant_is_permitted() { $client = $this->clientModelRepository->findActive(1); - $client->grant_types[] = 'password'; + $client->grant_types = ['password']; $client->secret = null; $this->assertTrue($this->repository->validateClient(1, null, 'password')); @@ -169,13 +169,10 @@ public function test_refresh_grant_is_prevented() class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client { - public $name = 'Client'; - - public $redirect_uris = ['http://localhost']; - - public $secret = '$2y$10$WgqU4wQpfsARCIQk.nPSOOiNkrMpPVxQiLCFUt8comvQwh1z6WFMG'; - - public $grant_types = ['authorization_code', 'refresh_token']; - - public $provider = null; + protected $attributes = [ + 'name' => 'Client', + 'redirect_uris' => '["http://localhost"]', + 'secret' => '$2y$10$WgqU4wQpfsARCIQk.nPSOOiNkrMpPVxQiLCFUt8comvQwh1z6WFMG', + 'grant_types' => '["authorization_code","refresh_token"]', + ]; } diff --git a/tests/Unit/ClientControllerTest.php b/tests/Unit/ClientControllerTest.php index aeff10120..66eba0c32 100644 --- a/tests/Unit/ClientControllerTest.php +++ b/tests/Unit/ClientControllerTest.php @@ -40,15 +40,14 @@ public function test_all_the_clients_for_the_current_user_can_be_retrieved() public function test_clients_can_be_stored() { $clients = m::mock(ClientRepository::class); + $user = new ClientControllerFakeUser; $request = Request::create('/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost']); - $request->setUserResolver(function () { - return new ClientControllerFakeUser; - }); + $request->setUserResolver(fn () => $user); $clients->shouldReceive('createAuthorizationCodeGrantClient') ->once() - ->with('client name', ['http://localhost'], true, 1) + ->with('client name', ['http://localhost'], true, $user) ->andReturn($client = new Client); $redirectRule = m::mock(RedirectRule::class); @@ -74,19 +73,18 @@ public function test_clients_can_be_stored() public function test_public_clients_can_be_stored() { $clients = m::mock(ClientRepository::class); + $user = new ClientControllerFakeUser; $request = Request::create( '/', 'GET', ['name' => 'client name', 'redirect' => 'http://localhost', 'confidential' => false] ); - $request->setUserResolver(function () { - return new ClientControllerFakeUser; - }); + $request->setUserResolver(fn () => $user); $clients->shouldReceive('createAuthorizationCodeGrantClient') ->once() - ->with('client name', ['http://localhost'], false, 1) + ->with('client name', ['http://localhost'], false, $user) ->andReturn($client = new Client); $redirectRule = m::mock(RedirectRule::class); @@ -229,7 +227,7 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() } } -class ClientControllerFakeUser +class ClientControllerFakeUser extends \Illuminate\Foundation\Auth\User { public $id = 1; From 6aeb388f3ded9ca0280ef871c58794f17a8c2ede Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 4 Aug 2024 11:22:59 +0330 Subject: [PATCH 24/26] formatting --- UPGRADE.md | 31 ------------------------------- src/ClientRepository.php | 6 +++--- 2 files changed, 3 insertions(+), 34 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 6b89986ea..0421175e6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -61,37 +61,6 @@ Passport's `oauth_personal_access_clients` table has been redundant and unnecess In addition, the `Laravel\Passport\PersonalAccessClient` model, `Passport::$personalAccessClientModel` property, `Passport::usePersonalAccessClientModel()`, `Passport::personalAccessClientModel()`, and `Passport::personalAccessClient()` methods have been removed. -### Clients Table - -PR: https://github.com/laravel/passport/pull/1744 - -The `oauth_clients` table now requires `grant_types` and `redirect_uris` columns as JSON array and `redirect`, `personal_access_client`, and `password_client` columns are removed: - -```php -Schema::table('oauth_clients', function (Blueprint $table) { - $table->after('provider', function (Blueprint $table) { - $table->text('redirect_uris'); - $table->text('grant_types'); - }); -}); - -foreach (Passport::client()->cursor() as $client) { - Model::withoutTimestamps(fn () => $client->forceFill([ - 'redirect_uris' => $client->redirect ? explode(',', $client->redirect) : [], - 'grant_types' => match (true) { - $client->personal_access_client => ['personal_access'], - $client->password_client => ['password', 'refresh_token'], - $client->secret && ! $client->redirect => ['client_credentials'], - default => ['authorization_code', 'refresh_token', 'implicit'], - }, - ])->save()); -} - -Schema::table('oauth_clients', function (Blueprint $table) { - $table->dropColumn(['redirect', 'personal_access_client', 'password_client']); -}); -``` - ## Upgrading To 12.0 From 11.x ### Migration Changes diff --git a/src/ClientRepository.php b/src/ClientRepository.php index e945bcc1d..4efaf68f5 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -99,12 +99,12 @@ protected function create( 'provider' => $provider, 'revoked' => false, ...(in_array('redirect_uris', $columns) ? [ - 'redirect_uris' => $redirectUris + 'redirect_uris' => $redirectUris, ] : [ - 'redirect' => implode(',', $redirectUris) + 'redirect' => implode(',', $redirectUris), ]), ...(in_array('grant_types', $columns) ? [ - 'grant_types' => $grantTypes + 'grant_types' => $grantTypes, ] : [ 'personal_access_client' => in_array('personal_access', $grantTypes), 'password_client' => in_array('password', $grantTypes), From 86694ee83aa1d40b686f760a3f41bcf98ec4f7b9 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 4 Aug 2024 12:12:20 +0330 Subject: [PATCH 25/26] fix bc on update --- src/ClientRepository.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 4efaf68f5..61ecab47d 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -173,9 +173,15 @@ public function createAuthorizationCodeGrantClient( */ public function update(Client $client, string $name, array $redirectUris): bool { + $columns = $client->getConnection()->getSchemaBuilder()->getColumnListing($client->getTable()); + return $client->forceFill([ 'name' => $name, - 'redirect_uris' => $redirectUris, + ...(in_array('redirect_uris', $columns) ? [ + 'redirect_uris' => $redirectUris, + ] : [ + 'redirect' => implode(',', $redirectUris), + ]), ])->save(); } From bf7ce0d0710799d8f2f91118295420c112ede932 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 4 Aug 2024 13:32:13 +0330 Subject: [PATCH 26/26] add tests for clients without grant_types --- tests/Feature/ClientTest.php | 47 +++++++++++++++++++++++ tests/Unit/BridgeClientRepositoryTest.php | 40 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/tests/Feature/ClientTest.php b/tests/Feature/ClientTest.php index e37635f17..fbaac9168 100644 --- a/tests/Feature/ClientTest.php +++ b/tests/Feature/ClientTest.php @@ -69,4 +69,51 @@ public function testGrantTypesWhenClientHasGrantType(): void $this->assertTrue($client->hasGrantType('foo')); } + + public function testGrantTypesWhenColumnDoesNotExist(): void + { + $client = new Client(); + $client->exists = true; + + $this->assertTrue($client->hasGrantType('foo')); + + $client->personal_access_client = false; + $client->password_client = false; + + $this->assertTrue($client->hasGrantType('authorization_code')); + + $client->personal_access_client = false; + $client->password_client = true; + + $this->assertTrue($client->hasGrantType('password')); + + $client->personal_access_client = true; + $client->password_client = false; + $client->secret = 'secret'; + + $this->assertTrue($client->hasGrantType('personal_access')); + $this->assertTrue($client->hasGrantType('client_credentials')); + } + + public function testGrantTypesWhenColumnIsNull(): void + { + $client = new Client(['grant_types' => null]); + $client->exists = true; + + $this->assertTrue($client->hasGrantType('foo')); + + $client->personal_access_client = false; + $client->password_client = false; + $this->assertTrue($client->hasGrantType('authorization_code')); + + $client->personal_access_client = false; + $client->password_client = true; + $this->assertTrue($client->hasGrantType('password')); + + $client->personal_access_client = true; + $client->password_client = false; + $client->secret = 'secret'; + $this->assertTrue($client->hasGrantType('personal_access')); + $this->assertTrue($client->hasGrantType('client_credentials')); + } } diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index 8d04017e3..12a0477c9 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -165,6 +165,46 @@ public function test_refresh_grant_is_prevented() { $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'refresh_token')); } + + public function test_without_grant_types() + { + $client = $this->clientModelRepository->findActive(1); + $client->grant_types = null; + + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'client_credentials')); + + $client->personal_access_client = true; + $client->password_client = false; + + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'personal_access')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'personal_access')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'password')); + + $client->personal_access_client = false; + $client->password_client = true; + + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'password')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'password')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'personal_access')); + + $client->personal_access_client = false; + $client->password_client = true; + $client->secret = null; + + $this->assertFalse($this->repository->validateClient(1, null, 'client_credentials')); + $this->assertTrue($this->repository->validateClient(1, null, 'password')); + + $client->personal_access_client = true; + $client->password_client = false; + $client->secret = null; + + $this->assertFalse($this->repository->validateClient(1, null, 'personal_access')); + } } class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client