From 2336ce20c85a1a80bd0894a6e5173e16a339a9f4 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 22 Apr 2024 17:25:34 +0200 Subject: [PATCH] Raise PHPStan level to max and allow multiple mailers (#23) * Work towards PHPStan level max * Handle invalid configuration * Fix tests on older pest versions --- .github/workflows/phpstan.yml | 5 +- phpstan.neon.dist | 3 +- src/Exceptions/ConfigurationInvalid.php | 14 ++ src/Exceptions/ConfigurationMissing.php | 19 +-- src/Exceptions/InvalidResponse.php | 9 ++ src/LaravelMsGraphMailServiceProvider.php | 47 ++++--- src/MicrosoftGraphTransport.php | 4 +- src/Services/MicrosoftGraphApiService.php | 9 +- tests/MicrosoftGraphTransportTest.php | 160 +++++++++++++++------- 9 files changed, 183 insertions(+), 87 deletions(-) create mode 100644 src/Exceptions/ConfigurationInvalid.php create mode 100644 src/Exceptions/InvalidResponse.php diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 3855a08..c5a1d08 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -11,7 +11,8 @@ jobs: name: phpstan runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Checkout code + uses: actions/checkout@v4 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -23,4 +24,4 @@ jobs: uses: ramsey/composer-install@v3 - name: Run PHPStan - run: ./vendor/bin/phpstan --error-format=github + run: vendor/bin/phpstan --error-format=github diff --git a/phpstan.neon.dist b/phpstan.neon.dist index cef36aa..9aa5399 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,7 +2,7 @@ includes: - phpstan-baseline.neon parameters: - level: 8 + level: max paths: - src tmpDir: build/phpstan @@ -10,4 +10,3 @@ parameters: checkModelProperties: true checkMissingIterableValueType: false checkGenericClassInNonGenericObjectType: false - diff --git a/src/Exceptions/ConfigurationInvalid.php b/src/Exceptions/ConfigurationInvalid.php new file mode 100644 index 0000000..1359315 --- /dev/null +++ b/src/Exceptions/ConfigurationInvalid.php @@ -0,0 +1,14 @@ +app->bind(MicrosoftGraphApiService::class, function () { - //throw exceptions when config is missing - throw_unless(filled(config('mail.mailers.microsoft-graph.tenant_id')), ConfigurationMissing::tenantId()); - throw_unless(filled(config('mail.mailers.microsoft-graph.client_id')), ConfigurationMissing::clientId()); - throw_unless(filled(config('mail.mailers.microsoft-graph.client_secret')), ConfigurationMissing::clientSecret()); - - return new MicrosoftGraphApiService( - tenantId: config('mail.mailers.microsoft-graph.tenant_id', ''), - clientId: config('mail.mailers.microsoft-graph.client_id', ''), - clientSecret: config('mail.mailers.microsoft-graph.client_secret', ''), - accessTokenTtl: config('mail.mailers.microsoft-graph.access_token_ttl', 3000), - ); - }); + Mail::extend('microsoft-graph', function (array $config): MicrosoftGraphTransport { + throw_if(blank($config['from']['address'] ?? []), new ConfigurationMissing('from.address')); - Mail::extend('microsoft-graph', function (array $config) { - throw_unless(filled($config['from']['address'] ?? []), ConfigurationMissing::fromAddress()); + $accessTokenTtl = $config['access_token_ttl'] ?? 3000; + if (! is_int($accessTokenTtl)) { + throw new ConfigurationInvalid('access_token_ttl', $accessTokenTtl); + } return new MicrosoftGraphTransport( - $this->app->make(MicrosoftGraphApiService::class) + new MicrosoftGraphApiService( + tenantId: $this->requireConfigString($config, 'tenant_id'), + clientId: $this->requireConfigString($config, 'client_id'), + clientSecret: $this->requireConfigString($config, 'client_secret'), + accessTokenTtl: $accessTokenTtl, + ), ); }); } + + /** + * @param array $config + * @return non-empty-string + */ + protected function requireConfigString(array $config, string $key): string + { + if (! array_key_exists($key, $config)) { + throw new ConfigurationMissing($key); + } + + $value = $config[$key]; + if (! is_string($value) || $value === '') { + throw new ConfigurationInvalid($key, $value); + } + + return $value; + } } diff --git a/src/MicrosoftGraphTransport.php b/src/MicrosoftGraphTransport.php index d232b02..9264699 100644 --- a/src/MicrosoftGraphTransport.php +++ b/src/MicrosoftGraphTransport.php @@ -82,7 +82,7 @@ protected function prepareAttachments(Email $email, ?string $html): array } /** - * @param Collection
$recipients + * @param Collection $recipients */ protected function transformEmailAddresses(Collection $recipients): array { @@ -101,7 +101,7 @@ protected function transformEmailAddress(Address $address): array } /** - * @return Collection
+ * @return Collection */ protected function getRecipients(Email $email, Envelope $envelope): Collection { diff --git a/src/Services/MicrosoftGraphApiService.php b/src/Services/MicrosoftGraphApiService.php index b6574ae..17e0c69 100644 --- a/src/Services/MicrosoftGraphApiService.php +++ b/src/Services/MicrosoftGraphApiService.php @@ -7,6 +7,7 @@ use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; +use InnoGE\LaravelMsGraphMail\Exceptions\InvalidResponse; class MicrosoftGraphApiService { @@ -48,7 +49,13 @@ protected function getAccessToken(): string $response->throw(); - return $response->json('access_token'); + $accessToken = $response->json('access_token'); + if (! is_string($accessToken)) { + $notString = var_export($accessToken, true); + throw new InvalidResponse("Expected response to contain key access_token of type string, got: {$notString}."); + } + + return $accessToken; }); } } diff --git a/tests/MicrosoftGraphTransportTest.php b/tests/MicrosoftGraphTransportTest.php index dc26213..259d7bd 100644 --- a/tests/MicrosoftGraphTransportTest.php +++ b/tests/MicrosoftGraphTransportTest.php @@ -6,7 +6,9 @@ use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Mail; use Illuminate\Support\Str; +use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationInvalid; use InnoGE\LaravelMsGraphMail\Exceptions\ConfigurationMissing; +use InnoGE\LaravelMsGraphMail\Exceptions\InvalidResponse; use InnoGE\LaravelMsGraphMail\Tests\Stubs\TestMail; use InnoGE\LaravelMsGraphMail\Tests\Stubs\TestMailWithInlineImage; @@ -219,69 +221,133 @@ ->toBe('foo_access_token'); }); -it('throws exceptions when config is missing', function (array $config, string $exceptionMessage) { +it('throws exceptions on invalid access token in response', function () { + Config::set('mail.mailers.microsoft-graph', [ + 'transport' => 'microsoft-graph', + 'client_id' => 'foo_client_id', + 'client_secret' => 'foo_client_secret', + 'tenant_id' => 'foo_tenant_id', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', + ], + ]); + Config::set('mail.default', 'microsoft-graph'); + + Http::fake([ + 'https://login.microsoftonline.com/foo_tenant_id/oauth2/v2.0/token' => Http::response(['access_token' => 123]), + ]); + + expect(fn () => Mail::to('caleb@livewire.com')->send(new TestMail(false))) + ->toThrow(InvalidResponse::class, 'Expected response to contain key access_token of type string, got: 123.'); +}); + +it('throws exceptions when config is invalid', function (array $config, Exception $exception) { Config::set('mail.mailers.microsoft-graph', $config); Config::set('mail.default', 'microsoft-graph'); - try { - Mail::to('caleb@livewire.com') - ->send(new TestMail(false)); - } catch (Exception $e) { - expect($e) - ->toBeInstanceOf(ConfigurationMissing::class) - ->getMessage()->toBe($exceptionMessage); - } -})->with( + expect(fn () => Mail::to('caleb@livewire.com')->send(new TestMail(false))) + ->toThrow(get_class($exception), $exception->getMessage()); +})->with([ [ [ - [ - 'transport' => 'microsoft-graph', - 'client_id' => 'foo_client_id', - 'client_secret' => 'foo_client_secret', - 'tenant_id' => '', - 'from' => [ - 'address' => 'taylor@laravel.com', - 'name' => 'Taylor Otwell', - ], + 'transport' => 'microsoft-graph', + 'client_id' => 'foo_client_id', + 'client_secret' => 'foo_client_secret', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', ], - 'The tenant id is missing from the configuration file.', ], + new ConfigurationMissing('tenant_id'), + ], + [ [ - [ - 'transport' => 'microsoft-graph', - 'client_id' => '', - 'client_secret' => 'foo_client_secret', - 'tenant_id' => 'foo_tenant_id', - 'from' => [ - 'address' => 'taylor@laravel.com', - 'name' => 'Taylor Otwell', - ], + 'transport' => 'microsoft-graph', + 'tenant_id' => 123, + 'client_id' => 'foo_client_id', + 'client_secret' => 'foo_client_secret', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', ], - 'The client id is missing from the configuration file.', ], + new ConfigurationInvalid('tenant_id', 123), + ], + [ [ - [ - 'transport' => 'microsoft-graph', - 'client_id' => 'foo_client_id', - 'client_secret' => '', - 'tenant_id' => 'foo_tenant_id', - 'from' => [ - 'address' => 'taylor@laravel.com', - 'name' => 'Taylor Otwell', - ], + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_secret' => 'foo_client_secret', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', ], - 'The client secret is missing from the configuration file.', ], + new ConfigurationMissing('client_id'), + ], + [ [ - [ - 'transport' => 'microsoft-graph', - 'client_id' => 'foo_client_id', - 'client_secret' => 'foo_client_secret', - 'tenant_id' => 'foo_tenant_id', + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_id' => '', + 'client_secret' => 'foo_client_secret', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', ], - 'The mail from address is missing from the configuration file.', ], - ]); + new ConfigurationInvalid('client_id', ''), + ], + [ + [ + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_id' => 'foo_client_id', + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', + ], + ], + new ConfigurationMissing('client_secret'), + ], + [ + [ + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_id' => 'foo_client_id', + 'client_secret' => null, + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', + ], + ], + new ConfigurationInvalid('client_secret', null), + ], + [ + [ + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_id' => 'foo_client_id', + 'client_secret' => 'foo_client_secret', + ], + new ConfigurationMissing('from.address'), + ], + [ + [ + 'transport' => 'microsoft-graph', + 'tenant_id' => 'foo_tenant_id', + 'client_id' => 'foo_client_id', + 'client_secret' => 'foo_client_secret', + 'access_token_ttl' => false, + 'from' => [ + 'address' => 'taylor@laravel.com', + 'name' => 'Taylor Otwell', + ], + ], + new ConfigurationInvalid('access_token_ttl', false), + ], +]); it('sends html mails with inline images with microsoft graph', function () { Config::set('mail.mailers.microsoft-graph', [