diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 6d4187925..4d704aa83 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -19,6 +19,7 @@ jobs: spec: - tests/01-spp-starter-spmis.spec.ts - tests/02-spp-starter-farmer-registry.spec.ts + - tests/03-spp-dci-compliance.spec.ts steps: - uses: actions/checkout@v4 @@ -57,6 +58,7 @@ jobs: case "${{ matrix.spec }}" in *spmis*) NAME="SP-MIS" ;; *farmer*) NAME="Farmer Registry" ;; + *dci*) NAME="DCI Compliance" ;; *) NAME="${{ matrix.spec }}" ;; esac @@ -109,7 +111,7 @@ jobs: FIELDS=$(jq -n \ --arg repo "${{ github.repository }}" \ --arg ref "${{ github.ref_name }}" \ - --arg specs "tests/01-spp-starter-spmis.spec.ts, tests/02-spp-starter-farmer-registry.spec.ts" \ + --arg specs "tests/01-spp-starter-spmis.spec.ts, tests/02-spp-starter-farmer-registry.spec.ts, tests/03-spp-dci-compliance.spec.ts" \ '[{"name": "Repository", "value": $repo, "inline": true}, {"name": "Ref", "value": $ref, "inline": true}, {"name": "Spec files", "value": $specs, "inline": false}]') for f in statuses/*.json; do diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0a4cac364..a274c1190 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -282,6 +282,17 @@ repos: # Exclude: scripts, tests, data, demo, and third-party modules exclude: ^scripts/|/tests/|/data/|/demo/|^(fastapi|job_worker|base_user_role|extendable|extendable_fastapi|endpoint_route_handler)/ pass_filenames: true + - id: openspp-check-odoo19-js + name: "OpenSPP: Odoo 19 compatibility (JavaScript)" + description: "Check for useService() calls on web services removed in Odoo 19" + entry: python scripts/lint/check_odoo19.py --js + language: python + additional_dependencies: + - PyYAML + types: [javascript] + # Exclude: scripts, e2e, vendored libs, tests, and third-party modules + exclude: ^scripts/|^e2e/|/static/lib/|/static/tests/|^(fastapi|job_worker|base_user_role|extendable|extendable_fastapi|endpoint_route_handler)/ + pass_filenames: true # API authentication enforcement - id: openspp-check-api-auth name: "OpenSPP: API endpoint authentication" diff --git a/e2e/README.md b/e2e/README.md index 222aabf73..fb6ff169d 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -27,10 +27,11 @@ Tests run sequentially (1 worker). Each test file resets the database before run ## Test files -| File | Module | Description | -| ---------------------------------------- | ----------------------------- | --------------------------------------------------------------- | -| `01-spp-starter-spmis.spec.ts` | `spp_starter_sp_mis` | Installs OpenSPP Starter SP-MIS and verifies nav menus | -| `02-spp-starter-farmer-registry.spec.ts` | `spp_starter_farmer_registry` | Installs OpenSPP Starter Farmer Registry and verifies nav menus | +| File | Module | Description | +| ---------------------------------------- | ----------------------------- | ---------------------------------------------------------------------- | +| `01-spp-starter-spmis.spec.ts` | `spp_starter_sp_mis` | Installs OpenSPP Starter SP-MIS and verifies nav menus | +| `02-spp-starter-farmer-registry.spec.ts` | `spp_starter_farmer_registry` | Installs OpenSPP Starter Farmer Registry and verifies nav menus | +| `03-spp-dci-compliance.spec.ts` | `spp_dci_compliance` | Installs DCI Compliance and verifies the security-warning systray item | ## How each test works diff --git a/e2e/tests/03-spp-dci-compliance.spec.ts b/e2e/tests/03-spp-dci-compliance.spec.ts new file mode 100644 index 000000000..3f75a0dca --- /dev/null +++ b/e2e/tests/03-spp-dci-compliance.spec.ts @@ -0,0 +1,185 @@ +// OpenSPP DCI Compliance — end-to-end test suite +// +// What this tests: +// 01 - Logs in as admin and installs the spp_dci_compliance module via the Apps menu +// 02 - Reloads the backend and confirms the webclient renders cleanly (navbar visible, +// no uncaught page error, no client error dialog) and that the DCI +// security-warning systray item shows a badge of 3, naming the three insecure +// dci.* settings the module's post-install hook turns on (unsigned requests, +// HTTP callbacks, internal callback IPs) +// 03 - Turns dci.allow_unsigned_requests back off in the System Parameters list, +// reloads, and confirms the badge drops to 2 and that setting is no longer +// listed in the dropdown +// +// The systray component runs for every backend user on every page load, so it is the +// one place a broken frontend service lookup (useService on a service Odoo 19 removed) +// takes the whole webclient down. Nothing else in CI mounts it — module tests run +// without a browser — which is why this spec exists. +// +// All tests run in order and share a single browser session (test.describe.serial). +// A fresh Docker stack is spun up in beforeAll so every run starts from a clean database. + +import {test, expect, Page} from "@playwright/test"; +import {resetStack} from "./helpers"; + +const SYSTRAY_ITEM = ".o_dci_security_warning"; +// Odoo's error service renders every uncaught client error into one of these. +const ERROR_DIALOG = + ".o_error_dialog, .modal:has-text('Odoo Client Error'), .modal:has-text('Odoo Error')"; + +async function login(page: Page) { + await page.goto("/web/login"); + await page.getByRole("textbox", {name: "Email"}).fill("admin"); + await page.getByRole("textbox", {name: "Password"}).fill("admin"); + await page.getByRole("button", {name: "Log in"}).click(); + await expect(page.locator(".o_main_navbar")).toBeVisible({timeout: 30_000}); +} + +async function installApp(page: Page, technicalName: string) { + console.log("✅ Clicking Apps menuitem"); + await page.getByRole("menuitem", {name: "Apps"}).click(); + await page.waitForLoadState("domcontentloaded"); + console.log("✅ Apps page loaded"); + + // Remove all preset filter chips (there are two by default); spp_dci_compliance is + // not flagged as an application, so the default "Apps" filter would hide it. + await page.getByRole("button", {name: "Remove"}).click(); + await page.waitForLoadState("domcontentloaded"); + console.log("✅ Filter cleared, page settled"); + + // Search by technical module name + await page.getByRole("searchbox", {name: "Search..."}).fill(technicalName); + await page.getByRole("searchbox", {name: "Search..."}).press("Enter"); + await page.waitForLoadState("domcontentloaded"); + console.log("✅ Search done, looking for Install button"); + + // Click Install on the matching card + const installBtn = page.getByRole("button", {name: "Activate"}).first(); + await expect(installBtn).toBeVisible({timeout: 15_000}); + await installBtn.click(); + + // Wait for installation — Odoo shows a loading spinner then reloads the webclient + console.log("✅ Waiting for installation to complete"); + await page.waitForLoadState("domcontentloaded", {timeout: 180_000}); + await page + .locator(".o_loading") + .waitFor({state: "hidden", timeout: 180_000}) + .catch(() => {}); + await expect(page.locator(".o_main_navbar")).toBeVisible({timeout: 180_000}); + console.log("✅ Installation complete — webclient is back"); +} + +async function reloadBackend(page: Page) { + await page.goto("/odoo"); + await page.waitForLoadState("domcontentloaded"); + await expect(page.locator(".o_main_navbar")).toBeVisible({timeout: 60_000}); +} + +test.describe.serial("OpenSPP DCI Compliance", () => { + let page: Page; + // Uncaught errors thrown while the webclient mounts. A component whose setup() + // throws (#450) never reaches an error dialog: the OWL root dies first and the + // page stays blank, so this is the assertion that encodes that failure mode. + const pageErrors: string[] = []; + + test.beforeAll(async ({browser}) => { + await resetStack(); + page = await browser.newPage(); + page.on("pageerror", (error) => pageErrors.push(error.message)); + }); + + test.afterAll(async () => { + // page is only assigned after resetStack() succeeds — guard so a beforeAll + // failure/timeout reports its own real error instead of this masking it. + if (!page) return; + await page.close(); + }); + + test.afterEach(async ({}, testInfo) => { + if (testInfo.status !== testInfo.expectedStatus && !process.env.CI) { + console.log( + `❌ "${testInfo.title}" failed — pausing for investigation (set CI=1 to skip)` + ); + await page.pause(); + } + }); + + test("01 - login and install DCI Compliance", async () => { + await login(page); + await installApp(page, "spp_dci_compliance"); + await page.screenshot({path: "reports/dci-post-install.png", fullPage: false}); + }); + + test("02 - webclient renders and the systray item lists the enabled settings", async () => { + await reloadBackend(page); + + // A broken service lookup in the component's setup() would surface here as a + // client error dialog and a webclient that never mounts. + await expect(page.locator(ERROR_DIALOG)).toHaveCount(0); + await expect(page.locator(".o_main_navbar .o_menu_systray")).toBeVisible(); + + // The module's post-install hook enables three insecure settings on purpose (it is + // a compliance test harness), so a fresh install must show all three. + const systrayItem = page.locator(SYSTRAY_ITEM); + await expect(systrayItem).toBeVisible({timeout: 30_000}); + await expect(systrayItem.locator(".badge")).toHaveText("3"); + + await systrayItem.click(); + await expect(page.getByText("Not safe for production!")).toBeVisible(); + await expect(page.getByText("Allow Unsigned Requests")).toBeVisible(); + await expect(page.getByText("Allow HTTP Callbacks")).toBeVisible(); + await expect(page.getByText("Allow Internal Callback IPs")).toBeVisible(); + await expect(page.getByText("Bypass Bearer Authentication")).toHaveCount(0); + await expect(page.locator(ERROR_DIALOG)).toHaveCount(0); + expect(pageErrors).toEqual([]); + + await page.screenshot({ + path: "reports/dci-systray-three-warnings.png", + fullPage: false, + }); + await page.keyboard.press("Escape"); + }); + + test("03 - turning a setting off removes it from the systray item", async () => { + // The System Parameters action is opened by its XML id; the Technical menu that + // normally leads to it needs developer mode, the action itself does not. + await page.goto("/odoo/action-base.ir_config_list_action"); + await page.waitForLoadState("domcontentloaded"); + const search = page.getByRole("searchbox", {name: "Search..."}); + await expect(search).toBeVisible({timeout: 30_000}); + await search.fill("dci.allow_unsigned_requests"); + await search.press("Enter"); + await page.waitForLoadState("domcontentloaded"); + + // The list is editable in place: clicking the Value cell turns it into a text + // field (a textarea, since ir.config_parameter.value is a Text field). Odoo puts + // the field name on the cell, which is the stable handle. + const row = page.getByRole("row", {name: /dci\.allow_unsigned_requests/}); + const valueCell = row.locator("td[name='value']"); + await valueCell.click(); + const valueField = valueCell.getByRole("textbox"); + await expect(valueField).toBeVisible({timeout: 15_000}); + await valueField.fill("false"); + await page.getByRole("button", {name: "Save", exact: true}).click(); + await expect(valueCell).toHaveText("false"); + console.log("✅ dci.allow_unsigned_requests = false saved"); + + await reloadBackend(page); + + await expect(page.locator(ERROR_DIALOG)).toHaveCount(0); + const systrayItem = page.locator(SYSTRAY_ITEM); + await expect(systrayItem).toBeVisible({timeout: 30_000}); + await expect(systrayItem.locator(".badge")).toHaveText("2"); + + await systrayItem.click(); + await expect(page.getByText("Allow HTTP Callbacks")).toBeVisible(); + await expect(page.getByText("Allow Internal Callback IPs")).toBeVisible(); + await expect(page.getByText("Allow Unsigned Requests")).toHaveCount(0); + expect(pageErrors).toEqual([]); + + await page.screenshot({ + path: "reports/dci-systray-two-warnings.png", + fullPage: false, + }); + }); +}); diff --git a/scripts/lint/check_odoo19.py b/scripts/lint/check_odoo19.py index 721d586f7..c7e9174a9 100755 --- a/scripts/lint/check_odoo19.py +++ b/scripts/lint/check_odoo19.py @@ -9,6 +9,9 @@ - group_expand signature: Detects old 3-parameter _read_group_* methods - Legacy _sql_constraints: Detects the _sql_constraints class attribute, which Odoo 19 ignores (constraints are silently never created); use models.Constraint +- Removed web services (--js): Detects useService("rpc") / useService("user") and + env.services.rpc / env.services.user in frontend JavaScript; both services are + gone and requesting them crashes the component at setup() Features: - Auto-fix support for Command API tuples (--fix) @@ -90,6 +93,22 @@ r"def\s+(_read_group_\w+|_group_expand_\w+)\s*\(\s*self\s*,\s*\w+\s*,\s*\w+\s*,\s*\w+\s*\)" ) +# Web services that no longer exist. useService() raises +# "Service is not available" for them at component setup(), so a +# component still requesting one crashes the moment it is mounted. Nothing +# fails at asset-build time because the useService import itself resolves. +REMOVED_WEB_SERVICES = { + "rpc": 'import {rpc} from "@web/core/network/rpc"; and call rpc(url, params) directly', + "user": 'import {user} from "@web/core/user";', +} +REMOVED_SERVICE_PATTERN = re.compile( + r"""(? bool: """Check if XML file contains view definitions.""" return "views" in path.parts or "view" in path.name.lower() or "wizard" in path.parts + def check_js_file(self, file_path: str) -> list[Violation]: + """Check a frontend JavaScript file for Odoo 19 issues.""" + violations = [] + path = Path(file_path) + + if not path.exists() or path.suffix != ".js": + return violations + + if self.config.should_ignore(file_path): + return violations + + try: + with open(file_path, encoding="utf-8") as f: + content = f.read() + except Exception: + return violations + + # Match the whole file (a formatter may wrap the call over several lines). + content = JS_COMMENT_PATTERN.sub(lambda m: re.sub(r"[^\n]", " ", m.group(0)), content) + + for match in REMOVED_SERVICE_PATTERN.finditer(content): + service = match.group(1) or match.group(2) + line_num = content.count("\n", 0, match.start()) + 1 + violations.append( + Violation( + file_path=file_path, + line=line_num, + message=( + f"the {service!r} web service no longer exists; requesting it crashes the component at setup()" + ), + rule_id="odoo19.removed_web_service", + severity=Severity.ERROR, + suggestion=f"Use {REMOVED_WEB_SERVICES[service]}", + doc_link="docs/principles/odoo19-compatibility.md#removed-web-services", + ) + ) + + return violations + def _check_search_view_groups(self, file_path: str, root: ET.Element, lines: list[str]) -> list[Violation]: """Check for or in search views.""" violations = [] @@ -566,6 +624,9 @@ def main(): # Check XML files python check_odoo19.py --xml spp_programs/views/*.xml + # Check frontend JavaScript files + python check_odoo19.py --js spp_programs/static/src/**/*.js + # Auto-fix Command API tuples python check_odoo19.py --fix spp_programs/models/*.py @@ -577,6 +638,7 @@ def main(): add_common_args(parser) parser.add_argument("files", nargs="*", help="Files to check") parser.add_argument("--xml", action="store_true", help="Check XML files for search view issues") + parser.add_argument("--js", action="store_true", help="Check JavaScript files for removed web services") parser.add_argument("--fix", action="store_true", help="Auto-fix Command API tuples") parser.add_argument( "--dry-run", @@ -602,6 +664,8 @@ def main(): if args.xml: files_to_check.extend(module_path.glob("views/*.xml")) files_to_check.extend(module_path.glob("wizard/*.xml")) + elif args.js: + files_to_check.extend(module_path.glob("static/src/**/*.js")) else: files_to_check.extend(module_path.glob("models/*.py")) files_to_check.extend(module_path.glob("wizard/*.py")) @@ -640,6 +704,8 @@ def main(): file_str = str(file_path) if args.xml or file_str.endswith(".xml"): violations = checker.check_xml_file(file_str) + elif args.js or file_str.endswith(".js"): + violations = checker.check_js_file(file_str) else: violations = checker.check_python_file(file_str) all_violations.extend(violations) diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst index 297815919..36188dd3c 100644 --- a/spp_change_request_v2/README.rst +++ b/spp_change_request_v2/README.rst @@ -853,6 +853,17 @@ Before declaring a new CR type complete: Changelog ========= +19.0.3.1.17 +~~~~~~~~~~~ + +- fix(change_request): ``CRReviewPanel`` no longer requests the ``rpc`` + web service in its setup. Odoo 19 has no such service and + ``useService("rpc")`` throws the moment a component asking for it is + mounted; the handle was never used anyway. The panel is not wired into + any view today, so no screen was affected, but whoever mounts it next + would have hit the crash (#450). Whether the panel is wired in or + removed is tracked in #524 + 19.0.3.1.16 ~~~~~~~~~~~ diff --git a/spp_change_request_v2/__manifest__.py b/spp_change_request_v2/__manifest__.py index b09a537a5..80f9dd341 100644 --- a/spp_change_request_v2/__manifest__.py +++ b/spp_change_request_v2/__manifest__.py @@ -1,6 +1,6 @@ { "name": "OpenSPP Change Request V2", - "version": "19.0.3.1.16", + "version": "19.0.3.1.17", "sequence": 50, "category": "OpenSPP", "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention", diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md index 4b8652f35..d0b0df975 100644 --- a/spp_change_request_v2/readme/HISTORY.md +++ b/spp_change_request_v2/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.3.1.17 + +- fix(change_request): `CRReviewPanel` no longer requests the `rpc` web service in its setup. Odoo 19 has no such service and `useService("rpc")` throws the moment a component asking for it is mounted; the handle was never used anyway. The panel is not wired into any view today, so no screen was affected, but whoever mounts it next would have hit the crash (#450). Whether the panel is wired in or removed is tracked in #524 + ### 19.0.3.1.16 - fix(change_request): an Edit Individual change request can be opened again on a registrant that already holds a future date of birth. `spp.change.request.create` prefills the detail from the registrant and that prefill is a write, so the guard added in 19.0.3.1.15 refused the copied value and the request could not be created at all — closing the very path field staff use to correct the date. A birthdate the guard would refuse is now dropped from the prefill mapping rather than offered, so the field arrives empty and a valid date has to be entered. The rule itself lives in one place (`_is_future_birthdate` on the mixin), so what prefill declines to offer and what the constraint refuses cannot drift apart. diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html index 90a552098..f3c87cd21 100644 --- a/spp_change_request_v2/static/description/index.html +++ b/spp_change_request_v2/static/description/index.html @@ -1339,6 +1339,18 @@

Changelog

+

19.0.3.1.17

+
    +
  • fix(change_request): CRReviewPanel no longer requests the rpc +web service in its setup. Odoo 19 has no such service and +useService("rpc") throws the moment a component asking for it is +mounted; the handle was never used anyway. The panel is not wired into +any view today, so no screen was affected, but whoever mounts it next +would have hit the crash (#450). Whether the panel is wired in or +removed is tracked in #524
    • +
+
+

19.0.3.1.16

  • fix(change_request): an Edit Individual change request can be opened @@ -1358,7 +1370,7 @@

    19.0.3.1.16

    which of several lines to fix.
-
+

19.0.3.1.15

  • fix(change_request): refuse a date of birth in the future while the @@ -1374,7 +1386,7 @@

    19.0.3.1.15

    recorded earlier that local day (#362)
-
+

19.0.3.1.14

  • fix(change_request): group-scope conflict rules work again. @@ -1391,7 +1403,7 @@

    19.0.3.1.14

    both directions, including that ended memberships are excluded.
-
+

19.0.3.1.13

  • fix(change_request): a selectable field on a dynamic-approval type may @@ -1408,7 +1420,7 @@

    19.0.3.1.13

    routing key is still not applied.
-
+

19.0.3.1.12

  • fix(change_request): auto-apply-on-approve runs through the public @@ -1423,7 +1435,7 @@

    19.0.3.1.12

    the applying user is still recorded as the approver.
-
+

19.0.3.1.11

  • fix(change_request): field-mapping transform expressions are evaluated @@ -1462,7 +1474,7 @@

    19.0.3.1.11

    the full traceback is logged only at DEBUG.
-
+

19.0.3.1.10

  • fix(security): conflict and duplicate detection now decide whether a @@ -1502,7 +1514,7 @@

    19.0.3.1.10

    configured mapping.
-
+

19.0.3.1.9

  • fix(security): duplicate detection now scores the fields both change @@ -1519,7 +1531,7 @@

    19.0.3.1.9

    requester-writable selected_field_name / field_to_modify.
-
+

19.0.3.1.8

  • fix(security): scope the Create-Group member wizards to the parent @@ -1537,7 +1549,7 @@

    19.0.3.1.8

    access-control entry grants.
-
+

19.0.3.1.7

  • fix(security): require change-request manager rights to apply a change @@ -1552,7 +1564,7 @@

    19.0.3.1.7

    endpoint.
-
+

19.0.3.1.6

  • fix(security): derive conflict and duplicate detection from the change @@ -1566,7 +1578,7 @@

    19.0.3.1.6

    an empty one, so detection cannot silently disable itself.
-
+

19.0.3.1.5

  • fix(security): scope the CR Requestor, Local Validator and HQ @@ -1578,7 +1590,7 @@

    19.0.3.1.5

    are noupdate.
-
+

19.0.3.1.4

  • fix(security): add ownership and area record rules to every concrete @@ -1595,7 +1607,7 @@

    19.0.3.1.4

    unrestricted delete their access-control entries grant.
-
+

19.0.3.1.3

  • fix(security): route and apply the same single field for @@ -1608,7 +1620,7 @@

    19.0.3.1.3

    the routing selector.
-
+

19.0.3.1.2

  • fix(change_request_v2): adding an ID now looks for a live one of that @@ -1617,7 +1629,7 @@

    19.0.3.1.2

    (#1136)
-
+

19.0.3.1.1

  • fix(change_request): enforce the (cr_type_id, reason) uniqueness @@ -1631,7 +1643,7 @@

    19.0.3.1.1

    applied) so the constraint applies cleanly on upgrade.
-
+

19.0.3.1.0

  • revert(change_request): restore the create-a-new-individual Add @@ -1649,7 +1661,7 @@

    19.0.3.1.0

    not restored here; reinstate separately if needed.
-
+

19.0.3.0.0

  • feat(change_request): redesign the group/membership CR flows (#242) — @@ -1671,7 +1683,7 @@

    19.0.3.0.0

    must adapt (see #1133).
-
+

19.0.2.0.8

  • fix(views): disable inline creation of CR document types on the Change @@ -1682,7 +1694,7 @@

    19.0.2.0.8

    Documents” modal (missing Name field) that blocked saving (#1125)
-
+

19.0.2.0.7

  • fix(security): align CR Requestor / CR Local Validator / CR HQ @@ -1694,7 +1706,7 @@

    19.0.2.0.7

    dependencies.
-
+

19.0.2.0.6

  • fix(views): route post-submit CRs (pending / approved / applied / @@ -1709,7 +1721,7 @@

    19.0.2.0.6

    list so row-click goes through the stage router.
-
+

19.0.2.0.5

  • fix(security): add a global ir.rule on spp.change.request that @@ -1722,27 +1734,27 @@

    19.0.2.0.5

    roles).
-
+

19.0.2.0.3

  • fix: add HTML escaping to all computed Html fields with sanitize=False to prevent stored XSS (#50)
-
+

19.0.2.0.2

  • fix: fix batch approval wizard line deletion (#130)
-
+

19.0.2.0.1

  • fix: skip field types before getattr and isolate detail prefetch (#129)
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_change_request_v2/static/src/components/review_panel/review_panel.js b/spp_change_request_v2/static/src/components/review_panel/review_panel.js index 7fb7517a2..4c25159cf 100644 --- a/spp_change_request_v2/static/src/components/review_panel/review_panel.js +++ b/spp_change_request_v2/static/src/components/review_panel/review_panel.js @@ -24,7 +24,6 @@ export class CRReviewPanel extends Component { this.action = useService("action"); this.notification = useService("notification"); this.dialog = useService("dialog"); - this.rpc = useService("rpc"); this.state = useState({ loading: true, diff --git a/spp_dci_compliance/README.rst b/spp_dci_compliance/README.rst index 3f1f6b084..1159e7ffd 100644 --- a/spp_dci_compliance/README.rst +++ b/spp_dci_compliance/README.rst @@ -144,6 +144,34 @@ Dependencies .. contents:: :local: +Changelog +========= + +19.0.1.0.1 +~~~~~~~~~~ + +- fix(dci_compliance): the security-warning systray item requested the + ``rpc`` web service, which Odoo 19 no longer provides. + ``useService("rpc")`` throws at component setup, and because the item + is registered for every backend user the whole webclient failed to + mount on any database with this module installed: a blank page after + login, for everyone. The component now calls ``rpc()`` from + ``@web/core/network/rpc`` directly, the same way the rest of OpenSPP + does (#450) +- fix(dci_compliance): the warnings are shown to system administrators + only. The item's one action opens the System Parameters list, which + nobody else can open, so other users were told about settings they + could neither see nor change. The ``/dci/security/warnings`` route now + answers with an empty summary for anyone outside + ``base.group_system``, and the component skips the call for them +- fix(dci_compliance): the route is declared ``type="jsonrpc"``; + ``type="json"`` is a deprecated alias on Odoo 19 that logged a warning + on every module load +- fix(dci_compliance): the systray button carries an accessible label + naming the number of warnings instead of exposing the bare count, the + "View DCI Settings" action filters on keys starting with ``dci.`` + rather than containing it, and its title is translatable + Bug Tracker =========== diff --git a/spp_dci_compliance/__manifest__.py b/spp_dci_compliance/__manifest__.py index 90100f0c8..a674a4abe 100644 --- a/spp_dci_compliance/__manifest__.py +++ b/spp_dci_compliance/__manifest__.py @@ -3,7 +3,7 @@ "name": "OpenSPP DCI Compliance Tests", "summary": "DCI compliance validation test suite (test-only, disabled by default)", "category": "OpenSPP/Integration", - "version": "19.0.1.0.0", + "version": "19.0.1.0.1", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", "license": "LGPL-3", diff --git a/spp_dci_compliance/controllers/security_warning.py b/spp_dci_compliance/controllers/security_warning.py index 4899fcc3a..29d583ecc 100644 --- a/spp_dci_compliance/controllers/security_warning.py +++ b/spp_dci_compliance/controllers/security_warning.py @@ -8,13 +8,18 @@ class DCISecurityWarningController(http.Controller): """Controller providing API endpoints for DCI security warnings.""" - @http.route("/dci/security/warnings", type="json", auth="user") + @http.route("/dci/security/warnings", type="jsonrpc", auth="user") def get_security_warnings(self): """Get current DCI security warnings. - Returns JSON with warning information for the systray widget. + Returns JSON with warning information for the systray widget. Only system + administrators can change the settings involved, so anyone else is told + there is nothing to show. Returns: dict: Security warning summary """ - return request.env["spp.dci.security.warning"].get_warning_summary() + warning_model = request.env["spp.dci.security.warning"] + if not request.env.user.has_group("base.group_system"): + return warning_model.summarize([]) + return warning_model.get_warning_summary() diff --git a/spp_dci_compliance/models/security_warning.py b/spp_dci_compliance/models/security_warning.py index d9486d3e3..f79d734ac 100644 --- a/spp_dci_compliance/models/security_warning.py +++ b/spp_dci_compliance/models/security_warning.py @@ -81,7 +81,18 @@ def get_warning_summary(self): Returns: dict: Summary with count and details """ - warnings = self.get_security_warnings() + return self.summarize(self.get_security_warnings()) + + @api.model + def summarize(self, warnings): + """Shape a list of warnings into the payload the systray widget reads. + + Args: + warnings: list of INSECURE_SETTINGS entries that are enabled + + Returns: + dict: Summary with count and details + """ return { "has_warnings": len(warnings) > 0, "warning_count": len(warnings), diff --git a/spp_dci_compliance/readme/HISTORY.md b/spp_dci_compliance/readme/HISTORY.md new file mode 100644 index 000000000..6f3326f78 --- /dev/null +++ b/spp_dci_compliance/readme/HISTORY.md @@ -0,0 +1,6 @@ +### 19.0.1.0.1 + +- fix(dci_compliance): the security-warning systray item requested the `rpc` web service, which Odoo 19 no longer provides. `useService("rpc")` throws at component setup, and because the item is registered for every backend user the whole webclient failed to mount on any database with this module installed: a blank page after login, for everyone. The component now calls `rpc()` from `@web/core/network/rpc` directly, the same way the rest of OpenSPP does (#450) +- fix(dci_compliance): the warnings are shown to system administrators only. The item's one action opens the System Parameters list, which nobody else can open, so other users were told about settings they could neither see nor change. The `/dci/security/warnings` route now answers with an empty summary for anyone outside `base.group_system`, and the component skips the call for them +- fix(dci_compliance): the route is declared `type="jsonrpc"`; `type="json"` is a deprecated alias on Odoo 19 that logged a warning on every module load +- fix(dci_compliance): the systray button carries an accessible label naming the number of warnings instead of exposing the bare count, the "View DCI Settings" action filters on keys starting with `dci.` rather than containing it, and its title is translatable diff --git a/spp_dci_compliance/static/description/index.html b/spp_dci_compliance/static/description/index.html index 926ad041f..a8cca8bb2 100644 --- a/spp_dci_compliance/static/description/index.html +++ b/spp_dci_compliance/static/description/index.html @@ -515,16 +515,40 @@

    Dependencies

    Table of contents

    + +
+
+

19.0.1.0.1

+
    +
  • fix(dci_compliance): the security-warning systray item requested the +rpc web service, which Odoo 19 no longer provides. +useService("rpc") throws at component setup, and because the item +is registered for every backend user the whole webclient failed to +mount on any database with this module installed: a blank page after +login, for everyone. The component now calls rpc() from +@web/core/network/rpc directly, the same way the rest of OpenSPP +does (#450)
    • +
  • fix(dci_compliance): the warnings are shown to system administrators +only. The item’s one action opens the System Parameters list, which +nobody else can open, so other users were told about settings they +could neither see nor change. The /dci/security/warnings route now +answers with an empty summary for anyone outside +base.group_system, and the component skips the call for them
    • +
  • fix(dci_compliance): the route is declared type="jsonrpc"; +type="json" is a deprecated alias on Odoo 19 that logged a warning +on every module load
    • +
  • fix(dci_compliance): the systray button carries an accessible label +naming the number of warnings instead of exposing the bare count, the +“View DCI Settings” action filters on keys starting with dci. +rather than containing it, and its title is translatable
    • +
-

Bug Tracker

+

Bug Tracker

Bugs are tracked on GitHub Issues. In case of trouble, please check there if your issue has already been reported. If you spotted it first, help us to smash it by providing a detailed and welcomed @@ -532,15 +556,15 @@

Bug Tracker

Do not contact contributors directly about support or help with technical issues.

-

Credits

+

Credits

-

Authors

+

Authors

  • OpenSPP.org
-

Maintainers

+

Maintainers

This module is part of the OpenSPP/OpenSPP2 project on GitHub.

You are welcome to contribute.

diff --git a/spp_dci_compliance/static/src/components/security_warning/security_warning.js b/spp_dci_compliance/static/src/components/security_warning/security_warning.js index f7c4b5b1d..57c5c0f18 100644 --- a/spp_dci_compliance/static/src/components/security_warning/security_warning.js +++ b/spp_dci_compliance/static/src/components/security_warning/security_warning.js @@ -2,13 +2,17 @@ import {Component, useState, onWillStart} from "@odoo/owl"; import {registry} from "@web/core/registry"; +import {rpc} from "@web/core/network/rpc"; +import {user} from "@web/core/user"; import {useService} from "@web/core/utils/hooks"; +import {_t} from "@web/core/l10n/translation"; import {Dropdown} from "@web/core/dropdown/dropdown"; import {DropdownItem} from "@web/core/dropdown/dropdown_item"; /** * Systray component that displays DCI security warnings. * Shows a red warning icon when development/testing security settings are enabled. + * Only system administrators are told: they are the ones who can change the settings. */ export class DCISecurityWarning extends Component { static template = "spp_dci_compliance.SecurityWarning"; @@ -16,7 +20,6 @@ export class DCISecurityWarning extends Component { static props = {}; setup() { - this.rpc = useService("rpc"); this.actionService = useService("action"); this.state = useState({ hasWarnings: false, @@ -27,13 +30,17 @@ export class DCISecurityWarning extends Component { }); onWillStart(async () => { - await this.loadWarnings(); + if (await user.hasGroup("base.group_system")) { + await this.loadWarnings(); + } else { + this.state.loaded = true; + } }); } async loadWarnings() { try { - const result = await this.rpc("/dci/security/warnings", {}); + const result = await rpc("/dci/security/warnings", {}); this.state.hasWarnings = result.has_warnings; this.state.warningCount = result.warning_count; this.state.warnings = result.warnings || []; @@ -45,6 +52,10 @@ export class DCISecurityWarning extends Component { } } + get warningLabel() { + return _t("%s DCI security warnings", this.state.warningCount); + } + openSettings() { this.actionService.doAction({ type: "ir.actions.act_window", @@ -53,13 +64,12 @@ export class DCISecurityWarning extends Component { [false, "list"], [false, "form"], ], - domain: [["key", "like", "dci.%"]], - name: "DCI Configuration Parameters", + domain: [["key", "=like", "dci.%"]], + name: _t("DCI Configuration Parameters"), }); } } -// Only show the systray item if there are warnings const systrayItem = { Component: DCISecurityWarning, // Always displayed; the component itself decides visibility diff --git a/spp_dci_compliance/static/src/components/security_warning/security_warning.xml b/spp_dci_compliance/static/src/components/security_warning/security_warning.xml index fdc040482..50ff25bc4 100644 --- a/spp_dci_compliance/static/src/components/security_warning/security_warning.xml +++ b/spp_dci_compliance/static/src/components/security_warning/security_warning.xml @@ -3,13 +3,15 @@ + diff --git a/spp_dci_compliance/tests/__init__.py b/spp_dci_compliance/tests/__init__.py index 8c32c02d1..9dcfeb75c 100644 --- a/spp_dci_compliance/tests/__init__.py +++ b/spp_dci_compliance/tests/__init__.py @@ -5,6 +5,7 @@ test_fastapi_endpoint_compliance, test_schemas, test_security_warning, + test_security_warning_controller, test_sr_sync_search, test_verification_router, ) diff --git a/spp_dci_compliance/tests/test_security_warning_controller.py b/spp_dci_compliance/tests/test_security_warning_controller.py new file mode 100644 index 000000000..9f0668b93 --- /dev/null +++ b/spp_dci_compliance/tests/test_security_warning_controller.py @@ -0,0 +1,88 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the ``/dci/security/warnings`` JSON route. + +The DCI security warning systray item +(``static/src/components/security_warning/security_warning.js``) calls this route on +every webclient load. These tests pin the contract the JavaScript depends on: the +response shape, that it tracks the insecure ``dci.*`` parameters, that only system +administrators receive the warnings, and that it refuses unauthenticated callers. +""" + +import json + +from odoo.tests import HttpCase, tagged + +EMPTY_SUMMARY = {"has_warnings": False, "warning_count": 0, "warnings": [], "message": ""} + + +@tagged("post_install", "-at_install") +class TestDCISecurityWarningController(HttpCase): + """Test cases for the security warning JSON route.""" + + ROUTE = "/dci/security/warnings" + + def setUp(self): + super().setUp() + self.ConfigParam = self.env["ir.config_parameter"].sudo() + for setting in self.env["spp.dci.security.warning"].INSECURE_SETTINGS: + self.ConfigParam.set_param(setting["key"], "false") + + def _call_route(self): + return self.url_open( + self.ROUTE, + data=json.dumps({"jsonrpc": "2.0", "method": "call", "params": {}}), + headers={"Content-Type": "application/json"}, + ) + + def _create_internal_user(self): + return self.env["res.users"].create( + { + "name": "Plain Internal User", + "login": "dci_plain_user", + "password": "dci_plain_user_pw", + "group_ids": [(6, 0, [self.env.ref("base.group_user").id])], + } + ) + + def test_unauthenticated_call_is_refused(self): + """An anonymous caller gets a session-expired JSON-RPC error, never the payload.""" + body = self._call_route().json() + + self.assertNotIn("result", body) + self.assertTrue(body["error"]["data"]["name"].endswith("SessionExpiredException")) + + def test_all_settings_secure_returns_no_warnings(self): + """With every insecure setting off the route reports nothing to show.""" + self.authenticate("admin", "admin") + + response = self._call_route() + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["result"], EMPTY_SUMMARY) + + def test_enabled_setting_is_reported(self): + """An insecure setting turned on shows up as one warning with its key.""" + self.ConfigParam.set_param("dci.allow_unsigned_requests", "true") + self.authenticate("admin", "admin") + + result = self._call_route().json()["result"] + + self.assertTrue(result["has_warnings"]) + self.assertEqual(result["warning_count"], 1) + self.assertEqual([w["key"] for w in result["warnings"]], ["dci.allow_unsigned_requests"]) + self.assertEqual(result["message"], "1 DCI security setting(s) are in development mode") + + def test_non_admin_gets_empty_summary(self): + """A plain internal user is told nothing, even while an insecure setting is on. + + Only system administrators can act on these settings (the action the systray + offers opens ``ir.config_parameter``), so only they are told about them. + """ + self.ConfigParam.set_param("dci.allow_unsigned_requests", "true") + self._create_internal_user() + self.authenticate("dci_plain_user", "dci_plain_user_pw") + + response = self._call_route() + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["result"], EMPTY_SUMMARY)