Development #25645
API pour obtenir la liste des rôles portés par un utilisateur
0%
Description
Dans Welco on affichait pour un usager la liste de ses rôles (#12630), ça ne me semble pas une possibilité aujourd'hui avec Authentic.
Fichiers
Demandes liées
Historique
Mis à jour par Thomas Noël il y a plus de 5 ans
Je ne connais pas le contexte exacte de l'utilisation de l'API voulue, mais en théorie, Welco étant provisionné (comme les autres SP) on peut afficher les rôles d'un utilisateur (ses groupes Django locaux) dès qu'on en connait l'UUID.
Mis à jour par Frédéric Péters il y a plus de 5 ans
Le contexte c'est reproduire dans Combo, en utilisant l'API d'authentic, ce qu'on fait dans Welco, avec l'API de w.c.s. (et c'est plus simple d'avoir ce qu'on veut via l'API que de devoir ensuite avoir du code pour merger une part de résultat de l'API avec une part d'infos provisionnées).
Mis à jour par Paul Marillonnet il y a plus de 5 ans
J'entends ici qu'il faut reproduire dans A2 l'API de liste des rôles pour un utilisateur, en s'inspirant de l'API déjà existante prévue à cet effet dans w.c.s.
Est-ce que ça doit aller jusqu'à un format d'interface identique (même formats d'URI, notamment) ? Par exemple, si on veut que le code qui utilise l'API w.c.s. dans Welco soit porté à l'identique dans, factorisé avec, Combo.
Mis à jour par Frédéric Péters il y a plus de 5 ans
Est-ce que ça doit aller jusqu'à un format d'interface identique (même formats d'URI, notamment) ? Par exemple, si on veut que le code qui utilise l'API w.c.s. dans Welco soit porté à l'identique dans, factorisé avec, Combo.
Non.
Mon souhait c'est que /api/users/?... renvoie une liste d'utilisateurs, et que dans les attributs pour un utililisateur, il y ait un attribut "roles", qui soit une liste, contenant au moins les libellés des rôles.
Mis à jour par Paul Marillonnet il y a plus de 5 ans
- Fichier 0001-WIP-include-roles-in-users-api-25645.patch 0001-WIP-include-roles-in-users-api-25645.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Bon, dans l'idée.
Après, deux options :
- on exploite ça en lecture uniquement, et on s'en fiche de créer/modifier des utilisateurs en mentionnant leurs rôles directement dans l'API.
- on inclut l'usage en écriture, et il faut gérer le contrôle d'accès.
Ça me plairait plus de partir sur la deuxième, tant qu'à faire. Je me plante ?
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
Part sur l'usage en lecture seul, par contre je pense que tu ne renvoies que les slugs là, il faudrait utiliser un serializer pour avoir au moins slug, uuid et name.
Mis à jour par Paul Marillonnet il y a plus de 5 ans
- Fichier 0001-WIP-include-roles-in-users-api-25645.patch 0001-WIP-include-roles-in-users-api-25645.patch ajouté
- Statut changé de Solution proposée à En cours
- Patch proposed changé de Oui à Non
Benjamin Dauvergne a écrit :
Part sur l'usage en lecture seul, par contre je pense que tu ne renvoies que les slugs là, il faudrait utiliser un serializer pour avoir au moins slug, uuid et name.
Peut-être quelque chose comment ça du coup ?
Je vais écrire quelques tests aussi.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Pourquoi définir roles dans init plutôt qu'au niveau des attributs de la classe ?
- Pourquoi ajouter roles à ordering_fields ?
Mis à jour par Paul Marillonnet il y a plus de 5 ans
Benjamin Dauvergne a écrit :
- Pourquoi définir roles dans init plutôt qu'au niveau des attributs de la classe ?
Je ne voulais pas déplacer la class RoleSerializer avant BaseUserSerialize, et c'est la solution la plus simple que j'ai trouvée pour ne pas me manger une "NameError ... not defined".
- Pourquoi ajouter roles à ordering_fields ?
Erreur de ma part.
Mis à jour par Paul Marillonnet il y a plus de 5 ans
- Patch proposed changé de Non à Oui
- Fichier 0001-include-roles-in-users-api-25645.patch 0001-include-roles-in-users-api-25645.patch ajouté
- Statut changé de En cours à Solution proposée
Avec les tests.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Lié à Development #19756: Personnalisation accrue du portail agent pour en faire aussi la page d'entrée des agents d'accueil ajouté
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Fichier 0001-api-include-roles-in-users-API-25645.patch 0001-api-include-roles-in-users-API-25645.patch ajouté
Je ne voulais pas déplacer la class RoleSerializer avant BaseUserSerialize, et c'est la solution la plus simple que j'ai trouvée pour ne pas me manger une "NameError ... not defined".
Arbitrage ici, je pense que j'aurais fait pareil, pour réduire la taille du patch, faciliter la relecture, mais comme ça a été évoqué, voici la version qui déplace RoleSerializer.
Bref, pour moi, l'un ou l'autre, mais pour avancer sur le portail agent, vu que c'était la remarque de Benjamin, je serais pour valider et pousser la version qui déplace.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
En l'état actuel ça ne va renvoyer que les rôles directs et pas ceux hérités, il faudrait passer par l'accesseur User.roles_and_parents()
.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Statut changé de Solution proposée à En cours
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
(via l'attribut source
sur de Field.__init__()
de django-rest-framework, https://www.django-rest-framework.org/api-guide/fields/#source)
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Fichier 0001-api-include-roles-in-users-API-25645.patch 0001-api-include-roles-in-users-API-25645.patch ajouté
- Statut changé de En cours à Solution proposée
Voilà j'ai corrigé le souci que je voyais, c'est bon pour moi.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
Paul c'est moi ou tu as bossé deux fois sur le même ticket ? Voir #21485.
Mis à jour par Frédéric Péters il y a environ 5 ans
- Lié à Development #21485: API pour avoir la liste des rôles d'un utilisateur ajouté
Mis à jour par Frédéric Péters il y a environ 5 ans
Paul c'est moi ou tu as bossé deux fois sur le même ticket ? Voir #21485.
On dirait bien, j'ai rejeté l'autre, qu'on relise ici.
Mis à jour par Paul Marillonnet il y a environ 5 ans
Frédéric Péters a écrit :
On dirait bien, j'ai rejeté l'autre, qu'on relise ici.
Merci. On mettra ça sur le compte de la fatigue :/
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Il me semble qu'on devrait aussi exporter l'OU du rôle sinon on est pas certain de lequel c'est (avec l'uuid on l'est, mais bon c'est plus simple de repérer avec l'OU), un truc genre :
"ou": { "uuid": "xxx", "slug": "xxx", "name": "yyy", }
en fait il faudrait que ça se rapproche de la forme prise par les rôles dans les exports (dans les exports il y a service
qui est aussi pris en compte).
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Statut changé de Solution proposée à En cours
Mis à jour par Paul Marillonnet il y a presque 5 ans
- Fichier 0001-api-include-roles-in-users-API-25645.patch 0001-api-include-roles-in-users-API-25645.patch ajouté
- Statut changé de En cours à Solution proposée
Ok.
Mis à jour par Paul Marillonnet il y a presque 5 ans
- Fichier 0001-api-include-roles-in-users-API-25645.patch 0001-api-include-roles-in-users-API-25645.patch ajouté
Avec les tests qui passent c'est encore mieux :)
(Deux corrections sur les autres tests qui définissaient en extension l'ensemble des clés renvoyées par l'API users).
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Paul Marillonnet a écrit :
Avec les tests qui passent c'est encore mieux :)
(Deux corrections sur les autres tests qui définissaient en extension l'ensemble des clés renvoyées par l'API users).
- Une fonction attributes_hash apparait sortant de nulle part et n'est pas utilisée, quid ?
- On a perdu l'utiliation de roles_and_parents pour obtenir la liste des rôles qui était dans ma version du patch (ça veut dire qu'on a pas de tests suffisant pour valider ça)
- L'utilisation actuelle de RoleSerializer étant en lecture-seule pour l'instant on peut virer create/update/partial_update (ou y mettre des raise NotImplementedError pour s'assurer qu'on y passe pas) pour l'instant on verra plus tard quand on réutilisera tout ça ou qu'on permettra des écritures; commiter du code pas testé c'est mal. Tu peux vérifier la couverture du code via le rapport coverage de jenkins : https://jenkins2.entrouvert.org/job/authentic-wip/job/wip%252F25645-roles-in-users-api/3/cobertura/authentic2/api_views_py/
- Ne pas exporter Role.external_id il n'est pas certain que ce champ survive (il n'a servi qu'au tout début du provisionning des rôles quand c'est w.c.s. qui était maître).
- La représentation de Service n'est pas donnée (idem que rôle ou presque, c'est (ou, slug, name) pas d'uuid).
- Je verrai bien des tests de la représentation complète d'un rôle :
assert response.json['roles'][0] == {'name': .., etc..}
un test explicite de ce genre apporte plus d'informations
Mis à jour par Paul Marillonnet il y a presque 5 ans
Benjamin Dauvergne a écrit :
- Une fonction attributes_hash apparait sortant de nulle part et n'est pas utilisée, quid ?
Erreur de ma part, je retire.
- On a perdu l'utiliation de roles_and_parents pour obtenir la liste des rôles qui était dans ma version du patch (ça veut dire qu'on a pas de tests suffisant pour valider ça)
Oui j'ai oublié de prendre en compte ton patch, c'est corrigé.
- L'utilisation actuelle de RoleSerializer étant en lecture-seule pour l'instant on peut virer create/update/partial_update (ou y mettre des raise NotImplementedError pour s'assurer qu'on y passe pas) pour l'instant on verra plus tard quand on réutilisera tout ça ou qu'on permettra des écritures; commiter du code pas testé c'est mal. Tu peux vérifier la couverture du code via le rapport coverage de jenkins : https://jenkins2.entrouvert.org/job/authentic-wip/job/wip%252F25645-roles-in-users-api/3/cobertura/authentic2/api_views_py/
Oui, ma faute, manque de tests dans #20706. Est-ce que ça veut dire qu'on déprécie les fonctionnalités apportées dans ce ticket ?
- Ne pas exporter Role.external_id il n'est pas certain que ce champ survive (il n'a servi qu'au tout début du provisionning des rôles quand c'est w.c.s. qui était maître).
Ok, je l'ai fait apparaître en imitant sans réfléchir le json d'export de rôles, je corrige ça.
- La représentation de Service n'est pas donnée (idem que rôle ou presque, c'est (ou, slug, name) pas d'uuid).
Pareil que plus haut, bête mimétisme. Je corrige aussi.
- Je verrai bien des tests de la représentation complète d'un rôle :
assert response.json['roles'][0] == {'name': .., etc..}
un test explicite de ce genre apporte plus d'informations
Ok, oui.
Mis à jour par Paul Marillonnet il y a presque 5 ans
- Fichier 0001-api-include-roles-in-users-API-25645.patch 0001-api-include-roles-in-users-API-25645.patch ajouté
Voilà, il reste à trancher pour l'utilisation en écriture du RoleSerializer, introduite dans #20706.
Pour le reste c'est ici.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
Paul Marillonnet a écrit :
Voilà, il reste à trancher pour l'utilisation en écriture du RoleSerializer, introduite dans #20706.
Pour le reste c'est ici.
On peut garder l'écriture, je pensais que ça ne servait pas (pas toujours clair d'avoir un Serializer qui sert à tout), je relis le reste.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Statut changé de Solution proposée à En cours
Toujours un peu embêté que les OUs soient sérialisées de deux manières différentes selon qu'on est en mode read-only ou pas; pour ne pas obérer l'avenir je verrai bien un remplacement de ou
par ou_slug
dans RoleSerializer et à la place une champ read_only avec OUConsiseSerializer.