Project

General

Profile

Development #25645

API pour obtenir la liste des rôles portés par un utilisateur

Added by Frédéric Péters 11 months ago. Updated 3 months ago.

Status:
En cours
Priority:
Normal
Category:
-
Target version:
-
Start date:
13 Aug 2018
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

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.

0001-WIP-include-roles-in-users-api-25645.patch View (2.32 KB) Paul Marillonnet, 30 Aug 2018 02:53 PM

0001-WIP-include-roles-in-users-api-25645.patch View (2.48 KB) Paul Marillonnet, 06 Sep 2018 12:33 PM

0001-include-roles-in-users-api-25645.patch View (4.15 KB) Paul Marillonnet, 20 Sep 2018 03:41 PM

0001-api-include-roles-in-users-API-25645.patch View (8.07 KB) Frédéric Péters, 30 Oct 2018 10:29 AM

0001-api-include-roles-in-users-API-25645.patch View (8.07 KB) Benjamin Dauvergne, 04 Dec 2018 07:41 PM

0001-api-include-roles-in-users-API-25645.patch View (8.84 KB) Paul Marillonnet, 22 Mar 2019 04:03 PM

0001-api-include-roles-in-users-API-25645.patch View (9.96 KB) Paul Marillonnet, 22 Mar 2019 05:33 PM

0001-api-include-roles-in-users-API-25645.patch View (11.5 KB) Paul Marillonnet, 29 Mar 2019 12:15 PM


Related issues

Related to Publik - Development #19756: Personnalisation accrue du portail agent pour en faire aussi la page d'entrée des agents d'accueil En cours 29 Oct 2017 30 Jun 2018
Related to Authentic 2 - Development #21485: API pour avoir la liste des rôles d'un utilisateur Rejeté 29 Jan 2018

History

#1 Updated by Thomas Noël 10 months ago

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.

#2 Updated by Frédéric Péters 10 months ago

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).

#3 Updated by Paul Marillonnet 10 months ago

  • Assignee set to Paul Marillonnet

#4 Updated by Paul Marillonnet 10 months ago

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.

#5 Updated by Frédéric Péters 10 months ago

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.

#6 Updated by Paul Marillonnet 10 months ago

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 ?

#7 Updated by Benjamin Dauvergne 10 months ago

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.

#8 Updated by Paul Marillonnet 10 months ago

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.

#9 Updated by Benjamin Dauvergne 10 months ago

  • Pourquoi définir roles dans init plutôt qu'au niveau des attributs de la classe ?
  • Pourquoi ajouter roles à ordering_fields ?

#10 Updated by Paul Marillonnet 10 months ago

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.

#11 Updated by Paul Marillonnet 9 months ago

Avec les tests.

#12 Updated by Frédéric Péters 9 months ago

  • Related to Development #19756: Personnalisation accrue du portail agent pour en faire aussi la page d'entrée des agents d'accueil added

#13 Updated by Frédéric Péters 8 months ago

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.

#14 Updated by Benjamin Dauvergne 7 months ago

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().

#15 Updated by Benjamin Dauvergne 7 months ago

  • Status changed from Solution proposée to En cours

#16 Updated by Benjamin Dauvergne 7 months ago

(via l'attribut source sur de Field.__init__() de django-rest-framework, https://www.django-rest-framework.org/api-guide/fields/#source)

#17 Updated by Benjamin Dauvergne 7 months ago

Voilà j'ai corrigé le souci que je voyais, c'est bon pour moi.

#18 Updated by Benjamin Dauvergne 7 months ago

Paul c'est moi ou tu as bossé deux fois sur le même ticket ? Voir #21485.

#19 Updated by Frédéric Péters 5 months ago

  • Related to Development #21485: API pour avoir la liste des rôles d'un utilisateur added

#20 Updated by Frédéric Péters 5 months ago

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.

#21 Updated by Paul Marillonnet 5 months ago

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 :/

#22 Updated by Benjamin Dauvergne 3 months ago

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).

#23 Updated by Benjamin Dauvergne 3 months ago

  • Status changed from Solution proposée to En cours

#24 Updated by Paul Marillonnet 3 months ago

Ok.

#25 Updated by Paul Marillonnet 3 months ago

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).

#26 Updated by Benjamin Dauvergne 3 months ago

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)
  • 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

#27 Updated by Paul Marillonnet 3 months ago

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é.

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.

#28 Updated by Paul Marillonnet 3 months ago

Voilà, il reste à trancher pour l'utilisation en écriture du RoleSerializer, introduite dans #20706.
Pour le reste c'est ici.

#29 Updated by Benjamin Dauvergne 3 months ago

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.

#30 Updated by Benjamin Dauvergne 3 months ago

  • Status changed from Solution proposée to 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.

Also available in: Atom PDF