Bug #10
openFax Tone/Send Module — Strategy Abstraction, Controller Decomposition & Security Hardening
0%
Description
User Story¶
As a backend engineer maintaining the Klearcom telecom-testing platform, I want the Fax Tone/Send module (test type 6) modernized through strategy-pattern abstraction, fat-controller decomposition, security hardening, and cron-processing improvements, so that every bug fix is applied once instead of twice, admin billing routes are protected against privilege escalation, REST API consumers receive clear deprecation signals, and the monitor:fax cron does not develop execution-time drift as the DID fleet grows.
Background¶
The Fax Tone/Send module schedules automated fax-tone-detection and fax-document-send tests against customer DIDs under test type 6. Its central architectural liability is full duplication of the entire data stack across two parallel families — Plain Fax (fax_test_type=1) and Verify+ Fax (fax_test_type=2) — driven by a single integer discriminator, resulting in 14 mirrored MySQL tables, 12 mirrored Eloquent models, and duplicated service/repository classes. Additional issues:
- Fat controller (
KcFaxSchedulerController) bundling 7+ distinct concerns - Three simultaneous REST API versions (v1/v2/v3) with no deprecation plan
- Missing conditional validation for
fax_document_idwhenfax_test_type=2— can cause runtime failure - Unconfirmed admin middleware on billing/stuck-test routes (OWASP A01)
-
monitor:faxcron iterating all DIDs sequentially — execution time grows linearly with DID count
Scope boundary: Only files within fax-tone-send-module.md §3.1–3.13. Cross-module consumers (KcCronPdfReports, KcNumberPerformanceTracker, KcDashboardController, KcBillingControllers) must not be modified.
Scope¶
In scope:
-
FaxSchedulerStrategyInterface,FaxStrategyFactory,FaxToneStrategy,FaxVerifyStrategy -
KcFaxSchedulerRepository(new — centralises all scheduler queries) -
KcFaxSchedulerServicerefactored to delegate via factory; allif ($fax_test_type)branches removed -
FaxResetStuckTestServiceunified;FaxVerifyResetStuckTestServicedeleted -
KcFaxSchedulerControllerdecomposed into 5 focused controllers (route paths unchanged) -
fax_document_idconditional validation added toStoreFaxRequest,UpdateFaxRequest, and REST v3 request - Admin middleware verified/enforced on billing/stuck-test routes
-
auth:apimiddleware verified/enforced on all REST fax endpoints - Verify+ entitlement check in
KcFaxSchedulerService -
ProcessFaxMonitorrefactored tochunk(50)DID iteration; per-DID logic moved toFaxMonitorService::processChunk() -
AddDeprecationHeadersmiddleware applied to REST v1/v2 groups (Sunset: 2026-11-30) - 4 MySQL index migrations (non-destructive)
- MongoDB field sanitization for DID and CLI values
- PHPUnit unit and feature tests for all components
Out of scope: DB table merging, cross-module consumer changes, UI/Blade changes, new architectural patterns beyond the strategy interface.
Acceptance Criteria¶
AC-01 — Strategy abstraction
-
FaxSchedulerStrategyInterfaceexists atapp/Contracts/Fax/FaxSchedulerStrategyInterface.php -
FaxStrategyFactory::make(1)→FaxToneStrategy;make(2)→FaxVerifyStrategy;make(0)throwsInvalidArgumentException - No
if ($fax_test_type == 1)discriminator blocks remain inKcFaxSchedulerService -
FaxVerifyResetStuckTestService.phpdeleted;FaxResetStuckTestServicehandles both families
AC-02 — fax_document_id conditional validation
-
POST /fax/schedulerwithfax_test_type=2+ nofax_document_id→ HTTP 422 witherrors.fax_document_id - Same request with valid
fax_document_idpasses validation;fax_test_type=1requests unaffected - REST v3 endpoint exhibits identical behaviour
AC-03 — Admin routes and REST auth
-
actingAs($regularUser)->post('/fax/billing/toggle')→ HTTP 403 - Unauthenticated
POST /api/v3/fax→ HTTP 401 - Unauthenticated
GET /api/v3/fax/reporting→ HTTP 401
AC-04 — Verify+ entitlement
- User without entitlement +
fax_test_type=2→ HTTP 403 on any create/update endpoint -
fax_test_type=1unaffected regardless of entitlement
AC-05 — Controller decomposition
- All existing route paths resolve correctly (
php artisan route:list— no broken references) - No controller handles more than one primary concern
- CDR list returns paginated response (not unbounded)
AC-06 — Cron chunked processing
-
ProcessFaxMonitor::handle()contains no per-DID business logic - 55 seeded DIDs →
FaxMonitorService::processChunk()called at least twice - No DID processed twice; MongoDB audit document includes
chunk_idandexecution_time_ms
AC-07 — REST deprecation headers
- v1/v2 responses include
Deprecation: true,Sunset: 2026-11-30,Link: </api/v3/fax>; rel="successor-version" - v3 responses do not include these headers
AC-08 — Index migrations and MongoDB hygiene
- 4 index migration files run and roll back without error
- No raw user-derived DID/CLI input written directly to MongoDB
AC-09 — Test gate
-
php artisan test --filter=Fax— zero failures - Zero linting errors on new/modified files
-
php artisan migrate:fresh --seedcompletes without error
Technical Notes¶
Stack: PHP / Laravel (MVC + Service Layer), MySQL (14 tables via Eloquent), MongoDB (audit), REST v1/v2/v3, Artisan Console.
Core decision: Introduce FaxSchedulerStrategyInterface — eliminates Plain/Verify+ duplication at code level without merging database tables (migration risk too high). Tables stay separate; the strategy absorbs all branching logic.
Implementation phases (each independently deployable):
| Phase | Scope | Deploy First? |
|---|---|---|
| 1 — Immediate Fixes |
fax_document_id validation, admin middleware, REST auth, entitlement check |
Yes |
| 2 — Strategy Abstraction | Interface, factory, 2 strategies, repository, service refactor, unified stuck-test | After Phase 1 |
| 3 — Controller Decomposition | 5 new controllers; FaxRoutes.php updated; paths unchanged |
After Phase 2 |
| 4 — Cron & Deprecation | Chunked DID processing, AddDeprecationHeaders middleware |
After Phase 3 |
| 5 — Index Migrations & MongoDB | 4 composite indexes, DID/CLI sanitization | Non-destructive; any time |
New files:
app/Contracts/Fax/FaxSchedulerStrategyInterface.phpapp/Services/Fax/FaxStrategyFactory.phpapp/Services/Fax/Strategies/FaxToneStrategy.phpapp/Services/Fax/Strategies/FaxVerifyStrategy.phpapp/Repositories/Fax/KcFaxSchedulerRepository.phpapp/Http/Controllers/Fax/FaxBillingController.phpapp/Http/Controllers/Fax/FaxCdrController.phpapp/Http/Controllers/Fax/FaxStuckTestController.phpapp/Http/Controllers/Fax/FaxAlertHistoryController.phpapp/Http/Middleware/AddDeprecationHeaders.php
Open questions (resolve before implementation):
- Does
fax_verify_scheduleralready have afax_document_idcolumn? - Does
FaxRoutes.phpalready apply admin middleware on billing/stuck-test routes? - Does
restapi.phpwrap fax endpoints inauth:api? - What is the existing Verify+ entitlement/feature-flag mechanism?
- Do REST v1/v2 response contracts diverge from v3?
Manual-review flags: T-16 (admin middleware on billing routes), T-17 (REST auth), T-18 (entitlement check), T-26 (deletion of FaxVerifyResetStuckTestService), T-28 (billing controller transaction), T-32 (controller rename — confirm no reflection-based class loading).
Rollback: All schema changes are non-destructive. Strategy, controller, and cron changes are independently reversible via git revert with no data loss.
Dependencies¶
- Source file inventory:
fax-tone-send-module.md §3.1–3.13 - Prior pipeline artifacts:
01-code-analysis.md,02-design-document.md,03-task-list.md/tasks.json(48 tasks) - Existing Laravel auth/subscription layer (provides Verify+ entitlement gate)
-
ScheduledClientReport::TEST_TYPE_FAXconstant (read-only; must not be modified) -
OutboundAlertSchedulerForDidmodel (DID list for cron chunking) - MongoDB connection (audit log writes)
- PHPUnit + PHP-CS-Fixer / PHPStan (CI gate)
Testing Needs¶
Unit tests: FaxStrategyFactoryTest, FaxToneStrategyTest, FaxVerifyStrategyTest, FaxResetStuckTestServiceTest
Feature tests: StoreFaxRequestValidationTest, AdminRouteAccessTest, RestApiAuthTest, VerifyPlusFaxEntitlementTest, FaxSchedulerCrudControllerTest, FaxBillingControllerTest, FaxCdrControllerTest, FaxStuckTestControllerTest, ProcessFaxMonitorTest (55-DID chunk test), KcFaxSchedulerRepositoryTest (both families, no cross-family leakage)
Labels: ai-created, modernization, fax-module, php-laravel, security, architecture
Labels: ai-created, modernization, fax-module, php-laravel, security
No data to display