Project

General

Profile

Actions

Bug #10

open

Fax Tone/Send Module — Strategy Abstraction, Controller Decomposition & Security Hardening

Added by Redmine Admin about 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Start date:
08/18/2026
Due date:
% Done:

0%

Estimated time:

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_id when fax_test_type=2 — can cause runtime failure
  • Unconfirmed admin middleware on billing/stuck-test routes (OWASP A01)
  • monitor:fax cron 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)
  • KcFaxSchedulerService refactored to delegate via factory; all if ($fax_test_type) branches removed
  • FaxResetStuckTestService unified; FaxVerifyResetStuckTestService deleted
  • KcFaxSchedulerController decomposed into 5 focused controllers (route paths unchanged)
  • fax_document_id conditional validation added to StoreFaxRequest, UpdateFaxRequest, and REST v3 request
  • Admin middleware verified/enforced on billing/stuck-test routes
  • auth:api middleware verified/enforced on all REST fax endpoints
  • Verify+ entitlement check in KcFaxSchedulerService
  • ProcessFaxMonitor refactored to chunk(50) DID iteration; per-DID logic moved to FaxMonitorService::processChunk()
  • AddDeprecationHeaders middleware 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

  • FaxSchedulerStrategyInterface exists at app/Contracts/Fax/FaxSchedulerStrategyInterface.php
  • FaxStrategyFactory::make(1) → FaxToneStrategy; make(2) → FaxVerifyStrategy; make(0) throws InvalidArgumentException
  • No if ($fax_test_type == 1) discriminator blocks remain in KcFaxSchedulerService
  • FaxVerifyResetStuckTestService.php deleted; FaxResetStuckTestService handles both families

AC-02 — fax_document_id conditional validation

  • POST /fax/scheduler with fax_test_type=2 + no fax_document_id → HTTP 422 with errors.fax_document_id
  • Same request with valid fax_document_id passes validation; fax_test_type=1 requests 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=1 unaffected 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_id and execution_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 --seed completes 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.php
  • app/Services/Fax/FaxStrategyFactory.php
  • app/Services/Fax/Strategies/FaxToneStrategy.php
  • app/Services/Fax/Strategies/FaxVerifyStrategy.php
  • app/Repositories/Fax/KcFaxSchedulerRepository.php
  • app/Http/Controllers/Fax/FaxBillingController.php
  • app/Http/Controllers/Fax/FaxCdrController.php
  • app/Http/Controllers/Fax/FaxStuckTestController.php
  • app/Http/Controllers/Fax/FaxAlertHistoryController.php
  • app/Http/Middleware/AddDeprecationHeaders.php

Open questions (resolve before implementation):

  1. Does fax_verify_scheduler already have a fax_document_id column?
  2. Does FaxRoutes.php already apply admin middleware on billing/stuck-test routes?
  3. Does restapi.php wrap fax endpoints in auth:api?
  4. What is the existing Verify+ entitlement/feature-flag mechanism?
  5. 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_FAX constant (read-only; must not be modified)
  • OutboundAlertSchedulerForDid model (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

Actions

Also available in: Atom PDF