Started aligning app-wide outbound http calling behaviour

diff --git a/app/Access/Oidc/OidcOAuthProvider.php b/app/Access/Oidc/OidcOAuthProvider.php
index 2ed8cd5c9726ff40a41f33301b4be508425cfd5b..d2dc829b729ef4e342853e29fe2c87e35f236afe 100644 (file)
--- a/app/Access/Oidc/OidcOAuthProvider.php
+++ b/app/Access/Oidc/OidcOAuthProvider.php
@@ -20,15 +20,8 @@ class OidcOAuthProvider extends AbstractProvider
 {
     use BearerAuthorizationTrait;
 
 {
     use BearerAuthorizationTrait;
 

-    /**
-     * @var string
-     */
-    protected $authorizationEndpoint;
-
-    /**
-     * @var string
-     */
-    protected $tokenEndpoint;
+    protected string $authorizationEndpoint;
+    protected string $tokenEndpoint;

 
     /**
      * Scopes to use for the OIDC authorization call.
 
     /**
      * 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
     {
      */
     public function addScope(string $scope): void
     {

diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php
index 9c8b1b2647241f11830ce46bc8af7a526fd78307..fa3f579b18a8db4a667fb4597d71f00c85743db6 100644 (file)
--- 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://');
         }
     }
             throw new InvalidArgumentException('Issuer value must start with https://');
         }
     }

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index 6d13fe8f1691ac8ef6dddd64e733a5def0e56ffd..d22b26eec67b5f6ae9b74acceacdc1e3bd0e807d 100644 (file)
--- 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\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 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
 
 /**
  * Class OpenIdConnectService


@@ -26,7 +26,7 @@ class OidcService
     public function __construct(
         protected RegistrationService $registrationService,
         protected LoginService $loginService,
     public function __construct(
         protected RegistrationService $registrationService,
         protected LoginService $loginService,

-        protected HttpClient $httpClient,
+        protected HttpRequestService $http,

         protected GroupSyncService $groupService
     ) {
     }
         protected GroupSyncService $groupService
     ) {
     }


@@ -94,7 +94,7 @@ class OidcService
         // Run discovery
         if ($config['discover'] ?? false) {
             try {
         // 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());
             }
             } 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(), [
     protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider
     {
         $provider = new OidcOAuthProvider($settings->arrayForProvider(), [

-            'httpClient'     => $this->httpClient,
+            'httpClient'     => $this->http->buildClient(5),

             'optionProvider' => new HttpBasicAuthOptionProvider(),
         ]);
 
             'optionProvider' => new HttpBasicAuthOptionProvider(),
         ]);
 

diff --git a/app/Activity/DispatchWebhookJob.php b/app/Activity/DispatchWebhookJob.php
index 405bca49cbee925b574ff7ed7c574d702df9e62c..09fa12785ea189949058f25132cefe175ed329f1 100644 (file)
--- 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\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;
 use BookStack\Theming\ThemeEvents;
 use BookStack\Users\Models\User;
 use BookStack\Util\SsrUrlValidator;


@@ -14,8 +15,8 @@ use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;

-use Illuminate\Support\Facades\Http;

 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Log;

+use Psr\Http\Client\ClientExceptionInterface;

 
 class DispatchWebhookJob implements ShouldQueue
 {
 
 class DispatchWebhookJob implements ShouldQueue
 {


@@ -49,25 +50,28 @@ class DispatchWebhookJob implements ShouldQueue
      *
      * @return void
      */
      *
      * @return void
      */

-    public function handle()
+    public function handle(HttpRequestService $http)

     {
         $lastError = null;
 
         try {
             (new SsrUrlValidator())->ensureAllowed($this->webhook->endpoint);
 
     {
         $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 (ClientExceptionInterface $error) {
+            $lastError = $error->getMessage();
+            Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");

         }
 
         $this->webhook->last_called_at = now();
         }
 
         $this->webhook->last_called_at = now();

diff --git a/app/App/Providers/AppServiceProvider.php b/app/App/Providers/AppServiceProvider.php
index deb664ba697d2ff639b42b00c812f4503d1286dc..0275a54891a18c70f5fa5cb852b754d94aad0f38 100644 (file)
--- 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\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 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 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
 {
 
 class AppServiceProvider extends ServiceProvider
 {


@@ -39,6 +38,7 @@ class AppServiceProvider extends ServiceProvider
         SettingService::class => SettingService::class,
         SocialAuthService::class => SocialAuthService::class,
         CspService::class => CspService::class,
         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) {
         // 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');
         }
             URL::forceRootUrl($appUrl);
             URL::forceScheme($isHttps ? 'https' : 'http');
         }


@@ -75,12 +75,6 @@ class AppServiceProvider extends ServiceProvider
      */
     public function register()
     {
      */
     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);
         });
         $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 (file)
index 0000000..7d019d7
--- /dev/null
+++ b/app/Http/HttpClientHistory.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace BookStack\Http;
+
+use GuzzleHttp\Psr7\Request as GuzzleRequest;
+
+class HttpClientHistory
+{
+    public function __construct(
+        protected &$container
+    ) {
+    }
+
+    public function requestCount(): int
+    {
+        return count($this->container);
+    }
+
+    public function requestAt(int $index): ?GuzzleRequest
+    {
+        return $this->container[$index]['request'] ?? null;
+    }
+
+    public function latestRequest(): ?GuzzleRequest
+    {
+        return $this->requestAt($this->requestCount() - 1);
+    }
+}

diff --git a/app/Http/HttpRequestService.php b/app/Http/HttpRequestService.php
new file mode 100644 (file)
index 0000000..8318474
--- /dev/null
+++ b/app/Http/HttpRequestService.php
@@ -0,0 +1,61 @@
+<?php
+
+namespace BookStack\Http;
+
+use GuzzleHttp\Client;
+use GuzzleHttp\Handler\MockHandler;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Middleware;
+use GuzzleHttp\Psr7\Request as GuzzleRequest;
+use Psr\Http\Client\ClientInterface;
+
+class HttpRequestService
+{
+    protected ?HandlerStack $handler = null;
+
+    /**
+     * Build a new http client for sending requests on.
+     */
+    public function buildClient(int $timeout, array $options): ClientInterface
+    {
+        $defaultOptions = [
+            'timeout' => $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 = []): HttpClientHistory
+    {
+        $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/tests/Actions/WebhookCallTest.php b/tests/Actions/WebhookCallTest.php
index 0746aa3a1b2e87d1c22e424517530012bb55003d..81bd7e7e8ce303df28e86f5fc07177c4a5762096 100644 (file)
--- 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\Activity\Models\Webhook;
 use BookStack\Activity\Tools\ActivityLogger;
 use BookStack\Api\ApiToken;

-use BookStack\Entities\Models\PageRevision;

 use BookStack\Users\Models\User;
 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\Bus;

-use Illuminate\Support\Facades\Http;

 use Tests\TestCase;
 
 class WebhookCallTest extends TestCase
 use Tests\TestCase;
 
 class WebhookCallTest extends TestCase


@@ -50,10 +49,10 @@ class WebhookCallTest extends TestCase
 
     public function test_webhook_runs_for_delete_actions()
     {
 
     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']);
         $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());
 
         $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();
     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);
 
         $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()
     {
 
     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']);
 
         $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');
     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);
 
         $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);
 
         $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()
     {
 
     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);
 
         $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)
     }
 
     protected function runEvent(string $event, $detail = '', ?User $user = null)

diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 191a25f8801e732a75a3df5d46d4c19de8389756..367e84816fa4d926b5a10676820fc96bf424b0ba 100644 (file)
--- 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 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;
 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');
 
         $this->post('/oidc/login');
         $state = session()->get('oidc_state');
 

-        $transactions = &$this->mockHttpClient([$this->getMockAuthorizationResponse([
+        $transactions = $this->mockHttpClient([$this->getMockAuthorizationResponse([

             'email' => '[email protected]',
             'sub'   => 'benny1010101',
         ])]);
             'email' => '[email protected]',
             '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('/');
         // 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]);
         $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();
 
     {
         $this->withAutodiscovery();
 

-        $transactions = &$this->mockHttpClient([
+        $transactions = $this->mockHttpClient([

             $this->getAutoDiscoveryResponse(),
             $this->getJwksResponse(),
         ]);
             $this->getAutoDiscoveryResponse(),
             $this->getJwksResponse(),
         ]);


@@ -289,11 +287,9 @@ class OidcTest extends TestCase
         $this->runLogin();
 
         $this->assertTrue(auth()->check());
         $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());
         $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();
 
     {
         $this->withAutodiscovery();
 

-        $transactions = &$this->mockHttpClient([
+        $transactions = $this->mockHttpClient([

             $this->getAutoDiscoveryResponse(),
             $this->getJwksResponse(),
             $this->getAutoDiscoveryResponse([
             $this->getAutoDiscoveryResponse(),
             $this->getJwksResponse(),
             $this->getAutoDiscoveryResponse([


@@ -327,15 +323,15 @@ class OidcTest extends TestCase
 
         // Initial run
         $this->post('/oidc/login');
 
         // Initial run
         $this->post('/oidc/login');

-        $this->assertCount(2, $transactions);
+        $this->assertEquals(2, $transactions->requestCount());

         // Second run, hits cache
         $this->post('/oidc/login');
         // 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');
 
         // 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()
     }
 
     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 0ab0792bd00228dfb306b7216e7f61a66be894e3..e3c47cd89588d0370be487db5c114ececc56a7dc 100644 (file)
--- a/tests/TestCase.php
+++ b/tests/TestCase.php
@@ -3,13 +3,11 @@
 namespace Tests;
 
 use BookStack\Entities\Models\Entity;
 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 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;
 use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Illuminate\Foundation\Testing\TestCase as BaseTestCase;


@@ -21,7 +19,6 @@ use Illuminate\Testing\Assert as PHPUnit;
 use Mockery;
 use Monolog\Handler\TestHandler;
 use Monolog\Logger;
 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;
 use Ssddanbrown\AssertHtml\TestsHtml;
 use Tests\Helpers\EntityProvider;
 use Tests\Helpers\FileProvider;


@@ -115,6 +112,7 @@ abstract class TestCase extends BaseTestCase
      */
     protected function mockHttpFetch($returnData, int $times = 1)
     {
      */
     protected function mockHttpFetch($returnData, int $times = 1)
     {

+        // TODO - Remove

         $mockHttp = Mockery::mock(HttpFetcher::class);
         $this->app[HttpFetcher::class] = $mockHttp;
         $mockHttp->shouldReceive('fetch')
         $mockHttp = Mockery::mock(HttpFetcher::class);
         $this->app[HttpFetcher::class] = $mockHttp;
         $mockHttp->shouldReceive('fetch')


@@ -123,21 +121,11 @@ abstract class TestCase extends BaseTestCase
     }
 
     /**
     }
 
     /**

-     * 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
+     * Mock the http client used in BookStack http calls.

      */
      */

-    protected function &mockHttpClient(array $responses = []): array
+    protected function mockHttpClient(array $responses = []): HttpClientHistory

     {
     {

-        $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 6976f23847cf136f257ad9735fa8540cd2ea86eb..08c99d297724e249402bb15bb9ad98372b89dd34 100644 (file)
--- 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 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\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
 use League\CommonMark\Environment\Environment;
 
 class ThemeTest extends TestCase


@@ -177,9 +174,7 @@ class ThemeTest extends TestCase
         };
         Theme::listen(ThemeEvents::WEBHOOK_CALL_BEFORE, $callback);
 
         };
         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();
 
         $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);
 
         $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()
     }
 
     public function test_event_activity_logged()