Project

General

Profile

Development #20690

Ajouter automatiquement des rôles correspondant aux OU

Added by Mikaël Ates over 1 year ago. Updated 5 months ago.

Status:
En cours
Priority:
Normal
Category:
-
Target version:
-
Start date:
14 Dec 2017
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Un rôle est créé pour chaque OU dès sa création, dont les membres sont mis à jour automatiquement selon les utilisateurs présents dans l'OU.

Ceci est nécessaire notamment pour le provisionning dans Publik.

Peut-être un lien avec #20689.

0001-WIP-support-OU-automatic-roles-20690.patch View (11 KB) Paul Marillonnet, 18 Jul 2018 05:55 PM

0001-WIP-support-OU-automatic-roles-20690.patch View (12.7 KB) Paul Marillonnet, 19 Jul 2018 10:30 AM

0001-support-organizational-unit-automatic-roles-20690.patch View (9.55 KB) Paul Marillonnet, 02 Aug 2018 05:58 PM

ou_ui_admin.png View (36.6 KB) Paul Marillonnet, 08 Aug 2018 09:57 AM

ou_ui_manage.png View (122 KB) Paul Marillonnet, 08 Aug 2018 10:19 AM

26998
27001

Related issues

Related to Authentic 2 - Development #20689: Créer automatiquement des rôles/groupes en se basant sur des attributs de l'utilisateur Nouveau 14 Dec 2017

History

#1 Updated by Mikaël Ates over 1 year ago

  • Related to Development #20689: Créer automatiquement des rôles/groupes en se basant sur des attributs de l'utilisateur added

#2 Updated by Mikaël Ates 12 months ago

  • Description updated (diff)

#3 Updated by Benjamin Dauvergne 11 months ago

  • Ajouter un M2M automatic_roles au modèle OrganizationalUnits
  • surveiller les post_save du modèle User et m2m_changed sur automatic_roles, dans les deux cas ajouter/retirer le rôle aux utilisateurs en fonction
  • modifier OrganizationalUnitSerializer pour pouvoir modifier/définir la liste des rôles automatiques via l'API (ajouter un test en conséquence), ce serait bien qu'on puisse passer le rôle de façon souple (à la façon des imports, en utilisant uuid/ou_sug,slug/ou_slug,name); il faut valider que l'utilisateur qui fait cela a bien le droit d'attribuer ce rôle.

#4 Updated by Benjamin Dauvergne 11 months ago

  • passer aussi dans le BO pour ajouter ce champ au formulaire d'édition des OUs.

#5 Updated by Paul Marillonnet 11 months ago

  • Assignee set to Paul Marillonnet

#6 Updated by Paul Marillonnet 11 months ago

Benjamin Dauvergne a écrit :

  • surveiller les post_save du modèle User et m2m_changed sur automatic_roles, dans les deux cas ajouter/retirer le rôle aux utilisateurs en fonction
Ce qui me chiffonne ce qu'on risque de retirer aux utilisateurs de l'ou des roles obtenus non automatiquement.
Ex:
  • User1 obtient le role admin
  • User1 passe dans l'OU globale dont l'un des rôles automatiquement attribué est le role admin
  • L'OU globale perd ce automatic_role admin
  • User1 perd illégitimement ce rôle

Non ?

#7 Updated by Paul Marillonnet 11 months ago

Je mets un patch de pseudo-code, brouillon en cours, pour donner une idée de la direction que prend l'affaire (et pour moi, aussi, connaissant ma capacité à perdre du code non commité...)

#8 Updated by Paul Marillonnet 11 months ago

  • Patch proposed changed from Yes to No

#9 Updated by Benjamin Dauvergne 11 months ago

Paul Marillonnet a écrit :

Benjamin Dauvergne a écrit :

  • surveiller les post_save du modèle User et m2m_changed sur automatic_roles, dans les deux cas ajouter/retirer le rôle aux utilisateurs en fonction
Ce qui me chiffonne ce qu'on risque de retirer aux utilisateurs de l'ou des roles obtenus non automatiquement.
Ex:
  • User1 obtient le role admin
  • User1 passe dans l'OU globale dont l'un des rôles automatiquement attribué est le role admin
  • L'OU globale perd ce automatic_role admin
  • User1 perd illégitimement ce rôle

Non ?

Très bonne remarque, et donc il ne faut pas faire comme je dis. Il va falloir faire ça indirectement et ne rien matérialiser et jouer Role.objects.for_user() et voir si on utiliser toujours cet accesseur là où il faut la liste des rôles (déjà c'est pas le cas dans User.roles_and_parents()).

#10 Updated by Benjamin Dauvergne 11 months ago

  • Status changed from Solution proposée to Nouveau

#11 Updated by Paul Marillonnet 11 months ago

Benjamin Dauvergne a écrit :

et voir si on utiliser toujours cet accesseur là où il faut la liste des rôles (déjà c'est pas le cas dans User.roles_and_parents()).

C'est ce que je suis en train de regarder, à coups de git-grep.

#12 Updated by Benjamin Dauvergne 11 months ago

Je ne sais pas pourquoi dans ton patch tu déplaces OrganizationalUnit, ça raccourcirait un peu de ne pas le faire.

Idem ligne en moins dans a2_rbac.apps.

En fait on pourrait remplacer complètement roles_and_parents() par for_user(), il faut juste voir si il y a des utilisations des annotations particulières, ensuite il faudrait juste surcharger cette méthode dans la sous-classe du manager dans a2_rbac.manager pour y ajouter les rôles venant de l'OU de l'utilisateur (à l'origine j'ai pensé ça comme une application générique django_rbac et du spécifique dans authentic2.a2_rbac, de fait j'ai jamais trouvé le temps de packager django_rbac pour enfaire un projet à part entière...).

On ne peut pas faire

roles.append(...)
si roles est un QuerySet, il faut faire une union puis .distinct(), genre (qs1 | qs2).distinct().

#13 Updated by Benjamin Dauvergne 11 months ago

Il faudra aussi toucher au code de provisionning pour reprovisionner tous les utilisateurs d'une OU sur changement des rôles automatiques de celle-ci.

#14 Updated by Paul Marillonnet 11 months ago

Je suis en train de me prendre la tête sur une erreur de résolution de dépendances à l'exécution des tests.
Je suspecte le lien M2M.

Le déplacement de la classe OU c'est pour pouvoir référence Role définie plus tôt. Il y a peut-être un moyen plus propre, quelque chose genre
https://docs.djangoproject.com/fr/1.11/ref/applications/#django.apps.AppConfig.get_model ?

(Ceci dit, pas essayé et je ne suis pas certain que cela fonctionne lors de la définition de l'attribut ManyToManyField)

#15 Updated by Paul Marillonnet 11 months ago

Ah bein oui.

M2M de OU vers Role, Role ayant une FK vers Service, qui lui-même a une FK vers OU. Je pense que c'est ça qui complique l'affaire :)

#16 Updated by Frédéric Péters 11 months ago

(sans rien regarder) tu peux spécifier le nom du modèle sous forme de chaine, comme ça pas besoin que le symbole existe déjà.

#17 Updated by Paul Marillonnet 11 months ago

Frédéric Péters a écrit :

(sans rien regarder) tu peux spécifier le nom du modèle sous forme de chaine, comme ça pas besoin que le symbole existe déjà.

Je prends le conseil, merci.

#18 Updated by Paul Marillonnet 11 months ago

Paul Marillonnet a écrit :

Ah bein oui.

M2M de OU vers Role, Role ayant une FK vers Service, qui lui-même a une FK vers OU. Je pense que c'est ça qui complique l'affaire :)

En fait je n'ai jamais eu affaire à ce genre d'erreur [1], qui semble concerner un problème de dépendances circulaires, mais que est levée à l'exécution des tests, alors que la migration des modèles se passe sans encombre.

Je continue de chercher.

[1] :


request = <SubRequest 'django_db_setup' for <Function 'test_registration_email_blacklist'>>, django_test_environment = None
django_db_blocker = <pytest_django.plugin._DatabaseBlocker object at 0x7fa0d07e4690>, django_db_use_migrations = True, django_db_keepdb = False, django_db_createdb = False
django_db_modify_db_settings = None

    @pytest.fixture(scope='session')
    def django_db_setup(
        request,
        django_test_environment,
        django_db_blocker,
        django_db_use_migrations,
        django_db_keepdb,
        django_db_createdb,
        django_db_modify_db_settings,
    ):
        """Top level fixture to ensure test databases are available""" 
        from .compat import setup_databases, teardown_databases

        setup_databases_args = {}

        if not django_db_use_migrations:
            _disable_native_migrations()

        if django_db_keepdb and not django_db_createdb:
            setup_databases_args['keepdb'] = True

        with django_db_blocker.unblock():
            db_cfg = setup_databases(
                verbosity=pytest.config.option.verbose,
                interactive=False,
>               **setup_databases_args
            )

/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/pytest_django/fixtures.py:96: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/test/runner.py:370: in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/db/backends/base/creation.py:376: in create_test_db
    self.connection._test_serialized_contents = self.serialize_db_to_string()
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/db/backends/base/creation.py:413: in serialize_db_to_string
    serializers.serialize("json", get_objects(), indent=None, stream=out)
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/core/serializers/__init__.py:129: in serialize
    s.serialize(queryset, **options)
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/core/serializers/base.py:52: in serialize
    for obj in queryset:
/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/db/backends/base/creation.py:405: in get_objects
    for model in serializers.sort_dependencies(app_list):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

app_list = [(<ContentTypesConfig: contenttypes>, None), (<CustomUserConfig: custom_user>, None), (<AuthConfig: auth>, None), (<SessionsConfig: sessions>, None), (<AdminConfig: admin>, None), (<AppConfig: nonce>, None), ...]

    def sort_dependencies(app_list):
        """Sort a list of (app_config, models) pairs into a single list of models.

        The single list of models is sorted so that any model with a natural key
        is serialized before a normal model, and any model with a natural key
        dependency has it's dependencies serialized first.
        """ 
        # Process the list of models, and get the list of dependencies
        model_dependencies = []
        models = set()
        for app_config, model_list in app_list:
            if model_list is None:
                model_list = app_config.get_models()

            for model in model_list:
                models.add(model)
                # Add any explicitly defined dependencies
                if hasattr(model, 'natural_key'):
                    deps = getattr(model.natural_key, 'dependencies', [])
                    if deps:
                        deps = [apps.get_model(dep) for dep in deps]
                else:
                    deps = []

                # Now add a dependency for any FK relation with a model that
                # defines a natural key
                for field in model._meta.fields:
                    if hasattr(field.rel, 'to'):
                        rel_model = field.rel.to
                        if hasattr(rel_model, 'natural_key') and rel_model != model:
                            deps.append(rel_model)
                # Also add a dependency for any simple M2M relation with a model
                # that defines a natural key.  M2M relations with explicit through
                # models don't count as dependencies.
                for field in model._meta.many_to_many:
                    if field.rel.through._meta.auto_created:
                        rel_model = field.rel.to
                        if hasattr(rel_model, 'natural_key') and rel_model != model:
                            deps.append(rel_model)
                model_dependencies.append((model, deps))

        model_dependencies.reverse()
        # Now sort the models to ensure that dependencies are met. This
        # is done by repeatedly iterating over the input list of models.
        # If all the dependencies of a given model are in the final list,
        # that model is promoted to the end of the final list. This process
        # continues until the input list is empty, or we do a full iteration
        # over the input models without promoting a model to the final list.
        # If we do a full iteration without a promotion, that means there are
        # circular dependencies in the list.
        model_list = []
        while model_dependencies:
            skipped = []
            changed = False
            while model_dependencies:
                model, deps = model_dependencies.pop()

                # If all of the models in the dependency list are either already
                # on the final model list, or not on the original serialization list,
                # then we've found another model with all it's dependencies satisfied.
                found = True
                for candidate in ((d not in models or d in model_list) for d in deps):
                    if not candidate:
                        found = False
                if found:
                    model_list.append(model)
                    changed = True
                else:
                    skipped.append((model, deps))
            if not changed:
                raise RuntimeError("Can't resolve dependencies for %s in serialized app list." %
                    ', '.join('%s.%s' % (model._meta.app_label, model._meta.object_name)
>                   for model, deps in sorted(skipped, key=lambda obj: obj[0].__name__))
                )
E               RuntimeError: Can't resolve dependencies for authentic2_idp_cas.Attribute, authentic2.AuthorizedRole, auth2_ssl.ClientCertificate, authentic2.DeletedUser, authentic2.LDAPUser, saml.LibertyFederation, saml.LibertyProvider, saml.LibertyServiceProvider, saml.LibertySession, admin.LogEntry, authentic2_idp_oidc.OIDCAccessToken, authentic2_auth_oidc.OIDCAccount, authentic2_idp_oidc.OIDCAuthorization, authentic2_idp_oidc.OIDCClaim, authentic2_idp_oidc.OIDCClient, authentic2_idp_oidc.OIDCCode, authentic2_auth_oidc.OIDCProvider, a2_rbac.OrganizationalUnit, authentic2.PasswordReset, a2_rbac.Permission, a2_rbac.Role, a2_rbac.RoleAttribute, a2_rbac.RoleParenting, authentic2_idp_cas.Service, authentic2.Service, authentic2_idp_cas.Ticket, custom_user.User, authentic2.UserExternalId, mellon.UserSAMLIdentifier in serialized app list.

/tmp/tox-paul/authentic/py2-coverage-dj18-authentic-pg/local/lib/python2.7/site-packages/django/core/serializers/__init__.py:232: RuntimeError
______________________________________________ ERROR at setup of test_api_role_add_member[user_ou2-role_random-member_rando] ______________________________________________

#19 Updated by Benjamin Dauvergne 11 months ago

Tu es bloqué parce que tous ces modèles déclarent des clés naturelles et les dépendances circulaires entre modèles déclarant des clés naturelles ne sont pas possibles.

Le seul moyen ici va être de créer un modèle de jointure explicite, voir ce bout de code pour l'explication:

                    if field.rel.through._meta.auto_created:

Pour toute relation M2M il faut une troisième table qui stocke les id du premier et deuxième modèle pour faire la jointure, par défaut ce modèle est implicite (i.e. auto_created vaut True) mais via le paramètre through de ManyToManyField tu peux spécifier un modèle explicite, ici:

class OUAutomaticRoles(models.Model):
    ou = models.ForeignKey('OrganizationalUnit')
    role = models.ForeignKey('Role')
....
    automatic_roles = models.ManyToManyField(Role, through=OUAutomaticRoles)

cela va casser la dépendance circulaire entre OU et Role.

#20 Updated by Paul Marillonnet 11 months ago

Merci pour ton aide

#21 Updated by Benjamin Dauvergne 11 months ago

Tu fais trop de .distinct() faut le faire uniquement en sortie d'une méthode généralement.

#22 Updated by Benjamin Dauvergne 11 months ago

Tu pourrais poser un screenshot pour voir ce que ça donne en terme de page de gestion dans le backoffice ?

Après si tu as un publik-dev-inst de monté il faudra regarder le code de provisionning (hobo.agent.authentic2.provisionning).

#23 Updated by Paul Marillonnet 11 months ago

26998

Oui, bon déjà pas de visibilité du changement du modèle dans l'interface admin (cf la capture).
Je regarde :
- s'il y a un moyen simple d'inclure la gestion de ces rôles automatiques sur la page de modification d'une OU
- si la page de gestion d'un utilisateur tient bien compte de ses rôles automatiques indirectement obtenus via l'OU

Edit: en fait, pour le point 1, je réalise seulement maintenant que c'est du côté du modèle de jointure qu'il faut regarder.

Edit2: Et je fournis une capture de l'interface admin alors que tu me parles du backoffice. Merde. Je recommence.

#24 Updated by Paul Marillonnet 11 months ago

Pas de surprise côté /manage/, le M2M n'apparaît pas non plus.
S'il faut intégrer ce M2M dans le BO, je vois deux solutions :
- une vue à part pour la gestion du modèle de jointure
- l'intégration de la gestion de ce modèle dans la vue de gestion des OU.

La solution deux me paraît préférable pour ce qui est de l'interface.

#25 Updated by Paul Marillonnet 11 months ago

27001

Et donc, comme demandé, la capture (même si aucun changement visible n'apparaît).

#26 Updated by Paul Marillonnet 11 months ago

Pour ce qui est de l'interface du backoffice, je verrais bien un affichage des rôles automatiques sur l'espace latéral droit de la vue de gestion d'une OU.
En gros comme le sont déjà affichés les rôles sur la page de gestion d'un utilisateur.
À la différence que l'édition d'une OU (bouton "Éditer") permettrait de gérer les rôles présents dans cette liste (alors que pour gérer les rôles d'un utilisateur il faut passer par la vue de gestion des rôles).

Est-ce que c'est une approche viable du point de vue du besoin exprimé dans ce ticket ?

#27 Updated by Benjamin Dauvergne 5 months ago

  • Status changed from Solution proposée to En cours

#28 Updated by Benjamin Dauvergne 5 months ago

Paul Marillonnet a écrit :

Est-ce que c'est une approche viable du point de vue du besoin exprimé dans ce ticket ?

Oui. Pour continuer rebasé et pousser branche pour voir le statut du build ici.

Also available in: Atom PDF