Files
formipay/FINDINGS.md
dwindown 35569923a5 docs: add comprehensive audit report and architectural recommendation
Checkpoint before implementation. Includes audit findings (FINDINGS.md),
architectural recommendation (RECOMMENDATION.md), and existing code changes
to Form, Order, Render, and form-action.js from recent development.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-17 17:00:47 +07:00

197 lines
18 KiB
Markdown

# 🔍 Formipay Plugin — Comprehensive Audit Report
**Date:** April 17, 2026
**Auditor:** GitHub Copilot
**Plugin Version:** 1.0.0
**Files Analyzed:** ~60+ files, ~15,000+ lines of PHP, JS, CSS, HTML
---
## Table of Contents
- [1. Bugs & Defects](#1-bugs--defects)
- [1.1 Critical Bugs](#11-critical-bugs)
- [1.2 Moderate Bugs](#12-moderate-bugs)
- [2. Security Concerns](#2-security-concerns)
- [3. Architecture & Code Quality Issues](#3-architecture--code-quality-issues)
- [4. Missing Features & Modules](#4-missing-features--modules)
- [5. Performance Issues](#5-performance-issues)
- [6. Missing Admin Pages / Settings](#6-missing-admin-pages--settings)
- [7. Code Cleanup Needed](#7-code-cleanup-needed)
- [8. Opportunities & Nice-to-Haves](#8-opportunities--nice-to-haves)
- [9. Summary Priority Matrix](#9-summary-priority-matrix)
- [10. Architectural Recommendation](#10-architectural-recommendation)
---
## 1. Bugs & Defects
### 1.1 Critical Bugs
| # | Location | Issue | Detail |
|---|----------|-------|--------|
| 1 | `includes/Customer.php` ~line 172 `update()` | **Undefined variable — fatal error** | Method builds `$insert_data` and `$where`, but the `$wpdb->update()` call references undefined `$table_name` and `$new_args`. Every customer update will throw a PHP fatal error. |
| 2 | `includes/Order.php` `delete()` | **Undefined `$id` variable** | Uses `$id` in the `$wpdb->delete()` where clause instead of the method parameter `$order_id`. Every order deletion call will fail. |
| 3 | `includes/Order.php` `formipay_bulk_delete_order()` | **Iterates wrong variable** | Loops `foreach($ids as $id)` but calls `$this->delete($order_id)``$order_id` comes from the outer scope (nonce check), not the loop variable. Bulk delete will repeatedly delete the same (or zero) orders. |
| 4 | `includes/Notification/Email.php` `send_email()` | **Wrong class reference — fatal error** | Calls `\Formipay_Notification::update_notification_data()` — this class does not exist. Should use `parent::update_notification_data()`. Email status tracking will crash. |
| 5 | `includes/Integration/Paypal.php` `auto_cancel_order_on_timeout()` | **Undefined `Order` class** | Calls `Order::update(...)` but unlike `BankTransfer.php`, `Paypal.php` does not import `use Formipay\Order as Order`. This will throw a class-not-found error on timeout. |
| 6 | `includes/Integration/Paypal.php` `process_payment()` | **Undefined `self::paypal_settings`** | The PayPal class never declares a `$paypal_settings` property. Accessing it leads to undefined property notices and broken payment flow. |
### 1.2 Moderate Bugs
| # | Location | Issue |
|---|----------|-------|
| 7 | `includes/Payment/BankTransfer.php` `check_unique_code()` | Uses `MAX(id)+1` for unique codes — predictable and subject to race conditions. Two concurrent orders can receive the same unique code. |
| 8 | `includes/Payment/BankTransfer.php` `add_unique_code_details()` | Calls `$this->check_unique_code()` **three times** per request (once for `'item'`, once for `'amount'`, once for `'subtotal'`). Each call queries the DB independently and may return different values. Displayed unique code may not match the stored one. |
| 9 | `admin/functions.php` `formipay_field_type_collection()` | Color field label says `'Number'` instead of `'Color'` — copy-paste error. |
| 10 | `includes/Render.php` field rendering | No `default` fallback rendered when field type doesn't match any case in the switch — unknown field types silently produce no output. |
| 11 | `includes/Order.php` `render_form_submit()` | `$field_value` is sometimes an array (from checkbox fields) but the code assumes string context. Nested `if(is_array($field_value))` only handles one level. |
| 12 | `includes/Thankyou.php` `check_parse_query()` vs `formipay_get_order()` | Old cookie-based URL (`base64_encode`) co-exists with new Token-based validation. `formipay_get_order()` generates the old-format URL for the `thankyou` link, but the new Token system expects a different format. Inconsistent access path. |
---
## 2. Security Concerns
| # | Severity | Location | Issue |
|---|----------|----------|-------|
| 1 | **High** | `includes/Order.php` `retrieve_form_data()` | Cookie `fp_access` uses `maybe_serialize()`**PHP object injection** risk if an attacker can manipulate cookie values. Should use `json_encode()`/`json_decode()`. |
| 2 | **High** | `includes/Thankyou.php` `set_endpoint()` | Calls `flush_rewrite_rules()` on **every `init`** hook — extremely expensive and causes race conditions under concurrent load. Should only flush on activation/deactivation or settings save. |
| 3 | **High** | `includes/Payment/Payment.php` `set_endpoint()` | Same `flush_rewrite_rules()` issue — fires on every page load. |
| 4 | **Medium** | `includes/Order.php` `retrieve_form_data()` | Thank-you URL uses `base64_encode(form_id:::order_id:::session_id)` — base64 is not encryption. Sequential order IDs can be guessed. The `Token` class provides proper tokens but the old path remains active. |
| 5 | **Medium** | `includes/LicenseAPI.php` | All REST endpoints use `permission_callback => '__return_true'`. The `revoke` endpoint has a stub permission callback that always returns `true`. Anyone can revoke or manipulate licenses without authentication. |
| 6 | **Medium** | `includes/Integration/Paypal.php` `webhook_endpoint()` | No PayPal webhook signature verification. An attacker could forge webhook calls to mark orders as paid without actual payment. |
| 7 | **Medium** | `includes/Customer.php` `formipay_tabledata_customers()` | No nonce check (`check_ajax_referer`). Any authenticated user can dump all customer data via direct AJAX call. |
| 8 | **Low** | `includes/Render.php` | Inline `<style>` blocks injected into `<body>` — Content Security Policy (CSP) headers may block these. Should use `wp_add_inline_style()`. |
| 9 | **Low** | `includes/Token.php` `validate()` | Uses MySQL `NOW()` for expiry comparison while token generation uses PHP timezone. Server timezone mismatch could cause valid tokens to be rejected or expired tokens to pass. |
| 10 | **Medium** | `includes/Render.php` (default timezone) | Hardcoded default timezone `'Asia/Jakarta'` in Render.php instead of using `wp_timezone_string()`. |
---
## 3. Architecture & Code Quality Issues
| # | Issue | Detail |
|---|-------|--------|
| 1 | **No Composer dependency management** | Custom `spl_autoload_register` works but there's no `composer.json`. The `vendor/` folder contains manually copied libraries with no version locking, no autoload optimization, and no security update path. |
| 2 | **SingletonTrait incomplete** | `__wakeup()` is `public` (should be `private`). Missing `__sleep()` to prevent serialization-based singleton bypass. |
| 3 | **No database migration system** | `dbDelta()` runs on every `init` for all custom tables. No version tracking for schema changes. Adding columns later requires manual SQL or relying on `dbDelta` diff detection. |
| 4 | **Inconsistent class instantiation** | Some classes use Singleton trait, others (`Token`) don't. `new \Formipay\Token` in `Init.php` bypasses the singleton pattern entirely. |
| 5 | **Global functions vs OOP** | `admin/functions.php` has ~40 global functions (e.g., `formipay_price_format`, `formipay_get_order`, `formipay_currency_array`). Should be in utility/service classes for testability and namespace hygiene. |
| 6 | **Insufficient capability checks** | Several admin-ajax handlers verify nonce but don't check `current_user_can('manage_options')`. Any authenticated user (even subscriber) with a valid nonce could potentially access admin functions. |
| 7 | **Hardcoded English strings** | Many UI strings in JS-localized data and some PHP output are hardcoded in English without `__()` translation functions. |
| 8 | **No proper REST API** | Form submission goes through `admin-ajax.php`. A proper REST API endpoint would be more cache-friendly, better structured, and follow WordPress standards. |
| 9 | **Static version constant** | `FORMIPAY_VERSION` is hardcoded to `'1.0.0'` and never updated programmatically. All cache-busting relies on this value. |
| 10 | **Backup file in production** | `includes/Integration/Paypal.phpbak` exists — should be removed from production codebase. |
| 11 | **No deactivation/uninstall cleanup** | No `uninstall.php` or deactivation hook to clean up custom tables, scheduled events, or options. |
| 12 | **No `.distignore` or build process** | No `.distignore` file for WordPress.org packaging. Development files would be shipped to production. |
---
## 4. Missing Features & Modules
| # | Module / Area | Status | Priority |
|---|---------------|--------|----------|
| 1 | **ExchangeRateAPI** | File exists but is **completely empty** — just an empty class extending Payment | 🔴 High |
| 2 | **License API implementation** | All 4 REST endpoints (`verify`, `activate`, `deactivate`, `revoke`) are **stubs returning hardcoded `ok: true`** | 🔴 High |
| 3 | **Subscription / Recurring payments** | Listed in `readme.txt` as "Planned" but no code exists | 🟡 Medium |
| 4 | **Donation forms** | `formipay_is_donation()` and `donation_config` filter exist, but no donation-specific frontend UI (no "pay what you want" input, no suggested amounts) | 🟡 Medium |
| 5 | **Inventory / Stock management** | Product has `stock_config` in settings but no stock decrement on order, no stock validation before submission, no "out of stock" frontend message | 🟡 Medium |
| 6 | **Product variations on frontend** | Variation config exists in Product admin, but `Order.php` doesn't process variation selections — no variation dropdown rendered on frontend | 🟡 Medium |
| 7 | **Tax system** | No tax calculation anywhere. Products have price, shipping has fees, but there is no tax engine. Essential for physical product sales in most jurisdictions. | 🟡 Medium |
| 8 | **Customer portal / dashboard** | Referenced in readme and `single-formipay.php` template exists, but no customer-facing order history page, no login/registration flow, no "My Orders" view | 🟡 Medium |
| 9 | **Refund system** | `refunded` status exists in status list, but no refund workflow, no partial refund, no payment reversal integration | 🟡 Medium |
| 10 | **Form analytics / reporting** | No form view tracking, no conversion rate calculation, no abandonment tracking, no dashboard charts | 🟢 Low |
| 11 | **Export / Import** | No CSV/Excel export for orders, customers, or products. No bulk import for products. | 🟢 Low |
| 12 | **Outgoing webhook system** | No way to notify external systems (Zapier, Slack, custom endpoints) on order events | 🟢 Low |
| 13 | **Form template library** | No pre-built form templates for common use cases (simple product, donation, registration) | 🟢 Low |
| 14 | **Multi-step form navigation** | Page break config exists in admin, step indicators render, but no frontend JS to actually navigate between steps | 🟡 Medium |
| 15 | **Email log admin page** | `formipay_notification_log` table exists but there's no admin page to view email history, resend failed emails, or debug delivery | 🟡 Medium |
---
## 5. Performance Issues
| # | Location | Issue |
|---|----------|-------|
| 1 | `includes/Thankyou.php` `set_endpoint()` | `flush_rewrite_rules()` on **every page load** — one of the most expensive WordPress operations. Should only run on plugin activation and settings save. |
| 2 | `includes/Payment/Payment.php` `set_endpoint()` | Same `flush_rewrite_rules()` issue. |
| 3 | `includes/Order.php` `formipay_tabledata_orders()` | Runs **two separate full-table queries** — one fetching ALL orders just to count statuses (`O(n)` in memory), plus the paginated query. Should use `COUNT(*) ... GROUP BY status` for the report. |
| 4 | `includes/Customer.php` `formipay_tabledata_customers()` | **No pagination** — loads ALL customers into memory and returns them. Will crash or timeout with large datasets. |
| 5 | `admin/functions.php` `formipay_currency_array()` | `file_get_contents()` of `currencies.json` on **every call** with no caching. Should use a static variable or WordPress transient. |
| 6 | `admin/functions.php` `formipay_country_array()` | Same issue — reads JSON file from disk on every function call. |
| 7 | `includes/Init.php` `default_config()` | The entire ~200-line default config array is loaded on every `plugin_loaded` action. Should be lazy-loaded or only when saving defaults. |
| 8 | `includes/Render.php` | All form CSS is output as inline `<style>` in the HTML `<body>` — not cacheable by browsers, duplicated on every page with the form shortcode. |
| 9 | `includes/Token.php` `create_db()` | Runs `dbDelta()` on every `init` — redundant after first install. |
| 10 | `includes/Customer.php` `create_db()` | Same `dbDelta()` on every `init`. |
| 11 | `includes/Order.php` `create_db()` | Same issue. |
---
## 6. Missing Admin Pages / Settings
| # | Missing Page | Description |
|---|-------------|-------------|
| 1 | **Notification Log** | Table `formipay_notification_log` exists but there's no admin page to view email history, resend failed emails, or debug delivery issues. |
| 2 | **License detail/edit** | License list page exists but there's no detail or edit view (can't manually activate, revoke, or extend a license). |
| 3 | **Dashboard / Analytics** | No overview dashboard with form views, submissions, revenue charts, or conversion rates. |
| 4 | **System Status / Tools** | No page to check PayPal connectivity, email delivery test, database health, or scheduled events. |
| 5 | **Customer order history** | Customer detail page exists but doesn't show linked order history or purchase timeline. |
| 6 | **Import/Export tools** | No admin UI to bulk import products or export order/customer data. |
---
## 7. Code Cleanup Needed
| # | Item | Detail |
|---|------|--------|
| 1 | **Commented-out code** | `Init.php` has commented-out PayPal init, taxonomy, and defer-attribute filter. Should be removed or tracked as TODOs. |
| 2 | **`Paypal.phpbak`** | Backup file in production directory `includes/Integration/Paypal.phpbak` — must be deleted. |
| 3 | **Dead code in `Notification.php`** | `$last_notification_field = count($notification_fields) - 1; $notification_fields[$last_notification_field]['group'] = 'ended'` — uses numeric array index instead of the key name, which breaks if filter order changes. |
| 4 | **Inconsistent nonce naming** | Different pages use different nonce names: `formipay-order-details`, `formipay-admin-coupon-page`, `formipay-admin-licenses`, `formipay-admin-product-page`, `formipay-admin-access-nonce`, `formipay-form-editor`, `formipay-thankyou-nonce`. Should follow a single convention. |
| 5 | **Deprecated `FILTER_SANITIZE_STRING`** | Used in `Customer.php` `customers_page()` — deprecated since PHP 8.1. Should use `FILTER_SANITIZE_SPECIAL_CHARS` or `sanitize_text_field()`. |
| 6 | **Mixed indentation** | Some files use tabs, some spaces, some mix both within the same file. |
| 7 | **Missing PHPDoc blocks** | Most methods have no docblocks. Return types are rarely declared. Makes IDE support and static analysis poor. |
| 8 | **No `.editorconfig`** | No project-wide formatting standard file. |
---
## 8. Opportunities & Nice-to-Haves
| # | Opportunity | Business Impact |
|---|------------|-----------------|
| 1 | **Composer package management** | Replace manual vendor copies with proper Composer dependencies for auto-updates, security patches, and smaller distribution |
| 2 | **React-based form builder** | Replace the partial Vue editor canvas with a full React drag-and-drop form builder for better UX and extensibility |
| 3 | **Webhook/API integrations** | Zapier, Make (Integromat), Slack notifications — extend the notification system for 3rd-party automation |
| 4 | **Stripe payment gateway** | Most requested gateway after PayPal. The abstract `Payment` class makes it architecturally straightforward to add |
| 5 | **PDF invoice generation** | Auto-generate PDF invoices/receipts for orders — high-value feature for business users |
| 6 | **Google Analytics / Facebook Pixel** | E-commerce event tracking (`purchase`, `add_to_cart`) for conversion optimization |
| 7 | **Multi-vendor / marketplace** | Architecture already supports per-form products — extend to multi-vendor marketplace |
| 8 | **Form A/B testing** | Duplicate form feature already exists — add conversion tracking and statistical comparison |
| 9 | **Cart / checkout recovery** | Store partial submissions in `localStorage` + send recovery emails for abandoned forms |
| 10 | **Full localization** | Generate `.pot` file, translate to top 10 languages. Currently only partially translatable |
| 11 | **WP-CLI commands** | `wp formipay order list`, `wp formipay product create`, `wp formipay license verify`, etc. |
| 12 | **Headless / REST API** | Full REST API for headless WordPress + Next.js / Nuxt frontends |
| 13 | **Gutenberg block** | Register a `formipay/form` block for native Gutenberg integration instead of shortcode-only |
| 14 | **Unit & integration tests** | Zero test coverage currently. Critical for a payment plugin handling money |
| 15 | **Rate limiting on public endpoints** | `retrieve_form_data` and `check_coupon_code` have no rate limiting. Vulnerable to brute-force and spam |
---
## 9. Summary Priority Matrix
| Priority | Count | Key Items |
|----------|-------|-----------|
| 🔴 **Critical (fix immediately)** | 6 | Customer update fatal error, Order delete undefined variable, Bulk delete wrong variable, Email send wrong class, flush_rewrite_rules performance, License API stubs |
| 🟡 **High (next sprint)** | 12 | Unique code race condition, PayPal webhook verification, ExchangeRateAPI empty class, Stock management, Donation UI, Notification log page, Customer portal, Tax system |
| 🟢 **Medium (backlog)** | 15 | Refund workflow, Analytics dashboard, Export/Import, Form templates, Gutenber block, Localization, Rate limiting |
| ⚪ **Nice-to-have (roadmap)** | 12 | WP-CLI, A/B testing, Headless API, PDF invoices, Multi-vendor, Cart recovery, CI/CD pipeline |
---
## 10. Architectural Recommendation
> See `RECOMMENDATION.md` for the detailed technical recommendation.
---
*End of audit report.*