Bug #13184
Ne pas émettre de 403 sur une requête à /api/user/forms avec un NameID inconnu
0%
Description
Vu à Montpellier en arrivant sur combo après la création d'un nouvel utilisateur, on a:
failed to get data from any /api/user/forms (set([403]))
Le endpoint devrait simplement renvoyer une liste vide dans ce cas.
Fichiers
Demandes liées
Révisions associées
Historique
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
- Fichier 0001-make-api-user-form-returns-on-unknown-NameID-13184.patch 0001-make-api-user-form-returns-on-unknown-NameID-13184.patch ajouté
- Patch proposed changé de Non à Oui
Mis à jour par Frédéric Péters il y a plus de 7 ans
Je ne suis pas sûr de vouloir ça1; mais si on fait ça il faut au moins que les autres endpoints sous /api/user/ se comportent de la même manière. (et des tests sur ces situations)
[1] (d'abord parce que ça vient comme réponse à une lenteur du provisioning)
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
Je peux aussi renvoyer un 404 pour pouvoir différencier les situations (il faut qu'on continue de considérer les 403, i.e. erreurs de signature comme des erreurs) mais il me faudra ensuite modifier le code des cellules. Dis mois ton choix entre 404 ou renvoyer une information vide et je ferai la modification aux autres endpoints.
Mis à jour par Frédéric Péters il y a plus de 7 ans
Absolument pas enthousiaste sur la 404, mais ce que je proposerais c'est retourner {"err": 1, "data": []}, et modifier le endpoint pour également retourner cette structure quand tout va bien; et là il est peut-être nécessaire d'avoir une url d'endpoint différente pour ne pas casser la compatibilité (je ne sais pas trop les appels qui peuvent être faits à Vincennes), puis modifier combo en conséquence.
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
- Fichier 0001-add-HttpRequest.get_version-to-help-in-API-versionni.patch 0001-add-HttpRequest.get_version-to-help-in-API-versionni.patch ajouté
- Fichier 0002-add-new-version-to-api-user-form-API-fixes-13184.patch 0002-add-new-version-to-api-user-form-API-fixes-13184.patch ajouté
Nouvelle proposition, j'ai ajouté une méthode à HttpRequest pour aider dans le versionning des APIs, il suffit d'ajouter &api_version=x pour obtenir une version particulière, par défaut get_request().get_api_version()
renvoie 0.
Ici l'api /user/forms/
continue à fonctionner normalement, mais si api_version > 0 alors on retourne le nouveau format.
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
- Fichier 0001-add-new-version-to-api-user-form-API-fixes-13184.patch 0001-add-new-version-to-api-user-form-API-fixes-13184.patch ajouté
Avec mise à jour de la doc.
Peut-être au passage devrais-je traiter les autres endpoints autour de l'utilisateur (/api/user et /api/user/drafts).
Mis à jour par Frédéric Péters il y a plus de 7 ans
J'ai regardé ce qui était utilisé comme API à Vincennes et user/forms n'en fait pas partie; c'est donc uniquement utilisé par Combo. Ma préférence du coup c'est de simplement modifier l'API dans w.c.s. après avoir modifié combo pour gérer correctement la présente et la nouvelle API (#13521).
Très basiquement, la partie "retours" de ce patch deviendrait :
--- a/wcs/api.py +++ b/wcs/api.py @@ -461,7 +461,7 @@ class ApiUserDirectory(Directory): d.update(form.get_json_export_dict(include_files=False)) drafts.append(d) - return json.dumps(drafts, + return json.dumps({'err': 0, 'data': drafts}, cls=misc.JSONEncoder, encoding=get_publisher().site_charset) @@ -499,7 +499,7 @@ class ApiUserDirectory(Directory): d.update(form.get_json_export_dict(include_files=False)) forms.append(d) - return json.dumps(forms, + return json.dumps({'err': 0, 'data': forms}, cls=misc.JSONEncoder, encoding=get_publisher().site_charset)
Mis à jour par Frédéric Péters il y a plus de 7 ans
Dans la partie "ne pas retourner de liste mais un dictionnaire avec une clé data contenant la liste", il y a aussi le retour du _q_index de ApiFormdefsDirectory.
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
- Fichier 0001-change-schema-of-api-formdefs-api-user-forms-and-api.patch 0001-change-schema-of-api-formdefs-api-user-forms-and-api.patch ajouté
J'ai supprimé tout ce qui concerne api_version, j'ai voulu passé aussi sur /api/user puis je me suis dit que ça pouvait attendre.
Mis à jour par Benjamin Dauvergne il y a plus de 7 ans
- Fichier 0001-change-schema-of-api-formdefs-api-user-forms-and-api.patch 0001-change-schema-of-api-formdefs-api-user-forms-and-api.patch ajouté
Avec les corrections de test pas commités...
Mis à jour par Frédéric Péters il y a environ 7 ans
- Lié à Development #13521: wcs : gérer la transition des API passant d'un retour de type liste à un retour de type {"data": []} ajouté
Mis à jour par Frédéric Péters il y a environ 7 ans
À rebaser, + préfixer le message de commit par "api: ".
Dans la documentation il y a aussi un exemple de retour de /api/user/drafts, à actualiser également.
Mis à jour par Benjamin Dauvergne il y a environ 7 ans
- Fichier 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch ajouté
Rebasé, préfixe "api: " ajouté au message de commit et exemple pour /api/user/drafts modifié. (j'ai aussi modifié l'adresse qui était /myspace/drafts dans l'exemple, par /api/user/drafts.
Mis à jour par Thomas Noël il y a plus de 6 ans
- Lié à Bug #18748: trace d'erreurs 403 quand l'utilisateur vient d'être crée, le temps du provisionning ajouté
Mis à jour par Thomas Noël il y a plus de 6 ans
- Fichier 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch ajouté
- Statut changé de Nouveau à En cours
Rebasé, parce que je ne vois pas d'autre moyen d'éviter les alertes "403" de combo sur un usager tout neuf.
Mis à jour par Benjamin Dauvergne il y a plus de 6 ans
Et qui c'est qui va acker si tu modifies mon patch ? :) C'est malin faut qu'on trouve une troisième personne maintenant.
Mis à jour par Thomas Noël il y a plus de 6 ans
:-D
On peut considérer qu'il est acké, allez... mais on va de toute façon attendre la MEP de jeudi 9 avant de pusher, donc plutôt attendre mardi 14 (et d'ici là, si un liégeois de passage...).
Mis à jour par Frédéric Péters il y a plus de 6 ans
J'ai regardé ce qui était utilisé comme API à Vincennes et user/forms n'en fait pas partie; c'est donc uniquement utilisé par Combo. Ma préférence du coup c'est de simplement modifier l'API dans w.c.s. après avoir modifié combo pour gérer correctement la présente et la nouvelle API (#13521).
Je me trouvais donc à faire ce travail mais c'était "il y a environ un an". Avant de reprendre ici, il me semble nécessaire de refaire un tour exhaustif de nos installations.
Mis à jour par Frédéric Péters il y a plus de 6 ans
- Lié à Development #19853: Provisionning "full" ajouté
Mis à jour par Benjamin Dauvergne il y a plus de 6 ans
- Lié à Development #20383: adapter wcs_api pour le changement de format retour de /api/formdefs/ ajouté
Mis à jour par Thomas Noël il y a plus de 6 ans
Avec le patch proposé par Benjamin, on ne change pas tout :
Les listes qui passent en {"err": 0, "data": ...} :- /api/formdefs/ et /api/category/<slug>/formdefs
- /api/user/forms
- /api/user/drafts/
- /api/forms/<formdef>/list
Donc je proposer de migrer ça aussi, afin que ceux qui jouent avec les API n'aient pas à deviner où ça a changé : quand c'était une liste, c'est maintenant une err/data. Ça simplifiera le discours à tenir pour informer du changement.
(avis bienvenus)
Mis à jour par Frédéric Péters il y a plus de 6 ans
(avis bienvenus)
Déjà arriver à faire évoluer Vincennes pour gérer les deux; et puis pour /api/forms/<formdef>/list il y aura aussi Liège qui utilise cette API.
Mis à jour par Benjamin Dauvergne il y a plus de 6 ans
Le versionning c'était pas si mal finalement :)
Mis à jour par Thomas Noël il y a environ 6 ans
- Fichier 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch ajouté
Patch rebasé et reconstruit suite à #20230
Mis à jour par Thomas Noël il y a environ 6 ans
- Fichier 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch ajouté
Et rerebasé suite à #21166
Mis à jour par Frédéric Péters il y a environ 6 ans
- Fichier 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch 0001-api-change-schema-of-formdefs-user-forms-and-user-dr.patch ajouté
Les tests font disparaitre un test sur form_receipt_datetime :
- assert len(resp.json) == 1 - assert resp.json[0]['form_name'] == 'test' - assert resp.json[0]['form_slug'] == 'test' - assert resp.json[0]['form_status'] == 'New' - assert datetime.datetime.strptime(resp.json[0]['form_receipt_datetime'], - '%Y-%m-%dT%H:%M:%S') - assert resp.json[0]['keywords'] == ['hello', 'world'] + assert resp.json['err'] == 0 + assert len(resp.json['data']) == 1 + assert resp.json['data'][0]['form_name'] == 'test' + assert resp.json['data'][0]['form_slug'] == 'test' + assert resp.json['data'][0]['form_status'] == 'New' + assert resp.json['data'][0]['keywords'] == ['hello', 'world']
Je l'ai remis.
Il y a aussi quelques appels qui se font avec ?¶ms=... dans l'URL, je les ai doublé en ?params=... pour garder la forme qui sera communément utilisée par les appelants.
Mis à jour par Thomas Noël il y a environ 6 ans
- Statut changé de En cours à Résolu (à déployer)
Parfait, merci ; j'ai poussé ainsi.
commit 5cc3847843e9106cc343a6ed1b2ee98e26b9fe12 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Sat Nov 4 03:01:13 2017 +0100 api: change schema of formdefs, user/forms and user/drafts APIs (#13184) All responses now have the form {"err": x, "data": y}.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
api: change schema of formdefs, user/forms and user/drafts APIs (#13184)
All responses now have the form {"err": x, "data": y}.