fix(billing): resolve code-review findings (authz, cross-billing, validation, webhook integrity)

- C1/H4: rating cron only rates subs on the charge's own plan_id
- C1: _fc_rate_usage skips creating a line when amount is 0 (still updates existing)
- C2/C4: /usage authorizes each event (exists + is_subscription + linked customer)
- C3: API handlers validate input and return 4xx-shaped errors instead of raising;
       controller maps status=='error' to HTTP 400
- H1: cron uses real billing window [last_invoice_date or start_date, next_invoice_date)
- H2: _aggregate uses half-open window anchored on period_start
- H3: idempotency scoped to (subscription_id, metric_id, idempotency_key)
- H5: webhook stores canonical body, signs+POSTs it verbatim, adds X-Fusion-Event-Id,
       caps backoff at 2**min(attempts,10)
- H6: SSRF guard rejects non-https / localhost / private / link-local webhook_url
- M7: charge_model reduced to standard/package (dropped unimplemented graduated/volume)
- L1: currency_id required on charge + reconciliation
- L2: charge price non-negative + unit_batch positive DB constraints

Adds 17 regression tests (suite 22 -> 39, all green via fcb_test_on_trial.sh).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsinghpal
2026-05-27 03:27:34 -04:00
parent a5db0fe71e
commit d770c0c3a9
11 changed files with 442 additions and 32 deletions

View File

@@ -2,9 +2,12 @@
# Copyright 2026 Nexa Systems Inc.
# License OPL-1
import hashlib
import ipaddress
import secrets
from urllib.parse import urlparse
from odoo import api, fields, models
from odoo.exceptions import ValidationError
class FusionBillingService(models.Model):
@@ -45,6 +48,33 @@ class FusionBillingService(models.Model):
for rec in self:
rec.account_link_count = len(rec.account_link_ids)
@api.constrains("webhook_url")
def _check_webhook_url(self):
"""Reject SSRF-prone webhook targets: a non-empty URL must be https and must
not point at localhost or a private / link-local / loopback IP literal. Empty
is allowed (no webhook configured)."""
for rec in self:
url = (rec.webhook_url or "").strip()
if not url:
continue
parsed = urlparse(url)
if parsed.scheme != "https":
raise ValidationError("Webhook URL must use https.")
host = parsed.hostname or ""
if not host or host.lower() in ("localhost", "ip6-localhost", "ip6-loopback"):
raise ValidationError(
"Webhook URL must not target localhost or a private address.")
try:
ip = ipaddress.ip_address(host)
except ValueError:
ip = None
if ip is not None and (
ip.is_private or ip.is_loopback or ip.is_link_local
or ip.is_reserved or ip.is_unspecified or ip.is_multicast
):
raise ValidationError(
"Webhook URL must not target a private or loopback address.")
def action_generate_api_key(self):
"""Generate a fresh bearer key, store only its hash, return the raw key.
@@ -64,7 +94,14 @@ class FusionBillingService(models.Model):
return self.search([('api_key_hash', '=', key_hash), ('active', '=', True)], limit=1)
def _api_upsert_customer(self, payload):
"""Resolve/create the partner link for an external account.
Defensive: a non-dict payload or a missing/empty ``external_id`` returns a
4xx-shaped error instead of raising (C3).
"""
self.ensure_one()
if not isinstance(payload, dict):
return {'status': 'error', 'error': 'invalid payload'}
ext = payload.get('external_id')
if not ext:
return {'status': 'error', 'error': 'external_id required'}
@@ -73,15 +110,53 @@ class FusionBillingService(models.Model):
return {'status': 'ok', 'partner_id': link.partner_id.id, 'external_id': ext}
def _api_record_usage(self, payload):
"""Ingest a batch of usage events.
Authorization (C2/C4): each event must target a subscription sale.order that
(a) exists, (b) is actually a subscription, and (c) belongs to a customer THIS
service is linked to. Any failing event is rejected and stops processing for
that event without writing a usage row.
Validation (C3): a non-dict payload, a non-list ``events``, missing required
keys, or non-numeric ``quantity``/ids return a 4xx-shaped error instead of
raising (no 500s).
"""
self.ensure_one()
events = payload.get('events') or []
if not isinstance(payload, dict):
return {'status': 'error', 'error': 'invalid payload'}
events = payload.get('events')
if events is None:
events = []
if not isinstance(events, list):
return {'status': 'error', 'error': 'events must be a list'}
Usage = self.env['fusion.billing.usage']
linked_partners = self.account_link_ids.mapped('partner_id')
accepted = 0
for ev in events:
sub = self.env['sale.order'].browse(int(ev['subscription_external_id']))
Usage._record_usage(
sub, ev['metric_code'], float(ev['quantity']),
ev['period_start'], ev['period_end'], idem=ev.get('idempotency_key'))
if not isinstance(ev, dict):
return {'status': 'error', 'error': 'invalid event'}
for key in ('subscription_external_id', 'metric_code', 'quantity',
'period_start', 'period_end'):
if ev.get(key) in (None, ''):
return {'status': 'error', 'error': 'missing %s' % key}
try:
sub_id = int(ev['subscription_external_id'])
except (TypeError, ValueError):
return {'status': 'error', 'error': 'invalid subscription_external_id'}
try:
quantity = float(ev['quantity'])
except (TypeError, ValueError):
return {'status': 'error', 'error': 'invalid quantity'}
sub = self.env['sale.order'].browse(sub_id)
if not sub.exists() or not sub.is_subscription \
or sub.partner_id not in linked_partners:
return {'status': 'error', 'error': 'unknown subscription'}
try:
Usage._record_usage(
sub, ev['metric_code'], quantity,
ev['period_start'], ev['period_end'], idem=ev.get('idempotency_key'))
except ValueError as e:
return {'status': 'error', 'error': str(e)}
accepted += 1
return {'status': 'ok', 'accepted': accepted}
@@ -100,21 +175,49 @@ class FusionBillingService(models.Model):
The product on each line must have recurring_invoice=True so that
Odoo recognises the order as a subscription with has_recurring_line and
action_confirm() reaches subscription_state='3_progress'.
Validation (C3): a non-dict payload, a missing/unknown customer, a missing
``plan_id``, a non-list ``lines``, or a non-numeric product id/quantity
return a 4xx-shaped error instead of raising (no 500s).
"""
self.ensure_one()
if not isinstance(payload, dict):
return {'status': 'error', 'error': 'invalid payload'}
if not payload.get('external_customer_id'):
return {'status': 'error', 'error': 'external_customer_id required'}
if not payload.get('plan_id'):
return {'status': 'error', 'error': 'plan_id required'}
try:
plan_id = int(payload['plan_id'])
except (TypeError, ValueError):
return {'status': 'error', 'error': 'invalid plan_id'}
link = self.env['fusion.billing.account.link'].search([
('service_id', '=', self.id),
('external_id', '=', payload.get('external_customer_id')),
], limit=1)
if not link:
return {'status': 'error', 'error': 'unknown customer'}
order_lines = [(0, 0, {
'product_id': line['product_id'],
'product_uom_qty': line.get('quantity', 1),
}) for line in payload.get('lines', [])]
lines = payload.get('lines')
if lines is None:
lines = []
if not isinstance(lines, list):
return {'status': 'error', 'error': 'lines must be a list'}
order_lines = []
for line in lines:
if not isinstance(line, dict) or line.get('product_id') in (None, ''):
return {'status': 'error', 'error': 'invalid line'}
try:
product_id = int(line['product_id'])
quantity = float(line.get('quantity', 1))
except (TypeError, ValueError):
return {'status': 'error', 'error': 'invalid line'}
order_lines.append((0, 0, {
'product_id': product_id,
'product_uom_qty': quantity,
}))
sub = self.env['sale.order'].sudo().create({
'partner_id': link.partner_id.id,
'plan_id': payload['plan_id'],
'plan_id': plan_id,
'order_line': order_lines,
})
sub.action_confirm()