PnX-SI / UsersHub-authentification-module

Module Flask d'authentification de UsersHub
GNU General Public License v3.0
5 stars 12 forks source link

Le décorateur "check_auth" attrape toute les exceptions #12

Closed TheoLechemia closed 6 years ago

TheoLechemia commented 6 years ago

ça faisait un moment que je me demandais pourquoi l'API de GeoNature renvoyait tout le temps des erreurs 403 à chaque erreur (synthaxe, internal serveur error etc...) Après investigation, c'est le décorateur @check_auth qui cause se comportement:

def _check_auth(fn):
    @wraps(fn)
    def __check_auth(*args, **kwargs):
        try:
            # TODO: better name and configurability for the token
            user = user_from_token(request.cookies['token'])

            if user.id_droit_max < level:
                # TODO: english error message ?
                print('Niveau de droit insufissants')
                return Response('Forbidden', 403)

            if get_role:
                kwargs['id_role'] = user.id_role

            g.user = user

            return fn(*args, **kwargs)

        except Exception as e:
            trap_all_exceptions = current_app.config.get(
                'TRAP_ALL_EXCEPTIONS',
                True
            )
            if not trap_all_exceptions:
                raise
            print('Exception')
            print(e)
            msg = json.dumps({'type': 'Exception', 'msg': repr(e)})
            return Response(msg, 403)

Le retour de la fonction qui est décoré se trouve à l’intérieur du bloc "try" qui intercepte toute les exceptions et retourne des 403 (si le paramètre TRAPP_ALL_EXCEPTIONS n'est pas redéfini dans l'application mère). Du coup toute les erreurs levé à l’intérieur d'une route qui utilise ce décorateur était toujours surchargé par ce bout de code !

ksamuel commented 6 years ago

Hello theo. Si tu fais ce changement tu vas casser la compatibilité avec les services existant qui utilisent la lib. Ce comportement n'est pas le bon, on est d'accord, mais il est celui attendu maintenant.

Pour cette raison, j'ai ajouté la variable d'environnement "TRAP_ALL_EXCEPTIONS" qui permet de récuperer le comportement plus sain que tu es en train de forcer.

Deux choix ici:

C'est la loi du code legacy.

Vois avec Amandine pour choisir la stratégie.

TheoLechemia commented 6 years ago

Il me semblait bien que ça pouvait avoir des répercussion, d'ou l'issue ouverte sur le sujet. On va voir ce qu'on fait.

Merci !