Project

General

Profile

Development #39650

connecteurs SMS, option pour refuser les numéros surchargés

Added by Frédéric Péters 12 months ago. Updated 2 months ago.

Status:
Solution proposée
Priority:
Normal
Assignee:
Target version:
-
Start date:
07 Feb 2020
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Plutôt qu'avoir à faire des validations côté client (ex: #39478) avoir une option, activée par défaut, qui fasse qu'un connecteur de diffusion SMS refuse les numéros particuliers.


Files

0002-sms-authorize-numbers-using-masks-39650.patch (9.09 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 07 May 2020 03:12 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (15.3 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 07 May 2020 03:12 PM
0002-sms-authorize-numbers-using-masks-39650.patch (9.49 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 19 May 2020 07:52 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (21.2 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 19 May 2020 07:52 PM
0002-sms-authorize-numbers-using-masks-39650.patch (9.51 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 29 May 2020 12:35 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (17.7 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 29 May 2020 12:35 PM
0003-adaptation-de-43128.patch (2.24 KB) 0003-adaptation-de-43128.patch Nicolas Roche, 29 May 2020 04:28 PM
0002-sms-authorize-numbers-using-masks-39650.patch (9.51 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 29 May 2020 06:38 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (17.5 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 29 May 2020 06:38 PM
0002-sms-authorize-numbers-using-masks-39650.patch (9.51 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 31 May 2020 02:18 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (17 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 31 May 2020 02:18 PM
0002-sms-authorize-numbers-using-masks-39650.patch (9.51 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 20 Jul 2020 10:24 AM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (18.5 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 20 Jul 2020 10:24 AM
0002-sms-authorize-numbers-using-masks-39650.patch (8.96 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 21 Sep 2020 12:05 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (18.9 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 21 Sep 2020 12:05 PM
0001-sms-add-number-authorization-masks-to-SMSResource-39.patch (19.4 KB) 0001-sms-add-number-authorization-masks-to-SMSResource-39.patch Nicolas Roche, 16 Nov 2020 10:11 AM
0002-sms-authorize-numbers-using-masks-39650.patch (9.12 KB) 0002-sms-authorize-numbers-using-masks-39650.patch Nicolas Roche, 16 Nov 2020 10:11 AM

Related issues

Related to w.c.s. - Development #39760: Pouvoir tester avec filtre Django dans une condition qu'un numéro de téléphone est un numéro de mobile Français.Rejeté11 Feb 2020

Actions
Related to Passerelle - Development #42426: Déplacer le code générique des connecteurs SMSSolution déployée04 May 2020

Actions
Related to Passerelle - Development #42427: Déplacer le code clean_number utilisés par les connecteurs SMS dans SMSResourceSolution déployée04 May 2020

Actions
Related to Passerelle - Development #42627: sms: ajouter une classe SMSConnectorFormRejeté07 May 2020

Actions

History

#1

Updated by Frédéric Péters 12 months ago

  • Related to Development #39760: Pouvoir tester avec filtre Django dans une condition qu'un numéro de téléphone est un numéro de mobile Français. added
#2

Updated by Frédéric Péters 12 months ago

+ parallèlement option pour interdire les numéros étranger.

#3

Updated by Thomas Noël 12 months ago

On pourrait partir vers des "masques d'autorisation", avec un format qui ne soit pas compliqué (pas de regex), dans les IPBX tels qu'Asterisk c'est ainsi :

Pour des numéros français commençant par 06 et avec 10 chiffres (le X est un seul chiffre) :

0[6-7]XXXXXXXX
0033[6-7]XXXXXXXX
+33[6-7]XXXXXXXX

Pour la belgique ou des pays où on ne connait pas la longueur du numéro, c'est le "." qui veut dire « n'importe quoi après »

0032.

Bref, on aurait dans le connecteur un textarea avec une ligne par masque. C'est un peu moche mais super facile à comprendre/lire, je trouverais ça mieux qu'une option un peu magique qui ne sera jamais parfaitement complète.

(A noter que certains opérateurs ont de vrais outils de vérification disponible, comme OVH et son HLR, mais ça serait l'objet d'un autre ticket spécifique, si un jour)

#4

Updated by Valentin Deniaud 11 months ago

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

une option, activée par défaut, qui fasse qu'un connecteur de diffusion SMS refuse les numéros particuliers
+ parallèlement option pour interdire les numéros étranger.

La définition de numéros particuliers varie de pays en pays, donc la première option implique la deuxième, non ? Plus que ça, il faut interdire des numéros autres que français (pas simplement étrangers). C'est un peu dommage d'avoir une case qui ne marche que sous de telles contraintes.

Thomas Noël a écrit :

On pourrait partir vers des "masques d'autorisation"

Je ne suis pas convaincu de l'intérêt de cette idée, et ça ne permet pas d'avoir quelque chose d'activé par défaut.
La notation XXXXXXXX me paraît être un nid à bug, mais les goûts et les couleurs...
Et tout comme une option magique, ça ne sera jamais parfaitement complet (ou alors ça devient des regex). En fait je crois que je préfèrerais les regex, sur des chaînes numériques c'est quand même assez lisible, et les 3 expressions copy/paste ready couvrant 99% des besoins seront dans la doc. Et pas de format à inventer et de parser à écrire/maintenir/faire évoluer au gré des nouveaux cas d'usage.

#5

Updated by Frédéric Péters 11 months ago

La définition de numéros particuliers varie de pays en pays.

Oui mais par fournisseur de SMS ce serait toujours la même, je me disais, peut-être. i.e. si je configure un connecteur Orange Business il considérera les numéros non-français comme étrangers. (en vrai je me dis qu'un abonnement OVH peut varier de pays en pays donc mon commentaire est peut-être à ignorer).

Et tout comme une option magique, ça ne sera jamais parfaitement complet (ou alors ça devient des regex). En fait je crois que je préfèrerais les regex, sur des chaînes numériques c'est quand même assez lisible, et les 3 expressions copy/paste ready couvrant 99% des besoins seront dans la doc. Et pas de format à inventer et de parser à écrire/maintenir/faire évoluer au gré des nouveaux cas d'usage.

Ok regex dans le code mais côté UI ça m'irait d'avoir un <select> avec les presets, plutôt qu'avoir à trouver d'où les copier/coller.

#6

Updated by Nicolas Roche 9 months ago

  • Assignee set to Nicolas Roche
#7

Updated by Frédéric Péters 9 months ago

spoiler alert : taper ces trucs spécifiques SMS dans passerelle/sms/whatever.py ou passerelle/utils/sms.py, ou autre, mais pas au milieu du reste.

#8

Updated by Nicolas Roche 9 months ago

  • Status changed from Nouveau to En cours

Ne pas toucher à forms.py ?

class GenericConnectorForm(forms.ModelForm):
     def clean_authorization_masks(self):

Ni à base/models.py ?
class SMSResource(BaseResource):
... 
     def get_authorization_masks_display(self):

ou simplement déplacer le corps des 2 fonctions ?

#9

Updated by Frédéric Péters 9 months ago

Generic Whatever ne doit pas contenir de trucs parlant de SMS.

SMSResource aurait aussi du se trouver ailleurs mais j'ai raté ce coche au bon moment; il est cependant encore temps de le déplacer.

#10

Updated by Frédéric Péters 9 months ago

Mais aussi, ce truc de masque, je trouve quand même ça bof et me cite :

Ok regex dans le code mais côté UI ça m'irait d'avoir un <select> avec les presets, plutôt qu'avoir à trouver d'où les copier/coller.

Et à développer sur base de l'UI, j'en serais à :

Pays autorisés
 [x] France métrop (+33)  [ ] France DOM/TOM (+262, etc.)  [ ] Belgique (+32)   [ ] Tout

Numéros surtaxés
 (x) Ne pas autoriser  ( ) Autoriser
 (attention s'applique uniquement pour la France)

Et de là ça donne dans le code juste quelques définitions des bonnes regex,

authorized = {
 'french': r'0033[67]\d{9}',
 'french-dom-tom': r'...',
 'belgian': r'00324\d{9}',
}
premium_rate = {
 'french': r'0033[89]\d{9}',
 'french-dom-tom': ?,
 'belgian': ?,
}

Et ce sont les numéros passés par clean_numbers qui arrivent ici, donc on connait la structure.

#11

Updated by Benjamin Dauvergne 9 months ago

Sinon y a python(3)-phonenumbers dans la dernière version sur toutes les debian (stretch, buster, etc..) et qui donne ces informations gratuitement, une textarea en moins :

In [23]: pn = phonenumbers.parse('+33630XXXXX', 'FR')

In [24]: phonenumbers.number_type(pn) in [getattr(phonenumbers.PhoneNumberType, name) for name in ['FIXED_LINE', 'MOBILE', 'FIXED_LINE_OR_MOBILE']]
Out[24]: True

In [25]: pn = phonenumbers.parse('0808080808', 'FR')

In [26]: phonenumbers.number_type(pn) in [getattr(phonenumbers.PhoneNumberType, name) for name in ['FIXED_LINE', 'MOBILE', 'FIXED_LINE_OR_MOBILE']]
Out[26]: False
#12

Updated by Frédéric Péters 9 months ago

une textarea en moins

Il n'y a pas de textarea dans ma proposition mais pointer qu'il n'y a pas besoin d'inventer tout un système de masques, ok, c'est ce que j'écrivais déjà, et ça pouvait passer passer par quelques regex, mais pas de soucis à ce que ça passe par une dépendance externe.

#13

Updated by Benjamin Dauvergne 9 months ago

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

une textarea en moins

Il n'y a pas de textarea dans ma proposition mais pointer qu'il n'y a pas besoin d'inventer tout un système de masques, ok, c'est ce que j'écrivais déjà, et ça pouvait passer passer par quelques regex, mais pas de soucis à ce que ça passe par une dépendance externe.

Désolé je n'avais pas lu tous les échanges, j'ai réagi dès que j'ai senti une implémentation custom de plus.

#14

Updated by Nicolas Roche 9 months ago

#15

Updated by Nicolas Roche 9 months ago

  • Related to Development #42427: Déplacer le code clean_number utilisés par les connecteurs SMS dans SMSResource added
#16

Updated by Nicolas Roche 9 months ago

Vu entre autres : https://fr.m.wikipedia.org/wiki/Num%C3%A9ro_de_t%C3%A9l%C3%A9phone#F
La bibliothèque phonenumbers ne me semble pas requise ici (elle valide juste le code pays et le nombre de chiffres).

France
  • +33 pour la France métropolitaine,
  • Numéro surtaxé : numéros allant de 0800xxxxxx à 0899xxxxxx
Belgique
  • Format international : +32 ZZ CC CC CC ou +32 Z CCC CC CC
  • GSM (international) : de +32 45C CC CC CC à +32 49C CC CC CC
  • Numéro payant a minimum 1 euro/min : 090C/ ?
Les départments d'outre-mer :
  • +262 pour la Réunion, Mayotte et Terres australes et antarctiques françaises (+262 262 CC CC CC)
  • +508 pour Saint-Pierre-et-Miquelon (+508 508 CC CC CC)
  • +590 pour la Guadeloupe, Saint-Barthélemy et Saint-Martin, (+590 590 CC CC CC)
  • +594 pour la Guyane, (+594 594 CC CC CC)
  • +596 pour la Martinique. (+596 596 CC CC CC)
  • +687 Nouvelle-Calédonie
    (Rien trouvé sur les numéros surtaxés et j'ai un doute sur les numéros de portables)
#17

Updated by Nicolas Roche 9 months ago

#20

Updated by Frédéric Péters 8 months ago

Mais non, ici ou ailleurs, il n'est pas question d'importer dans passerelle/base/ quoique ce soit de spécifique aux SMS.

#21

Updated by Frédéric Péters 8 months ago

Mais je pense enfin pouvoir exprimer ton problème :

  • tu dois pouvoir définir des widgets particuliers sur certains champs du formulaire de création/édition du connecteur;
  • il y a dans GenericConnectorMixin quelque chose qui semble aller vers là;
  • mais ça cherche une possibilité de get_form_class sur la classe d'application;
  • où ça prend un truc qui part de rien;
  • alors que ce serait pratique de pouvoir partir de ce qui est créé de base.

Si c'est bien ça, je me dis que ton souhait est dans https://git.entrouvert.org/passerelle.git/commit/?id=5e8b06043e893e9dca90fe02d80fee38570229ca (pas testé du tout)

#22

Updated by Frédéric Péters 8 months ago

De manière alternative et plus courte,

https://git.entrouvert.org/passerelle.git/commit/?id=58769c378afc2f056526ab73d393ea12f3b16f72

~~

Si tout ça correspond bien au problème que tu rencontres, je verrai pour agencer/ajuster ça, peut-être en combinant les deux approches.

#23

Updated by Nicolas Roche 8 months ago

Oui, ça colle. Merci. (#43128 : j'ai rebasé ma branche dessus)

#25

Updated by Frédéric Péters 8 months ago

Branche rebasée (mais toujours avec cette adaptation : https://dev.entrouvert.org/issues/43128#note-2)

Encore ici, jeu de piste, invitation à aller voir un commentaire ailleurs et ticket où je n'arrive pas à comprendre ce que tu essaies de passer comme message.

#26

Updated by Nicolas Roche 8 months ago

Il s'agit de ce troisième patch que je dois ajouter pour utiliser #43128.
Sans ce patch :
  • je dois expliciter exclude = ('slug', 'users') dans le Meta de la classe SMSConnectorForm (et donc je perd le fait que le slug soit présenté uniquement lors de l'ajout des connecteurs sms).
  • je dois ajouter un attribut id à la classe SMSConnector sans quoi __init__ se plaint de son absence lors de la configuration du logger du connecteur.

(Je cherche encore l'explication de ces 2 problèmes.)

#27

Updated by Frédéric Péters 8 months ago

Tu peux dégager ton bricolage ici, valider l'autre ticket, et comme ça il y a une base de travail ?

#28

Updated by Frédéric Péters 8 months ago

À nouveau traduire la demande, ce pourrait être "c'est bien beau de pouvoir définir une classe de formulaire custom mais j'aimerais bien que celle-ci soit différente pour la création ou l'édition".

Mais j'ai l'impression que ma réponse pourrait être différente aussi, de l'ordre de : il suffit d'ajouter un get_manager_form_class() qui se base sur le paramètre exclude pour retourner une classe ou l'autre, et là on pourra se dire que c'est quand même pas très naturel, qu'un paramètre explicite serait mieux, et il pourrait être ajouté.

Ou une autre réponse, qui serait pourquoi pas juste si on en est toujours à ce que j'ai cru deviner (commentaire 21), vouloir définir des widgets particuliers, pourquoi pas juste overrider get_manager_form_class et modifier le widget là-derrière, sur la mode de ce qui est fait pour ClearableFileInput.

#29

Updated by Nicolas Roche 8 months ago

juste overrider get_manager_form_class et modifier le widget là-derrière

Oui, tu a anticipé ma réponse.

#43128 fait que les classes peuvent definir elles même leur form_class via l'ajout d'une méthode get_manager_form_class.

class GenericConnectorMixin(object):
    exclude_fields = ('slug', 'users')
    ...
    def init_stuff(self, request, *args, **kwargs):
        ...
        self.form_class = self.model.get_manager_form_class(exclude=self.exclude_fields)


Ce qui est le cas pour le connecteur pastell :
class Pastell(BaseResource):
...
    @classmethod
    def get_manager_form_class(cls, *args, **kwargs):
        from .forms import PastellForm
        return PastellForm

Je cherchais à définir de la même façon une classe formulaire pour les connecteurs sms.
Mais je souhaite également bénéficier de la factory pour gagner les fonctionnalités de BaseRessource (gestion du slug, pas de champ user d'affiché, ... cf https://dev.entrouvert.org/issues/42627#note-6)
Je peux m'en sortir en redéfinissant la factory dans la méthode get_manager_form_class :

class SMSResource(BaseResource):
    @classmethod
    def get_manager_form_class(cls, *args, **kwargs):
        from .forms import SMSConnectorForm

        form_class = modelform_factory(
            cls,
            form=SMSConnectorForm,
            **kwargs)
        for field in form_class.base_fields.values():
            if isinstance(field.widget, ClearableFileInput):
                field.widget.template_with_initial = ''\
                        '%(initial_text)s: %(initial)s '\
                        '%(clear_template)s<br />%(input_text)s: %(input)s'
        return form_class

#31

Updated by Frédéric Péters 8 months ago

Le fait de devoir moi poser les réponses puis avoir un commentaire qui n'oriente pas la discussion reste compliqué.

Il me semble que tu finis par dire que le seul truc dont tu as besoin, c'est pouvoir passer la classe que tu souhaites dans l'appel à modelform_factory.

Si c'est le cas; est-ce que ça ne peut pas juste être fait en mettant manager_form_base_class = SMSConnectorForm dans la classe, end of story ?

#32

Updated by Nicolas Roche 8 months ago

Non (mais sans encore avoir re-testé), il y a des dépendances circulaires entre le modèle et le formulaire
(et sinon c'est exactement ce que j'essayais de faire jusqu'à présent).

#33

Updated by Frédéric Péters 8 months ago

(et sinon c'est exactement ce que j'essayais de faire jusqu'à présent).

C'est la première fois que tu parles de dépendances circulaires. Là-dessus je note en regardant le patch que ce serait parce que "model = SMSResource" dans ton SMSConnectorForm, ça ne me semble pas nécessaire.

#34

Updated by Nicolas Roche 8 months ago

"model = SMSResource" dans ton SMSConnectorForm, ça ne me semble pas nécessaire.

Oui ça fonctionne sans.

manager_form_base_class = SMSConnectorForm dans la classe, end of story ?

Oui, c'est bon tout fonctionne.

Also available in: Atom PDF