From 78da716621455abdd3e4eea04ef5a89160dcdba9 Mon Sep 17 00:00:00 2001 From: litnimax Date: Sun, 19 Jul 2026 15:42:09 +0300 Subject: [PATCH 1/3] Fix security hardening regressions --- .../odusite_base/models/odusite_rate_limit.py | 72 ++++++++++++------- addons/odusite_base/tests/__init__.py | 1 + addons/odusite_base/tests/common.py | 8 +++ addons/odusite_base/tests/test_rate_limit.py | 56 +++++++++++++++ addons/odusite_crm/tests/test_contact_form.py | 14 ++-- site/src/blocks/events/pages/event.astro | 5 +- site/src/blocks/forms/pages/contact.astro | 6 +- site/src/blocks/jobs/api/apply.ts | 2 +- site/src/blocks/jobs/pages/job.astro | 6 +- .../components/NewsletterForm.astro | 5 +- site/src/lib/api/client.ts | 5 +- 11 files changed, 137 insertions(+), 43 deletions(-) create mode 100644 addons/odusite_base/tests/test_rate_limit.py diff --git a/addons/odusite_base/models/odusite_rate_limit.py b/addons/odusite_base/models/odusite_rate_limit.py index 1dda2b6..3a98e3a 100644 --- a/addons/odusite_base/models/odusite_rate_limit.py +++ b/addons/odusite_base/models/odusite_rate_limit.py @@ -4,12 +4,12 @@ event registration / auth). One row per key with an atomic ``INSERT ... ON CONFLICT`` counter: concurrent requests cannot lose updates (unlike a single JSON ``ir.config_parameter``) and each key is an isolated row, -so there is no global write hotspot. The client IP is the Cloudflare-forwarded -``CF-Connecting-IP`` (the site forwards it), not ``remote_addr`` — behind a -tunnel the latter is the proxy address, which would bucket every visitor -together. +so there is no global write hotspot. The site forwards Cloudflare's client IP +in ``X-Odusite-Client-IP`` because Cloudflare replaces ``CF-Connecting-IP`` on +cross-zone Worker subrequests. """ +import ipaddress import time from odoo import api, fields, models @@ -30,6 +30,7 @@ class OdusiteRateLimit(models.Model): key = fields.Char(required=True) window_start = fields.Integer(required=True) + expires_at = fields.Integer(required=True, default=0) hits = fields.Integer(default=0) _key_unique = models.Constraint('UNIQUE(key)', 'One throttle row per key.') @@ -37,12 +38,18 @@ class OdusiteRateLimit(models.Model): @api.model def _client_ip(self): headers = request.httprequest.headers - return ( - headers.get('CF-Connecting-IP') - or headers.get('X-Forwarded-For', '').split(',')[0].strip() - or request.httprequest.remote_addr - or 'unknown' + candidates = ( + headers.get('X-Odusite-Client-IP'), + headers.get('CF-Connecting-IP'), + headers.get('X-Forwarded-For', '').split(',')[0].strip(), + request.httprequest.remote_addr, ) + for candidate in candidates: + try: + return str(ipaddress.ip_address(candidate)) + except (TypeError, ValueError): + continue + return 'unknown' @api.model def _enforce(self, scope='form', limit=None, window=None, key=None): @@ -67,22 +74,35 @@ def _enforce(self, scope='form', limit=None, window=None, key=None): return full_key = '%s:%s' % (scope, key or self._client_ip()) now = int(time.time()) - self.env.cr.execute( - """ - INSERT INTO odusite_rate_limit (key, window_start, hits) - VALUES (%(k)s, %(now)s, 1) - ON CONFLICT (key) DO UPDATE SET - hits = CASE - WHEN odusite_rate_limit.window_start > %(now)s - %(w)s - THEN odusite_rate_limit.hits + 1 ELSE 1 END, - window_start = CASE - WHEN odusite_rate_limit.window_start > %(now)s - %(w)s - THEN odusite_rate_limit.window_start ELSE %(now)s END - RETURNING hits - """, - {'k': full_key, 'now': now, 'w': window}, - ) - hits = self.env.cr.fetchone()[0] + # API errors roll back the request cursor. Commit the throttle update on + # an isolated cursor so rejected attempts still count toward the limit. + with self.env.registry.cursor() as cr: + cr.execute( + """ + INSERT INTO odusite_rate_limit + (key, window_start, expires_at, hits) + VALUES (%(k)s, %(now)s, %(expires)s, 1) + ON CONFLICT (key) DO UPDATE SET + hits = CASE + WHEN odusite_rate_limit.expires_at > %(now)s + THEN LEAST(odusite_rate_limit.hits + 1, %(cap)s) + ELSE 1 END, + window_start = CASE + WHEN odusite_rate_limit.expires_at > %(now)s + THEN odusite_rate_limit.window_start ELSE %(now)s END, + expires_at = CASE + WHEN odusite_rate_limit.expires_at > %(now)s + THEN odusite_rate_limit.expires_at ELSE %(expires)s END + RETURNING hits + """, + { + 'k': full_key, + 'now': now, + 'expires': now + max(1, window), + 'cap': limit + 1, + }, + ) + hits = cr.fetchone()[0] if hits > limit: raise ApiError(429, 'too_many_requests', 'Too many submissions, please try again later.') @@ -91,6 +111,6 @@ def _enforce(self, scope='form', limit=None, window=None, key=None): def _gc_rate_limit(self): # Drop rows whose window ended over a day ago to keep the table small. self.env.cr.execute( - "DELETE FROM odusite_rate_limit WHERE window_start < %s", + "DELETE FROM odusite_rate_limit WHERE expires_at < %s", (int(time.time()) - 86400,), ) diff --git a/addons/odusite_base/tests/__init__.py b/addons/odusite_base/tests/__init__.py index c9ac400..add96fb 100644 --- a/addons/odusite_base/tests/__init__.py +++ b/addons/odusite_base/tests/__init__.py @@ -1,4 +1,5 @@ from . import test_api_core from . import test_jwt +from . import test_rate_limit from . import test_search from . import test_webhook_queue diff --git a/addons/odusite_base/tests/common.py b/addons/odusite_base/tests/common.py index 3eecb4e..169716b 100644 --- a/addons/odusite_base/tests/common.py +++ b/addons/odusite_base/tests/common.py @@ -74,6 +74,14 @@ def assert_api_error(self, response, body, status, code): self.assertTrue(body and 'error' in body, f'expected error body, got: {body}') self.assertEqual(body['error']['code'], code) + def clear_rate_limit(self, key): + self.env['odusite.rate.limit'].sudo().search([('key', '=', key)]).unlink() + + def rate_limit_hits(self, key): + row = self.env['odusite.rate.limit'].sudo().search( + [('key', '=', key)], limit=1) + return row.hits if row else None + # -- Auth helpers ---------------------------------------------------- @classmethod diff --git a/addons/odusite_base/tests/test_rate_limit.py b/addons/odusite_base/tests/test_rate_limit.py new file mode 100644 index 0000000..2348298 --- /dev/null +++ b/addons/odusite_base/tests/test_rate_limit.py @@ -0,0 +1,56 @@ +import time + +from odoo.tests.common import TransactionCase, tagged + +from ..controllers.api import ApiError + + +@tagged('post_install', '-at_install') +class TestRateLimit(TransactionCase): + + def _delete_committed_key(self, key): + with self.env.registry.cursor() as cr: + cr.execute("DELETE FROM odusite_rate_limit WHERE key = %s", (key,)) + + def test_rejected_hit_commits_on_isolated_cursor(self): + limiter_key = 'test:rollback-persistence' + RateLimit = self.env['odusite.rate.limit'].sudo() + self.env['ir.config_parameter'].sudo().set_param( + 'odusite.rate_limit_force_in_tests', '1') + self._delete_committed_key(limiter_key) + try: + RateLimit._enforce( + scope='test', key='rollback-persistence', limit=1, window=60) + with self.assertRaises(ApiError): + RateLimit._enforce( + scope='test', key='rollback-persistence', limit=1, window=60) + + with self.env.registry.cursor() as cr: + cr.execute( + "SELECT hits FROM odusite_rate_limit WHERE key = %s", + (limiter_key,), + ) + self.assertEqual(cr.fetchone(), (2,)) + finally: + self._delete_committed_key(limiter_key) + + def test_gc_keeps_active_long_window(self): + now = int(time.time()) + RateLimit = self.env['odusite.rate.limit'].sudo() + active = RateLimit.create({ + 'key': 'test:active-long-window', + 'window_start': now - 2 * 86400, + 'expires_at': now + 86400, + 'hits': 3, + }) + expired = RateLimit.create({ + 'key': 'test:expired-window', + 'window_start': now - 3 * 86400, + 'expires_at': now - 2 * 86400, + 'hits': 2, + }) + + RateLimit._gc_rate_limit() + + self.assertTrue(active.exists(), 'an active long window must survive GC') + self.assertFalse(expired.exists(), 'an old expired window should be deleted') diff --git a/addons/odusite_crm/tests/test_contact_form.py b/addons/odusite_crm/tests/test_contact_form.py index 26e9496..2270f85 100644 --- a/addons/odusite_crm/tests/test_contact_form.py +++ b/addons/odusite_crm/tests/test_contact_form.py @@ -83,20 +83,26 @@ def test_contact_utm(self): def test_contact_rate_limit(self): icp = self.env['ir.config_parameter'].sudo() + client_ip = '198.51.100.20' + limiter_key = f'contact:{client_ip}' + headers = {'X-Odusite-Client-IP': client_ip} # Enforcement is skipped under the test runner by default (counters # accumulate across unrelated cases); this test opts in explicitly. icp.set_param('odusite.rate_limit_force_in_tests', '1') icp.set_param('odusite.form_rate_limit', '1') - self.env['odusite.rate.limit'].sudo().search([]).unlink() + self.clear_rate_limit(limiter_key) try: - response, body = self.api('POST', '/forms/contact', VALID_PAYLOAD) + response, body = self.api( + 'POST', '/forms/contact', VALID_PAYLOAD, headers=headers) self.assertEqual(response.status_code, 200, body) - response, body = self.api('POST', '/forms/contact', VALID_PAYLOAD) + self.assertEqual(self.rate_limit_hits(limiter_key), 1) + response, body = self.api( + 'POST', '/forms/contact', VALID_PAYLOAD, headers=headers) self.assert_api_error(response, body, 429, 'too_many_requests') finally: icp.set_param('odusite.rate_limit_force_in_tests', False) icp.set_param('odusite.form_rate_limit', False) - self.env['odusite.rate.limit'].sudo().search([]).unlink() + self.clear_rate_limit(limiter_key) def test_generic_form_unknown_model(self): # res.users is never in the odusite.api form whitelist. (The full diff --git a/site/src/blocks/events/pages/event.astro b/site/src/blocks/events/pages/event.astro index da42488..a4fad22 100644 --- a/site/src/blocks/events/pages/event.astro +++ b/site/src/blocks/events/pages/event.astro @@ -161,7 +161,7 @@ const Tickets = canRegister ? 'form' : 'div'; <>
{turnstileSiteKey && ( -
+
)} @@ -319,7 +319,8 @@ const Tickets = canRegister ? 'form' : 'div'; } finally { if (submit) submit.disabled = false; // A consumed Turnstile token cannot be reused — reset for a retry. - (window as unknown as { turnstile?: { reset?: () => void } }).turnstile?.reset?.(); + (window as unknown as { turnstile?: { reset?: (widget?: string) => void } }) + .turnstile?.reset?.('#event-register-turnstile'); } }); } diff --git a/site/src/blocks/forms/pages/contact.astro b/site/src/blocks/forms/pages/contact.astro index 3ff2f10..138aa1d 100644 --- a/site/src/blocks/forms/pages/contact.astro +++ b/site/src/blocks/forms/pages/contact.astro @@ -59,7 +59,7 @@ const turnstileSiteKey = getEnv(Astro).PUBLIC_TURNSTILE_SITE_KEY; {turnstileSiteKey && ( -
+
)} @@ -140,8 +140,8 @@ const turnstileSiteKey = getEnv(Astro).PUBLIC_TURNSTILE_SITE_KEY; errorBox.hidden = false; } // A consumed Turnstile token cannot be reused — reset for a retry. - const turnstile = (window as unknown as { turnstile?: { reset?: () => void } }).turnstile; - turnstile?.reset?.(); + const turnstile = (window as unknown as { turnstile?: { reset?: (widget?: string) => void } }).turnstile; + turnstile?.reset?.('#contact-turnstile'); }; form.addEventListener('submit', async (event) => { diff --git a/site/src/blocks/jobs/api/apply.ts b/site/src/blocks/jobs/api/apply.ts index b300b9a..b3453d8 100644 --- a/site/src/blocks/jobs/api/apply.ts +++ b/site/src/blocks/jobs/api/apply.ts @@ -94,7 +94,7 @@ export const POST: APIRoute = async (context) => { const applyHeaders: Record = { 'X-Odusite-Token': env.ODUSITE_TOKEN }; const clientIp = context.request.headers.get('CF-Connecting-IP'); - if (clientIp) applyHeaders['CF-Connecting-IP'] = clientIp; + if (clientIp) applyHeaders['X-Odusite-Client-IP'] = clientIp; const response = await fetch(url, { method: 'POST', diff --git a/site/src/blocks/jobs/pages/job.astro b/site/src/blocks/jobs/pages/job.astro index c875bf1..028a3b9 100644 --- a/site/src/blocks/jobs/pages/job.astro +++ b/site/src/blocks/jobs/pages/job.astro @@ -90,7 +90,7 @@ Astro.response.headers.set(TAGS_HEADER, `jobs,jobs:${job.id}`); {turnstileSiteKey && ( -
+
)} @@ -128,8 +128,8 @@ Astro.response.headers.set(TAGS_HEADER, `jobs,jobs:${job.id}`); errorBox.hidden = false; } // A consumed Turnstile token cannot be reused — reset for a retry. - const turnstile = (window as unknown as { turnstile?: { reset?: () => void } }).turnstile; - turnstile?.reset?.(); + const turnstile = (window as unknown as { turnstile?: { reset?: (widget?: string) => void } }).turnstile; + turnstile?.reset?.('#job-apply-turnstile'); }; form.addEventListener('submit', async (event) => { diff --git a/site/src/blocks/newsletter/components/NewsletterForm.astro b/site/src/blocks/newsletter/components/NewsletterForm.astro index 0bdfba2..91c31ea 100644 --- a/site/src/blocks/newsletter/components/NewsletterForm.astro +++ b/site/src/blocks/newsletter/components/NewsletterForm.astro @@ -38,7 +38,7 @@ const turnstileSiteKey = getEnv(Astro).PUBLIC_TURNSTILE_SITE_KEY; /> {turnstileSiteKey && ( -
+
)} @@ -85,7 +85,8 @@ const turnstileSiteKey = getEnv(Astro).PUBLIC_TURNSTILE_SITE_KEY; } finally { if (button) button.disabled = false; // A consumed Turnstile token cannot be reused — reset for a retry. - (window as unknown as { turnstile?: { reset?: () => void } }).turnstile?.reset?.(); + (window as unknown as { turnstile?: { reset?: (widget?: string) => void } }) + .turnstile?.reset?.('#newsletter-turnstile'); } }); } diff --git a/site/src/lib/api/client.ts b/site/src/lib/api/client.ts index f2ce3d0..53492a6 100644 --- a/site/src/lib/api/client.ts +++ b/site/src/lib/api/client.ts @@ -61,9 +61,10 @@ export async function apiFetch( ...odooAccessHeaders(env), }; // Forward the real client IP so Odoo's per-IP throttle keys on the visitor, - // not on this Worker. Trusted because the whole API is gated by the token. + // not on this Worker. A custom header survives cross-zone Worker requests; + // Cloudflare replaces CF-Connecting-IP in that case. const clientIp = (ctx as { request?: Request }).request?.headers.get('CF-Connecting-IP'); - if (clientIp) headers['CF-Connecting-IP'] = clientIp; + if (clientIp) headers['X-Odusite-Client-IP'] = clientIp; if (options.auth !== false) { const token = getAccessToken(ctx); if (token) headers['Authorization'] = `Bearer ${token}`; From 9490c20489b31855af08a0fc38175ed06f8da2a2 Mon Sep 17 00:00:00 2001 From: litnimax Date: Sun, 19 Jul 2026 15:54:42 +0300 Subject: [PATCH 2/3] Fix signup test config isolation --- addons/odusite_portal/tests/test_signup_confirm.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/addons/odusite_portal/tests/test_signup_confirm.py b/addons/odusite_portal/tests/test_signup_confirm.py index 4d270a8..e9b2fe8 100644 --- a/addons/odusite_portal/tests/test_signup_confirm.py +++ b/addons/odusite_portal/tests/test_signup_confirm.py @@ -23,6 +23,13 @@ def setUpClass(cls): super().setUpClass() cls.ICP = cls.env['ir.config_parameter'].sudo() + def setUp(self): + super().setUp() + # The stable config cache outlives each test's savepoint rollback. + # Clear it at both boundaries so HTTP requests see the restored value. + self.env.registry.clear_cache('stable') + self.addCleanup(self.env.registry.clear_cache, 'stable') + def _enable_b2c(self): self.ICP.set_param('auth_signup.invitation_scope', 'b2c') From a68f4b7346280634d68db02441e3d8b60c4ec772 Mon Sep 17 00:00:00 2001 From: litnimax Date: Sun, 19 Jul 2026 16:00:12 +0300 Subject: [PATCH 3/3] Isolate signup flow tests from config cache --- addons/odusite_portal/tests/test_signup_confirm.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/addons/odusite_portal/tests/test_signup_confirm.py b/addons/odusite_portal/tests/test_signup_confirm.py index e9b2fe8..7736d24 100644 --- a/addons/odusite_portal/tests/test_signup_confirm.py +++ b/addons/odusite_portal/tests/test_signup_confirm.py @@ -23,15 +23,12 @@ def setUpClass(cls): super().setUpClass() cls.ICP = cls.env['ir.config_parameter'].sudo() - def setUp(self): - super().setUp() - # The stable config cache outlives each test's savepoint rollback. - # Clear it at both boundaries so HTTP requests see the restored value. - self.env.registry.clear_cache('stable') - self.addCleanup(self.env.registry.clear_cache, 'stable') - def _enable_b2c(self): - self.ICP.set_param('auth_signup.invitation_scope', 'b2c') + # Keep flow tests independent of the process-wide config cache. The + # settings-to-parameter integration is covered by its dedicated test. + self.patch( + self.registry['res.users'], '_get_signup_invitation_scope', + lambda _users: 'b2c') def _find_user(self, login): Users = self.env['res.users'].sudo().with_context(active_test=False)