From 315afe5d2d1ecdfd69eb30434d388f23a3e94c7a Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Mon, 19 Sep 2022 08:54:02 +0200
Subject: [PATCH 1/2] custom_user: add phone and phone verification fields
 (#65173)

---
 src/authentic2/admin.py                       | 21 ++++++--
 src/authentic2/app_settings.py                |  2 +-
 .../migrations/0032_auto_20220919_1230.py     | 50 +++++++++++++++++++
 src/authentic2/custom_user/models.py          | 32 ++++++++++--
 src/authentic2/validators.py                  |  6 +++
 tests/api/test_all.py                         | 30 +++++------
 tests/idp_oidc/test_misc.py                   |  4 +-
 tests/test_all.py                             |  2 +
 8 files changed, 122 insertions(+), 25 deletions(-)
 create mode 100644 src/authentic2/custom_user/migrations/0032_auto_20220919_1230.py

diff --git a/src/authentic2/admin.py b/src/authentic2/admin.py
index 792b06def..9e3a9eb4a 100644
--- a/src/authentic2/admin.py
+++ b/src/authentic2/admin.py
@@ -286,7 +286,20 @@ class UserCreationForm(BaseUserForm):
 class AuthenticUserAdmin(UserAdmin):
     fieldsets = (
         (None, {'fields': ('uuid', 'ou', 'password')}),
-        (_('Personal info'), {'fields': ('username', 'first_name', 'last_name', 'email', 'email_verified')}),
+        (
+            _('Personal info'),
+            {
+                'fields': (
+                    'username',
+                    'first_name',
+                    'last_name',
+                    'email',
+                    'email_verified',
+                    'phone',
+                    'phone_verified_on',
+                )
+            },
+        ),
         (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser', 'groups')}),
         (_('Important dates'), {'fields': ('last_login', 'date_joined', 'deactivation')}),
     )
@@ -302,6 +315,8 @@ class AuthenticUserAdmin(UserAdmin):
                     'last_name',
                     'email',
                     'email_verified',
+                    'phone',
+                    'phone_verified_on',
                     'password1',
                     'password2',
                 ),
@@ -310,7 +325,7 @@ class AuthenticUserAdmin(UserAdmin):
     )
     readonly_fields = ('uuid',)
     list_filter = UserAdmin.list_filter + (UserRealmListFilter, ExternalUserListFilter)
-    list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email']
+    list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email', 'phone']
     actions = UserAdmin.actions + ['mark_as_inactive']
 
     def get_fieldsets(self, request, obj=None):
@@ -351,7 +366,7 @@ class AuthenticUserAdmin(UserAdmin):
             qs = models.Attribute.objects.all()
         else:
             qs = models.Attribute.objects.filter(required=True)
-        non_model_fields = {a.name for a in qs} - {'first_name', 'last_name'}
+        non_model_fields = {a.name for a in qs} - {'first_name', 'last_name', 'phone'}
         fields = list(set(fields) - set(non_model_fields))
         kwargs['fields'] = fields
         return super().get_form(request, obj=obj, **kwargs)
diff --git a/src/authentic2/app_settings.py b/src/authentic2/app_settings.py
index 54f6b9e75..fc22288c9 100644
--- a/src/authentic2/app_settings.py
+++ b/src/authentic2/app_settings.py
@@ -298,7 +298,7 @@ default_settings = dict(
         default='3/d', definition='Maximum rate of emails sent to the same email address.'
     ),
     A2_USER_DELETED_KEEP_DATA=Setting(
-        default=['email', 'uuid'], definition='User data to keep after deletion'
+        default=['email', 'uuid', 'phone'], definition='User data to keep after deletion'
     ),
     A2_USER_DELETED_KEEP_DATA_DAYS=Setting(
         default=365, definition='Number of days to keep data on deleted users'
diff --git a/src/authentic2/custom_user/migrations/0032_auto_20220919_1230.py b/src/authentic2/custom_user/migrations/0032_auto_20220919_1230.py
new file mode 100644
index 000000000..e2d2bce9c
--- /dev/null
+++ b/src/authentic2/custom_user/migrations/0032_auto_20220919_1230.py
@@ -0,0 +1,50 @@
+# Generated by Django 2.2.26 on 2022-09-19 10:30
+
+from django.db import migrations, models
+
+import authentic2.validators
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('custom_user', '0031_profile_email'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='deleteduser',
+            name='old_phone',
+            field=models.CharField(blank=True, max_length=64, null=True, verbose_name='Old phone number'),
+        ),
+        migrations.AddField(
+            model_name='user',
+            name='phone',
+            field=models.CharField(
+                blank=True,
+                max_length=64,
+                null=True,
+                validators=[authentic2.validators.PhoneNumberValidator],
+                verbose_name='phone number',
+            ),
+        ),
+        migrations.AddField(
+            model_name='user',
+            name='phone_verified_on',
+            field=models.DateTimeField(
+                blank=True, default=None, null=True, verbose_name='phone verification date'
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name='user',
+            constraint=models.CheckConstraint(
+                check=models.Q(
+                    ('username__isnull', False),
+                    ('email__isnull', False),
+                    ('phone__isnull', False),
+                    _connector='OR',
+                ),
+                name='constraint_at_least_one_identifier',
+            ),
+        ),
+    ]
diff --git a/src/authentic2/custom_user/models.py b/src/authentic2/custom_user/models.py
index baeade85a..a114cd539 100644
--- a/src/authentic2/custom_user/models.py
+++ b/src/authentic2/custom_user/models.py
@@ -23,6 +23,7 @@ import uuid
 from django.core.exceptions import MultipleObjectsReturned, ValidationError
 from django.core.mail import send_mail
 from django.db import models, transaction
+from django.db.models import Q
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -42,7 +43,7 @@ from authentic2.models import Attribute, AttributeValue, Service
 from authentic2.utils import misc as utils_misc
 from authentic2.utils.cache import RequestCache
 from authentic2.utils.models import generate_slug
-from authentic2.validators import email_validator
+from authentic2.validators import PhoneNumberValidator, email_validator
 from django_rbac.models import PermissionMixin
 
 from .managers import UserManager, UserQuerySet
@@ -156,6 +157,15 @@ class User(AbstractBaseUser, PermissionMixin):
     email_verified_date = models.DateTimeField(
         default=None, blank=True, null=True, verbose_name=_('email verified date')
     )
+    phone = models.CharField(
+        _('phone number'), null=True, blank=True, max_length=64, validators=[PhoneNumberValidator]
+    )
+    phone_verified_on = models.DateTimeField(
+        null=True,
+        blank=True,
+        default=None,
+        verbose_name=_('phone verification date'),
+    )
     is_staff = models.BooleanField(
         _('staff status'),
         default=False,
@@ -202,6 +212,12 @@ class User(AbstractBaseUser, PermissionMixin):
         verbose_name = _('user')
         verbose_name_plural = _('users')
         ordering = ('last_name', 'first_name', 'email', 'username')
+        constraints = [
+            models.CheckConstraint(
+                check=Q(username__isnull=False) | Q(email__isnull=False) | Q(phone__isnull=False),
+                name='constraint_at_least_one_identifier',
+            )
+        ]
 
     def get_full_name(self):
         """
@@ -247,11 +263,11 @@ class User(AbstractBaseUser, PermissionMixin):
         return '<User: %s (%s)>' % (human_name, short_id)
 
     def clean(self):
-        if not (self.username or self.email or (self.first_name and self.last_name)):
+        if not (self.username or self.email or self.phone or (self.first_name and self.last_name)):
             raise ValidationError(
                 _(
-                    'An account needs at least one identifier: username, email or a full name (first and last'
-                    ' name).'
+                    'An account needs at least one identifier: username, email, phone numbor or a full name '
+                    '(first and last name).'
                 )
             )
 
@@ -406,6 +422,8 @@ class User(AbstractBaseUser, PermissionMixin):
             deleted_user.old_email = self.email.rsplit('#', 1)[0]
         if 'uuid' in app_settings.A2_USER_DELETED_KEEP_DATA:
             deleted_user.old_uuid = self.uuid
+        if 'phone' in app_settings.A2_USER_DELETED_KEEP_DATA:
+            deleted_user.old_phone = self.phone
 
         # save LDAP account references
         external_ids = self.userexternalid_set.order_by('id')
@@ -454,6 +472,7 @@ class DeletedUser(models.Model):
     old_uuid = models.TextField(verbose_name=_('Old UUID'), null=True, blank=True)
     old_user_id = models.PositiveIntegerField(verbose_name=_('Old user id'), null=True, blank=True)
     old_email = models.EmailField(verbose_name=_('Old email adress'), null=True, blank=True)
+    old_phone = models.CharField(verbose_name=_('Old phone number'), null=True, blank=True, max_length=64)
     old_data = JSONField(verbose_name=_('Old data'), null=True, blank=True)
 
     @classmethod
@@ -464,16 +483,19 @@ class DeletedUser(models.Model):
         cls.objects.filter(deleted__lt=threshold).delete()
 
     def __repr__(self):
-        return 'DeletedUser(old_id=%s, old_uuid=%s…, old_email=%s)' % (
+        return 'DeletedUser(old_id=%s, old_uuid=%s…, old_email=%s, old_phone=%s)' % (
             self.old_user_id or '-',
             (self.old_uuid or '')[:6],
             self.old_email or '-',
+            self.old_phone or '-',
         )
 
     def __str__(self):
         data = ['#%d' % self.old_user_id]
         if self.old_email:
             data.append(self.old_email)
+        if self.old_phone:
+            data.append(self.old_phone)
         return _('deleted user (%s)') % ', '.join(data)
 
     class Meta:
diff --git a/src/authentic2/validators.py b/src/authentic2/validators.py
index 414166e8d..f454f565a 100644
--- a/src/authentic2/validators.py
+++ b/src/authentic2/validators.py
@@ -99,6 +99,12 @@ class EmailValidator:
 email_validator = EmailValidator()
 
 
+class PhoneNumberValidator(RegexValidator):
+    def __init__(self, *args, **kwargs):
+        self.regex = r'^\+?\d{,20}$'
+        super().__init__(*args, **kwargs)
+
+
 class UsernameValidator(RegexValidator):
     def __init__(self, *args, **kwargs):
         self.regex = app_settings.A2_REGISTRATION_FORM_USERNAME_REGEX
diff --git a/tests/api/test_all.py b/tests/api/test_all.py
index ae27a0934..26f9363d3 100644
--- a/tests/api/test_all.py
+++ b/tests/api/test_all.py
@@ -66,6 +66,8 @@ USER_ATTRIBUTES_SET = {
     'modified',
     'email_verified',
     'email_verified_date',
+    'phone',
+    'phone_verified_on',
     'last_account_deletion_alert',
     'deactivation',
     'deactivation_reason',
@@ -2390,59 +2392,59 @@ def test_api_address_autocomplete(app, admin, settings):
 
 def test_phone_normalization_ok(settings, app, admin):
     headers = basic_authorization_header(admin)
-    Attribute.objects.create(name='phone', label='phone', kind='phone_number')
+    Attribute.objects.create(name='extra_phone', label='extra phone', kind='phone_number')
     payload = {
         'username': 'janedoe',
-        'phone': ' + 334-99 98.56/43',
+        'extra_phone': ' + 334-99 98.56/43',
         'first_name': 'Jane',
         'last_name': 'Doe',
     }
     resp = app.post_json('/api/users/', headers=headers, params=payload, status=201)
-    assert resp.json['phone'] == '+33499985643'
-    assert User.objects.get(username='janedoe').attributes.phone == '+33499985643'
+    assert resp.json['extra_phone'] == '+33499985643'
+    assert User.objects.get(username='janedoe').attributes.extra_phone == '+33499985643'
 
 
 def test_phone_normalization_nok(settings, app, admin):
     headers = basic_authorization_header(admin)
-    Attribute.objects.create(name='phone', label='phone', kind='phone_number')
+    Attribute.objects.create(name='extra_phone', label='extra phone', kind='phone_number')
     payload = {
         'username': 'janedoe',
         'first_name': 'Jane',
         'last_name': 'Doe',
     }
-    payload['phone'] = (' + 334-99+98.56/43',)
+    payload['extra_phone'] = (' + 334-99+98.56/43',)
     app.post_json('/api/users/', headers=headers, params=payload, status=400)
 
-    payload['phone'] = '1#2'
+    payload['extra_phone'] = '1#2'
     app.post_json('/api/users/', headers=headers, params=payload, status=400)
 
 
 def test_fr_phone_normalization_ok(settings, app, admin):
     headers = basic_authorization_header(admin)
-    Attribute.objects.create(name='phone', label='phone', kind='fr_phone_number')
+    Attribute.objects.create(name='extra_phone', label='extra phone', kind='fr_phone_number')
     payload = {
         'username': 'janedoe',
-        'phone': ' 04-99 98.56/43',
+        'extra_phone': ' 04-99 98.56/43',
         'first_name': 'Jane',
         'last_name': 'Doe',
     }
     resp = app.post_json('/api/users/', headers=headers, params=payload, status=201)
-    assert resp.json['phone'] == '0499985643'
-    assert User.objects.get(username='janedoe').attributes.phone == '0499985643'
+    assert resp.json['extra_phone'] == '0499985643'
+    assert User.objects.get(username='janedoe').attributes.extra_phone == '0499985643'
 
 
 def test_fr_phone_normalization_nok(settings, app, admin):
     headers = basic_authorization_header(admin)
-    Attribute.objects.create(name='phone', label='phone', kind='fr_phone_number')
+    Attribute.objects.create(name='extra_phone', label='extra phone', kind='fr_phone_number')
     payload = {
         'username': 'janedoe',
-        'phone': '+33499985643',
+        'extra_phone': '+33499985643',
         'first_name': 'Jane',
         'last_name': 'Doe',
     }
     app.post_json('/api/users/', headers=headers, params=payload, status=400)
 
-    payload['phone'] = '1#2'
+    payload['extra_phone'] = '1#2'
     app.post_json('/api/users/', headers=headers, params=payload, status=400)
 
 
diff --git a/tests/idp_oidc/test_misc.py b/tests/idp_oidc/test_misc.py
index 8e36b1a84..61e11b00e 100644
--- a/tests/idp_oidc/test_misc.py
+++ b/tests/idp_oidc/test_misc.py
@@ -1167,7 +1167,7 @@ def test_claim_default_value(oidc_settings, normal_oidc_client, simple_user, app
     assert claims['given_name'] == simple_user.first_name
     assert claims['family_name'] == simple_user.last_name
     assert claims['email'] == simple_user.email
-    assert claims['phone'] is None
+    assert claims['phone'] == simple_user.phone
     assert claims['email_verified'] is False
 
     assert user_info['sub'] == make_sub(oidc_client, simple_user)
@@ -1175,7 +1175,7 @@ def test_claim_default_value(oidc_settings, normal_oidc_client, simple_user, app
     assert user_info['given_name'] == simple_user.first_name
     assert user_info['family_name'] == simple_user.last_name
     assert user_info['email'] == simple_user.email
-    assert user_info['phone'] is None
+    assert user_info['phone'] == simple_user.phone
     assert user_info['email_verified'] is False
 
     params['scope'] = 'openid email'
diff --git a/tests/test_all.py b/tests/test_all.py
index 0fe41bd6d..0430e3b3c 100644
--- a/tests/test_all.py
+++ b/tests/test_all.py
@@ -73,6 +73,8 @@ class SerializerTests(TestCase):
                     'email_verified_date': None,
                     'username': 'john.doe',
                     'email': '',
+                    'phone': None,
+                    'phone_verified_on': None,
                     'first_name': '',
                     'last_name': '',
                     'is_active': True,
-- 
2.37.2

