From 86b8e59c95da5d81978abdc9e281658a18076b42 Mon Sep 17 00:00:00 2001
From: gsinghpal <gur_preet.singh@outlook.com>
Date: Tue, 26 May 2026 21:50:24 -0400
Subject: [PATCH] feat(fusion_login_audit): async geo enrichment cron

5-min cron processes up to 100 pending rows per pass: private IPs
short-circuit to state=private_ip; same-IP cache (30 days) avoids
duplicate ip-api.com calls; reverse DNS via socket with 1.5s timeout;
HTTP lookup respects ip-api''s X-Rl rate-limit header. Tests cover
private-IP shortcut, cache hit (no HTTP), and internal-state skip --
no network calls needed.

Per-row isolation uses cr.savepoint() instead of cr.commit() because
Odoo 19 TestCursor raises AssertionError on commit/rollback. Recorded
the gotcha as CLAUDE.md rule #14.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 CLAUDE.md                                     |   1 +
 fusion_login_audit/data/ir_cron_data.xml      |  11 ++
 .../models/fusion_login_audit.py              | 153 +++++++++++++++++-
 fusion_login_audit/tests/test_login_audit.py  |  61 +++++++
 4 files changed, 225 insertions(+), 1 deletion(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 579ee46b..32e8f731 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -29,6 +29,7 @@
 11. **Inherited `ir.ui.view` records cannot have `groups`/`group_ids` on the record itself.** Odoo 19 raises `ParseError: Inherited view cannot have 'groups' defined on the record. Use 'groups' attributes inside the view definition` at install time. Move the gate to the inner XML nodes — every `<button>`, `<page>`, `<field>`, `<xpath>`, `<group>` etc. supports a `groups="base.group_system"` attribute. For an inherited form with a smart button + admin tab, put `groups=` on the button and the page individually; leave the `<record model="ir.ui.view">` clean.
 12. **`mail.template` QWeb/inline_template `ctx` IS `self.env.context`** — not a nested dict you can pass. `MailRenderMixin._render_eval_context()` sets `ctx = self.env.context`, so `ctx.get('foo')` in subject/body resolves to `env.context.get('foo')`. To pass dynamic data to a template, spread keys directly into the context: `tmpl.with_context(**my_data).send_mail(res_id, ...)`. Calling `tmpl.with_context(ctx=my_data)` puts the dict at `env.context['ctx']`, and the template's `ctx.get('foo')` becomes `env.context.get('foo')` → `None` (looks like a silent rendering bug — subject ends up blank).
 13. **`ir.cron` dropped `numbercall`** in Odoo 19. Old recipes set `<field name="numbercall">-1</field>` for "run forever"; that now raises `ValueError: Invalid field 'numbercall' in 'ir.cron'` at install time. Just omit the field — recurring crons keep running as long as `active=True`. Source: `odoo/addons/base/models/ir_cron.py` field list.
+14. **`cr.commit()` / `cr.rollback()` raise AssertionError inside `TransactionCase`** — they are NOT silent no-ops in Odoo 19. The test cursor explicitly refuses both ("Cannot commit or rollback a cursor from inside a test, this will lead to a broken cursor when trying to rollback the test. Please rollback to a specific savepoint instead..."). For cron/worker code that needs per-row isolation so one bad row doesn't roll back the whole batch, use `with self.env.cr.savepoint(): ...` inside the loop instead of `cr.commit()`. Savepoints work in both prod (under the outer cron transaction) and tests (under the outer test transaction). The cron transaction commits the whole batch when the method returns; in tests everything rolls back cleanly. Source: `odoo/sql_db.py::TestCursor.commit` and `Cursor.savepoint()`.
 
 15. **There is NO `sale.subscription` model in Odoo 19** (Enterprise `sale_subscription`). A subscription is a **`sale.order`** with `is_subscription=True`, `plan_id` → **`sale.subscription.plan`** (the recurrence), plus `subscription_state` / `next_invoice_date` / `recurring_monthly`. Any Many2one or relation that targets "a subscription" must point at `sale.order` (filter `domain=[('is_subscription','=',True)]`) — **not** `sale.subscription`, which does not exist and fails at install. The surviving `sale.subscription.*` records are only the plan + wizards/reports (`sale.subscription.plan`, `sale.subscription.report`, `sale.subscription.change.customer.wizard`, `sale.subscription.close.reason.wizard`). Verified on live `nexamain` (odoo-nexa, 19.0): `SELECT model FROM ir_model WHERE model LIKE 'sale.subscription%'`.
 
diff --git a/fusion_login_audit/data/ir_cron_data.xml b/fusion_login_audit/data/ir_cron_data.xml
index a7fee2cc..aaeaf103 100644
--- a/fusion_login_audit/data/ir_cron_data.xml
+++ b/fusion_login_audit/data/ir_cron_data.xml
@@ -12,5 +12,16 @@
             <field name="active" eval="True"/>
         </record>
 
+        <record id="cron_geo_enrich" model="ir.cron">
+            <field name="name">Fusion Login Audit: Geo Enrichment</field>
+            <field name="model_id" ref="model_fusion_login_audit"/>
+            <field name="state">code</field>
+            <field name="code">model._fc_geo_enrich_pending(limit=100)</field>
+            <field name="interval_number">5</field>
+            <field name="interval_type">minutes</field>
+            <field name="active" eval="True"/>
+            <field name="priority">10</field>
+        </record>
+
     </data>
 </odoo>
diff --git a/fusion_login_audit/models/fusion_login_audit.py b/fusion_login_audit/models/fusion_login_audit.py
index 4bd29d55..3cc5ebc2 100644
--- a/fusion_login_audit/models/fusion_login_audit.py
+++ b/fusion_login_audit/models/fusion_login_audit.py
@@ -1,6 +1,15 @@
 # -*- coding: utf-8 -*-
+import ipaddress
+import logging
+import socket
+from datetime import timedelta
+
+import requests
+
 from odoo import api, fields, models
 
+_logger = logging.getLogger(__name__)
+
 
 class FusionLoginAudit(models.Model):
     _name = 'fusion.login.audit'
@@ -88,7 +97,6 @@ class FusionLoginAudit(models.Model):
     def _fc_retention_gc(self):
         """Delete audit rows older than `fusion_login_audit.retention_days`.
         Called daily by ir.cron. retention_days=0 means keep forever."""
-        from datetime import timedelta
         ICP = self.env['ir.config_parameter'].sudo()
         try:
             days = int(ICP.get_param(
@@ -103,3 +111,146 @@ class FusionLoginAudit(models.Model):
         if old:
             old.unlink()
         return count
+
+    _FC_PRIVATE_NETWORKS = (
+        ipaddress.ip_network('10.0.0.0/8'),
+        ipaddress.ip_network('172.16.0.0/12'),
+        ipaddress.ip_network('192.168.0.0/16'),
+        ipaddress.ip_network('127.0.0.0/8'),
+        ipaddress.ip_network('::1/128'),
+        ipaddress.ip_network('fe80::/10'),
+    )
+
+    @api.model
+    def _fc_is_private_ip(self, ip):
+        if not ip or ip == 'internal':
+            return False  # 'internal' uses its own state
+        try:
+            addr = ipaddress.ip_address(ip)
+        except ValueError:
+            return False
+        return any(addr in net for net in self._FC_PRIVATE_NETWORKS)
+
+    @api.model
+    def _fc_geo_cache_hit(self, ip):
+        """Return a dict of geo fields if we've resolved this IP in the last
+        30 days, else None."""
+        if not ip:
+            return None
+        cutoff = fields.Datetime.now() - timedelta(days=30)
+        cached = self.sudo().search([
+            ('ip_address', '=', ip),
+            ('geo_lookup_state', '=', 'done'),
+            ('event_time', '>=', cutoff),
+        ], limit=1, order='event_time desc')
+        if not cached:
+            return None
+        return {
+            'country_code': cached.country_code,
+            'country_name': cached.country_name,
+            'city': cached.city,
+            'geo_state': cached.geo_state,
+            'ip_hostname': cached.ip_hostname,
+        }
+
+    @api.model
+    def _fc_geo_reverse_dns(self, ip):
+        try:
+            socket.setdefaulttimeout(1.5)
+            host, _aliases, _ips = socket.gethostbyaddr(ip)
+            return (host or '')[:255]
+        except (socket.herror, socket.gaierror, OSError):
+            return ''
+        finally:
+            socket.setdefaulttimeout(None)
+
+    @api.model
+    def _fc_geo_http_lookup(self, ip):
+        """Call ip-api.com. Returns (vals_dict, rate_limited_bool).
+        Falls back to ({}, False) on any error."""
+        try:
+            resp = requests.get(
+                'http://ip-api.com/json/' + ip,
+                params={'fields': 'status,country,countryCode,regionName,city'},
+                timeout=3,
+                headers={'User-Agent': 'Odoo-FusionLoginAudit/19.0'},
+            )
+            rate_limited = resp.headers.get('X-Rl', '') == '0'
+            if resp.status_code != 200:
+                return ({}, rate_limited)
+            data = resp.json() or {}
+            if data.get('status') != 'success':
+                return ({}, rate_limited)
+            return ({
+                'country_code': (data.get('countryCode') or '')[:2],
+                'country_name': (data.get('country') or '')[:64],
+                'geo_state': (data.get('regionName') or '')[:64],
+                'city': (data.get('city') or '')[:128],
+            }, rate_limited)
+        except (requests.RequestException, ValueError):
+            _logger.warning("fusion_login_audit: geo lookup failed for %s",
+                            ip, exc_info=True)
+            return ({}, False)
+
+    @api.model
+    def _fc_geo_enrich_pending(self, limit=100):
+        """Cron worker: process up to `limit` pending rows.
+
+        Per-row isolation is provided by `cr.savepoint()` rather than
+        `cr.commit()`/`cr.rollback()` — the latter raises an AssertionError
+        inside a TransactionCase (Odoo's test cursor refuses commit/rollback).
+        Savepoints work in both prod and tests; the outer cron transaction
+        commits the lot once the method returns. One bad IP rolls back only
+        its own savepoint, so the rest of the batch still lands.
+        """
+        pending = self.sudo().search(
+            [('geo_lookup_state', '=', 'pending')],
+            order='event_time asc', limit=limit,
+        )
+        if not pending:
+            return 0
+        processed = 0
+        stop_after_this = False
+        for row in pending:
+            ip = row.ip_address
+            try:
+                with self.env.cr.savepoint():
+                    if self._fc_is_private_ip(ip):
+                        row.write({
+                            'geo_lookup_state': 'private_ip',
+                            'country_code': '--',
+                            'country_name': 'Private network',
+                            'city': 'Private network',
+                        })
+                        processed += 1
+                        continue
+
+                    cached = self._fc_geo_cache_hit(ip)
+                    if cached:
+                        cached['geo_lookup_state'] = 'done'
+                        row.write(cached)
+                        processed += 1
+                        continue
+
+                    hostname = self._fc_geo_reverse_dns(ip) if ip and ip != 'internal' else ''
+                    vals, rate_limited = self._fc_geo_http_lookup(ip) if ip and ip != 'internal' else ({}, False)
+                    if vals:
+                        vals['ip_hostname'] = hostname
+                        vals['geo_lookup_state'] = 'done'
+                        row.write(vals)
+                    else:
+                        row.write({
+                            'geo_lookup_state': 'failed',
+                            'ip_hostname': hostname,
+                        })
+                    processed += 1
+                    if rate_limited:
+                        _logger.info("fusion_login_audit: ip-api rate limit "
+                                     "hit, stopping batch early")
+                        stop_after_this = True
+            except Exception:
+                _logger.exception(
+                    "fusion_login_audit: geo enrich failed for row %s", row.id)
+            if stop_after_this:
+                break
+        return processed
diff --git a/fusion_login_audit/tests/test_login_audit.py b/fusion_login_audit/tests/test_login_audit.py
index 38802f9f..d7767c77 100644
--- a/fusion_login_audit/tests/test_login_audit.py
+++ b/fusion_login_audit/tests/test_login_audit.py
@@ -440,3 +440,64 @@ class TestFusionLoginAuditModel(TransactionCase):
 
         self.assertTrue(Audit.browse(ancient_id).exists(),
                         "retention_days=0 must keep everything")
+
+    def test_geo_private_ip_shortcut(self):
+        """Private IPs short-circuit to state='private_ip' without HTTP."""
+        Audit = self.env['fusion.login.audit'].sudo()
+        rec = Audit.create({
+            'attempted_login': 'lan@example.com',
+            'result': 'success',
+            'ip_address': '192.168.1.40',
+            'geo_lookup_state': 'pending',
+        })
+        Audit._fc_geo_enrich_pending(limit=10)
+        rec.invalidate_recordset()
+        self.assertEqual(rec.geo_lookup_state, 'private_ip')
+        self.assertEqual(rec.country_code, '--')
+
+    def test_geo_cache_hit_avoids_http(self):
+        """A second row with the same recent IP copies from cache."""
+        from unittest.mock import patch
+        Audit = self.env['fusion.login.audit'].sudo()
+        # Seed a "done" row from the same IP.
+        Audit.create({
+            'attempted_login': 'seed@example.com',
+            'result': 'success',
+            'ip_address': '203.0.113.99',
+            'geo_lookup_state': 'done',
+            'country_code': 'CA',
+            'country_name': 'Canada',
+            'city': 'Toronto',
+            'geo_state': 'Ontario',
+        })
+        target = Audit.create({
+            'attempted_login': 'hit@example.com',
+            'result': 'success',
+            'ip_address': '203.0.113.99',
+            'geo_lookup_state': 'pending',
+        })
+
+        with patch(
+            'odoo.addons.fusion_login_audit.models.fusion_login_audit.requests.get'
+        ) as mock_get:
+            Audit._fc_geo_enrich_pending(limit=10)
+            mock_get.assert_not_called()
+
+        target.invalidate_recordset()
+        self.assertEqual(target.geo_lookup_state, 'done')
+        self.assertEqual(target.country_code, 'CA')
+        self.assertEqual(target.city, 'Toronto')
+
+    def test_geo_internal_skipped(self):
+        """Rows with geo_lookup_state='internal' are not picked up."""
+        Audit = self.env['fusion.login.audit'].sudo()
+        rec = Audit.create({
+            'attempted_login': 'cron@example.com',
+            'result': 'success',
+            'ip_address': 'internal',
+            'geo_lookup_state': 'internal',
+        })
+        # Should be a no-op for 'internal' state (cron only picks 'pending').
+        Audit._fc_geo_enrich_pending(limit=10)
+        rec.invalidate_recordset()
+        self.assertEqual(rec.geo_lookup_state, 'internal')