fix: two install/runtime crashes from the DCBO report (TOTP column leak + auditlog rule collision) - #6
Open
rutgerhofste wants to merge 2 commits into
Open
Conversation
`auth_totp.device` inherits `res.users.apikeys` by prototype inheritance (`_inherit` with a distinct `_name`, `_auto = False`), so Odoo copies our x_role_id / x_state / x_last_used / x_use_count fields onto its own `auth_totp_device` table. Our `init()` hard-coded `res_users_apikeys` as the ALTER/CREATE INDEX target, so those columns were never created on `auth_totp_device`. Any ORM read of a trusted TOTP device — e.g. the user form snapshotting `totp_trusted_device_ids` while granting another user MCP Pro admin rights — then raised `psycopg2.errors.UndefinedColumn: auth_totp_device.x_role_id`. `init()` is inherited by `auth_totp.device` (which defines none of its own), so keying every statement on `self._table` provisions the columns on whichever table is being initialised. Existing databases heal on upgrade; no manual migration needed. Reproduced and verified on a fresh DB (auth_totp + module): before, the columns were absent on auth_totp_device and present on res_users_apikeys; after upgrade, both tables carry all four columns and the previously crashing ORM read succeeds. Reported by Mil Cuyvers (DCBO Open Solutions) via support@pantalytics.com. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… the model `auditlog.rule` enforces `unique(model_id)` (one rule per model), but `post_init_hook` only skipped a model when a rule with OUR exact name was already present. A database that already had an audit rule on a target model under a different name — e.g. migrated from a pre-existing OCA `auditlog` install — hit the constraint when the hook tried to create a second rule, aborting the whole module install. Skip a model when ANY auditlog.rule already exists for it, leaving the operator's own rule untouched. Reproduced on a fresh DB: with a pre-existing partner rule under a different name, the old hook raised `UniqueViolation: auditlog_rule_model_uniq`; after the change the hook skips the model and installs cleanly. Second issue from the same report by Mil Cuyvers (DCBO Open Solutions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two separate issues reported by Mil Cuyvers (DCBO Open Solutions) via support@pantalytics.com, both hit while installing/using MCP Pro Governance on a database that already ran OCA
auditlogwith 2FA in use.1. User form crashes:
UndefinedColumn: auth_totp_device.x_role_idRoot cause
auth_totp.deviceuses prototype inheritance of our extended model:So Odoo copies our
x_role_id,x_state,x_last_used,x_use_countfields ontoauth_totp.devicetoo, but ourinit()hard-codedres_users_apikeysas the DDL target, so the columns were never created onauth_totp_device. Reading a user's trusted TOTP devices (the user-form onchange snapshotstotp_trusted_device_ids) then hits the missing column. The customer's own diagnosis was spot-on.Fix
init()is inherited byauth_totp.device(which defines none of its own), so it already runs for both models — it just always wrote to the wrong table. Keying every statement onself._table(viaodoo.tools.SQL) provisions the columns on whichever table is being initialised. Existing DBs heal on upgrade; no manual migration.Verified
On a fresh DB (
auth_totp+ module): before,auth_totp_devicehad none of the columns and the read crashed; after, both tables carry all four columns + indexes and the previously-crashingsearch_readreturns cleanly.2. Install aborts:
UniqueViolation: auditlog_rule_model_uniqRoot cause
post_init_hookseeds a draftauditlog.ruleper AI-target model, butauditlog.ruleenforcesunique(model_id)(one rule per model). The hook only skipped a model when a rule with our exact name already existed. A DB that already had an audit rule on that model under a different name (e.g. migrated from a pre-existing OCAauditloginstall) tripped the constraint and the whole install failed.Fix
Skip a model when any
auditlog.rulealready exists for it, leaving the operator's rule untouched.Verified
On a fresh DB with a pre-existing partner rule under a different name, the old hook raised
UniqueViolation: auditlog_rule_model_uniq; after the change the hook skips the model and installs cleanly.Housekeeping
TestApiKeyFieldsOnTotpDeviceandTestPostInitHook.test_preexisting_rule_on_model_does_not_break_install.19.0.1.20.3, CHANGELOG updated.19.0branch serves 17/18/19; the customer is on the18.0.xbuild and both fixes are version-agnostic (odoo.tools.SQLexists since 17.0), so all published builds are covered.🤖 Generated with Claude Code