Produits Entr'ouvert » Authentic 2

À reculer un peu,

• on aurait des conditions "wants_frontoffice" / "wants_backoffice"
• genre une configuration pourrait être : FranceConnect:wants_frontoffice, KeyCloak:wants_backoffice.
  • l'usager arrive, les filtres sont joués, reste une seule méthode, l'autorun de #28216 est lancé, etc.

Mais, la situation GNM est différente, il y a une seule méthode, "oidc", mais celle-ci en cache deux (oidc vers GLC, oidc vers GLC/agents). Résultat cette mécanique de filtre ne peut pas y fonctionner.

(c'est un commentaire que j'ai fait dans les notes "bilan J1" du pad, sans doute j'aurais du la mettre en avant).

Selon moi, on a besoin d'une découpe supplémentaire, un module d'authentification (OIDC, FC, etc.) doit pouvoir exposer plusieurs frontends(?), et c'est à ce niveau que pourrait alors se jouer le filtrage de ce ticket.

• OIDC (module d'authent)
  • explode
  • → frontends : OIDC[usagers], OIDC[agents]
  • filtre des frontends
  • → OIDC[agents]
    • un seul → autorun

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

À reculer un peu,

• on aurait des conditions "wants_frontoffice" / "wants_backoffice"
• genre une configuration pourrait être : FranceConnect:wants_frontoffice, KeyCloak:wants_backoffice.
  • l'usager arrive, les filtres sont joués, reste une seule méthode, l'autorun de #28216 est lancé, etc.

Mais, la situation GNM est différente, il y a une seule méthode, "oidc", mais celle-ci en cache deux (oidc vers GLC, oidc vers GLC/agents). Résultat cette mécanique de filtre ne peut pas y fonctionner.

Si parce que pour le frontend OIDC, chaque fournisseur OIDC représente une méthode différente.

(c'est un commentaire que j'ai fait dans les notes "bilan J1" du pad, sans doute j'aurais du la mettre en avant).

Selon moi, on a besoin d'une découpe supplémentaire, un module d'authentification (OIDC, FC, etc.) doit pouvoir exposer plusieurs frontends(?), et c'est à ce niveau que pourrait alors se jouer le filtrage de ce ticket.

• OIDC (module d'authent)
  • explode
  • → frontends : OIDC[usagers], OIDC[agents]
  • filtre des frontends
  • → OIDC[agents]
    • un seul → autorun

Dans mon plan initial ce n'est pas la vue login() qui décide de lancer autorun, mais le frontend il faut pour cela deux méthodes (can_autorun() puis autorun()) ou bien prévoir qu'autorun retourne possiblement None.

• GNM -> un seul frontend = OIDC
• appel à OIDCFrontend.autorun()
  • sélection d'au moins deux fournisseurs : autorun() retourne None
  • sélection d'un seul fournisseur : autorun() retourne HttpRedirect

Au passage il faut passer à autorun tout un tas de paramètre reçus par login, comme le nonce, next_url, etc..

Si parce que pour le frontend OIDC, chaque fournisseur OIDC représente une méthode différente.

C'est peut-être un problème de vocabulaire mais il me semble que c'est ma proposition aussi, et qu'elle ne correspond pas à la série de patchs en cours de développement, #14475 change juste des noms, on reste avec OIDC = 1 Frontend^WAuthenticator, et ce ticket, il itère sur ces objets, pas un objet "méthode d'authentification" dessous.

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

Si parce que pour le frontend OIDC, chaque fournisseur OIDC représente une méthode différente.

C'est peut-être un problème de vocabulaire mais il me semble que c'est ma proposition aussi.

Oui. Je revois mon idée pour prendre en compte ce cas d'usage.

Si on introduit maintenant un changement de structure avec des AuthMethod il faut passer sur authentic2-auth-fc aussi et peut-être d'autres modules externes, je préfèrerai que ce soit fait une fois ces modules externes réintégrés dans authnetic.

(peut-être que tout ceci devrait être dans authentic, pas authentik)

À redéfinir le fonctionnement des frontends/méthodes d'authentification il faut peut-être inclure dans la réflexion également la partie "inscription" (qui aujourd'hui mélange un truc un peu en dur pour l'inscription locale/email et un mécanique "accumulation de blocs de frontends" simulaire à la page de login).

Concernant la problèmatique des multiples fournisseurs OIDC je vois la possibilité de pouvoir les filtrer dans la condition d'affichage, genre: request.session.get('is_agent', False) and providers.filter(slug="idp-agents")

Je trouve vraiment dommage de garder cette association "1 module d'authentification = 1 pavé de connexion", qui oblige derrière à inventer une notion de sous-pavé (un fournisseur oidc), qui fait que tu parles déjà de devoir dupliquer une mécanique de conditions. Je ne fais que me répéter.

    A2_AUTH_PASSWORD_ENABLE_CONDITION=Setting(default=None, definition='Filters',
                                              names=('FRONTEND ENABLE CONDITION',)),

Je lis le code de app_settings et names me semble être un attribut permettant d'aller chercher, si le paramètre demandé n'existe pas, le paramètre sous un autre nom. Et ici, sous le nom "FRONTEND ENABLE CONDITION", avec des espaces, ça me semble farfelu.

                return eval(condition, context)

Discussion de fond, je serais pour éviter le Python tapable partout, que se passe par une condition Django. (mais je pense qu'il n'y aura pas accord sur ce commentaire, ne te lance surtout pas là-dedans tout de suite).

            except Exception, e:

Pensons à Python 3, Exception as e.

            providers = self.eval_enable_condition(app_settings.IDP_ENABLE_CONDITION, request,
                                                   providers=providers)

mais self.eval_enable_condition retourne un booléen. (?)

Ici, et dans l'autre, côté SAML, où on passe idps=idps, je ne pige pas trop le sens de l'affaire, pourquoi on ferait une comparaison sur base de la liste des IdP possibles. Alors ça s'éclaire en lisant le test, providers.filter(name='My IDP'), mais pour moi c'est retourner à mon commentaire 13 ci-dessus, un partage bizarre d'une même mécanique pour faire deux choses :

• pour une méthode d'authentification comme le mot de passe avoir une condition qui retourne un booléen.
• pour une méthode d'authentification comme oidc/saml, avoir une condition qui n'en est pas une qui est plutôt une expression filtrant une liste.

Pas fan du tout il y a dix mois, pas fan du tout aujourd'hui.

Très concrètement, pour mon usage GNM, j'aurais aimé pouvoir :

"A2_AUTH_MODULES_CONDITIONS": {
  "password": "request|is_for_backoffice",
  "oidc/glc": "not request|is_for_backoffice" 
}

ou dans la version précédente avec deux GLC,

"A2_AUTH_MODULES_CONDITIONS": {
  "oidc/glc-agents": "request|is_for_backoffice",
  "oidc/glc": "not request|is_for_backoffice" 
}

Ailleurs, pour un Chambéry,

"A2_AUTH_MODULES_CONDITIONS": {
  "password": "not request|is_for_backoffice",
  "franceconnect": "not request|is_for_backoffice",
  "saml/adfs": "request|is_for_backoffice" 
}

(et j'en reste ici à des settings, idéalement j'aimerais la condition portée par l'objet OIDCProvider, etc.)

Je veux bien une relecture du dernier patch.

Ok. J'avais pensé au début à l'usage de utils/evaluate mais l'idée d'uniformiser la syntaxe des expressions en Django me plasait bien aussi.

Pour revenir à l'écriture des expressions de filtrage, on reste sur la définition des celles-ci dans le settings.json du tenant comme proposé plus haut, genre ?:

"A2_AUTH_MODULES_CONDITIONS": {
  "password": "is_for_backoffice(request)",
  "franceconnect": "is_for_backoffice(request)",
  "saml/adfs": "is_for_backoffice(request)" 
}

Serghei Mihai a écrit :

Pour revenir à l'écriture des expressions de filtrage, on reste sur la définition des celles-ci dans le settings.json du tenant comme proposé plus haut, genre ?:
[...]

1. Idéalement on va vouloir ça par instance, donc non pas comme cela, déjà on ne va pas vouloir que ça accède à request il faut un truc plus contraignant que cela et je ne mettrai pas ça au niveau de get_backends() plutôt dans login() ; je veux vraiment éviter des expressions du style request.GET.get('idee-du-jour') == 'ok',
2. ne pas mélanger les histoires de login-hint SAML on verra ça dans un autre ticket.

• dans login on filtre authenticators ou les instances sur l'absence d'une fonction shown qui a pour signature shown(insance_id, **kwargs) ou alors sur le résultat de son appel,
• dans kwargs dans un premier temps on peut mettre des trucs évidents genre remote_addr, service_id

On verra plus tard quoi faire de login_hint et comment le passer, mais la mécanique étant faite ce sera plus simple.

PS: on a déjà dans get_backends() de quoi passer des paramètre à Authenticator.__init__, donc une première implémentation de base tirerai l'expression ce ça, genre

class BaseAuthenticator:
    def __init__(self, shown_condition=None):
        self.shown_condition = shown_condition

    def get_shown_condition(self, instance_id=None, instance=None):
        if isinstance(self.shown_condition, dict):
            if instance_id and instance_id in self.shown_condition:
                return self.shown_condition[instance_id]
            else:
                return self.shown_condition.get('')
        else:
            return self.shown_condition

    def shown(self, instance_id=None, instance=None, remote_addr=None, service_id=None):
        condition = self.get_shown_condition(instance_id=instance_id, instance=None)
        if not condition:
            return True
        return eval(condition, {'id': instance_id, 'remote_addr': remote_addr, 'service_id': service_id})

Ça se configure avec :

A2_AUTH_FRONTENDS_KWARGS = {
    'password': {
        'shown_condition': '\'admin\' in login_hint',
    },
    'oidc': {
        'shown_condition': '\'admin\' not in login_hint',
    }
}

coté auth_oidc on pourra avoir

    def get_shown_condition(self, instance_id, instance):
         if instance.shown_condition:
             return instance.shown_condition
         return super(OIDCAuthenticator, self).get_shown_condition(instance_id=instance_id, instance=instance)

avec ajout d'un champ au modèle.

Benjamin Dauvergne a écrit :

coté auth_oidc on pourra avoir

[...]

avec ajout d'un champ au modèle.

Je pense qu'il faut soit rajouter un champ au modèle et gérer l'affichage uniquement sur la base celui-ci, soit le faire uniquement dans le settings, genre:

{
 "oidc": {
  "show_condition: {
    "idp-agents": "<condition>",
    "idp-usagers": "<condition>" 
  }
} 

Pareil pour auth_saml. Soit:

{
 "saml": {
  "show_condition": {
    "idp_ville": "<condition>",
    "idp_departement": "<condition>" 
  }
} 

Et donc avoir une seule manière de gérer l'affichage des backends proposant plusieurs instances.

• je ne mélangerai pas enabled et shown pour l'instant (1 parce que je ne vois pas à quoi ça sert là maintenant, 2 parce que dans les cas d'authentification déporté ça peut avoir un sens justement, typiquement OIDC ou SAML on peut se connecter en utilisant une truc initié par l'IdP ou une URL d'initiation sur authentic sans passer par un bouton sur le front).

Sinon pour le reste c'est ok pour moi.

Serghei Mihai a écrit :

Tu as raison.

Je préfère un db explicite.

pytestmark = pytest.mark.django_db