feat(mail): add mail configuration audit command - #29
Conversation
c3b2d84 to
816643c
Compare
7ff8309 to
72080b8
Compare
xaviedoanhduy
left a comment
There was a problem hiding this comment.
requesting changes on three things that i think are wrong rather than merely improvable, all reproduced against real databases:
login = 'admin'silently drops the admin row when that login was renamed, which is the normal state on odoo.sh. verified on a production copy of ours: no admin line in the output, and nothing saying it was looked for and not found.- the known-relay port constraints contradict Google's own documentation, so
smtp.gmail.com:587with STARTTLS, the most common Odoo setup there is, is not recognised. a miss on a positive-confirmation flag reads as "not a known relay", so the audit quietly withholds the thing the feature exists to say. - the audit says nothing about a neutralized database, which is the most frequent reason mail never leaves a staging instance (that one is in an earlier comment thread, still open).
everything else below is smaller: case-sensitive is_default, substring matching that flags lookalike domains, a missing Mailtrap, a version claim that is off by two, a summary line that does not name the server it is talking about, and an AGENTS.md entry carrying review history.
the SQL underneath is solid and i want to be clear about that: the masking, the to_regclass probes and the "raw SQL sees past the ORM's active=True" reasoning all hold up against v16 through v19. most comments carry a suggestion you can apply directly, and three earlier threads (neutralization, the archived-row gauges, the alias-domain gauge) are still open above.
72080b8 to
9ddd38e
Compare
Hi @xaviedoanhduy, |
|
2 inputs:
|
9ddd38e to
bf40581
Compare
|
Thanks @nilshamerlinck, @xaviedoanhduy, I have updated the PR. |
xaviedoanhduy
left a comment
There was a problem hiding this comment.
thanks for the fast turnaround on this, everything from the last round is addressed and the client database names are out of the code now. a few things are still left over, each with a suggestion inline.
bf40581 to
9cb378a
Compare
|
Hi @xaviedoanhduy, I have updated the PR. |
xaviedoanhduy
left a comment
There was a problem hiding this comment.
verified against real v16/v17/v19 databases plus a scratch one shaped like an odoo.sh staging build: the renamed-admin lookup, the case-folded defaults, the neutralization flag and stub, the active-only gauges and the Google/M365 ports all behave as described. two non-blocking comments below.
9cb378a to
6d23317
Compare
Summary
Adds
odoo-db mail <db>: a direct-SQL audit of outbound mail configuration, porting a script that checked the same things through the Odoo API (odooly) — none of this data needs auth. Returns one bundle with five sections:config_parameters— their_config_parameterkeys mail cares about (mail.bounce.alias,mail.catchall.alias/.domain,mail.default.from/.from_filter, plus Trobz'sdefault_email). Keys not set still get a row withvalue: None("(not defined)"); a key present with an empty string is preserved as"", distinct from absent (verified against real data). None of these keys are masked.alias_domains(Odoo 17+ only,Noneotherwise) — the per-companymail.alias.domainrecords that actually control bounce/catchall/default-from routing since v17; the legacy ICP keys above are read only by a one-time migration helper past that version (verified against a real Odoo 18 source tree).bounce_email/catchall_email/default_from_emailare computed in SQL mirroringAliasDomain's own compute methods. A company with no alias domain assigned showsalias_domain: None— a worse, distinct misconfiguration from "still on ICP defaults". Filtersres_company.active = true(an archived company isn't sending real mail).addresses— company partner (id 1),__system__/OdooBot, andadminemails, each flaggedis_defaultif still an Odoo/demo-data default. Raw SQL has no implicitactive=Truefilter, so archived records surface here rather than silently vanishing (unlike the original ORM-based script). Shown as-is, not masked — organizational mailboxes, not individual PII (per reviewer feedback, reversed from an earlier default-masking design).mail_servers—ir.mail_server, ordered bysequence.smtp_authentication/from_filter(Odoo 17+ columns) are probed viato_regclass/pg_attributeandNULLon older versions.smtp_user/smtp_passare real credentials and stay masked by default (per reviewer call, username included since it may itself be a real mailbox address) — reuses the existing global--include-sensitive-informationrather than adding a per-command flag.modules— install state of modules that materially change mail behavior (currently justmass_mailing).Text output prints each section as its own table, with a note explaining the legacy-vs-authoritative relationship between
config_parametersandalias_domainswhen the latter is present. Prometheus output addsodoo_db_mail_servers,odoo_db_mail_default_addresses, and (whenalias_domainsapplies)odoo_db_mail_companies_missing_alias_domain.Test plan
tests/test_smoke.py—mail --help,get_mail_config_parameters(including absent-vs-blank key handling),get_mail_addresses(default-flagging, missing records, empty emails),get_mail_servers(masking,--include-sensitive-information, v17+ column detection)AGENTS.md,README.md,site-docs/docs/cli-reference.mdupdated