PB-1106: Use cognito's preferred username

Pipfile

@@ -13,6 +13,7 @@ pyyaml = "~=6.0.2"
 gevent = "~=24.2"
 logging-utilities = "~=4.4.1"
 boto3 = "~=1.35"
+nanoid = "*"
 
 [dev-packages]
 yapf = "*"
@@ -34,6 +35,7 @@ debugpy = "*"
 boto3-stubs = {extras = ["cognito-idp"], version = "~=1.35"}
 bandit = "*"
 pytest-xdist = "*"
+types-nanoid = "*"
 
 [requires]
 python_version = "3.12"

app/access/admin.py

@@ -8,7 +8,7 @@
 @admin.register(User)
 class UserAdmin(admin.ModelAdmin):  # type:ignore[type-arg]
     '''Admin View for User'''
-    list_display = ('provider', 'username', 'deleted_at')
+    list_display = ('provider', 'user_id', 'username', 'deleted_at')
     actions = ["disable"]
 
     @admin.action(description="Disable selected users")

app/access/migrations/0003_user_user_id.py

@@ -0,0 +1,36 @@
+# Generated by Django 5.0.9 on 2024-11-20 11:44
+
+from utils.short_id import generate_short_id
+
+from django.apps.registry import Apps
+from django.db import migrations
+from django.db import models
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+
+
+def populate_user_id(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    User = apps.get_model('access', 'User')
+    for obj in User.objects.all():
+        obj.user_id = generate_short_id()
+        obj.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('access', '0002_user_deleted_at'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='user',
+            name='user_id',
+            field=models.CharField(default=generate_short_id, null=True, verbose_name='User ID'),
+        ),
+        migrations.RunPython(populate_user_id, migrations.RunPython.noop),
+        migrations.AlterField(
+            model_name='user',
+            name='user_id',
+            field=models.CharField(default=generate_short_id, unique=True, verbose_name='User ID'),
+        ),
+    ]

app/access/models.py

@@ -2,6 +2,7 @@
 from typing import Iterable
 
 from cognito.utils.client import Client
+from utils.short_id import generate_short_id
 
 from django.db import models
 from django.db import transaction
@@ -34,6 +35,7 @@ def __str__(self) -> str:
         return f'{self.first_name} {self.last_name}'
 
     username = models.CharField(_(_context, "User name"), unique=True)
+    user_id = models.CharField(_(_context, "User ID"), unique=True, default=generate_short_id)
     first_name = models.CharField(_(_context, "First name"))
     last_name = models.CharField(_(_context, "Last name"))
     email = models.EmailField(_(_context, "Email"))
@@ -63,14 +65,14 @@ def save(
         with transaction.atomic():
             if self._state.adding:
                 super().save(force_insert=True, using=using, update_fields=update_fields)
-                if not client.create_user(self.username, self.email):
-                    logger.critical("User %s already exists in cognito, not created", self.username)
+                if not client.create_user(self.user_id, self.username, self.email):
+                    logger.critical("User %s already exists in cognito, not created", self.user_id)
                     raise CognitoInconsistencyError()
             else:
                 User.objects.select_for_update().filter(pk=self.pk).get()
                 super().save(force_update=True, using=using, update_fields=update_fields)
-                if not client.update_user(self.username, self.email):
-                    logger.critical("User %s does not exist in cognito, not updated", self.username)
+                if not client.update_user(self.user_id, self.username, self.email):
+                    logger.critical("User %s does not exist in cognito, not updated", self.user_id)
                     raise CognitoInconsistencyError()
 
     def delete(self,
@@ -82,8 +84,8 @@ def delete(self,
         with transaction.atomic():
             User.objects.select_for_update().filter(pk=self.pk).get()
             result = super().delete(using=using, keep_parents=keep_parents)
-            if not client.delete_user(self.username):
-                logger.critical("User %s does not exist in cognito, not deleted", self.username)
+            if not client.delete_user(self.user_id):
+                logger.critical("User %s does not exist in cognito, not deleted", self.user_id)
                 raise CognitoInconsistencyError()
             return result
 
@@ -94,6 +96,6 @@ def disable(self) -> None:
             # use django.utils.timezone over datetime to use timezone aware objects.
             self.deleted_at = timezone.now()
             super().save(force_update=True)
-            if not client.disable_user(self.username):
-                logger.critical("User %s does not exist in cognito, not disabled", self.username)
+            if not client.disable_user(self.user_id):
+                logger.critical("User %s does not exist in cognito, not disabled", self.user_id)
                 raise CognitoInconsistencyError()

app/cognito/management/commands/cognito_sync.py

@@ -27,87 +27,93 @@ def clear_users(self) -> None:
 
         for user in self.client.list_users():
             self.counts['deleted'] += 1
-            username = user['Username']
-            self.print(f'deleting user {username}')
+            user_id = user['Username']
+            self.print(f'deleting user {user_id}')
             if not self.dry_run:
-                deleted = self.client.delete_user(username)
+                deleted = self.client.delete_user(user_id)
                 if not deleted:
                     self.print_error(
-                        'Could not delete %s, might not exist or might be unmanged', username
+                        'Could not delete %s, might not exist or might be unmanged', user_id
                     )
 
     def add_user(self, user: User) -> None:
         """ Add a local user to cognito. """
 
         self.counts['added'] += 1
-        self.print(f'adding user {user.username}')
+        self.print(f'adding user {user.user_id}')
         if not self.dry_run:
-            created = self.client.create_user(user.username, user.email)
+            created = self.client.create_user(user.user_id, user.username, user.email)
             if not created:
                 self.print_error(
-                    'Could not create %s, might already exist as unmanaged user', user.username
+                    'Could not create %s, might already exist as unmanaged user', user.user_id
                 )
 
-    def delete_user(self, username: str) -> None:
+    def delete_user(self, user_id: str) -> None:
         """ Delete a remote user from cognito. """
 
         self.counts['deleted'] += 1
-        self.print(f'deleting user {username}')
+        self.print(f'deleting user {user_id}')
         if not self.dry_run:
-            deleted = self.client.delete_user(username)
+            deleted = self.client.delete_user(user_id)
             if not deleted:
                 self.print_error(
-                    'Could not delete %s, might not exist or might be unmanaged', username
+                    'Could not delete %s, might not exist or might be unmanaged', user_id
                 )
 
     def update_user(self, local_user: User, remote_user: 'UserTypeTypeDef') -> None:
         """ Update a remote user in cognito. """
 
         remote_attributes = user_attributes_to_dict(remote_user['Attributes'])
-        if local_user.email != remote_attributes.get('email'):
+        changed = (
+            local_user.email != remote_attributes.get('email') or
+            local_user.username != remote_attributes.get('preferred_username')
+        )
+        if changed:
             self.counts['updated'] += 1
-            self.print(f'updating user {local_user.username}')
+            self.print(f'updating user {local_user.user_id}')
             if not self.dry_run:
-                updated = self.client.update_user(local_user.username, local_user.email)
+                updated = self.client.update_user(
+                    local_user.user_id, local_user.username, local_user.email
+                )
                 if not updated:
                     self.print_error(
                         'Could not update %s, might not exist or might be unmanaged',
-                        local_user.username
+                        local_user.user_id
                     )
 
         if local_user.is_active != remote_user['Enabled']:
             if local_user.is_active:
                 self.counts['enabled'] += 1
-                self.print(f'enabling user {local_user.username}')
+                self.print(f'enabling user {local_user.user_id}')
                 if not self.dry_run:
-                    enabled = self.client.enable_user(local_user.username)
+                    enabled = self.client.enable_user(local_user.user_id)
                     if not enabled:
-                        self.print_error('Could not enable %s', local_user.username)
+                        self.print_error('Could not enable %s', local_user.user_id)
             else:
                 self.counts['disabled'] += 1
-                self.print(f'disabling user {local_user.username}')
+                self.print(f'disabling user {local_user.user_id}')
                 if not self.dry_run:
-                    disabled = self.client.disable_user(local_user.username)
+                    disabled = self.client.disable_user(local_user.user_id)
                     if not disabled:
-                        self.print_error('Could not disable %s', local_user.username)
+                        self.print_error('Could not disable %s', local_user.user_id)
 
     def sync_users(self) -> None:
         """ Synchronizes local and cognito users. """
 
         # Get all remote and local users
-        local_users = {user.username: user for user in User.all_objects.all()}
-        local_usernames = set(local_users.keys())
+        local_users = {user.user_id: user for user in User.all_objects.all()}
+        local_user_ids = set(local_users.keys())
         remote_users = {user['Username']: user for user in self.client.list_users()}
-        remote_usernames = set(remote_users.keys())
+        remote_user_ids = set(remote_users.keys())
 
-        for username in local_usernames.difference(remote_usernames):
-            self.add_user(local_users[username])
+        for user_id in local_user_ids.difference(remote_user_ids):
+            self.add_user(local_users[user_id])
 
-        for username in remote_usernames.difference(local_usernames):
-            self.delete_user(username)
+        for user_id in remote_user_ids.difference(local_user_ids):
+            self.delete_user(user_id)
 
-        for username in local_usernames.intersection(remote_usernames):
-            self.update_user(local_users[username], remote_users[username])
+        for user_id in local_user_ids.intersection(remote_user_ids):
+            self.update_user(local_users[user_id], remote_users[user_id])
 
     def run(self) -> None:
         """ Main entry point of command. """

app/cognito/utils/client.py

@@ -50,7 +50,7 @@ def get_user(
         username: str,
         return_unmanaged: bool = False
     ) -> 'AdminGetUserResponseTypeDef | None':
-        """ Get the user with the given username.
+        """ Get the user with the given cognito username.
 
         Returns None if the user does not exist or doesn't have the managed flag and
         return_unamanged is False.
@@ -68,7 +68,7 @@ def get_user(
 
         return response
 
-    def create_user(self, username: str, email: str) -> bool:
+    def create_user(self, username: str, preferred_username: str, email: str) -> bool:
         """ Create a new user.
 
         Returns False, if a (managed or unmanaged) user already exist.
@@ -82,6 +82,8 @@ def create_user(self, username: str, email: str) -> bool:
                 Username=username,
                 UserAttributes=[{
                     "Name": "email", "Value": email
+                }, {
+                    "Name": "preferred_username", "Value": preferred_username
                 }, {
                     "Name": self.managed_flag_name, "Value": "true"
                 }],
@@ -91,7 +93,7 @@ def create_user(self, username: str, email: str) -> bool:
         return False
 
     def delete_user(self, username: str) -> bool:
-        """ Delete the given user.
+        """ Delete the user with the given cognito username.
 
         Returns False, if the user does not exist or doesn't have the managed flag.
 
@@ -103,8 +105,8 @@ def delete_user(self, username: str) -> bool:
             return True
         return False
 
-    def update_user(self, username: str, email: str) -> bool:
-        """ Update the given user.
+    def update_user(self, username: str, preferred_username: str, email: str) -> bool:
+        """ Update the user with the given cognito username.
 
         Returns False, if the user does not exist or doesn't have the managed flag.
 
@@ -117,13 +119,15 @@ def update_user(self, username: str, email: str) -> bool:
                 Username=username,
                 UserAttributes=[{
                     "Name": "email", "Value": email
+                }, {
+                    "Name": "preferred_username", "Value": preferred_username
                 }]
             )
             return True
         return False
 
     def enable_user(self, username: str) -> bool:
-        """Enable the given user.
+        """Enable the user with the given cognito username.
 
         Returns False, if the user does not exist, or doesn't have the managed flag.
         """
@@ -135,7 +139,7 @@ def enable_user(self, username: str) -> bool:
         return True
 
     def disable_user(self, username: str) -> bool:
-        """Disable the given user.
+        """Disable the user with the given cognito username.
 
         Returns False if the user does not exist, or doesn't have the managed flag.
         """

app/config/settings_base.py

@@ -152,3 +152,7 @@
 
 # Testing
 TESTING = False
+
+# nanoid
+SHORT_ID_SIZE = env.int('SHORT_ID_SIZE', 12)
+SHORT_ID_ALPHABET = env.str('SHORT_ID_ALPHABET', '0123456789abcdefghijklmnopqrstuvwxyz')

app/tests/access/test_access_models.py

@@ -31,6 +31,7 @@ def test_user_stored_as_expected_for_valid_input(client, provider):
     User.objects.create(**model_fields)
     actual = User.objects.first()
 
+    assert len(actual.user_id) == 12
     assert actual.username == "dude"
     assert actual.first_name == "Jeffrey"
     assert actual.last_name == "Lebowski"
@@ -88,6 +89,7 @@ def test_create_user_raises_cognito_exception(logger, client, provider):
     client.return_value.create_user.return_value = False
 
     model_fields = {
+        "user_id": "2ihg2ox304po",
         "username": "dude",
         "first_name": "Jeffrey",
         "last_name": "Lebowski",
@@ -101,7 +103,7 @@ def test_create_user_raises_cognito_exception(logger, client, provider):
     assert User.objects.count() == 0
     assert client.return_value.create_user.called
     assert call.critical(
-        'User %s already exists in cognito, not created', 'dude'
+        'User %s already exists in cognito, not created', '2ihg2ox304po'
     ) in logger.mock_calls