[FIX] spreadsheet: Catch access errors on spreadsheet

Currently, a user opening a spreadsheet containing pivots/lists/graphs
that they don't have the rights to access (be it because of the parent
model or the domain applied) will end up in an infinite loop.

While starting the datasources, the fetch step will throw, potentially
spamming the user with access errors.

Manual forward port of https://github.com/odoo/enterprise/pull/35146

Task 3107650

closes odoo/odoo#110113

X-original-commit: 1555e79be0b52c30df43e8de50f5db56fbbda630
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
This commit is contained in:
Rémi Rahir
2023-01-17 14:16:34 +01:00
parent e554f5688c
commit 3e7bdd40fa
7 changed files with 163 additions and 6 deletions
@@ -39,6 +39,9 @@ export default class ChartDataSource extends OdooViewsDataSource {
this.load();
return { datasets: [], labels: [] };
}
if (!this._isValid) {
return { datasets: [], labels: [] };
}
return this._model.data;
}
}
@@ -1,6 +1,7 @@
/** @odoo-module */
import { LoadingDataError } from "@spreadsheet/o_spreadsheet/errors";
import { RPCError } from "@web/core/network/rpc_service";
import { KeepLast } from "@web/core/utils/concurrency";
/**
@@ -30,6 +31,8 @@ export class LoadableDataSource {
*/
this._loadPromise = undefined;
this._isFullyLoaded = false;
this._isValid = true;
this._loadErrorMessage = "";
}
/**
@@ -45,11 +48,19 @@ export class LoadableDataSource {
}
if (!this._loadPromise) {
this._isFullyLoaded = false;
this._loadPromise = this._concurrency.add(this._load()).then(() => {
this._lastUpdate = Date.now();
this._isFullyLoaded = true;
this._notify();
});
this._isValid = true;
this._loadErrorMessage = "";
this._loadPromise = this._concurrency
.add(this._load())
.catch((e) => {
this._isValid = false;
this._loadErrorMessage = e instanceof RPCError ? e.data.message : e.message;
})
.finally(() => {
this._lastUpdate = Date.now();
this._isFullyLoaded = true;
this._notify();
});
}
return this._loadPromise;
}
@@ -73,6 +84,9 @@ export class LoadableDataSource {
this.load();
throw new LoadingDataError();
}
if (!this._isValid) {
throw new Error(this._loadErrorMessage);
}
}
/**
@@ -5,8 +5,9 @@ import { OdooChart } from "@spreadsheet/chart/odoo_chart/odoo_chart";
import { OdooLineChart } from "@spreadsheet/chart/odoo_chart/odoo_line_chart";
import { nextTick } from "@web/../tests/helpers/utils";
import { createSpreadsheetWithChart, insertChartInSpreadsheet } from "../../utils/chart";
import { createModelWithDataSource } from "../../utils/model";
import { createModelWithDataSource, waitForDataSourcesLoaded } from "../../utils/model";
import spreadsheet from "@spreadsheet/o_spreadsheet/o_spreadsheet_extended";
import { RPCError } from "@web/core/network/rpc_service";
const { toZone } = spreadsheet.helpers;
@@ -398,4 +399,35 @@ QUnit.module("spreadsheet > odoo chart plugin", {}, () => {
model.getters.getChartRuntime(chartId).chartJsConfig.options.scales.yAxes[0].stacked
);
});
QUnit.test(
"Load odoo chart spreadsheet with models that cannot be accessed",
async function (assert) {
let hasAccessRights = true;
const { model } = await createSpreadsheetWithChart({
mockRPC: async function (route, args) {
if (
args.model === "partner" &&
args.method === "web_read_group" &&
!hasAccessRights
) {
const error = new RPCError();
error.data = { message: "ya done!" };
throw error;
}
},
});
const chartId = model.getters.getFigures(model.getters.getActiveSheetId())[0].id;
const chartDataSource = model.getters.getChartDataSource(chartId);
await waitForDataSourcesLoaded(model);
const data = chartDataSource.getData();
assert.equal(data.datasets.length, 1);
assert.equal(data.labels.length, 2);
hasAccessRights = false;
chartDataSource.load({ reload: true });
await waitForDataSourcesLoaded(model);
assert.deepEqual(chartDataSource.getData(), { datasets: [], labels: [] });
}
);
});
@@ -3,6 +3,7 @@
import { nextTick } from "@web/../tests/helpers/utils";
import { LoadableDataSource } from "@spreadsheet/data_sources/data_source";
import { Deferred } from "@web/core/utils/concurrency";
import { RPCError } from "@web/core/network/rpc_service";
QUnit.module("spreadsheet data source", {}, () => {
QUnit.test(
@@ -43,4 +44,31 @@ QUnit.module("spreadsheet data source", {}, () => {
assert.strictEqual(dataSource.isReady(), true);
}
);
QUnit.test("Datasources handle errors thrown at _load", async (assert) => {
class TestDataSource extends LoadableDataSource {
constructor() {
super(...arguments);
this.data = null;
}
async _load() {
this.data = await this._orm.call();
}
}
const dataSource = new TestDataSource({
notify: () => {},
orm: {
call: () => {
const error = new RPCError();
error.data = { message: "Ya done!" };
throw error;
},
},
});
await dataSource.load();
assert.ok(dataSource._isFullyLoaded);
assert.notOk(dataSource._isValid);
assert.equal(dataSource._loadErrorMessage, "Ya done!");
});
});
@@ -16,6 +16,7 @@ import {
} from "../utils/getters";
import { createSpreadsheetWithList } from "../utils/list";
import { registry } from "@web/core/registry";
import { RPCError } from "@web/core/network/rpc_service";
QUnit.module("spreadsheet > list plugin", {}, () => {
QUnit.test("List export", async (assert) => {
@@ -506,4 +507,43 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
},
});
});
QUnit.test(
"Load list spreadsheet with models that cannot be accessed",
async function (assert) {
let hasAccessRights = true;
const { model } = await createSpreadsheetWithList({
mockRPC: async function (route, args) {
if (
args.model === "partner" &&
args.method === "search_read" &&
!hasAccessRights
) {
const error = new RPCError();
error.data = { message: "ya done!" };
throw error;
}
},
});
let headerCell;
let cell;
await waitForDataSourcesLoaded(model);
headerCell = getEvaluatedCell(model, "A3");
cell = getEvaluatedCell(model, "C3");
assert.equal(headerCell.value, 1);
assert.equal(cell.value, 42669);
hasAccessRights = false;
model.dispatch("REFRESH_ODOO_LIST", { listId: "1" });
await waitForDataSourcesLoaded(model);
headerCell = getEvaluatedCell(model, "A3");
cell = getEvaluatedCell(model, "C3");
assert.equal(headerCell.value, "#ERROR");
assert.equal(headerCell.error.message, "ya done!");
assert.equal(cell.value, "#ERROR");
assert.equal(cell.error.message, "ya done!");
}
);
});
@@ -16,6 +16,7 @@ import {
} from "@spreadsheet/../tests/utils/model";
import { makeDeferred, nextTick, patchWithCleanup } from "@web/../tests/helpers/utils";
import { session } from "@web/session";
import { RPCError } from "@web/core/network/rpc_service";
QUnit.module("spreadsheet > pivot plugin", {}, () => {
QUnit.test("can select a Pivot from cell formula", async function (assert) {
@@ -715,4 +716,42 @@ QUnit.module("spreadsheet > pivot plugin", {}, () => {
assert.deepEqual(model.getters.getPivotFieldMatching("1", filter.id), undefined);
assert.deepEqual(model.getters.getPivotDataSource("1").getComputedDomain(), []);
});
QUnit.test(
"Load pivot spreadsheet with models that cannot be accessed",
async function (assert) {
let hasAccessRights = true;
const { model } = await createSpreadsheetWithPivot({
mockRPC: async function (route, args) {
if (
args.model === "partner" &&
args.method === "read_group" &&
!hasAccessRights
) {
const error = new RPCError();
error.data = { message: "ya done!" };
throw error;
}
},
});
let headerCell;
let cell;
await waitForDataSourcesLoaded(model);
headerCell = getEvaluatedCell(model, "A3");
cell = getEvaluatedCell(model, "C3");
assert.equal(headerCell.value, "No");
assert.equal(cell.value, 15);
hasAccessRights = false;
model.dispatch("REFRESH_PIVOT", { id: "1" });
await waitForDataSourcesLoaded(model);
headerCell = getEvaluatedCell(model, "A3");
cell = getEvaluatedCell(model, "C3");
assert.equal(headerCell.value, "#ERROR");
assert.equal(headerCell.error.message, "ya done!");
assert.equal(cell.value, "#ERROR");
assert.equal(cell.error.message, "ya done!");
}
);
});
@@ -13,6 +13,7 @@ const { Model } = spreadsheet;
/**
* @typedef {import("@spreadsheet/../tests/utils/data").ServerData} ServerData
* @typedef {import("@spreadsheet/o_spreadsheet/o_spreadsheet").Model} Model
*/
export function setupDataSourceEvaluation(model) {