Project

General

Profile

Development #35352

fargo, ne pas logger d'erreur de GET sur une 403

Added by Benjamin Dauvergne 8 months ago. Updated 8 days ago.

Status:
Solution déployée
Priority:
Normal
Target version:
-
Start date:
10 Aug 2019
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Si l'utilisateur n'existe pas dans fargo on obtient une 403, ça ne sert à rien de la logger en erreur ni en warning d'ailleurs.

Report

failed to GET https://porte-documents.clissonsevremaine.fr/api/documents/recently-added/?NameID=15cde3228f7745f08bd55599cdae777a&orig=monespace.clissonsevremaine.fr (403)

Django Version:     1.11.20
Python Executable:     /usr/bin/uwsgi-core
Python Version:     2.7.13
Python Path:     

['.',
 '',
 '/usr/lib/python2.7',
 '/usr/lib/python2.7/plat-x86_64-linux-gnu',
 '/usr/lib/python2.7/lib-tk',
 '/usr/lib/python2.7/lib-old',
 '/usr/lib/python2.7/lib-dynload',
 '/usr/local/lib/python2.7/dist-packages',
 '/usr/lib/python2.7/dist-packages']

Server time:     sam, 10 Aoû 2019 09:23:58 +0200

Les informations de signature ont disparu des logs à cause de #35225, il faut juste s'en souvenir.

Report

failed to GET https://porte-documents.clissonsevremaine.fr/api/documents/recently-added/?NameID=fcc37bc2ba144264b573feaf3ff1a811&orig=monespace.clissonsevremaine.fr&algo=sha256&timestamp=2019-07-06T12%3A20%3A27Z&nonce=8f4a60573d5755adb9da654c66994817L&signature=j/F2R1hLZsOE2cPnTGaDtPsFop28jsH8SNLNjBuwTfQ%3D (403)

Django Version:     1.11.20
Python Executable:     /usr/bin/uwsgi-core
Python Version:     2.7.13
Python Path:     

['.',
 '',
 '/usr/lib/python2.7',
 '/usr/lib/python2.7/plat-x86_64-linux-gnu',
 '/usr/lib/python2.7/lib-tk',
 '/usr/lib/python2.7/lib-old',
 '/usr/lib/python2.7/lib-dynload',
 '/usr/local/lib/python2.7/dist-packages',
 '/usr/lib/python2.7/dist-packages']

Server time:     sam, 6 Jul 2019 14:20:27 +0200

0001-fargo-do-not-log-403-response-as-errors-35352.patch View (2.25 KB) Benjamin Dauvergne, 10 Aug 2019 11:58 AM

0001-fargo-hide-user-not-found-API-errors-and-only-that-3.patch View (3.02 KB) Benjamin Dauvergne, 22 Mar 2020 12:09 PM


Related issues

Related to Hobo - Development #39911: rest_authentication : lever des exceptions spécifiques en cas d'échec Solution déployée 14 Feb 2020

Associated revisions

Revision 3baed95c (diff)
Added by Benjamin Dauvergne 15 days ago

fargo: hide user-not-found API errors and only that (#35352)

History

#1 Updated by Benjamin Dauvergne 8 months ago

#2 Updated by Thomas Noël 8 months ago

Je dirais que c'est plutôt à fargo de répondre "vide" dans ce cas (et donc ne faire le 403 que sur les pépins de signature), voire répondre un err:1 ... mais ce n'est pas possible dans fargo car tout se joue trop tôt (Django Rest Framework).

Bref, je ne suis pas chaud, ne pas avoir d'alerte en cas de panne du système de signature, ça m'ennuie un peu. Et je n'ai pas souvenir d'avoir tant rencontré la trace ci-dessus (toi, si ?)

#3 Updated by Benjamin Dauvergne 8 months ago

Thomas Noël a écrit :

Je dirais que c'est plutôt à fargo de répondre "vide" dans ce cas (et donc ne faire le 403 que sur les pépins de signature), voire répondre un err:1 ... mais ce n'est pas possible dans fargo car tout se joue trop tôt (Django Rest Framework).

À vrai dire je n'ai même pas trouvé dans le code DRF d'où vient ce 403, c'est une exception PermissionDenied normalement et je n'en vois pas au moment du check d'authentification, on lève un AuthenticationFailed qui devrait donner une 401.

Bref, je ne suis pas chaud, ne pas avoir d'alerte en cas de panne du système de signature, ça m'ennuie un peu. Et je n'ai pas souvenir d'avoir tant rencontré la trace ci-dessus (toi, si ?)

Oui tous les déploiements avec un fargo dès qu'il y a une cellule dernier document, ça en produit sur les utilisateurs non provisionnés.

#4 Updated by Thomas Noël 8 months ago

Benjamin Dauvergne a écrit :

Oui tous les déploiements avec un fargo dès qu'il y a une cellule dernier document, ça en produit sur les utilisateurs non provisionnés.

C'est un peu ça qui m'étonne, les "utilisateurs non provisionnés", c'est pas si fréquent normalement... le cas que je vois c'est dans la seconde après la création du compte, retour sur la page d'accueil qui présente les derniers documents, et boum ?

#5 Updated by Benjamin Dauvergne 8 months ago

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Oui tous les déploiements avec un fargo dès qu'il y a une cellule dernier document, ça en produit sur les utilisateurs non provisionnés.

C'est un peu ça qui m'étonne, les "utilisateurs non provisionnés", c'est pas si fréquent normalement... le cas que je vois c'est dans la seconde après la création du compte, retour sur la page d'accueil qui présente les derniers documents, et boum ?

Ben c'est les gens inscrits avant qu'on déploie fargo, à moins d'être sûr de toujours déployer fargo avant toute mise en prod on en aura d'autres, jusqu'à ce qu'on provisionne autrement.

#6 Updated by Thomas Noël 8 months ago

Benjamin Dauvergne a écrit :

Thomas Noël a écrit :

Benjamin Dauvergne a écrit :

Oui tous les déploiements avec un fargo dès qu'il y a une cellule dernier document, ça en produit sur les utilisateurs non provisionnés.

C'est un peu ça qui m'étonne, les "utilisateurs non provisionnés", c'est pas si fréquent normalement... le cas que je vois c'est dans la seconde après la création du compte, retour sur la page d'accueil qui présente les derniers documents, et boum ?

Ben c'est les gens inscrits avant qu'on déploie fargo, à moins d'être sûr de toujours déployer fargo avant toute mise en prod on en aura d'autres, jusqu'à ce qu'on provisionne autrement.

« authentic hobo_provision --users » et hop

#7 Updated by Benjamin Dauvergne about 2 months ago

Je ne suis pas tellement d'accord, supposer que le provisionning des usagers fonctionne à 100% en temps réel c'est casse gueule, je pense que ce patch est utile.

#8 Updated by Thomas Noël about 2 months ago

Benjamin Dauvergne a écrit :

Je ne suis pas tellement d'accord, supposer que le provisionning des usagers fonctionne à 100% en temps réel c'est casse gueule, je pense que ce patch est utile.

Fargo a aussi un front, donc on a aussi du provisionning via SAML (et pour le coup "100% temps réel") dès que la personne clique sur "mes documents". Je reste pas chaud à cacher les 403 (et comme dit plus haut je préférerais qu'on creuse hobo/rest_authentication.py pour voir si vraiment on peut envoyer un err:1 quand l'utilisateur n'existe pas... même si j'ai pas vu de chemin pour faire ça)

#9 Updated by Benjamin Dauvergne about 2 months ago

J'ai trouvé l'origine de la 403 (ça convertit en 403 tout ce qui est 401 mais ne vient pas d'un mode d'authentification HTTP classique, i.e. via un entête Authorization, parce que c'est des nazis du code HTTP) :

    def handle_exception(self, exc):
        """ 
        Handle any exception that occurs, by returning an appropriate response,
        or re-raising the error.
        """ 
        if isinstance(exc, (exceptions.NotAuthenticated,
                            exceptions.AuthenticationFailed)):
            # WWW-Authenticate header for 401 responses, else coerce to 403
            auth_header = self.get_authenticate_header(self.request)

            if auth_header:
                exc.auth_header = auth_header
            else:
                exc.status_code = status.HTTP_403_FORBIDDEN

On pourrait passer par une erreur APIException et faire une 401 avec err=1 en loucedé, ça t'irait ? Mais ça va devoir aller dans hobo.

#10 Updated by Benjamin Dauvergne about 2 months ago

  • Status changed from Solution proposée to Information nécessaire
  • Assignee changed from Benjamin Dauvergne to Thomas Noël

#11 Updated by Thomas Noël about 2 months ago

  • Assignee changed from Thomas Noël to Benjamin Dauvergne

On pourrait passer par une erreur APIException et faire une 401 avec err=1 en loucedé, ça t'irait ? Mais ça va devoir aller dans hobo.

Oui, que ça soit dans hobo/rest_authentication.py c'est le bon endroit. Et cela grâce à un raise d'une APIException avec un detail={err:1, err_desc:"user-not-found"} et status_code=401, pour arriver ici dans le code de DRF :

def exception_handler(exc, context):

    if isinstance(exc, exceptions.APIException):
        headers = {}
        if getattr(exc, 'auth_header', None):
            headers['WWW-Authenticate'] = exc.auth_header
        if getattr(exc, 'wait', None):
            headers['Retry-After'] = '%d' % exc.wait

        if isinstance(exc.detail, (list, dict)):
            data = exc.detail
        else:
            data = {'detail': exc.detail}

        set_rollback()
        return Response(data, status=exc.status_code, headers=headers)
</per>

Et donc dans combo, petit patch quand même : quand on appelle une API DRF on saura qu'un 401 avec un payload err=1, on sait que ça peut arriver (soucis de provisionning sur un nouveau compte) : loguer juste en info, ou au pire warning.

J'ai bon ?

#12 Updated by Benjamin Dauvergne about 2 months ago

Thomas Noël a écrit :

Et donc dans combo, petit patch quand même : quand on appelle une API DRF on saura qu'un 401 avec un payload err=1, on sait que ça peut arriver (soucis de provisionning sur un nouveau compte) : loguer juste en info, ou au pire warning.

J'ai bon ?

Yep c'est ça l'idée.

#13 Updated by Benjamin Dauvergne about 2 months ago

  • Related to Development #39911: rest_authentication : lever des exceptions spécifiques en cas d'échec added

#14 Updated by Benjamin Dauvergne 17 days ago

#15 Updated by Thomas Noël 16 days ago

  • Status changed from Solution proposée to Solution validée

#16 Updated by Benjamin Dauvergne 15 days ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 60add2525c032ec2e7608d9d43ec8bc0923b5ee7
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sun Mar 22 12:07:14 2020 +0100

    fargo: hide user-not-found API errors and only that (#35352)

#17 Updated by Frédéric Péters 8 days ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF