Restreindre la contribution / édition au catalogue d'appartenance

florimondmanca commented 2 years ago

Dépend de :

284
122
288

User Stories

En tant que responsable de mon organisation, je ne souhaite pas que des membres d'autres organisations puissent contribuer (créer, modifier des fiches) au catalogue de mon organisation. En effet, des membres extérieurs n'ont pas les connaissances des jeux de données gérés en interne. Par ailleurs, en cas de problème, par exemple à l'occasion d'un désaccord sur des contributions, je ne pourrais pas discuter simplement avec eux pour résoudre le problème.

Critères d'acceptation

[x] Les utilisateurs ne peuvent créer et modifier des fiches que dans le catalogue de l'organisation dont ils sont membres.

Implémentation

Côté API :

Sous-catégoriser les endpoints : POST /catalogs/:siret/datasets/, GET /catalogs/:siret/datasets/:id/, PUT /catalogs/:siret/datasets/:id/, DELETE /catalogs/:siret/datasets/:id/. En effet sinon il faut récupérer le dataset en base avant de pouvoir vérifier les permissions basées sur la correspondance du SIRET entre le dataset de l'utilisateur.
"On n'a pas le droit de contribuer à un catalogue tiers" => Renvoyer une 403 sur POST /catalogs/:siret/datasets/, PUT /catalogs/:siret/datasets/:id/ et DELETE /catalogs/:siret/datasets/:id/ si le SIRET ne correspond pas à celui de l'utilisateur authentifié.

Côté front :

Pour les catalogues autres que celui de l'utilisateur, désactiver le bouton "Modifier" et afficher un texte en tooltip (grâce à un <button disable title="...">), du type :

Vous ne pouvez pas modifier ce jeu de données car il n'appartient pas au catalogue de votre organisation.
La page /contribuer doit devenir /catalogues/:siret/contribuer. Si l'utilisateur tente d'accéder au formulaire d'un catalogue autre que le sien, afficher une page d'erreur. (Elle ne peut pas provenir de l'API car POST /datasets/:siret/ est appelé en fin de formulaire.)
La page /fiches/:id/ doit devenir /catalogues/:siret/fiches/:id. Interpréter une 403 issue de l'API.
La page /fiches/:id/edit/ doit devenir /catalogues/:siret/fiches/:id/modifier. Interpréter une 403 issue de l'API.

PR :

[x] #441
[x] #437
[x] #455

florimondmanca commented 2 years ago

@DaFrenchFrog Comment envisages-tu cette restriction ? Doit-on cacher le bouton sans plus d'explication ? L'afficher en mode "Désactivé" avec un tooltip "Vous ne pouvez pas modifier ce jeu de données car il n'appartient pas à votre catalogue", ou qqc comme ça ?

DaFrenchFrog commented 2 years ago

@DaFrenchFrog Comment envisages-tu cette restriction ? Doit-on cacher le bouton sans plus d'explication ? L'afficher en mode "Désactivé" avec un tooltip "Vous ne pouvez pas modifier ce jeu de données car il n'appartient pas à votre catalogue", ou qqc comme ça ?

L'idée de la désactivation + tooltip est une bonne idée en revanche sur le texte je remplacerais plutôt le mot "catalogue" par "organisation".

Cela m'a fait penser qu'il pourrait y avoir une confusion si on consulte un catalogue d'une autre organisation et que l'on clique sur contribuer. Un utilisateur pourrait avoir l'impression qu'il contribue au catalogue qu'il consulte alors qu'il contribue à un autre catalogue (le sien). J'ai modifié la maquette du header de contribution et je rajoute ce point dans #372

johanricher commented 2 years ago

si on consulte un catalogue d'une autre organisation et que l'on clique sur contribuer

Du point de vue utilisateur sur la page de recherche filtrée par une organisation (je suppose que c'est ce dont tu parles), il me semble clair qu'on ne consulte pas "le catalogue d'une autre organisation". De même que quand on filtre par couverture géographique "France" on ne consulte pas "le catalogue de la France" mais juste un résultat de recherche.

Un utilisateur qui clique sur l'onglet "Contribuer" sort de cette page de recherche ce qui clarifie je pense que la contribution n'est pas liée d'une quelconque façon au résultat de la recherche consulté.

afficher le bouton "Modifier" en mode "Désactivé" avec un tooltip "Vous ne pouvez pas modifier ce jeu de données car il n'appartient pas au catalogue de votre organisation"

Ca me paraît en effet une bonne option pour une première itération.

On verra les retours des utilisateurs sur un parcours complet (y compris vérifier les éventuelles confusion avec la recherche filtrée) pour itérer dessus.

johanricher commented 2 years ago

J'ai créé 3 nouveaux tickets afin d'extraire certains aspects plus complexes qui ne sont pas des must have pour la livraison :

451
452
453

On les priorisera une fois les must have terminés.

florimondmanca commented 2 years ago

@johanricher Merci. À ce stade il faut donc traiter l'actuel rôle ADMIN comme voulant dire "responsable d'organisation". C'est donc un rôle qui doit être cantonné à une certaine organisation (ne pas permettre à un ADMIN de supprimer n'importe quelle fiche, ce qui est le cas actuellement).

Note technique - Dans le cadre de ce ticket, l'implémentation des "permissions" (capacités pour un utilisateur donné vis-à-vis des différentes entités) va être un peu "à la mano", en particulier il y aura de la duplication de logique entre le back (qui fera les vérifs à des fins de sécurité, cf #437) et le front (qui fera les vérifs à des fins d'UX, cf #441). Il est possible de n'avoir qu'une seule source de vérité (côté back) mais ça demande un peu de travail pour être fait proprement. Pour le moment il n'y a que 3 "permissions" (contribuer à un catalogue, modifier un jeu de données, supprimer un jeu de données) donc ça reste gérable sans un système fait pour passer à l'échelle. Je ferai un ticket "refacto" expliquant ce que j'aurais en tête une fois l'implémentation "à la mano" faite.

johanricher commented 2 years ago

Ok ça me va si c'est plus simple pour une première implé et refacto plus tard. :+1:

florimondmanca commented 2 years ago

@johanricher Je pense que ce ticket est prêt pour une revue des critères d'acceptation. :+1: Tester le comportement sur https://demo.catalogue.multi.coop par exemple. Si besoin d'autres indications n'hésite pas.

johanricher commented 2 years ago

J'ai testé sur l'instance https://demo.catalogue.multi.coop/

Je n'ai pas le même comportement d'un point de vue utilisateur si j'utilise la connexion legacy ou Mon Compte Pro (dans les 2 cas avec l'adresse catalogue.demo@yopmail.com).

Si je me connecte avec Mon Compte Pro je ne vois pas le bouton "Contribuer" ce qui laisse entendre que le catalogue de l'organisation legacy est absent.

Quels sont les catalogues présents sur l'instance ? Est-ce qu'il y a 2 users différents liés à catalogue.demo@yopmail.com ?

florimondmanca commented 2 years ago

@johanricher Merci du signalement. Si, sur demo/staging l'utilisateur demo (qu'on se connecte avec mdp ou MonComptePro) est lié à une organisation non-legacy (on utilise le SIRET du MC, mais c'est pour l'exemple). Ce que tu observes est en fait d'un bug. Je le corrige ici https://github.com/etalab/catalogage-donnees/pull/463

Tu peux retester sur staging ?

johanricher commented 2 years ago

Depuis l'instance https://staging.catalogue.multi.coop/, connecté avec Mon Compte Pro (catalogue.demo@yopmail.com) :

La page "Contribuer" indique "Ministère de la Culture" ce qui laisse entendre que c'est l'organisation à laquelle j'appartiens alors que je devrais appartenir à "Organisation par défaut" (je crois ?)

florimondmanca commented 2 years ago

La page "Contribuer" indique "Ministère de la Culture" ce qui laisse entendre que c'est l'organisation à laquelle j'appartiens alors que je devrais appartenir à "Organisation par défaut" (je crois ?)

Non, cf mon commentaire ci-dessus :

sur demo/staging l'utilisateur demo (qu'on se connecte avec mdp ou MonComptePro) est lié à une organisation non-legacy (on utilise le SIRET du MC, mais c'est pour l'exemple).

Pour le dire autrement, on a configuré staging et demo pour y créer une organisation "Ministère de la Culture" à laquelle on a rattaché catalogue.demo@yopmail.com.

De même, sur staging l'initdata configure un admin sur cette organisation "Ministère de la Culture"... Je peux modifier ça pour que ça reflète ce qu'on va faire sur prod (cf #460).

johanricher commented 2 years ago

Ah d'accord, merci pour la clarification.

Sur demo, le catalogue de "Organisation par défaut" est vide. Du coup je ne peux pas vérifier les critères d'acceptation de ce ticket. Il faudrait qu'il y ait des jeux de données que je ne pourrais pas modifier.

florimondmanca commented 2 years ago

Une fois mergé #465 tu pourras te connecter avec le compte admin (qui sera déplacé vers "Organisation par défaut"), créer un jeu de données, puis voir avec catalogue.demo que tu ne peux pas le modifier. Ou alors te connecter avec le compte admin, et voir que tu ne peux pas modifier les jeux de données de l'orga "Ministère de la Culture" (puisqu'on n'a pas encore #451).

johanricher commented 2 years ago

Entendu, c'est très clair Florimond :ok_hand:

florimondmanca commented 2 years ago

Voilà j'ai fait la manip, normalement c'est bon, tu peux retester. :+1:

johanricher commented 2 years ago

J'ai testé sur demo en me connectant (login legacy) en tant que admin@catalogue.data.gouv.fr (appartenant à "Organisation par défaut").

J'ai pu créer et modifier une fiche dans le catalogue de mon organisation.

Je peux voir les fiches de l'organisation "Ministère de la culture" mais je peux pas les modifier (ni créer une fiche dans ce catalogue)

C'est parfait, bravo à tous !

etalab / catalogage-donnees