Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check user belongs to dataset organization before creation #437

Merged
merged 4 commits into from
Sep 30, 2022

Conversation

florimondmanca
Copy link
Collaborator

@florimondmanca florimondmanca commented Sep 20, 2022

Nouvelle tentative suite à #435

Refs #388

Motivation

Cette PR "officialise" le fait que dans notre modèle de données, un utilisateur a le droit d'ajouter un jeu de données à un catalogue seulement s'il fait partie de l'organisation.

(Jusqu'ici le front s'en assurait, mais il n'y avait aucune garantie côté règles métier dans le backend.)

Comme pour le front (https://github.com/etalab/catalogage-donnees/pull/441/files#diff-6debc9c1c928e75751dcea3a25cc55f9ac2b13c2a462592ccb0c9164b91920f1R5), cette contrainte s'applique aussi au rôle ADMIN, à comprendre désormais comme un respo d'organisation (cf #288). La possibilité de le faire pour les "super-admins" est remise à plus tard, cf #452.

Implémentation

Concrètement, la commande CreateDataset() requiert désormais une valeur account qui est utilisée pour vérifier que l'utilisateur en question a le droit de créer un dataset au sein du catalogue visé par le organization_siret. Le cas échéant, une exception est levée et l'API l'intercepte pour renvoyer une 403.

La vérification de cette règle métier se fait donc au sein des couches métiers (dans la couche application).

Un artifice avec une classe-marqueur Skip permet de bypasser cette vérification lorsque le contexte ne fait pas intervenir d'utilisateur (en l'occurrence, dans un initdata ou dans make randomdatasets).

La même approche sera utilisée pour l'update et la suppression.

@florimondmanca florimondmanca force-pushed the fm/dataset-perms branch 3 times, most recently from f58e3f0 to 50f0d7c Compare September 20, 2022 15:37
@florimondmanca florimondmanca force-pushed the fm/dataset-perms branch 4 times, most recently from 26bffcb to 2b0a48e Compare September 21, 2022 11:42
@florimondmanca florimondmanca marked this pull request as ready for review September 21, 2022 11:44
@florimondmanca florimondmanca force-pushed the fm/dataset-perms branch 2 times, most recently from 50ad50f to 9951500 Compare September 27, 2022 09:18
@florimondmanca florimondmanca added please review Cette PR est en attente d'une revue and removed priorité-0-bas labels Sep 27, 2022
server/application/datasets/commands.py Show resolved Hide resolved
server/domain/common/types.py Show resolved Hide resolved
tests/api/test_datasets.py Outdated Show resolved Hide resolved
@florimondmanca florimondmanca removed the please review Cette PR est en attente d'une revue label Sep 27, 2022
* Check user belongs to organization before update

* Test ADMIN cannot update in other org either
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants