src/authentic2/admin.py

@@ -287,7 +287,6 @@ class AuthenticUserAdmin(UserAdmin):
                     'last_name',
                     'email',
                     'email_verified',
-                    'phone',
                     'phone_verified_on',
                 )
             },
@@ -307,7 +306,6 @@ class AuthenticUserAdmin(UserAdmin):
                     'last_name',
                     'email',
                     'email_verified',
-                    'phone',
                     'phone_verified_on',
                     'password1',
                     'password2',
@@ -317,7 +315,7 @@ class AuthenticUserAdmin(UserAdmin):
     )
     readonly_fields = ('uuid',)
     list_filter = UserAdmin.list_filter + (UserRealmListFilter, ExternalUserListFilter)
-    list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email', 'phone']
+    list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email']
     actions = UserAdmin.actions + ['mark_as_inactive']
 
     def get_fieldsets(self, request, obj=None):
@@ -358,7 +356,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', 'phone'}
+        non_model_fields = {a.name for a in qs} - {'first_name', 'last_name'}
         fields = list(set(fields) - set(non_model_fields))
         kwargs['fields'] = fields
         return super().get_form(request, obj=obj, **kwargs)

src/authentic2/api_views.py

@@ -554,7 +554,7 @@ class BaseUserSerializer(serializers.ModelSerializer):
                 'required': False,
             }
         }
-        exclude = ('phone', 'user_permissions', 'groups', 'keepalive')
+        exclude = ('user_permissions', 'groups', 'keepalive')
 
 
 class DuplicateUserSerializer(BaseUserSerializer):

src/authentic2/custom_user/migrations/0037_migrate_phone_to_profile_attribute.py

@@ -0,0 +1,50 @@
+import phonenumbers
+from django.conf import settings
+from django.db import migrations
+
+
+def migrate_phone_to_profile_attribute(apps, schema_editor):
+    User = apps.get_model('custom_user', 'User')
+    Attribute = apps.get_model('authentic2', 'Attribute')
+    AttributeValue = apps.get_model('authentic2', 'AttributeValue')
+    ContentType = apps.get_model('contenttypes', 'ContentType')
+
+    if not Attribute.all_objects.filter(name='phone'):
+        return
+
+    phone = Attribute.all_objects.get(name='phone')
+    user_ct = ContentType.objects.get_for_model(User)
+    default_country = settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE]['region']
+
+    for user in User.objects.filter(phone__isnull=False):
+        atv, created = AttributeValue.all_objects.get_or_create(
+            content_type=user_ct,
+            object_id=user.id,
+            attribute=phone,
+        )
+        if created or not atv.content:
+            try:
+                phone_number = phonenumbers.parse(user.phone)
+            except phonenumbers.NumberParseException:
+                try:
+                    phone_number = phonenumbers.parse(user.phone, default_country)
+                except phonenumbers.NumberParseException:
+                    phone_number = user.phone
+
+            if isinstance(phone_number, phonenumbers.PhoneNumber) and phonenumbers.is_valid_number(
+                phone_number
+            ):
+                phone_number = phonenumbers.format_number(phone_number, phonenumbers.PhoneNumberFormat.E164)
+
+            atv.content = phone_number
+            atv.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('custom_user', '0036_remove_user_constraint_at_least_one_identifier'),
+    ]
+
+    operations = [
+        migrations.RunPython(migrate_phone_to_profile_attribute, migrations.RunPython.noop),
+    ]

src/authentic2/custom_user/migrations/0038_remove_user_phone.py

@@ -0,0 +1,16 @@
+# Generated by Django 3.2.18 on 2023-09-12 12:06
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('custom_user', '0037_migrate_phone_to_profile_attribute'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='user',
+            name='phone',
+        ),
+    ]

src/authentic2/custom_user/models.py

@@ -54,7 +54,7 @@ from authentic2.utils import misc as utils_misc
 from authentic2.utils.cache import RequestCache
 from authentic2.utils.misc import get_password_authenticator
 from authentic2.utils.models import generate_slug
-from authentic2.validators import PhoneNumberValidator, email_validator
+from authentic2.validators import email_validator
 
 from .backends import DjangoRBACBackend
 from .managers import UserManager, UserQuerySet
@@ -192,9 +192,6 @@ class User(AbstractBaseUser):
         default=False,
         help_text=_('Designates that this user has all permissions without explicitly assigning them.'),
     )
-    phone = models.CharField(
-        _('phone number'), null=True, blank=True, max_length=64, validators=[PhoneNumberValidator]
-    )
     phone_verified_on = models.DateTimeField(
         null=True,
         blank=True,
@@ -405,7 +402,7 @@ class User(AbstractBaseUser):
         return '<User: %s (%s)>' % (human_name, short_id)
 
     def clean(self):
-        if not (self.username or self.email or self.phone or (self.first_name and self.last_name)):
+        if not (self.username or self.email or (self.first_name and self.last_name)):
             raise ValidationError(
                 _(
                     'An account needs at least one identifier: username, email, phone numbor or a full name '
@@ -579,8 +576,6 @@ class User(AbstractBaseUser):
             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')

tests/api/test_all.py

@@ -3567,7 +3567,7 @@ def test_api_authn_healthcheck(app, settings, superuser, simple_user):
     app.get('/api/authn-healthcheck/', status=403)
 
 
-def test_api_users_create_no_phone_model_field_writes(settings, app, admin, phone_activated_authn):
+def test_api_users_create_phone_attribute(settings, app, admin, phone_activated_authn):
     payload = {
         'username': 'janedoe',
         'email': 'jane.doe@nowhere.null',
@@ -3579,16 +3579,10 @@ def test_api_users_create_no_phone_model_field_writes(settings, app, admin, phon
     headers = basic_authorization_header(admin)
     resp = app.post_json('/api/users/', headers=headers, params=payload, status=201)
     user = User.objects.get(uuid=resp.json['uuid'])
-    assert not user.phone
     assert user.phone_identifier == '+33122334455'
 
 
-def test_api_users_update_no_phone_model_field_writes(
-    settings, app, admin, simple_user, phone_activated_authn
-):
-    simple_user.phone = None
-    simple_user.save()
-
+def test_api_users_update_phone_attribute(settings, app, admin, simple_user, phone_activated_authn):
     payload = {
         'username': simple_user.username,
         'id': simple_user.id,
@@ -3600,15 +3594,9 @@ def test_api_users_update_no_phone_model_field_writes(
     headers = basic_authorization_header(admin)
     app.put_json(f'/api/users/{simple_user.uuid}/', params=payload, headers=headers, status=200)
     user = User.objects.get(id=simple_user.id)
-    assert not user.phone
     assert user.phone_identifier == '+33122334455'
 
-    # already existing value should be left unchanged
-    simple_user.phone = '+33123456789'
-    simple_user.save()
-
     payload['phone'] = '+33155555555'
     app.put_json(f'/api/users/{simple_user.uuid}/', params=payload, headers=headers, status=200)
     user = User.objects.get(id=simple_user.id)
-    assert user.phone == '+33123456789'
     assert user.phone_identifier == '+33155555555'

tests/conftest.py

@@ -113,7 +113,6 @@ def simple_user(db, ou1):
         last_name='Dôe',
         email='user@example.net',
         ou=get_default_ou(),
-        phone='+33123456789',
     )
 
 
@@ -124,7 +123,6 @@ def nomail_user(db, ou1):
         first_name='Jôhn',
         last_name='Dôe',
         ou=get_default_ou(),
-        phone='+33123456789',
         email='',
         email_verified=False,
     )

tests/idp_oidc/test_misc.py

@@ -1316,7 +1316,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'] == simple_user.phone
+    assert claims['phone'] == simple_user.attributes.phone
     assert claims['email_verified'] is False
 
     assert user_info['sub'] == make_sub(oidc_client, simple_user)
@@ -1324,7 +1324,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'] == simple_user.phone
+    assert user_info['phone'] == simple_user.attributes.phone
     assert user_info['email_verified'] is False
 
     params['scope'] = 'openid email'

tests/test_all.py

@@ -75,7 +75,6 @@ class SerializerTests(TestCase):
                     'email_verified_sources': '[]',  # weird ArrayField serialization behavior
                     'username': 'john.doe',
                     'email': '',
-                    'phone': None,
                     'phone_verified_on': None,
                     'first_name': '',
                     'last_name': '',

tests/test_backends.py

@@ -47,9 +47,9 @@ def test_model_backend_phone_number(settings, db, simple_user, nomail_user, ou1,
     nomail_user.save()
     simple_user.attributes.phone = '+33123456789'
     simple_user.save()
-    assert authenticate(username=simple_user.phone, password=simple_user.username)
+    assert authenticate(username=simple_user.attributes.phone, password=simple_user.username)
     assert is_user_authenticable(simple_user)
-    assert authenticate(username=nomail_user.phone, password=nomail_user.username)
+    assert authenticate(username=nomail_user.attributes.phone, password=nomail_user.username)
     assert is_user_authenticable(nomail_user)
 
 
@@ -92,5 +92,5 @@ def test_model_backend_phone_number_email(settings, db, simple_user, phone_activ
     simple_user.save()
     # user with both phone number and username can authenticate in two different ways
     assert authenticate(username=simple_user.username, password=simple_user.username)
-    assert authenticate(username=simple_user.phone, password=simple_user.username)
+    assert authenticate(username=simple_user.attributes.phone, password=simple_user.username)
     assert is_user_authenticable(simple_user)

tests/test_migrations.py

@@ -60,7 +60,7 @@ def test_migration_custom_user_0028_user_email_verified_date(transactional_db, m
     assert user.email_verified_date == user.date_joined
 
 
-def test_migration_custom_user_0047_initialize_services_runtime_settings(transactional_db, migration):
+def test_migration_authentic2_0047_initialize_services_runtime_settings(transactional_db, migration):
     old_apps = migration.before([('authentic2', '0046_runtimesetting')])
 
     Setting = old_apps.get_model('authentic2', 'Setting')
@@ -72,3 +72,54 @@ def test_migration_custom_user_0047_initialize_services_runtime_settings(transac
     assert Setting.objects.filter(key__startswith='sso:').count() == 4
     for setting in Setting.objects.filter(key__startswith='sso:'):
         assert setting.value == ''
+
+
+def test_migration_custom_user_0037_migrate_phone_to_profile_attribute(transactional_db, migration):
+    old_apps = migration.before([('custom_user', '0036_remove_user_constraint_at_least_one_identifier')])
+
+    User = old_apps.get_model('custom_user', 'User')
+    Attribute = old_apps.get_model('authentic2', 'Attribute')
+    AttributeValue = old_apps.get_model('authentic2', 'AttributeValue')
+    ContentType = old_apps.get_model('contenttypes', 'ContentType')
+
+    phone = Attribute.all_objects.create(name='phone', label='Phone', kind='phone_number')
+    user_ct = ContentType.objects.get_for_model(User)
+
+    # correct international number
+    User.objects.create(email='john.doe@example.com', phone='+33122334455')
+    # erroneous value
+    User.objects.create(
+        email='john.doe2@example.com', phone='foo'
+    )  # suspicious source value will be kept when no target is know
+    # correct local number
+    User.objects.create(email='john.doe3@example.com', phone='0133334455')
+
+    assert not AttributeValue.all_objects.filter(attribute=phone, content__isnull=False)
+
+    # correct international number with existing profile attribute value
+    user4 = User.objects.create(email='john.doe4@example.com', phone='+33122334455')
+    AttributeValue.all_objects.create(
+        object_id=user4.id,
+        content_type=user_ct,
+        attribute=phone,
+        content='abc',  # suspicious pre-existing target value will still be kept
+    )
+
+    new_apps = migration.apply([('custom_user', '0037_migrate_phone_to_profile_attribute')])
+    User = new_apps.get_model('custom_user', 'User')
+    Attribute = new_apps.get_model('authentic2', 'Attribute')
+    AttributeValue = new_apps.get_model('authentic2', 'AttributeValue')
+
+    phone = Attribute.all_objects.get(name='phone')
+
+    user = User.objects.get(email='john.doe@example.com')
+    assert AttributeValue.all_objects.get(attribute=phone, object_id=user.id).content == '+33122334455'
+
+    user2 = User.objects.get(email='john.doe2@example.com')
+    assert AttributeValue.all_objects.get(attribute=phone, object_id=user2.id).content == 'foo'
+
+    user3 = User.objects.get(email='john.doe3@example.com')
+    assert AttributeValue.all_objects.get(attribute=phone, object_id=user3.id).content == '+33133334455'
+
+    user4 = User.objects.get(email='john.doe4@example.com')
+    assert AttributeValue.all_objects.get(attribute=phone, object_id=user4.id).content == 'abc'

tests/test_password_reset.py

@@ -64,7 +64,7 @@ def test_send_password_reset_email(app, simple_user, mailoutbox):
 def test_send_password_reset_by_sms_code_improperly_configured(
     app, nomail_user, settings, phone_activated_authn
 ):
-    nomail_user.attributes.phone = nomail_user.phone
+    nomail_user.attributes.phone = '+33123456789'
     nomail_user.save()
     settings.SMS_URL = 'https://foo.whatever.none/'
 

tests/test_registration.py

@@ -1162,7 +1162,6 @@ def test_phone_registration_nondefault_attribute(app, db, settings):
     assert user.attributes.another_phone == '+33612345678'
     # no standard attribute set nor even created
     assert not getattr(user.attributes, 'phone', None)
-    assert not user.phone
 
 
 def test_phone_registration_redirect_url(app, db, settings, phone_activated_authn):

tests/test_user_model.py

@@ -188,14 +188,12 @@ def test_save_does_not_reset_verified_attributes_phone(db):
     user.verified_attributes.phone = '+33123456789'
 
     user = User.objects.get()
-    assert user.phone is None  # deprecated model field
     assert user.is_verified.phone
     assert user.verified_attributes.phone == '+33123456789'
     assert user.attributes.phone == '+33123456789'
     user.save()
 
     user = User.objects.get()
-    assert user.phone is None  # deprecated model field
     assert user.is_verified.phone
     assert user.verified_attributes.phone == '+33123456789'
     assert user.attributes.phone == '+33123456789'

tests/test_views.py

@@ -60,6 +60,14 @@ def test_profile(app, simple_user):
 def test_phone_number_change_invalid_number(settings, app, simple_user):
     settings.A2_PROFILE_FIELDS = ('phone', 'mobile')
 
+    Attribute.objects.create(
+        kind='phone_number',
+        name='phone',
+        label='Phone',
+        user_visible=True,
+        user_editable=True,
+    )
+
     Attribute.objects.create(
         kind='phone_number',
         name='mobile',