Implement org-based write permissions in frontend

.env.example

@@ -1,4 +1,4 @@
 APP_SECRET_KEY="h(tz!4a)llr+gm9g%f7vo%@br8q5!ec$7vifheax267av3lnk+"
 APP_DATABASE_URL="postgresql+asyncpg://user:pass@localhost:5432/${DB:-catalogage}"
 
-TOOLS_PASSWORDS='{"admin@catalogue.data.gouv.fr": "admin"}'
+TOOLS_PASSWORDS='{"admin@catalogue.data.gouv.fr": "admin", "admin.sante@catalogue.data.gouv.fr": "admin"}'

.github/workflows/ci.yml

@@ -22,9 +22,7 @@ jobs:
         options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
 
     env:
-      APP_SECRET_KEY: "<testing>"
       APP_DATABASE_URL: "postgresql+asyncpg://username:password@localhost:5432/catalogage"
-      TOOLS_PASSWORDS: '{"admin@catalogue.data.gouv.fr": "ci-admin"}'
 
     steps:
       - uses: actions/checkout@v3
@@ -45,6 +43,9 @@ jobs:
           path: ~/.cache/ms-playwright
           key: ${{ runner.os }}-node-${{ hashFiles('**/client/package-lock.json') }}
 
+      - name: "Setup .env"
+        run: cp .env.example .env
+
       - name: "Install dependencies"
         run: make install
 

client/src/lib/components/Header/Header.svelte

@@ -2,29 +2,10 @@
   import { goto } from "$app/navigation";
   import { page } from "$app/stores";
   import { logout, account } from "$lib/stores/auth";
+  import { navigationItems } from "$lib/stores/layout";
   import paths from "$lib/paths";
   import { Maybe } from "$lib/util/maybe";
 
-  type NavItem = {
-    label: string;
-    href: string;
-  };
-
-  const navigationItems: NavItem[] = [
-    {
-      label: "Accueil",
-      href: paths.home,
-    },
-    {
-      label: "Rechercher",
-      href: paths.datasetSearch,
-    },
-    {
-      label: "Contribuer",
-      href: paths.contribute,
-    },
-  ];
-
   $: path = $page.url.pathname;
 
   const onClickLogout = async () => {
@@ -129,7 +110,7 @@
           data-fr-js-navigation="true"
         >
           <ul class="fr-nav__list">
-            {#each navigationItems as { label, href }}
+            {#each $navigationItems as { label, href }}
               <li class="fr-nav__item" data-fr-js-navigaton-item="true">
                 <a
                   {href}

client/src/lib/permissions.spec.ts

@@ -0,0 +1,24 @@
+import { getFakeAccount } from "src/tests/factories/accounts";
+import { getFakeCatalogRecord } from "src/tests/factories/catalog_records";
+import { getFakeDataset } from "src/tests/factories/dataset";
+import { getFakeOrganization } from "src/tests/factories/organizations";
+import { canEditDataset } from "./permissions";
+
+describe("permissions", () => {
+  describe("canEditDataset", () => {
+    test("Account with same SIRET as dataset can edit", () => {
+      const organization = getFakeOrganization();
+      const dataset = getFakeDataset({
+        catalogRecord: getFakeCatalogRecord({ organization }),
+      });
+      const account = getFakeAccount({ organizationSiret: organization.siret });
+      expect(canEditDataset(dataset, account)).toBeTruthy();
+    });
+
+    test("Account with different SIRET than dataset cannot edit", () => {
+      const dataset = getFakeDataset();
+      const account = getFakeAccount();
+      expect(canEditDataset(dataset, account)).toBeFalsy();
+    });
+  });
+});

client/src/lib/permissions.ts

@@ -0,0 +1,6 @@
+import type { Account } from "src/definitions/auth";
+import type { Dataset } from "src/definitions/datasets";
+
+export const canEditDataset = (dataset: Dataset, account: Account): boolean => {
+  return dataset.catalogRecord.organization.siret === account.organizationSiret;
+};

client/src/lib/stores/layout/index.ts

@@ -0,0 +1,52 @@
+import { derived, get, writable } from "svelte/store";
+import type { Catalog } from "src/definitions/catalogs";
+import { Maybe } from "$lib/util/maybe";
+import { debounce } from "$lib/util/store";
+import paths from "$lib/paths";
+import { getCatalogBySiret } from "$lib/repositories/catalogs";
+import { account, apiToken } from "../auth";
+
+type NavItem = {
+  label: string;
+  href: string;
+};
+
+const currentCatalog = writable<Maybe<Catalog>>();
+
+// XXX: Upon login, $account may be set a few times in a row.
+// Make only one HTTP request once things have settled.
+debounce(account, 100).subscribe((accountValue) => {
+  if (!Maybe.Some(accountValue)) {
+    currentCatalog.set(null);
+    return;
+  }
+
+  getCatalogBySiret({
+    fetch,
+    apiToken: Maybe.expect(get(apiToken), "$apiToken"),
+    siret: accountValue.organizationSiret,
+  }).then((catalog) => {
+    currentCatalog.set(catalog);
+  });
+});
+
+export const navigationItems = derived(currentCatalog, (catalog): NavItem[] => {
+  return [
+    {
+      label: "Accueil",
+      href: paths.home,
+    },
+    {
+      label: "Rechercher",
+      href: paths.datasetSearch,
+    },
+    ...(Maybe.Some(catalog)
+      ? [
+          {
+            label: "Contribuer",
+            href: paths.contribute,
+          },
+        ]
+      : []),
+  ];
+});

client/src/lib/util/array.spec.ts

@@ -1,7 +1,7 @@
 import { range } from "./array";
 
 describe("range", () => {
-  const cases: [number, number, number[]][] = [
+  const startEndCases: [number, number, number[]][] = [
     [1, 1, []],
     [2, 1, []],
     [2, 2, []],
@@ -11,7 +11,21 @@ describe("range", () => {
     [-3, 2, [-3, -2, -1, 0, 1]],
   ];
 
-  test.each(cases)("when start=%s and end=%s", (start, end, expected) => {
-    expect(range(start, end)).toStrictEqual(expected);
+  test.each(startEndCases)(
+    "when start=%s and end=%s",
+    (start, end, expected) => {
+      expect(range(start, end)).toStrictEqual(expected);
+    }
+  );
+
+  const endCases: [number, number[]][] = [
+    [0, []],
+    [1, [0]],
+    [4, [0, 1, 2, 3]],
+    [-3, []],
+  ];
+
+  test.each(endCases)("when end=%s", (end, expected) => {
+    expect(range(end)).toStrictEqual(expected);
   });
 });

client/src/lib/util/array.ts

@@ -1,4 +1,11 @@
-export const range = (start: number, end: number): number[] => {
+/**
+ * @returns An array of integers from `start` (included) to `end` (excluded).
+ */
+export const range = (start: number, end?: number): number[] => {
+  if (end === undefined) {
+    end = start;
+    start = 0;
+  }
   const length = end - start;
   return Array.from({ length }, (_, idx) => start + idx);
 };

client/src/lib/util/random.ts

@@ -0,0 +1,6 @@
+/**
+ * @returns A random integer between `start` (included) and `end` (excluded)
+ */
+export const randint = (start: number, end: number): number => {
+  return Math.floor(start + Math.random() * (end - start));
+};

client/src/lib/util/store.ts

@@ -0,0 +1,21 @@
+import { writable, type Readable } from "svelte/store";
+
+/**
+ * @returns A new store that sends a value once `store` has not been called for `delay` milliseconds.
+ *
+ * Inspired by: https://docs-lodash.com/v4/debounce/
+ */
+export const debounce = <T>(store: Readable<T>, delay: number): Readable<T> => {
+  const newStore = writable<T>();
+
+  let timer: NodeJS.Timeout;
+
+  store.subscribe((value) => {
+    clearTimeout(timer);
+    timer = setTimeout(() => {
+      newStore.set(value);
+    }, delay);
+  });
+
+  return newStore;
+};

client/src/playwright.config.ts

@@ -2,23 +2,11 @@ import path from "path";
 import dotenv from "dotenv";
 import type { PlaywrightTestConfig } from "@playwright/test";
 import type { AppTestArgs } from "src/tests/e2e/fixtures";
-import { ADMIN_EMAIL } from "./tests/e2e/constants.js";
 
 dotenv.config({
   path: path.resolve("..", ".env"),
 });
 
-const getAdminTestPassword = (): string => {
-  const passwords = JSON.parse(process.env.TOOLS_PASSWORDS || "{}");
-  const adminPassword = passwords[ADMIN_EMAIL];
-  if (!adminPassword) {
-    throw new Error(
-      `Password for ${ADMIN_EMAIL} not defined in TOOLS_PASSWORDS`
-    );
-  }
-  return adminPassword;
-};
-
 const config: PlaywrightTestConfig<AppTestArgs> = {
   testDir: "./tests/e2e/",
   globalSetup: "./tests/e2e/global-setup.ts",
@@ -30,14 +18,6 @@ const config: PlaywrightTestConfig<AppTestArgs> = {
     screenshot: "only-on-failure",
     video: "on-first-retry",
   },
-  projects: [
-    {
-      name: "default",
-      use: {
-        adminTestPassword: getAdminTestPassword(),
-      },
-    },
-  ],
 };
 
 export default config;

client/src/routes/(app)/fiches/[id]/+page.svelte

@@ -3,7 +3,9 @@
   import paths from "$lib/paths";
   import { UPDATE_FREQUENCY_LABELS } from "src/constants";
   import { formatFullDate, splitParagraphs } from "src/lib/util/format";
+  import { account } from "$lib/stores/auth";
   import { Maybe } from "$lib/util/maybe";
+  import { canEditDataset } from "$lib/permissions";
   import AsideItem from "./_AsideItem.svelte";
   import ExtraFieldsList from "./_ExtraFieldsList.svelte";
 
@@ -45,15 +47,17 @@
       <ul
         class="fr-grid-row fr-grid-row--right fr-btns-group fr-btns-group--inline fr-btns-group--icon-right fr-my-5w"
       >
-        <li>
-          <a
-            href={editUrl}
-            class="fr-btn fr-btn--secondary fr-icon-edit-fill"
-            title="Modifier ce jeu de données"
-          >
-            Modifier
-          </a>
-        </li>
+        {#if canEditDataset(dataset, Maybe.expect($account, "$account"))}
+          <li>
+            <a
+              href={editUrl}
+              class="fr-btn fr-btn--secondary fr-icon-edit-fill"
+              title="Modifier ce jeu de données"
+            >
+              Modifier
+            </a>
+          </li>
+        {/if}
         <li>
           <a
             class="fr-btn fr-btn--secondary fr-icon-mail-line"

client/src/routes/(app)/fiches/[id]/edit/+page.ts

@@ -5,16 +5,22 @@ import { getDatasetByID } from "$lib/repositories/datasets";
 import { getTags } from "src/lib/repositories/tags";
 import { getLicenses } from "src/lib/repositories/licenses";
 import { getDatasetFiltersInfo } from "src/lib/repositories/datasetFilters";
-import { apiToken as apiTokenStore, account } from "$lib/stores/auth";
+import { apiToken as apiTokenStore } from "$lib/stores/auth";
 import { Maybe } from "$lib/util/maybe";
 
 export const load: PageLoad = async ({ fetch, params }) => {
   const apiToken = get(apiTokenStore);
-  const siret = Maybe.expect(get(account), "$account").organizationSiret;
 
-  const [catalog, dataset, tags, licenses, filtersInfo] = await Promise.all([
-    getCatalogBySiret({ fetch, apiToken, siret }),
-    getDatasetByID({ fetch, apiToken, id: params.id }),
+  const dataset = await getDatasetByID({ fetch, apiToken, id: params.id });
+
+  const [catalog, tags, licenses, filtersInfo] = await Promise.all([
+    Maybe.map(dataset, (dataset) =>
+      getCatalogBySiret({
+        fetch,
+        apiToken,
+        siret: dataset.catalogRecord.organization.siret,
+      })
+    ),
     getTags({ fetch, apiToken }),
     getLicenses({ fetch, apiToken }),
     getDatasetFiltersInfo({ fetch, apiToken }),

client/src/routes/(app)/fiches/[id]/page.spec.ts

@@ -11,12 +11,15 @@ import type {
   ExtraField,
   ExtraFieldValue,
 } from "src/definitions/catalogs";
+import { login, logout } from "src/lib/stores/auth";
+import { getFakeOrganization } from "src/tests/factories/organizations";
+import { getFakeCatalogRecord } from "src/tests/factories/catalog_records";
+import { getFakeAccount } from "src/tests/factories/accounts";
+
+const organization = getFakeOrganization();
 
 const catalog: Catalog = {
-  organization: {
-    siret: "<siret>",
-    name: "Org 1",
-  },
+  organization,
   extraFields: [],
 };
 
@@ -27,10 +30,19 @@ const dataset = getFakeDataset({
   formats: ["other"],
   producerEmail: "service@mydomain.org",
   contactEmails: ["service@mydomain.org"],
+  catalogRecord: getFakeCatalogRecord({
+    organization,
+  }),
 });
 
 const data = { catalog, dataset };
 
+beforeAll(() =>
+  login(getFakeAccount({ organizationSiret: organization.siret }), "abcd1234")
+);
+
+afterAll(() => logout());
+
 describe("Dataset detail page header", () => {
   test("The dataset title is present", () => {
     const { getByRole } = render(index, { data });
@@ -45,6 +57,12 @@ describe("Dataset detail page action buttons", () => {
     expect(modifyButton).toBeInTheDocument();
     expect(modifyButton.getAttribute("href")).toContain(dataset.id);
   });
+  test("The button to modify the dataset is absent if user does not belong to organization", async () => {
+    login(getFakeAccount({ organizationSiret: "<other_siret>" }), "1234");
+    const { queryByText } = render(index, { data });
+    const modifyButton = queryByText("Modifier");
+    expect(modifyButton).not.toBeInTheDocument();
+  });
   test("The button to contact the author is present", () => {
     const { getByText } = render(index, { data });
     const contactButton = getByText("Contacter le producteur");