Produits Entr'ouvert » Authentic 2

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

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

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

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

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

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

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.

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)

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

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

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.

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] ______________________________________________

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.

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

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

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.

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 ?

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.