From f058177cc7f42a35d9c6b5cd82ff661ddba60e2d Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Mon, 2 Dec 2024 11:05:34 -0600 Subject: [PATCH 1/7] Exclude 'active_at' field from TaskResource and requestor serialization to prevent ETag inconsistencies --- ProcessMaker/Http/Resources/V1_1/TaskResource.php | 1 - ProcessMaker/Traits/TaskResourceIncludes.php | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ProcessMaker/Http/Resources/V1_1/TaskResource.php b/ProcessMaker/Http/Resources/V1_1/TaskResource.php index c577942c8f..cec32ced7e 100644 --- a/ProcessMaker/Http/Resources/V1_1/TaskResource.php +++ b/ProcessMaker/Http/Resources/V1_1/TaskResource.php @@ -82,7 +82,6 @@ class TaskResource extends ApiResource 'is_administrator', 'expires_at', 'loggedin_at', - 'active_at', 'created_at', 'updated_at', 'delegation_user_id', diff --git a/ProcessMaker/Traits/TaskResourceIncludes.php b/ProcessMaker/Traits/TaskResourceIncludes.php index ec12530a4c..f2597ae68c 100644 --- a/ProcessMaker/Traits/TaskResourceIncludes.php +++ b/ProcessMaker/Traits/TaskResourceIncludes.php @@ -41,7 +41,12 @@ private function includeUser() private function includeRequestor() { - return ['requestor' => new Users($this->processRequest->user)]; + $user = $this->processRequest->user; + + // Exclude 'active_at' to prevent ETag inconsistencies. + $user->makeHidden(['active_at']); + + return ['requestor' => new Users($user)]; } private function includeProcessRequest() From 48cd7d672ba91c3e18ee8a7122bc5371236542f4 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Mon, 2 Dec 2024 11:06:30 -0600 Subject: [PATCH 2/7] Implement ETag Caching for Task Responses --- routes/api.php | 4 +++- routes/v1_1/api.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/routes/api.php b/routes/api.php index 947297d3de..8966fc0fc7 100644 --- a/routes/api.php +++ b/routes/api.php @@ -208,7 +208,9 @@ Route::post('tasks/{task}/setViewed', [TaskController::class, 'setViewed'])->name('tasks.set_viewed')->middleware('can:viewScreen,task,screen'); Route::put('tasks/{task}/setPriority', [TaskController::class, 'setPriority'])->name('tasks.priority'); Route::put('tasks/updateReassign', [TaskController::class, 'updateReassign'])->name('tasks.updateReassign'); - Route::get('tasks/user-can-reassign', [TaskController::class, 'userCanReassign'])->name('tasks.user_can_reassign'); + Route::get('tasks/user-can-reassign', [TaskController::class, 'userCanReassign']) + ->middleware('etag:user') + ->name('tasks.user_can_reassign'); // TaskDrafts Route::put('drafts/{task}', [TaskDraftController::class, 'update'])->name('taskdraft.update'); diff --git a/routes/v1_1/api.php b/routes/v1_1/api.php index ead0037612..57ecc2a694 100644 --- a/routes/v1_1/api.php +++ b/routes/v1_1/api.php @@ -18,7 +18,7 @@ // Route to show a task Route::get('/{task}', [TaskController::class, 'show']) ->name('show') - ->middleware(['bindings', 'can:view,task']); + ->middleware(['bindings', 'can:view,task', 'etag:user']); // Route to show the screen of a task Route::get('/{taskId}/screen', [TaskController::class, 'showScreen']) From 8b327c8a016ed5674acda8cb68511a0679e9bd0e Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Tue, 3 Dec 2024 15:13:16 -0600 Subject: [PATCH 3/7] restrict ETag handling to GET/HEAD and enforce user-specific caching - Process only GET and HEAD methods to ensure middleware relevance and avoid unnecessary processing for non-cacheable HTTP methods. - Add a check to determine if the response is cacheable, filtering out non-cacheable responses (e.g., those with 'no-store' directive or non-cacheable status codes). - Default ETag generation now includes user-specific data (auth()->id()) to enforce personalized caching by default. - Removed 'scope' and 'includeUser' logic for simplified and consistent caching behavior. --- .../Http/Middleware/Etag/HandleEtag.php | 33 ++++++++++++------- .../Http/Resources/Caching/EtagManager.php | 18 ++++------ 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php index 7238c1cb81..3dca9e62f2 100644 --- a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php +++ b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php @@ -14,22 +14,23 @@ class HandleEtag /** * Handle an incoming request. */ - public function handle(Request $request, Closure $next, ?string $scope = null): Response + public function handle(Request $request, Closure $next): Response { - // Save original method and support HEAD requests. - $originalMethod = $request->getMethod(); - if ($request->isMethod('HEAD')) { - $request->setMethod('GET'); + // Process only GET and HEAD methods. + if (!$request->isMethod('GET') && !$request->isMethod('HEAD')) { + return $next($request); } // Handle response. $response = $next($request); - // Determine if the ETag should include user-specific data. - $includeUser = $scope === 'user'; + // Check if the response is cacheable. + if (!$this->isCacheableResponse($response)) { + return $response; // Skip ETag for non-cacheable responses. + } // Generate ETag for the response. - $etag = EtagManager::getEtag($request, $response, $includeUser); + $etag = EtagManager::getEtag($request, $response); if ($etag) { // Set the ETag header. $response->setEtag($etag); @@ -43,9 +44,6 @@ public function handle(Request $request, Closure $next, ?string $scope = null): } } - // Restore original method and return the response. - $request->setMethod($originalMethod); - return $response; } @@ -56,4 +54,17 @@ private function stripWeakTags(string $etag): string { return str_replace('W/', '', $etag); } + + /** + * Determine if a response is cacheable. + */ + private function isCacheableResponse(Response $response): bool + { + $cacheableStatusCodes = [200, 203, 204, 206, 304]; + $cacheControl = $response->headers->get('Cache-Control', ''); + + // Verify if the status code is cacheable and does not contain "no-store". + return in_array($response->getStatusCode(), $cacheableStatusCodes) + && !str_contains($cacheControl, 'no-store'); + } } diff --git a/ProcessMaker/Http/Resources/Caching/EtagManager.php b/ProcessMaker/Http/Resources/Caching/EtagManager.php index e1f335e1e8..5344df06a0 100644 --- a/ProcessMaker/Http/Resources/Caching/EtagManager.php +++ b/ProcessMaker/Http/Resources/Caching/EtagManager.php @@ -23,26 +23,22 @@ public static function etagGenerateUsing(?Closure $callback): void } /** - * Get ETag value for this request and response. + * Get the ETag value for this request and response. */ - public static function getEtag(Request $request, Response $response, bool $includeUser = false): string + public static function getEtag(Request $request, Response $response): string { $etag = static::$etagGenerateCallback - ? call_user_func(static::$etagGenerateCallback, $request, $response, $includeUser) - : static::defaultGetEtag($response, $includeUser); + ? call_user_func(static::$etagGenerateCallback, $request, $response) + : static::defaultGetEtag($response); return (string) Str::of($etag)->start('"')->finish('"'); } /** - * Generate an ETag, optionally including user-specific data. + * Generate a default ETag, including user-specific data by default. */ - private static function defaultGetEtag(Response $response, bool $includeUser = false): string + private static function defaultGetEtag(Response $response): string { - if ($includeUser) { - return md5(auth()->id() . $response->getContent()); - } - - return md5($response->getContent()); + return md5(auth()->id() . $response->getContent()); } } From dae24684fea905125f148b8db8e5318aeef6f4d4 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 5 Dec 2024 12:02:15 -0600 Subject: [PATCH 4/7] Add ETag middleware to API group to support package routes --- ProcessMaker/Providers/ProcessMakerServiceProvider.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index ce11b8e437..f8994be5a4 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -10,6 +10,7 @@ use Illuminate\Notifications\Events\BroadcastNotificationCreated; use Illuminate\Notifications\Events\NotificationSent; use Illuminate\Support\Facades; +use Illuminate\Support\Facades\Route; use Illuminate\Support\Facades\URL; use Laravel\Dusk\DuskServiceProvider; use Laravel\Horizon\Horizon; @@ -19,6 +20,7 @@ use ProcessMaker\Events\ActivityAssigned; use ProcessMaker\Events\ScreenBuilderStarting; use ProcessMaker\Helpers\PmHash; +use ProcessMaker\Http\Middleware\Etag\HandleEtag; use ProcessMaker\ImportExport\Extension; use ProcessMaker\ImportExport\SignalHelper; use ProcessMaker\Jobs\SmartInbox; @@ -52,6 +54,8 @@ public function boot(): void $this->setupFactories(); parent::boot(); + + Route::pushMiddlewareToGroup('api', HandleEtag::class); } public function register(): void From 8a94369fd213988a5bae094667eccf5ca5143074 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 5 Dec 2024 12:03:22 -0600 Subject: [PATCH 5/7] Apply ETag middleware to v1.1 routes for consistent caching --- ProcessMaker/Providers/RouteServiceProvider.php | 2 +- routes/api.php | 4 +--- routes/v1_1/api.php | 3 +-- tests/Feature/Etag/HandleEtagTest.php | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/ProcessMaker/Providers/RouteServiceProvider.php b/ProcessMaker/Providers/RouteServiceProvider.php index 1315214a1d..e12823a721 100644 --- a/ProcessMaker/Providers/RouteServiceProvider.php +++ b/ProcessMaker/Providers/RouteServiceProvider.php @@ -89,7 +89,7 @@ protected function mapApiRoutes() { Route::middleware('api') ->group(base_path('routes/api.php')); - Route::middleware('auth:api') + Route::middleware(['auth:api', 'etag']) ->group(base_path('routes/v1_1/api.php')); } diff --git a/routes/api.php b/routes/api.php index 8966fc0fc7..947297d3de 100644 --- a/routes/api.php +++ b/routes/api.php @@ -208,9 +208,7 @@ Route::post('tasks/{task}/setViewed', [TaskController::class, 'setViewed'])->name('tasks.set_viewed')->middleware('can:viewScreen,task,screen'); Route::put('tasks/{task}/setPriority', [TaskController::class, 'setPriority'])->name('tasks.priority'); Route::put('tasks/updateReassign', [TaskController::class, 'updateReassign'])->name('tasks.updateReassign'); - Route::get('tasks/user-can-reassign', [TaskController::class, 'userCanReassign']) - ->middleware('etag:user') - ->name('tasks.user_can_reassign'); + Route::get('tasks/user-can-reassign', [TaskController::class, 'userCanReassign'])->name('tasks.user_can_reassign'); // TaskDrafts Route::put('drafts/{task}', [TaskDraftController::class, 'update'])->name('taskdraft.update'); diff --git a/routes/v1_1/api.php b/routes/v1_1/api.php index 57ecc2a694..a2b2328dc9 100644 --- a/routes/v1_1/api.php +++ b/routes/v1_1/api.php @@ -18,11 +18,10 @@ // Route to show a task Route::get('/{task}', [TaskController::class, 'show']) ->name('show') - ->middleware(['bindings', 'can:view,task', 'etag:user']); + ->middleware(['bindings', 'can:view,task']); // 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 diff --git a/tests/Feature/Etag/HandleEtagTest.php b/tests/Feature/Etag/HandleEtagTest.php index 08ee467e9b..e933260f04 100644 --- a/tests/Feature/Etag/HandleEtagTest.php +++ b/tests/Feature/Etag/HandleEtagTest.php @@ -75,7 +75,7 @@ public function testDefaultGetEtagGeneratesCorrectEtagWithUser() $user = User::factory()->create(); $this->actingAs($user); - Route::middleware('etag:user')->any(self::TEST_ROUTE, function () { + Route::middleware('etag')->any(self::TEST_ROUTE, function () { return response($this->response, 200); }); From d748403ba9da105358ab0b575e1abaf89ad1e324 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Thu, 5 Dec 2024 15:53:28 -0600 Subject: [PATCH 6/7] feat: add flexibility to ETag generation with sources - Refactored `EtagManager` to support dynamic ETag generation based on configurable sources (`updated_at`). - Introduced `generateEtagFromTables` with a `source` parameter for flexibility in determining the source of truth. This update prepares the app for future scalability and allows switching between different ETag generation strategies. --- .../Http/Middleware/Etag/HandleEtag.php | 92 ++++++++++++++----- .../Http/Resources/Caching/EtagManager.php | 27 ++++++ routes/v1_1/api.php | 1 + 3 files changed, 97 insertions(+), 23 deletions(-) diff --git a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php index 3dca9e62f2..fe51c6d35e 100644 --- a/ProcessMaker/Http/Middleware/Etag/HandleEtag.php +++ b/ProcessMaker/Http/Middleware/Etag/HandleEtag.php @@ -21,40 +21,36 @@ public function handle(Request $request, Closure $next): Response return $next($request); } - // Handle response. - $response = $next($request); + // Check if specific tables are defined for the route and calculate ETag. + $etag = $this->generateEtagFromTablesIfNeeded($request); - // Check if the response is cacheable. - if (!$this->isCacheableResponse($response)) { - return $response; // Skip ETag for non-cacheable responses. - } + // If the client has a matching ETag, return a 304 response. + // Otherwise, continue with the controller execution. + $response = $etag && $this->etagMatchesRequest($etag, $request) + ? $this->buildNotModifiedResponse($etag) + : $next($request); - // Generate ETag for the response. - $etag = EtagManager::getEtag($request, $response); + // Add the pre-calculated ETag to the response if available. if ($etag) { - // Set the ETag header. - $response->setEtag($etag); + $response = $this->setEtagOnResponse($response, $etag); + } - // Get and strip weak ETags from request headers. - $noneMatch = array_map([$this, 'stripWeakTags'], $request->getETags()); + // If no ETag was calculated from tables, generate it based on the response. + if (!$etag && $this->isCacheableResponse($response)) { + $etag = EtagManager::getEtag($request, $response); + if ($etag) { + $response = $this->setEtagOnResponse($response, $etag); - // Compare ETags and set response as not modified if applicable. - if (in_array($etag, $noneMatch)) { - $response->setNotModified(); + // If the client has a matching ETag, set the response to 304. + if ($this->etagMatchesRequest($etag, $request)) { + $response = $this->buildNotModifiedResponse($etag); + } } } return $response; } - /** - * Remove the weak indicator (W/) from an ETag. - */ - private function stripWeakTags(string $etag): string - { - return str_replace('W/', '', $etag); - } - /** * Determine if a response is cacheable. */ @@ -67,4 +63,54 @@ private function isCacheableResponse(Response $response): bool return in_array($response->getStatusCode(), $cacheableStatusCodes) && !str_contains($cacheControl, 'no-store'); } + + /** + * Generate an ETag based on the tables defined in the route, if applicable. + */ + private function generateEtagFromTablesIfNeeded(Request $request): ?string + { + $tables = $request->route()->defaults['etag_tables'] ?? null; + + return $tables ? EtagManager::generateEtagFromTables(explode(',', $tables)) : null; + } + + /** + * Check if the ETag matches the request. + */ + private function etagMatchesRequest(string $etag, Request $request): bool + { + $noneMatch = array_map([$this, 'stripWeakTags'], $request->getETags()); + + return in_array($etag, $noneMatch); + } + + /** + * Build a 304 Not Modified response with the given ETag. + */ + private function buildNotModifiedResponse(string $etag): Response + { + $response = new Response(); + $response->setNotModified(); + $response->setEtag($etag); + + return $response; + } + + /** + * Set the ETag on a given response. + */ + private function setEtagOnResponse(Response $response, string $etag): Response + { + $response->setEtag($etag); + + return $response; + } + + /** + * Remove the weak indicator (W/) from an ETag. + */ + private function stripWeakTags(string $etag): string + { + return str_replace('W/', '', $etag); + } } diff --git a/ProcessMaker/Http/Resources/Caching/EtagManager.php b/ProcessMaker/Http/Resources/Caching/EtagManager.php index 5344df06a0..edd6a822b6 100644 --- a/ProcessMaker/Http/Resources/Caching/EtagManager.php +++ b/ProcessMaker/Http/Resources/Caching/EtagManager.php @@ -4,6 +4,8 @@ use Closure; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; use Symfony\Component\HttpFoundation\Response; @@ -41,4 +43,29 @@ private static function defaultGetEtag(Response $response): string { return md5(auth()->id() . $response->getContent()); } + + /** + * Generate an ETag based on the latest update timestamps from multiple tables. + */ + public static function generateEtagFromTables(array $tables, string $source = 'updated_at'): string + { + // Fetch the latest update timestamp from each table. + // If the source is 'etag_version', use a cached version key as the source of truth. + $lastUpdated = collect($tables)->map(function ($table) use ($source) { + if ($source === 'etag_version') { + // This is not currently implemented but serves as a placeholder for future flexibility. + // The idea is to use a cached version key (e.g., "etag_version_table_name") as the source of truth. + // This would allow us to version the ETag dynamically and invalidate it using model observers or other mechanisms. + // If implemented, observers can increment this version key whenever the corresponding table is updated. + return Cache::get("etag_version_{$table}", 0); + } + + // Default to the updated_at column in the database. + return DB::table($table)->max('updated_at'); + })->max(); + + $etag = md5(auth()->id() . $lastUpdated); + + return (string) Str::of($etag)->start('"')->finish('"'); + } } diff --git a/routes/v1_1/api.php b/routes/v1_1/api.php index a2b2328dc9..70ed572f5c 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']) + ->defaults('etag_tables', 'screens,screen_versions') ->name('show.screen'); // Route to show the interstitial screen of a task From cdaa80d2b04e0be00d4fc9bbc349c8bb745381a3 Mon Sep 17 00:00:00 2001 From: Eleazar Resendez Date: Fri, 6 Dec 2024 09:38:34 -0600 Subject: [PATCH 7/7] add ETag middleware to startProcesses route - Applied ETag middleware to the 'startProcesses' route for improved caching and reduced payload size. - Added default 'etag_tables' parameter set to 'processes' to optimize ETag generation for this route. --- routes/engine.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/routes/engine.php b/routes/engine.php index 92f67def38..6ca0557b18 100644 --- a/routes/engine.php +++ b/routes/engine.php @@ -8,7 +8,10 @@ // Engine Route::prefix('api/1.0')->name('api.')->group(function () { // List of Processes that the user can start - Route::get('start_processes', [ProcessController::class, 'startProcesses'])->name('processes.start'); // Filtered in controller + Route::get('start_processes', [ProcessController::class, 'startProcesses']) + ->middleware('etag') + ->defaults('etag_tables', 'processes') + ->name('processes.start'); // Filtered in controller // Start a process Route::post('process_events/{process}', [ProcessController::class, 'triggerStartEvent'])->name('process_events.trigger')->middleware('can:start,process');