From d187bd46f294d540205f6abe10ba7636901f1958 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Tue, 26 Nov 2024 14:09:16 -0600 Subject: [PATCH 1/5] feat(etag): add ETag middleware and customizable ETag generation - Introduced IfNoneMatch middleware to handle ETag-based caching: - Supports safe HTTP methods (GET, HEAD). - Returns "304 Not Modified" for matching ETags. - Handles multiple ETags and restores original request methods. - Added EtagManager for flexible ETag generation: - Default ETag generation uses MD5 hash. - Supports custom hash algorithms like SHA-256 via callbacks. - Included test cases: - Validate default ETag generation. - Test custom callback logic for ETag creation. This commit adds foundational support for ETag-based caching in the application. --- ProcessMaker/Http/Kernel.php | 4 +- .../Http/Middleware/Etag/IfNoneMatch.php | 48 +++++++++++++++++ ProcessMaker/Http/Middleware/Etag/SetEtag.php | 39 ++++++++++++++ .../Http/Resources/Caching/EtagManager.php | 44 ++++++++++++++++ tests/Feature/Etag/EtagManagerTest.php | 51 +++++++++++++++++++ tests/Feature/Etag/IfNoneMatchTest.php | 48 +++++++++++++++++ tests/Feature/Etag/SetEtagTest.php | 36 +++++++++++++ 7 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php create mode 100644 ProcessMaker/Http/Middleware/Etag/SetEtag.php create mode 100644 ProcessMaker/Http/Resources/Caching/EtagManager.php create mode 100644 tests/Feature/Etag/EtagManagerTest.php create mode 100644 tests/Feature/Etag/IfNoneMatchTest.php create mode 100644 tests/Feature/Etag/SetEtagTest.php diff --git a/ProcessMaker/Http/Kernel.php b/ProcessMaker/Http/Kernel.php index 5fb8432f48..eb15cdd029 100644 --- a/ProcessMaker/Http/Kernel.php +++ b/ProcessMaker/Http/Kernel.php @@ -41,7 +41,7 @@ class Kernel extends HttpKernel \Illuminate\Routing\Middleware\SubstituteBindings::class, Middleware\GenerateMenus::class, \Laravel\Passport\Http\Middleware\CreateFreshApiToken::class, - \ProcessMaker\Http\Middleware\IgnoreMapFiles::class, + Middleware\IgnoreMapFiles::class, ], 'api' => [ // API Middleware is defined with routeMiddleware below. @@ -83,6 +83,8 @@ class Kernel extends HttpKernel 'session_kill' => Middleware\SessionControlKill::class, 'no-cache' => Middleware\NoCache::class, 'admin' => Middleware\IsAdmin::class, + 'etag.set' => Middleware\Etag\SetEtag::class, + 'etag.if-none-match' => Middleware\Etag\IfNoneMatch::class, ]; /** diff --git a/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php b/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php new file mode 100644 index 0000000000..a6569996b3 --- /dev/null +++ b/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php @@ -0,0 +1,48 @@ +getMethod(); + + // Support using HEAD method for checking If-None-Match. + if ($request->isMethod('HEAD')) { + $request->setMethod('GET'); + } + + //Handle response. + $response = $next($request); + + // If the response is not modified, return it. + if ($response->isNotModified($request)) { + return $response; + } + + // Get the ETag value. + $etag = EtagManager::getEtag($request, $response); + + // Check if the ETag matches the If-None-Match header. + $noneMatch = array_map('trim', $request->getETags()); + if (in_array($etag, $noneMatch)) { + $response->setNotModified(); + } + + $request->setMethod($method); + + return $response; + } +} diff --git a/ProcessMaker/Http/Middleware/Etag/SetEtag.php b/ProcessMaker/Http/Middleware/Etag/SetEtag.php new file mode 100644 index 0000000000..169aee3e27 --- /dev/null +++ b/ProcessMaker/Http/Middleware/Etag/SetEtag.php @@ -0,0 +1,39 @@ +getMethod(); + + // Support using HEAD method for checking If-None-Match. + if ($request->isMethod('HEAD')) { + $request->setMethod('GET'); + } + + // Handle response. + $response = $next($request); + + // Setting ETag. + $etag = EtagManager::getEtag($request, $response); + $response->setEtag($etag); + $request->setMethod($method); + + return $response; + } +} diff --git a/ProcessMaker/Http/Resources/Caching/EtagManager.php b/ProcessMaker/Http/Resources/Caching/EtagManager.php new file mode 100644 index 0000000000..80a9cd4f6b --- /dev/null +++ b/ProcessMaker/Http/Resources/Caching/EtagManager.php @@ -0,0 +1,44 @@ +start('"')->finish('"'); + } + + /** + * Get default ETag value. + */ + private static function defaultGetEtag(Response $response): string + { + return md5($response->getContent()); + } +} diff --git a/tests/Feature/Etag/EtagManagerTest.php b/tests/Feature/Etag/EtagManagerTest.php new file mode 100644 index 0000000000..010c28ebbb --- /dev/null +++ b/tests/Feature/Etag/EtagManagerTest.php @@ -0,0 +1,51 @@ +response, 200); + + $this->assertEquals('"e0aa021e21dddbd6d8cecec71e9cf564"', EtagManager::getEtag($request, $response)); + } + + public function testGetEtagWithCallbackMd5() + { + $request = Request::create('/', 'GET'); + $response = response($this->response, 200); + + EtagManager::etagGenerateUsing(function (Request $request, Response $response) { + return md5($response->getContent()); + }); + + $this->assertEquals('"e0aa021e21dddbd6d8cecec71e9cf564"', EtagManager::getEtag($request, $response)); + } + + public function testGetEtagWithCallbackSha256() + { + $request = Request::create('/', 'GET'); + $response = response($this->response, 200); + + EtagManager::etagGenerateUsing(function (Request $request, Response $response) { + return hash('sha256', $response->getContent()); + }); + + $expectedHash = hash('sha256', $this->response); + $this->assertEquals("\"$expectedHash\"", EtagManager::getEtag($request, $response)); + } +} diff --git a/tests/Feature/Etag/IfNoneMatchTest.php b/tests/Feature/Etag/IfNoneMatchTest.php new file mode 100644 index 0000000000..15455aa2b3 --- /dev/null +++ b/tests/Feature/Etag/IfNoneMatchTest.php @@ -0,0 +1,48 @@ +any(self::TEST_ROUTE, function () { + return response($this->response, 200); + }); + } + + public function testGetRequestStatus200WithNoneMatchingIfNoneMatch() + { + $noneMatch = '"' . md5($this->response . 'NoneMatch') . '"'; + $response = $this + ->withHeaders([ + 'If-None-Match' => $noneMatch, + ]) + ->get(self::TEST_ROUTE); + + $response->assertStatus(200); + } + + public function testGetRequestStatus304WithMatchingIfNoneMatch() + { + $noneMatch = '"' . md5($this->response) . '"'; + $response = $this + ->withHeaders([ + 'If-None-Match' => $noneMatch, + ]) + ->get(self::TEST_ROUTE); + + $response->assertStatus(304); + } +} diff --git a/tests/Feature/Etag/SetEtagTest.php b/tests/Feature/Etag/SetEtagTest.php new file mode 100644 index 0000000000..754bdd5597 --- /dev/null +++ b/tests/Feature/Etag/SetEtagTest.php @@ -0,0 +1,36 @@ +any(self::TEST_ROUTE, function () { + return $this->response; + }); + } + + public function testMiddlewareSetsEtagHeader() + { + $response = $this->get(self::TEST_ROUTE); + $response->assertHeader('ETag', null); + } + + public function testEtagHeaderHasCorrectValue() + { + $value = '"' . md5($this->response) . '"'; + $response = $this->get(self::TEST_ROUTE); + $response->assertHeader('ETag', $value); + } +} From d3e4d2e1a028a49f2e95dfa2e8684a4e72f23dfe Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 28 Nov 2024 10:34:24 -0600 Subject: [PATCH 2/5] refactor: combine ETag generation and validation into a single middleware - Merged `etag.set` and `etag.if-none-match` middlewares into a single `etag.handle` middleware. - Simplified logic to reduce redundancy and improve maintainability. - Ensured ETag validation (`If-None-Match`) and generation are handled in the same flow. - Preserved compatibility with HEAD requests for consistency. This refactor improves clarity, reduces potential misconfigurations, and keeps the ETag logic centralized. --- ProcessMaker/Http/Kernel.php | 3 +- .../Http/Middleware/Etag/HandleEtag.php | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 ProcessMaker/Http/Middleware/Etag/HandleEtag.php diff --git a/ProcessMaker/Http/Kernel.php b/ProcessMaker/Http/Kernel.php index eb15cdd029..522bfba009 100644 --- a/ProcessMaker/Http/Kernel.php +++ b/ProcessMaker/Http/Kernel.php @@ -83,8 +83,7 @@ class Kernel extends HttpKernel 'session_kill' => Middleware\SessionControlKill::class, 'no-cache' => Middleware\NoCache::class, 'admin' => Middleware\IsAdmin::class, - 'etag.set' => Middleware\Etag\SetEtag::class, - 'etag.if-none-match' => Middleware\Etag\IfNoneMatch::class, + 'etag' => Middleware\Etag\HandleEtag::class, ]; /** diff --git a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php new file mode 100644 index 0000000000..122e70c45c --- /dev/null +++ b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php @@ -0,0 +1,56 @@ +getMethod(); + if ($request->isMethod('HEAD')) { + $request->setMethod('GET'); + } + + // Handle response. + $response = $next($request); + + // Generate ETag for the response. + $etag = EtagManager::getEtag($request, $response); + if ($etag) { + // Set the ETag header. + $response->setEtag($etag); + + // Get and strip weak ETags from request headers. + $noneMatch = array_map([$this, 'stripWeakTags'], $request->getETags()); + + // Compare ETags and set response as not modified if applicable. + if (in_array($etag, $noneMatch)) { + $response->setNotModified(); + } + } + + // Restore original method and return the response. + $request->setMethod($originalMethod); + + return $response; + } + + /** + * Remove the weak indicator (W/) from an ETag. + */ + private function stripWeakTags(string $etag): string + { + return str_replace('W/', '', $etag); + } +} From d5dd9a8bf6c04b412e06c686bacdb708cab6614e Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 28 Nov 2024 10:39:09 -0600 Subject: [PATCH 3/5] refactor: add tests for combined ETag middleware and remove deprecated tests - Created tests for the new `HandleEtag` middleware: - Validates ETag generation and correctness. - Tests responses for both matching and non-matching `If-None-Match` headers. - Ensures proper handling of weak ETags (`W/`). - Removed old tests for `SetEtag` and `IfNoneMatch` middlewares as they are no longer needed. This commit improves test clarity and ensures the new ETag middleware behaves as expected. --- .../Http/Middleware/Etag/IfNoneMatch.php | 48 ------------- ProcessMaker/Http/Middleware/Etag/SetEtag.php | 39 ----------- tests/Feature/Etag/HandleEtagTest.php | 70 +++++++++++++++++++ tests/Feature/Etag/IfNoneMatchTest.php | 48 ------------- tests/Feature/Etag/SetEtagTest.php | 36 ---------- 5 files changed, 70 insertions(+), 171 deletions(-) delete mode 100644 ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php delete mode 100644 ProcessMaker/Http/Middleware/Etag/SetEtag.php create mode 100644 tests/Feature/Etag/HandleEtagTest.php delete mode 100644 tests/Feature/Etag/IfNoneMatchTest.php delete mode 100644 tests/Feature/Etag/SetEtagTest.php diff --git a/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php b/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php deleted file mode 100644 index a6569996b3..0000000000 --- a/ProcessMaker/Http/Middleware/Etag/IfNoneMatch.php +++ /dev/null @@ -1,48 +0,0 @@ -getMethod(); - - // Support using HEAD method for checking If-None-Match. - if ($request->isMethod('HEAD')) { - $request->setMethod('GET'); - } - - //Handle response. - $response = $next($request); - - // If the response is not modified, return it. - if ($response->isNotModified($request)) { - return $response; - } - - // Get the ETag value. - $etag = EtagManager::getEtag($request, $response); - - // Check if the ETag matches the If-None-Match header. - $noneMatch = array_map('trim', $request->getETags()); - if (in_array($etag, $noneMatch)) { - $response->setNotModified(); - } - - $request->setMethod($method); - - return $response; - } -} diff --git a/ProcessMaker/Http/Middleware/Etag/SetEtag.php b/ProcessMaker/Http/Middleware/Etag/SetEtag.php deleted file mode 100644 index 169aee3e27..0000000000 --- a/ProcessMaker/Http/Middleware/Etag/SetEtag.php +++ /dev/null @@ -1,39 +0,0 @@ -getMethod(); - - // Support using HEAD method for checking If-None-Match. - if ($request->isMethod('HEAD')) { - $request->setMethod('GET'); - } - - // Handle response. - $response = $next($request); - - // Setting ETag. - $etag = EtagManager::getEtag($request, $response); - $response->setEtag($etag); - $request->setMethod($method); - - return $response; - } -} diff --git a/tests/Feature/Etag/HandleEtagTest.php b/tests/Feature/Etag/HandleEtagTest.php new file mode 100644 index 0000000000..43b702bd6f --- /dev/null +++ b/tests/Feature/Etag/HandleEtagTest.php @@ -0,0 +1,70 @@ +any(self::TEST_ROUTE, function () { + return response($this->response, 200); + }); + } + + public function testMiddlewareSetsEtagHeader() + { + $response = $this->get(self::TEST_ROUTE); + $response->assertHeader('ETag'); + } + + public function testEtagHeaderHasCorrectValue() + { + $expectedEtag = '"' . md5($this->response) . '"'; + $response = $this->get(self::TEST_ROUTE); + $response->assertHeader('ETag', $expectedEtag); + } + + public function testRequestReturns200WhenIfNoneMatchDoesNotMatch() + { + $noneMatch = '"' . md5($this->response . 'NoneMatch') . '"'; + $response = $this + ->withHeaders(['If-None-Match' => $noneMatch]) + ->get(self::TEST_ROUTE); + + $response->assertStatus(200); + $response->assertHeader('ETag'); + } + + public function testRequestReturns304WhenIfNoneMatchMatches() + { + $matchingEtag = '"' . md5($this->response) . '"'; + $response = $this + ->withHeaders(['If-None-Match' => $matchingEtag]) + ->get(self::TEST_ROUTE); + + $response->assertStatus(304); + $response->assertHeader('ETag', $matchingEtag); + } + + public function testRequestIgnoresWeakEtagsInIfNoneMatch() + { + $weakEtag = 'W/"' . md5($this->response) . '"'; + $response = $this + ->withHeaders(['If-None-Match' => $weakEtag]) + ->get(self::TEST_ROUTE); + + $response->assertStatus(304); + $response->assertHeader('ETag', '"' . md5($this->response) . '"'); + } +} diff --git a/tests/Feature/Etag/IfNoneMatchTest.php b/tests/Feature/Etag/IfNoneMatchTest.php deleted file mode 100644 index 15455aa2b3..0000000000 --- a/tests/Feature/Etag/IfNoneMatchTest.php +++ /dev/null @@ -1,48 +0,0 @@ -any(self::TEST_ROUTE, function () { - return response($this->response, 200); - }); - } - - public function testGetRequestStatus200WithNoneMatchingIfNoneMatch() - { - $noneMatch = '"' . md5($this->response . 'NoneMatch') . '"'; - $response = $this - ->withHeaders([ - 'If-None-Match' => $noneMatch, - ]) - ->get(self::TEST_ROUTE); - - $response->assertStatus(200); - } - - public function testGetRequestStatus304WithMatchingIfNoneMatch() - { - $noneMatch = '"' . md5($this->response) . '"'; - $response = $this - ->withHeaders([ - 'If-None-Match' => $noneMatch, - ]) - ->get(self::TEST_ROUTE); - - $response->assertStatus(304); - } -} diff --git a/tests/Feature/Etag/SetEtagTest.php b/tests/Feature/Etag/SetEtagTest.php deleted file mode 100644 index 754bdd5597..0000000000 --- a/tests/Feature/Etag/SetEtagTest.php +++ /dev/null @@ -1,36 +0,0 @@ -any(self::TEST_ROUTE, function () { - return $this->response; - }); - } - - public function testMiddlewareSetsEtagHeader() - { - $response = $this->get(self::TEST_ROUTE); - $response->assertHeader('ETag', null); - } - - public function testEtagHeaderHasCorrectValue() - { - $value = '"' . md5($this->response) . '"'; - $response = $this->get(self::TEST_ROUTE); - $response->assertHeader('ETag', $value); - } -} From bc39c242f9e4d78a7ce535ce1c558338112d9106 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 28 Nov 2024 11:48:09 -0600 Subject: [PATCH 4/5] test: add middleware tests for user-specific ETag generation - Added a test to validate ETag generation for user-specific routes using `etag:user`. - Simulates an authenticated user and verifies the ETag includes the user ID. These tests ensure the ETag middleware behaves correctly for both user-specific and common routes. --- .../Http/Middleware/Etag/HandleEtag.php | 7 +++++-- .../Http/Resources/Caching/EtagManager.php | 14 +++++++++----- tests/Feature/Etag/HandleEtagTest.php | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php index 122e70c45c..7238c1cb81 100644 --- a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php +++ b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php @@ -14,7 +14,7 @@ class HandleEtag /** * Handle an incoming request. */ - public function handle(Request $request, Closure $next): Response + public function handle(Request $request, Closure $next, ?string $scope = null): Response { // Save original method and support HEAD requests. $originalMethod = $request->getMethod(); @@ -25,8 +25,11 @@ public function handle(Request $request, Closure $next): Response // Handle response. $response = $next($request); + // Determine if the ETag should include user-specific data. + $includeUser = $scope === 'user'; + // Generate ETag for the response. - $etag = EtagManager::getEtag($request, $response); + $etag = EtagManager::getEtag($request, $response, $includeUser); if ($etag) { // Set the ETag header. $response->setEtag($etag); diff --git a/ProcessMaker/Http/Resources/Caching/EtagManager.php b/ProcessMaker/Http/Resources/Caching/EtagManager.php index 80a9cd4f6b..e1f335e1e8 100644 --- a/ProcessMaker/Http/Resources/Caching/EtagManager.php +++ b/ProcessMaker/Http/Resources/Caching/EtagManager.php @@ -25,20 +25,24 @@ public static function etagGenerateUsing(?Closure $callback): void /** * Get ETag value for this request and response. */ - public static function getEtag(Request $request, Response $response): string + public static function getEtag(Request $request, Response $response, bool $includeUser = false): string { $etag = static::$etagGenerateCallback - ? call_user_func(static::$etagGenerateCallback, $request, $response) - : static::defaultGetEtag($response); + ? call_user_func(static::$etagGenerateCallback, $request, $response, $includeUser) + : static::defaultGetEtag($response, $includeUser); return (string) Str::of($etag)->start('"')->finish('"'); } /** - * Get default ETag value. + * Generate an ETag, optionally including user-specific data. */ - private static function defaultGetEtag(Response $response): string + private static function defaultGetEtag(Response $response, bool $includeUser = false): string { + if ($includeUser) { + return md5(auth()->id() . $response->getContent()); + } + return md5($response->getContent()); } } diff --git a/tests/Feature/Etag/HandleEtagTest.php b/tests/Feature/Etag/HandleEtagTest.php index 43b702bd6f..08ee467e9b 100644 --- a/tests/Feature/Etag/HandleEtagTest.php +++ b/tests/Feature/Etag/HandleEtagTest.php @@ -4,6 +4,8 @@ use Illuminate\Support\Facades\Route; use ProcessMaker\Http\Middleware\Etag\HandleEtag; +use ProcessMaker\Http\Resources\Caching\EtagManager; +use ProcessMaker\Models\User; use Tests\TestCase; class HandleEtagTest extends TestCase @@ -67,4 +69,19 @@ public function testRequestIgnoresWeakEtagsInIfNoneMatch() $response->assertStatus(304); $response->assertHeader('ETag', '"' . md5($this->response) . '"'); } + + public function testDefaultGetEtagGeneratesCorrectEtagWithUser() + { + $user = User::factory()->create(); + $this->actingAs($user); + + Route::middleware('etag:user')->any(self::TEST_ROUTE, function () { + return response($this->response, 200); + }); + + $response = $this->get(self::TEST_ROUTE); + + $expectedEtag = '"' . md5($user->id . $this->response) . '"'; + $response->assertHeader('ETag', $expectedEtag); + } } From ea9795cfd55d1e1b53abf00b26c2eda8e493d6ab Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 28 Nov 2024 13:04:48 -0600 Subject: [PATCH 5/5] Add ETag to Screen Responses --- routes/v1_1/api.php | 1 + 1 file changed, 1 insertion(+) diff --git a/routes/v1_1/api.php b/routes/v1_1/api.php index a2b2328dc9..ead0037612 100644 --- a/routes/v1_1/api.php +++ b/routes/v1_1/api.php @@ -22,6 +22,7 @@ // Route to show the screen of a task Route::get('/{taskId}/screen', [TaskController::class, 'showScreen']) + ->middleware('etag') ->name('show.screen'); // Route to show the interstitial screen of a task