Project

General

Profile

Actions

Bug #14

open

Fax Tone/Send Module - Modernization: Enum Consolidation, IDOR Guard, Controller Decomposition & API Deprecation

Added by Redmine Admin about 2 months ago.

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

0%

Estimated time:

Description

User Story

As a platform engineer maintaining the kc-web application, I want to modernize the Fax Tone/Send module by eliminating dual-family code duplication, decomposing the god-controller, closing the IDOR/broken-access-control gap, and establishing a baseline test suite, so that each defect fix or schema change applies once (not twice), the module is independently testable, and the OWASP A01 exposure is closed before the next compliance review.


Background

The Fax Tone/Send module implements automated fax-line verification (tone-detection and document-send sub-modes) for the KlearCom platform. A static code analysis (run 20260819T153730_ary8ch) identified four high-severity structural issues:

  1. Dual plain/Verify+ family duplication — every entity (model, service, migration, repository) exists in two near-identical copies selected at runtime by the client-supplied fax_test_type discriminator (1 = plain, 2 = Verify+). This doubles the maintenance surface for all schema changes and bug fixes.
  2. God-controller KcFaxSchedulerController — handles 10+ distinct responsibilities (CRUD, CDR, stuck-test reset, billable/non-billable, alert history, filter dropdowns, bulk actions). Untestable in isolation.
  3. fax_test_type IDOR risk — client-supplied discriminator routes requests to the DB family without a confirmed cross-validation of record ownership + family membership (OWASP A01:2021).
  4. API version drift (v1/v2/v3) — three parallel REST API copies with no deprecation timeline; older versions likely lack security/validation fixes applied to v3.

Scope

In scope: app/Http/Controllers/Fax/, RestAPIControllers/v1|v2|v3/ (fax only), app/Http/Requests/Fax/, app/Services/Fax/, app/Services/OutboundFax/, app/Models/Fax/, app/Repositories/Fax/, ProcessFaxMonitor, FaxScheduleAlertEmail, FaxRoutes.php, OutboundFaxRoutes.php, 14 fax migrations, fax-related enums and constants.

Out of scope (must not be changed): KcDashboardController, KcBillingControllers, KcCronPdfReports, KcNumberPerformanceTracker, ScheduledClientReport, any non-Fax module, the telephony/fax carrier test engine.


Acceptance Criteria

AC-1: FaxTestType PHP Enum introduced

  • FaxTestType enum exists with PLAIN = 1 and VERIFY_PLUS = 2 cases.
  • All magic literals 1/2 for fax_test_type replaced with enum across all fax controllers, services, models, and Form Requests.
  • TEST_TYPE_FAX = 6 replaced with a named constant; integer value remains 6 for cross-module compatibility.

AC-2: IDOR / broken-access-control guard in place

  • Every action accepting fax_test_type + resource ID validates: (a) record belongs to the submitted family, (b) record belongs to the authenticated account.
  • PHPUnit feature test asserts mismatched fax_test_type returns HTTP 403 or 404.

AC-3: KcFaxSchedulerController decomposed

  • Split into ≥ 4 focused controllers: FaxSchedulerController (CRUD), FaxBillingController, FaxAlertHistoryController, FaxCDRController, FaxStuckTestController.
  • All existing routes remain at the same URLs; only the backing controller class changes.

AC-4: REST API v1/v2 deprecated; v3 security parity confirmed

  • v1/v2 responses include Deprecation: true and Link headers pointing to v3.
  • Migration guide document (markdown) listing v1→v3 and v2→v3 field/endpoint mapping written.
  • Missing auth middleware or validation rules backported to v1/v2 or documented as accepted risk with a removal target date.

AC-5: Dual-family architecture consolidated (Phase 1)

  • FaxServiceContract interface and FaxServiceFactory introduced; controllers resolve service via factory rather than inline if/else on fax_test_type.
  • FaxServices and FaxVerifyServices implement the contract. Existing MySQL tables unchanged.
  • Phase 2 (full model/table merge) deferred to a follow-on ticket, blocked on AC-7.

AC-6: Dashboard query guards added

  • Dashboard services enforce a 90-day maximum date-range window (HTTP 422 if exceeded).
  • Reporting queries enforce query-layer pagination (LIMIT applied before hydration).

AC-7: Baseline PHPUnit test suite established

  • Tests under tests/Unit/Services/Fax/ and tests/Feature/Controllers/Fax/ covering: service happy paths, Form Request validation rules (including conditional fax_verify_doc_id), CRUD happy path, and IDOR guard.
  • All tests pass on global-logic branch CI.

AC-8: ProcessFaxMonitor cron chunked

  • Cron uses chunk(100) or chunkById; no full-table in-memory load.

AC-9: MongoDB PII review completed

  • FaxMonitorService documents data classification of each logged field.
  • CLI values masked/hashed OR a decision record confirms legal basis for plain-text storage.

Field Definitions

Field Type Mandatory Default Rules
fax_test_type FaxTestType enum (int: 1 or 2) Yes — 1 = PLAIN, 2 = VERIFY_PLUS
did string Yes — Valid phone number; must belong to authenticated account
schedule string/int Yes — Non-empty scheduler config
fax_verify_doc_id int Conditional — Required when fax_test_type = 2; FK to fax_verify_doc
timezone string No Account default IANA timezone string
cli (monitor) string No — Phone number format; PII — see AC-9
start_date / end_date date Yes (reporting) — end >= start; ≤ 90 days for dashboard
ids (bulk) int[] Yes (bulk) — Non-empty; all IDs must belong to authenticated account
action (bulk) string Yes (bulk) — Enum: archive, activate, billable, non_billable
history_id int Yes (rerun) — FK to correct family history table, scoped to account
TEST_TYPE_FAX int constant N/A 6 Value must remain 6 for cross-module compatibility

Technical Notes

Implementation order (recommended):

  1. AC-1 (FaxTestType enum) — prerequisite for all
  2. AC-7 (baseline tests) — required before structural refactor
  3. AC-2 (IDOR guard) — high priority, low effort; unblocked after AC-1
  4. AC-8 (cron chunking) + AC-9 (MongoDB PII) — low effort; unblocked immediately
  5. AC-4 (REST deprecation) + AC-6 (query guards) — medium effort; unblocked after AC-1
  6. AC-3 (controller decomposition) — blocked on AC-7
  7. AC-5 Phase 1 (service factory) — blocked on AC-7 + AC-3

Key pattern — FaxServiceFactory:

class FaxServiceFactory {
    public function make(FaxTestType $type): FaxServiceContract {
        return match($type) {
            FaxTestType::PLAIN       => app(FaxServices::class),
            FaxTestType::VERIFY_PLUS => app(FaxVerifyServices::class),
        };
    }
}

Key pattern — IDOR guard:

$record = $factory->make($request->fax_test_type)->findForAccount($id, auth()->user()->account_id);
abort_if(!$record, 404);

Key constraint: MySQL table names and column names must not change in Phase 1. Cross-module consumers (KcDashboardController, KcBillingControllers, KcCronPdfReports, etc.) read these tables directly and are out of scope.

Risk: No baseline tests currently exist; AC-7 is a hard prerequisite for AC-3 and AC-5 to avoid undetected regressions.


Dependencies

  • PHP 8.1+ (backed enums)
  • Baseline test suite (AC-7) must precede controller decomposition (AC-3) and family consolidation (AC-5)
  • TEST_TYPE_FAX = 6 constant contract with ScheduledClientReport — value must not change
  • Cross-module table contracts with Dashboard/Billing teams — table structure frozen in Phase 1
  • MongoDB access-control review (AC-9) — coordinate with DevOps/DBA
  • v1/v2 REST API consumer inventory — needed before deprecation headers go live

Testing Requirements

Unit: FaxTestType enum edge cases; FaxServiceFactory dispatch; FaxServices/FaxVerifyServices schedule CRUD; FaxAlertServices threshold logic; ProcessFaxMonitor chunk behaviour.

Feature: StoreFaxRequest/UpdateFaxRequest all validation rules; IDOR guard (mismatched family → 403/404; cross-account ID → 403); scheduler CRUD happy path; bulk action with cross-account IDs → 403; dashboard date-range > 90 days → 422; reporting pagination meta.

Regression: All existing global-logic CI tests remain green; cross-module smoke tests (dashboard fax stats, billing fax count, PDF report fax bucket) return unchanged results after Phase 1.


Labels: ai-created, modernization, fax-tone-send, security, refactor, php, laravel

Labels: ai-created, modernization, fax-tone-send, security, refactor, php

No data to display

Actions

Also available in: Atom PDF