diff --git a/app/Access/Oidc/OidcOAuthProvider.php b/app/Access/Oidc/OidcOAuthProvider.php index 2ed8cd5c9..d2dc829b7 100644 --- a/app/Access/Oidc/OidcOAuthProvider.php +++ b/app/Access/Oidc/OidcOAuthProvider.php @@ -20,15 +20,8 @@ class OidcOAuthProvider extends AbstractProvider { use BearerAuthorizationTrait; - /** - * @var string - */ - protected $authorizationEndpoint; - - /** - * @var string - */ - protected $tokenEndpoint; + protected string $authorizationEndpoint; + protected string $tokenEndpoint; /** * Scopes to use for the OIDC authorization call. @@ -60,7 +53,7 @@ class OidcOAuthProvider extends AbstractProvider } /** - * Add an additional scope to this provider upon the default. + * Add another scope to this provider upon the default. */ public function addScope(string $scope): void { diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php index 9c8b1b264..fa3f579b1 100644 --- a/app/Access/Oidc/OidcProviderSettings.php +++ b/app/Access/Oidc/OidcProviderSettings.php @@ -59,7 +59,7 @@ class OidcProviderSettings } } - if (strpos($this->issuer, 'https://') !== 0) { + if (!str_starts_with($this->issuer, 'https://')) { throw new InvalidArgumentException('Issuer value must start with https://'); } } diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php index 6d13fe8f1..d22b26eec 100644 --- a/app/Access/Oidc/OidcService.php +++ b/app/Access/Oidc/OidcService.php @@ -9,13 +9,13 @@ use BookStack\Exceptions\JsonDebugException; use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Exceptions\UserRegistrationException; use BookStack\Facades\Theme; +use BookStack\Http\HttpRequestService; use BookStack\Theming\ThemeEvents; use BookStack\Users\Models\User; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Cache; use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider; use League\OAuth2\Client\Provider\Exception\IdentityProviderException; -use Psr\Http\Client\ClientInterface as HttpClient; /** * Class OpenIdConnectService @@ -26,7 +26,7 @@ class OidcService public function __construct( protected RegistrationService $registrationService, protected LoginService $loginService, - protected HttpClient $httpClient, + protected HttpRequestService $http, protected GroupSyncService $groupService ) { } @@ -94,7 +94,7 @@ class OidcService // Run discovery if ($config['discover'] ?? false) { try { - $settings->discoverFromIssuer($this->httpClient, Cache::store(null), 15); + $settings->discoverFromIssuer($this->http->buildClient(5), Cache::store(null), 15); } catch (OidcIssuerDiscoveryException $exception) { throw new OidcException('OIDC Discovery Error: ' . $exception->getMessage()); } @@ -111,7 +111,7 @@ class OidcService protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider { $provider = new OidcOAuthProvider($settings->arrayForProvider(), [ - 'httpClient' => $this->httpClient, + 'httpClient' => $this->http->buildClient(5), 'optionProvider' => new HttpBasicAuthOptionProvider(), ]); diff --git a/app/Activity/DispatchWebhookJob.php b/app/Activity/DispatchWebhookJob.php index 405bca49c..e1771b114 100644 --- a/app/Activity/DispatchWebhookJob.php +++ b/app/Activity/DispatchWebhookJob.php @@ -6,6 +6,7 @@ use BookStack\Activity\Models\Loggable; use BookStack\Activity\Models\Webhook; use BookStack\Activity\Tools\WebhookFormatter; use BookStack\Facades\Theme; +use BookStack\Http\HttpRequestService; use BookStack\Theming\ThemeEvents; use BookStack\Users\Models\User; use BookStack\Util\SsrUrlValidator; @@ -14,7 +15,6 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; -use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; class DispatchWebhookJob implements ShouldQueue @@ -49,25 +49,28 @@ class DispatchWebhookJob implements ShouldQueue * * @return void */ - public function handle() + public function handle(HttpRequestService $http) { $lastError = null; try { (new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint); - $response = Http::asJson() - ->withOptions(['allow_redirects' => ['strict' => true]]) - ->timeout($this->webhook->timeout) - ->post($this->webhook->endpoint, $this->webhookData); - } catch (\Exception $exception) { - $lastError = $exception->getMessage(); - Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\""); - } + $client = $http->buildClient($this->webhook->timeout, [ + 'connect_timeout' => 10, + 'allow_redirects' => ['strict' => true], + ]); - if (isset($response) && $response->failed()) { - $lastError = "Response status from endpoint was {$response->status()}"; - Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$response->status()}"); + $response = $client->sendRequest($http->jsonRequest('POST', $this->webhook->endpoint, $this->webhookData)); + $statusCode = $response->getStatusCode(); + + if ($statusCode >= 400) { + $lastError = "Response status from endpoint was {$statusCode}"; + Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$statusCode}"); + } + } catch (\Exception $error) { + $lastError = $error->getMessage(); + Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\""); } $this->webhook->last_called_at = now(); diff --git a/app/App/Providers/AppServiceProvider.php b/app/App/Providers/AppServiceProvider.php index deb664ba6..0275a5489 100644 --- a/app/App/Providers/AppServiceProvider.php +++ b/app/App/Providers/AppServiceProvider.php @@ -9,16 +9,15 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; use BookStack\Exceptions\BookStackExceptionHandlerPage; +use BookStack\Http\HttpRequestService; use BookStack\Permissions\PermissionApplicator; use BookStack\Settings\SettingService; use BookStack\Util\CspService; -use GuzzleHttp\Client; use Illuminate\Contracts\Foundation\ExceptionRenderer; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Facades\URL; use Illuminate\Support\ServiceProvider; -use Psr\Http\Client\ClientInterface as HttpClientInterface; class AppServiceProvider extends ServiceProvider { @@ -39,6 +38,7 @@ class AppServiceProvider extends ServiceProvider SettingService::class => SettingService::class, SocialAuthService::class => SocialAuthService::class, CspService::class => CspService::class, + HttpRequestService::class => HttpRequestService::class, ]; /** @@ -51,7 +51,7 @@ class AppServiceProvider extends ServiceProvider // Set root URL $appUrl = config('app.url'); if ($appUrl) { - $isHttps = (strpos($appUrl, 'https://') === 0); + $isHttps = str_starts_with($appUrl, 'https://'); URL::forceRootUrl($appUrl); URL::forceScheme($isHttps ? 'https' : 'http'); } @@ -75,12 +75,6 @@ class AppServiceProvider extends ServiceProvider */ public function register() { - $this->app->bind(HttpClientInterface::class, function ($app) { - return new Client([ - 'timeout' => 3, - ]); - }); - $this->app->singleton(PermissionApplicator::class, function ($app) { return new PermissionApplicator(null); }); diff --git a/app/Http/HttpClientHistory.php b/app/Http/HttpClientHistory.php new file mode 100644 index 000000000..f224b1779 --- /dev/null +++ b/app/Http/HttpClientHistory.php @@ -0,0 +1,33 @@ +container); + } + + public function requestAt(int $index): ?GuzzleRequest + { + return $this->container[$index]['request'] ?? null; + } + + public function latestRequest(): ?GuzzleRequest + { + return $this->requestAt($this->requestCount() - 1); + } + + public function all(): array + { + return $this->container; + } +} diff --git a/app/Http/HttpRequestService.php b/app/Http/HttpRequestService.php new file mode 100644 index 000000000..f59c298a6 --- /dev/null +++ b/app/Http/HttpRequestService.php @@ -0,0 +1,70 @@ + $timeout, + 'handler' => $this->handler, + ]; + + return new Client(array_merge($options, $defaultOptions)); + } + + /** + * Create a new JSON http request for use with a client. + */ + public function jsonRequest(string $method, string $uri, array $data): GuzzleRequest + { + $headers = ['Content-Type' => 'application/json']; + return new GuzzleRequest($method, $uri, $headers, json_encode($data)); + } + + /** + * Mock any http clients built from this service, and response with the given responses. + * Returns history which can then be queried. + * @link https://docs.guzzlephp.org/en/stable/testing.html#history-middleware + */ + public function mockClient(array $responses = [], bool $pad = true): HttpClientHistory + { + // By default, we pad out the responses with 10 successful values so that requests will be + // properly recorded for inspection. Otherwise, we can't later check if we're received + // too many requests. + if ($pad) { + $response = new Response(200, [], 'success'); + $responses = array_merge($responses, array_fill(0, 10, $response)); + } + + $container = []; + $history = Middleware::history($container); + $mock = new MockHandler($responses); + $this->handler = HandlerStack::create($mock); + $this->handler->push($history, 'history'); + + return new HttpClientHistory($container); + } + + /** + * Clear mocking that has been set up for clients. + */ + public function clearMocking(): void + { + $this->handler = null; + } +} diff --git a/app/Uploads/HttpFetcher.php b/app/Uploads/HttpFetcher.php deleted file mode 100644 index fcb4147e9..000000000 --- a/app/Uploads/HttpFetcher.php +++ /dev/null @@ -1,38 +0,0 @@ - $uri, - CURLOPT_RETURNTRANSFER => 1, - CURLOPT_CONNECTTIMEOUT => 5, - ]); - - $data = curl_exec($ch); - $err = curl_error($ch); - curl_close($ch); - - if ($err) { - $errno = curl_errno($ch); - throw new HttpFetchException($err, $errno); - } - - return $data; - } -} diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php index 3cd37812a..9692b3f38 100644 --- a/app/Uploads/UserAvatars.php +++ b/app/Uploads/UserAvatars.php @@ -3,20 +3,20 @@ namespace BookStack\Uploads; use BookStack\Exceptions\HttpFetchException; +use BookStack\Http\HttpRequestService; use BookStack\Users\Models\User; use Exception; +use GuzzleHttp\Psr7\Request; use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; +use Psr\Http\Client\ClientExceptionInterface; class UserAvatars { - protected $imageService; - protected $http; - - public function __construct(ImageService $imageService, HttpFetcher $http) - { - $this->imageService = $imageService; - $this->http = $http; + public function __construct( + protected ImageService $imageService, + protected HttpRequestService $http + ) { } /** @@ -112,8 +112,10 @@ class UserAvatars protected function getAvatarImageData(string $url): string { try { - $imageData = $this->http->fetch($url); - } catch (HttpFetchException $exception) { + $client = $this->http->buildClient(5); + $response = $client->sendRequest(new Request('GET', $url)); + $imageData = (string) $response->getBody(); + } catch (ClientExceptionInterface $exception) { throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]), $exception->getCode(), $exception); } @@ -127,7 +129,7 @@ class UserAvatars { $fetchUrl = $this->getAvatarUrl(); - return is_string($fetchUrl) && strpos($fetchUrl, 'http') === 0; + return str_starts_with($fetchUrl, 'http'); } /** diff --git a/tests/Actions/WebhookCallTest.php b/tests/Actions/WebhookCallTest.php index 0746aa3a1..81bd7e7e8 100644 --- a/tests/Actions/WebhookCallTest.php +++ b/tests/Actions/WebhookCallTest.php @@ -7,11 +7,10 @@ use BookStack\Activity\DispatchWebhookJob; use BookStack\Activity\Models\Webhook; use BookStack\Activity\Tools\ActivityLogger; use BookStack\Api\ApiToken; -use BookStack\Entities\Models\PageRevision; use BookStack\Users\Models\User; -use Illuminate\Http\Client\Request; +use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Psr7\Response; use Illuminate\Support\Facades\Bus; -use Illuminate\Support\Facades\Http; use Tests\TestCase; class WebhookCallTest extends TestCase @@ -50,10 +49,10 @@ class WebhookCallTest extends TestCase public function test_webhook_runs_for_delete_actions() { + // This test must not fake the queue/bus since this covers an issue + // around handling and serialization of items now deleted from the database. $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']); - Http::fake([ - '*' => Http::response('', 500), - ]); + $this->mockHttpClient([new Response(500)]); $user = $this->users->newUser(); $resp = $this->asAdmin()->delete($user->getEditUrl()); @@ -69,9 +68,7 @@ class WebhookCallTest extends TestCase public function test_failed_webhook_call_logs_error() { $logger = $this->withTestLogger(); - Http::fake([ - '*' => Http::response('', 500), - ]); + $this->mockHttpClient([new Response(500)]); $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']); $this->assertNull($webhook->last_errored_at); @@ -86,7 +83,7 @@ class WebhookCallTest extends TestCase public function test_webhook_call_exception_is_caught_and_logged() { - Http::shouldReceive('asJson')->andThrow(new \Exception('Failed to perform request')); + $this->mockHttpClient([new ConnectException('Failed to perform request', new \GuzzleHttp\Psr7\Request('GET', ''))]); $logger = $this->withTestLogger(); $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']); @@ -104,11 +101,11 @@ class WebhookCallTest extends TestCase public function test_webhook_uses_ssr_hosts_option_if_set() { config()->set('app.ssr_hosts', 'https://*.example.com'); - $http = Http::fake(); + $responses = $this->mockHttpClient(); $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.co.uk'], ['all']); $this->runEvent(ActivityType::ROLE_CREATE); - $http->assertNothingSent(); + $this->assertEquals(0, $responses->requestCount()); $webhook->refresh(); $this->assertEquals('The URL does not match the configured allowed SSR hosts', $webhook->last_error); @@ -117,29 +114,24 @@ class WebhookCallTest extends TestCase public function test_webhook_call_data_format() { - Http::fake([ - '*' => Http::response('', 200), - ]); + $responses = $this->mockHttpClient([new Response(200, [], '')]); $webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']); $page = $this->entities->page(); $editor = $this->users->editor(); $this->runEvent(ActivityType::PAGE_UPDATE, $page, $editor); - Http::assertSent(function (Request $request) use ($editor, $page, $webhook) { - $reqData = $request->data(); - - return $request->isJson() - && $reqData['event'] === 'page_update' - && $reqData['text'] === ($editor->name . ' updated page "' . $page->name . '"') - && is_string($reqData['triggered_at']) - && $reqData['triggered_by']['name'] === $editor->name - && $reqData['triggered_by_profile_url'] === $editor->getProfileUrl() - && $reqData['webhook_id'] === $webhook->id - && $reqData['webhook_name'] === $webhook->name - && $reqData['url'] === $page->getUrl() - && $reqData['related_item']['name'] === $page->name; - }); + $request = $responses->latestRequest(); + $reqData = json_decode($request->getBody(), true); + $this->assertEquals('page_update', $reqData['event']); + $this->assertEquals(($editor->name . ' updated page "' . $page->name . '"'), $reqData['text']); + $this->assertIsString($reqData['triggered_at']); + $this->assertEquals($editor->name, $reqData['triggered_by']['name']); + $this->assertEquals($editor->getProfileUrl(), $reqData['triggered_by_profile_url']); + $this->assertEquals($webhook->id, $reqData['webhook_id']); + $this->assertEquals($webhook->name, $reqData['webhook_name']); + $this->assertEquals($page->getUrl(), $reqData['url']); + $this->assertEquals($page->name, $reqData['related_item']['name']); } protected function runEvent(string $event, $detail = '', ?User $user = null) diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php index 191a25f88..367e84816 100644 --- a/tests/Auth/OidcTest.php +++ b/tests/Auth/OidcTest.php @@ -7,7 +7,6 @@ use BookStack\Facades\Theme; use BookStack\Theming\ThemeEvents; use BookStack\Users\Models\Role; use BookStack\Users\Models\User; -use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use Illuminate\Testing\TestResponse; use Tests\Helpers\OidcJwtHelper; @@ -137,7 +136,7 @@ class OidcTest extends TestCase $this->post('/oidc/login'); $state = session()->get('oidc_state'); - $transactions = &$this->mockHttpClient([$this->getMockAuthorizationResponse([ + $transactions = $this->mockHttpClient([$this->getMockAuthorizationResponse([ 'email' => 'benny@example.com', 'sub' => 'benny1010101', ])]); @@ -146,9 +145,8 @@ class OidcTest extends TestCase // App calls token endpoint to get id token $resp = $this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=' . $state); $resp->assertRedirect('/'); - $this->assertCount(1, $transactions); - /** @var Request $tokenRequest */ - $tokenRequest = $transactions[0]['request']; + $this->assertEquals(1, $transactions->requestCount()); + $tokenRequest = $transactions->latestRequest(); $this->assertEquals('https://oidc.local/token', (string) $tokenRequest->getUri()); $this->assertEquals('POST', $tokenRequest->getMethod()); $this->assertEquals('Basic ' . base64_encode(OidcJwtHelper::defaultClientId() . ':testpass'), $tokenRequest->getHeader('Authorization')[0]); @@ -279,7 +277,7 @@ class OidcTest extends TestCase { $this->withAutodiscovery(); - $transactions = &$this->mockHttpClient([ + $transactions = $this->mockHttpClient([ $this->getAutoDiscoveryResponse(), $this->getJwksResponse(), ]); @@ -289,11 +287,9 @@ class OidcTest extends TestCase $this->runLogin(); $this->assertTrue(auth()->check()); - /** @var Request $discoverRequest */ - $discoverRequest = $transactions[0]['request']; - /** @var Request $discoverRequest */ - $keysRequest = $transactions[1]['request']; + $discoverRequest = $transactions->requestAt(0); + $keysRequest = $transactions->requestAt(1); $this->assertEquals('GET', $keysRequest->getMethod()); $this->assertEquals('GET', $discoverRequest->getMethod()); $this->assertEquals(OidcJwtHelper::defaultIssuer() . '/.well-known/openid-configuration', $discoverRequest->getUri()); @@ -316,7 +312,7 @@ class OidcTest extends TestCase { $this->withAutodiscovery(); - $transactions = &$this->mockHttpClient([ + $transactions = $this->mockHttpClient([ $this->getAutoDiscoveryResponse(), $this->getJwksResponse(), $this->getAutoDiscoveryResponse([ @@ -327,15 +323,15 @@ class OidcTest extends TestCase // Initial run $this->post('/oidc/login'); - $this->assertCount(2, $transactions); + $this->assertEquals(2, $transactions->requestCount()); // Second run, hits cache $this->post('/oidc/login'); - $this->assertCount(2, $transactions); + $this->assertEquals(2, $transactions->requestCount()); // Third run, different issuer, new cache key config()->set(['oidc.issuer' => 'https://auto.example.com']); $this->post('/oidc/login'); - $this->assertCount(4, $transactions); + $this->assertEquals(4, $transactions->requestCount()); } public function test_auth_login_with_autodiscovery_with_keys_that_do_not_have_alg_property() diff --git a/tests/TestCase.php b/tests/TestCase.php index 0ab0792bd..f8f59977a 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -3,13 +3,10 @@ namespace Tests; use BookStack\Entities\Models\Entity; +use BookStack\Http\HttpClientHistory; +use BookStack\Http\HttpRequestService; use BookStack\Settings\SettingService; -use BookStack\Uploads\HttpFetcher; use BookStack\Users\Models\User; -use GuzzleHttp\Client; -use GuzzleHttp\Handler\MockHandler; -use GuzzleHttp\HandlerStack; -use GuzzleHttp\Middleware; use Illuminate\Contracts\Console\Kernel; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Foundation\Testing\TestCase as BaseTestCase; @@ -18,10 +15,8 @@ use Illuminate\Support\Env; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; use Illuminate\Testing\Assert as PHPUnit; -use Mockery; use Monolog\Handler\TestHandler; use Monolog\Logger; -use Psr\Http\Client\ClientInterface; use Ssddanbrown\AssertHtml\TestsHtml; use Tests\Helpers\EntityProvider; use Tests\Helpers\FileProvider; @@ -111,33 +106,11 @@ abstract class TestCase extends BaseTestCase } /** - * Mock the HttpFetcher service and return the given data on fetch. + * Mock the http client used in BookStack http calls. */ - protected function mockHttpFetch($returnData, int $times = 1) + protected function mockHttpClient(array $responses = []): HttpClientHistory { - $mockHttp = Mockery::mock(HttpFetcher::class); - $this->app[HttpFetcher::class] = $mockHttp; - $mockHttp->shouldReceive('fetch') - ->times($times) - ->andReturn($returnData); - } - - /** - * Mock the http client used in BookStack. - * Returns a reference to the container which holds all history of http transactions. - * - * @link https://docs.guzzlephp.org/en/stable/testing.html#history-middleware - */ - protected function &mockHttpClient(array $responses = []): array - { - $container = []; - $history = Middleware::history($container); - $mock = new MockHandler($responses); - $handlerStack = new HandlerStack($mock); - $handlerStack->push($history); - $this->app[ClientInterface::class] = new Client(['handler' => $handlerStack]); - - return $container; + return $this->app->make(HttpRequestService::class)->mockClient($responses); } /** diff --git a/tests/ThemeTest.php b/tests/ThemeTest.php index 6976f2384..08c99d297 100644 --- a/tests/ThemeTest.php +++ b/tests/ThemeTest.php @@ -12,13 +12,10 @@ use BookStack\Facades\Theme; use BookStack\Theming\ThemeEvents; use BookStack\Users\Models\User; use Illuminate\Console\Command; -use Illuminate\Http\Client\Request as HttpClientRequest; use Illuminate\Http\Request; use Illuminate\Http\Response; use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\File; -use Illuminate\Support\Facades\Http; -use League\CommonMark\ConfigurableEnvironmentInterface; use League\CommonMark\Environment\Environment; class ThemeTest extends TestCase @@ -177,9 +174,7 @@ class ThemeTest extends TestCase }; Theme::listen(ThemeEvents::WEBHOOK_CALL_BEFORE, $callback); - Http::fake([ - '*' => Http::response('', 200), - ]); + $responses = $this->mockHttpClient([new \GuzzleHttp\Psr7\Response(200, [], '')]); $webhook = new Webhook(['name' => 'Test webhook', 'endpoint' => 'https://example.com']); $webhook->save(); @@ -193,9 +188,10 @@ class ThemeTest extends TestCase $this->assertEquals($webhook->id, $args[1]->id); $this->assertEquals($detail->id, $args[2]->id); - Http::assertSent(function (HttpClientRequest $request) { - return $request->isJson() && $request->data()['test'] === 'hello!'; - }); + $this->assertEquals(1, $responses->requestCount()); + $request = $responses->latestRequest(); + $reqData = json_decode($request->getBody(), true); + $this->assertEquals('hello!', $reqData['test']); } public function test_event_activity_logged() diff --git a/tests/Uploads/AvatarTest.php b/tests/Uploads/AvatarTest.php index 363c1fa95..f5b49a9fc 100644 --- a/tests/Uploads/AvatarTest.php +++ b/tests/Uploads/AvatarTest.php @@ -3,9 +3,11 @@ namespace Tests\Uploads; use BookStack\Exceptions\HttpFetchException; -use BookStack\Uploads\HttpFetcher; use BookStack\Uploads\UserAvatars; use BookStack\Users\Models\User; +use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; use Tests\TestCase; class AvatarTest extends TestCase @@ -22,27 +24,16 @@ class AvatarTest extends TestCase return User::query()->where('email', '=', $user->email)->first(); } - protected function assertImageFetchFrom(string $url) - { - $http = $this->mock(HttpFetcher::class); - - $http->shouldReceive('fetch') - ->once()->with($url) - ->andReturn($this->files->pngImageData()); - } - - protected function deleteUserImage(User $user) + protected function deleteUserImage(User $user): void { $this->files->deleteAtRelativePath($user->avatar->path); } public function test_gravatar_fetched_on_user_create() { - config()->set([ - 'services.disable_services' => false, - ]); + $requests = $this->mockHttpClient([new Response(200, ['Content-Type' => 'image/png'], $this->files->pngImageData())]); + config()->set(['services.disable_services' => false]); $user = User::factory()->make(); - $this->assertImageFetchFrom('https://www.gravatar.com/avatar/' . md5(strtolower($user->email)) . '?s=500&d=identicon'); $user = $this->createUserRequest($user); $this->assertDatabaseHas('images', [ @@ -50,6 +41,9 @@ class AvatarTest extends TestCase 'created_by' => $user->id, ]); $this->deleteUserImage($user); + + $expectedUri = 'https://www.gravatar.com/avatar/' . md5(strtolower($user->email)) . '?s=500&d=identicon'; + $this->assertEquals($expectedUri, $requests->latestRequest()->getUri()); } public function test_custom_url_used_if_set() @@ -61,24 +55,22 @@ class AvatarTest extends TestCase $user = User::factory()->make(); $url = 'https://example.com/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500'; - $this->assertImageFetchFrom($url); + $requests = $this->mockHttpClient([new Response(200, ['Content-Type' => 'image/png'], $this->files->pngImageData())]); $user = $this->createUserRequest($user); + $this->assertEquals($url, $requests->latestRequest()->getUri()); $this->deleteUserImage($user); } public function test_avatar_not_fetched_if_no_custom_url_and_services_disabled() { - config()->set([ - 'services.disable_services' => true, - ]); - + config()->set(['services.disable_services' => true]); $user = User::factory()->make(); - - $http = $this->mock(HttpFetcher::class); - $http->shouldNotReceive('fetch'); + $requests = $this->mockHttpClient([new Response()]); $this->createUserRequest($user); + + $this->assertEquals(0, $requests->requestCount()); } public function test_avatar_not_fetched_if_avatar_url_option_set_to_false() @@ -89,21 +81,18 @@ class AvatarTest extends TestCase ]); $user = User::factory()->make(); - - $http = $this->mock(HttpFetcher::class); - $http->shouldNotReceive('fetch'); + $requests = $this->mockHttpClient([new Response()]); $this->createUserRequest($user); + + $this->assertEquals(0, $requests->requestCount()); } public function test_no_failure_but_error_logged_on_failed_avatar_fetch() { - config()->set([ - 'services.disable_services' => false, - ]); + config()->set(['services.disable_services' => false]); - $http = $this->mock(HttpFetcher::class); - $http->shouldReceive('fetch')->andThrow(new HttpFetchException()); + $this->mockHttpClient([new ConnectException('Failed to connect', new Request('GET', ''))]); $logger = $this->withTestLogger(); @@ -122,17 +111,16 @@ class AvatarTest extends TestCase $user = User::factory()->make(); $avatar = app()->make(UserAvatars::class); - $url = 'http_malformed_url/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500'; $logger = $this->withTestLogger(); + $this->mockHttpClient([new ConnectException('Could not resolve host http_malformed_url', new Request('GET', ''))]); $avatar->fetchAndAssignToUser($user); + $url = 'http_malformed_url/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500'; $this->assertTrue($logger->hasError('Failed to save user avatar image')); $exception = $logger->getRecords()[0]['context']['exception']; - $this->assertEquals(new HttpFetchException( - 'Cannot get image from ' . $url, - 6, - (new HttpFetchException('Could not resolve host: http_malformed_url', 6)) - ), $exception); + $this->assertInstanceOf(HttpFetchException::class, $exception); + $this->assertEquals('Cannot get image from ' . $url, $exception->getMessage()); + $this->assertEquals('Could not resolve host http_malformed_url', $exception->getPrevious()->getMessage()); } }