diff --git a/Pipfile b/Pipfile index 7c9f24e..dff9b5b 100644 --- a/Pipfile +++ b/Pipfile @@ -13,6 +13,7 @@ pyyaml = "~=6.0.2" gevent = "~=24.2" logging-utilities = "~=4.4.1" boto3 = "~=1.35" +nanoid = "~=2.0" [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" diff --git a/Pipfile.lock b/Pipfile.lock index 8f9ee27..bf683c2 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "5a137e6fa1e400507598c20ec4643b7823abd88670ddcdb99084d3e227706446" + "sha256": "4f946281c2a4bdb062633ddc50f68d188608be5652153d122ff4e12c843b935e" }, "pipfile-spec": 6, "requires": { @@ -226,6 +226,14 @@ "markers": "python_version >= '3.0'", "version": "==4.4.1" }, + "nanoid": { + "hashes": [ + "sha256:5a80cad5e9c6e9ae3a41fa2fb34ae189f7cb420b2a5d8f82bd9d23466e4efa68", + "sha256:90aefa650e328cffb0893bbd4c236cfd44c48bc1f2d0b525ecc53c3187b653bb" + ], + "index": "pypi", + "version": "==2.0.0" + }, "packaging": { "hashes": [ "sha256:026ed72c8ed3fcce5bf8950572258698927fd1dbda10a5e981cdf0ac37f4f002", @@ -1182,6 +1190,15 @@ "markers": "python_version >= '3.8'", "version": "==3.1.0.20240924" }, + "types-nanoid": { + "hashes": [ + "sha256:5219f721b80748d82b14edd5429cc888d32eb816966d2a4719730ef3f2e3a417", + "sha256:7246ee685ed55a367fc160d87cde75a550296ecca26e0602619552dae72efda0" + ], + "index": "pypi", + "markers": "python_version >= '3.8'", + "version": "==2.0.0.20240601" + }, "types-psutil": { "hashes": [ "sha256:61f836f8ba48f28f0d290d3bcd902f9130ce5057a1676e6ecbefb6141e2743f4", diff --git a/app/access/admin.py b/app/access/admin.py index 0188962..b18f5da 100644 --- a/app/access/admin.py +++ b/app/access/admin.py @@ -1,5 +1,6 @@ from django.contrib import admin from django.db import models +from django.db.models import Model from django.http import HttpRequest from .models import User @@ -8,7 +9,7 @@ @admin.register(User) class UserAdmin(admin.ModelAdmin): # type:ignore[type-arg] '''Admin View for User''' - list_display = ('username', 'deleted_at', 'provider') + list_display = ('user_id', 'username', 'deleted_at', 'provider') list_filter = ('deleted_at', ('provider', admin.RelatedOnlyFieldListFilter)) actions = ('disable',) @@ -19,3 +20,9 @@ def disable(self, request: HttpRequest, queryset: models.QuerySet[User]) -> None def get_queryset(self, request: HttpRequest) -> models.QuerySet[User]: return User.all_objects.get_queryset() + + def get_readonly_fields(self, request: HttpRequest, obj: Model | None = None) -> list[str]: + # Make user_id readonly when editing, but not when creating + if obj: + return ['user_id'] + return [] diff --git a/app/access/migrations/0003_user_user_id.py b/app/access/migrations/0003_user_user_id.py new file mode 100644 index 0000000..abcac27 --- /dev/null +++ b/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'), + ), + ] diff --git a/app/access/models.py b/app/access/models.py index 080eaa1..8be7576 100644 --- a/app/access/models.py +++ b/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() diff --git a/app/cognito/management/commands/cognito_sync.py b/app/cognito/management/commands/cognito_sync.py index 6f22400..6e38eeb 100644 --- a/app/cognito/management/commands/cognito_sync.py +++ b/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. """ diff --git a/app/cognito/utils/client.py b/app/cognito/utils/client.py index 16ce539..8ce5539 100644 --- a/app/cognito/utils/client.py +++ b/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. """ diff --git a/app/config/settings_base.py b/app/config/settings_base.py index e9ea3a3..d98631d 100644 --- a/app/config/settings_base.py +++ b/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') diff --git a/app/tests/access/test_access_models.py b/app/tests/access/test_access_models.py index 21a80f3..e6c875e 100644 --- a/app/tests/access/test_access_models.py +++ b/app/tests/access/test_access_models.py @@ -38,6 +38,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" @@ -95,6 +96,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", @@ -108,7 +110,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 diff --git a/app/tests/cognito/test_cognito_client.py b/app/tests/cognito/test_cognito_client.py index d948243..986f611 100644 --- a/app/tests/cognito/test_cognito_client.py +++ b/app/tests/cognito/test_cognito_client.py @@ -13,8 +13,8 @@ def test_user_attributes_to_dict(): @patch('cognito.utils.client.client') def test_list_users_returns_only_managed(boto3, cognito_user_response_factory): - managed = cognito_user_response_factory('1234', managed=True) - unmanaged = cognito_user_response_factory('1234', managed=False) + managed = cognito_user_response_factory('2ihg2ox304po', '1234', managed=True) + unmanaged = cognito_user_response_factory('2ihg2ox304po', '1234', managed=False) boto3.return_value.list_users.return_value = {'Users': [managed, unmanaged]} client = Client() @@ -25,7 +25,10 @@ def test_list_users_returns_only_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_list_users_pagination(boto3, cognito_user_response_factory): - users = [cognito_user_response_factory(str(count), True) for count in range(1, 131)] + users = [ + cognito_user_response_factory(f'2ihg2ox304po{count}', str(count), True) + for count in range(1, 131) + ] response_1 = {'Users': users[0:60], 'PaginationToken': '1'} response_2 = {'Users': users[60:120], 'PaginationToken': '2'} response_3 = {'Users': users[120:130]} @@ -34,7 +37,7 @@ def test_list_users_pagination(boto3, cognito_user_response_factory): client = Client() users = client.list_users() - assert [user['Username'] for user in users] == [str(x) for x in range(1, 131)] + assert [user['Username'] for user in users] == [f'2ihg2ox304po{x}' for x in range(1, 131)] assert call().list_users(UserPoolId=client.user_pool_id, Limit=60) in boto3.mock_calls assert call().list_users( UserPoolId=client.user_pool_id, Limit=60, PaginationToken='2' @@ -46,7 +49,9 @@ def test_list_users_pagination(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_get_user_returns_managed(boto3, cognito_user_response_factory): - response = cognito_user_response_factory('1234', managed=True, attributes_key='UserAttributes') + response = cognito_user_response_factory( + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' + ) boto3.return_value.admin_get_user.return_value = response client = Client() @@ -60,7 +65,7 @@ def test_get_user_returns_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_get_user_does_not_return_unmanaged(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() @@ -73,7 +78,9 @@ def test_get_user_does_not_return_unmanaged(boto3, cognito_user_response_factory @patch('cognito.utils.client.client') def test_get_user_returns_unmanaged(boto3, cognito_user_response_factory): - response = cognito_user_response_factory('1234', managed=False, attributes_key='UserAttributes') + response = cognito_user_response_factory( + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' + ) boto3.return_value.admin_get_user.return_value = response client = Client() @@ -89,13 +96,16 @@ def test_create_user_creates_managed(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = None client = Client() - created = client.create_user('1234', 'test@example.org') + created = client.create_user('2ihg2ox304po', '1234', 'test@example.org') assert created is True + assert '().admin_create_user' in [call[0] for call in boto3.mock_calls] assert call().admin_create_user( UserPoolId=client.user_pool_id, - Username='1234', + Username='2ihg2ox304po', UserAttributes=[{ 'Name': 'email', 'Value': 'test@example.org' + }, { + 'Name': 'preferred_username', 'Value': '1234' }, { 'Name': client.managed_flag_name, 'Value': 'true' }], @@ -106,49 +116,31 @@ def test_create_user_creates_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_create_user_does_not_create_if_managed_exists(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=True, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' ) client = Client() - created = client.create_user('1234', 'test@example.org') + created = client.create_user('2ihg2ox304po', '1234', 'test@example.org') assert created is False - assert call().admin_create_user( - UserPoolId=client.user_pool_id, - Username='1234', - UserAttributes=[{ - 'Name': 'email', 'Value': 'test@example.org' - }, { - 'Name': client.managed_flag_name, 'Value': 'true' - }], - DesiredDeliveryMediums=['EMAIL'] - ) not in boto3.mock_calls + assert '().admin_create_user' not in [call[0] for call in boto3.mock_calls] @patch('cognito.utils.client.client') def test_create_user_does_not_create_if_unmanaged_exists(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() - created = client.create_user('1234', 'test@example.org') + created = client.create_user('2ihg2ox304po', '1234', 'test@example.org') assert created is False - assert call().admin_create_user( - UserPoolId=client.user_pool_id, - Username='1234', - UserAttributes=[{ - 'Name': 'email', 'Value': 'test@example.org' - }, { - 'Name': client.managed_flag_name, 'Value': 'true' - }], - DesiredDeliveryMediums=['EMAIL'] - ) not in boto3.mock_calls + assert '().admin_create_user' not in [call[0] for call in boto3.mock_calls] @patch('cognito.utils.client.client') def test_delete_user_deletes_managed(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=True, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' ) client = Client() @@ -162,7 +154,7 @@ def test_delete_user_deletes_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_delete_user_does_not_delete_unmanaged(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() @@ -176,17 +168,20 @@ def test_delete_user_does_not_delete_unmanaged(boto3, cognito_user_response_fact @patch('cognito.utils.client.client') def test_update_user_updates_managed(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=True, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' ) client = Client() - updated = client.update_user('1234', 'test@example.org') + updated = client.update_user('2ihg2ox304po', '1234', 'test@example.org') assert updated is True + assert '().admin_update_user_attributes' in [call[0] for call in boto3.mock_calls] assert call().admin_update_user_attributes( UserPoolId=client.user_pool_id, - Username='1234', + Username='2ihg2ox304po', UserAttributes=[{ 'Name': 'email', 'Value': 'test@example.org' + }, { + 'Name': 'preferred_username', 'Value': '1234' }] ) in boto3.mock_calls @@ -194,27 +189,19 @@ def test_update_user_updates_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_update_user_does_not_update_unmanaged(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() - updated = client.update_user('1234', 'test@example.org') + updated = client.update_user('2ihg2ox304po', '1234', 'test@example.org') assert updated is False - assert call().admin_update_user_attributes( - UserPoolId=client.user_pool_id, - Username='1234', - UserAttributes=[{ - 'Name': 'email', 'Value': 'test@example.org' - }, { - 'Name': client.managed_flag_name, 'Value': 'true' - }] - ) not in boto3.mock_calls + assert '().admin_update_user_attributes' not in [call[0] for call in boto3.mock_calls] @patch('cognito.utils.client.client') def test_disable_user_disables_managed(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=True, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' ) client = Client() @@ -228,7 +215,7 @@ def test_disable_user_disables_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_disable_user_does_not_disable_unmanaged(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() @@ -242,7 +229,7 @@ def test_disable_user_does_not_disable_unmanaged(boto3, cognito_user_response_fa @patch('cognito.utils.client.client') def test_enable_user_enables_managed(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=True, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=True, attributes_key='UserAttributes' ) client = Client() @@ -256,7 +243,7 @@ def test_enable_user_enables_managed(boto3, cognito_user_response_factory): @patch('cognito.utils.client.client') def test_enable_user_does_not_enable_unmanaged(boto3, cognito_user_response_factory): boto3.return_value.admin_get_user.return_value = cognito_user_response_factory( - '1234', managed=False, attributes_key='UserAttributes' + '2ihg2ox304po', '1234', managed=False, attributes_key='UserAttributes' ) client = Client() diff --git a/app/tests/cognito/test_cognito_sync_command.py b/app/tests/cognito/test_cognito_sync_command.py index 1238a74..e6673f5 100644 --- a/app/tests/cognito/test_cognito_sync_command.py +++ b/app/tests/cognito/test_cognito_sync_command.py @@ -31,7 +31,12 @@ def fixture_user(provider): with patch('access.models.Client') as client: client.return_value.create_user.return_value = True yield User.objects.create( - username='1', first_name='1', last_name='1', email='1@example.org', provider=provider + user_id='2ihg2ox304po', + username='1', + first_name='1', + last_name='1', + email='1@example.org', + provider=provider ) @@ -42,37 +47,37 @@ def test_command_adds(cognito_client, user): out = StringIO() call_command('cognito_sync', verbosity=2, stdout=out) - assert 'adding user 1' in out.getvalue() + assert 'adding user 2ihg2ox304po' in out.getvalue() assert '1 user(s) added' in out.getvalue() - assert call().create_user('1', '1@example.org') in cognito_client.mock_calls + assert call().create_user('2ihg2ox304po', '1', '1@example.org') in cognito_client.mock_calls @patch('cognito.management.commands.cognito_sync.Client') def test_command_deletes(cognito_client, db, cognito_user_response_factory): cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '1@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org') ] out = StringIO() call_command('cognito_sync', verbosity=2, stdout=out) - assert 'deleting user 1' in out.getvalue() + assert 'deleting user 2ihg2ox304po' in out.getvalue() assert '1 user(s) deleted' in out.getvalue() - assert call().delete_user('1') in cognito_client.mock_calls + assert call().delete_user('2ihg2ox304po') in cognito_client.mock_calls @patch('cognito.management.commands.cognito_sync.Client') def test_command_updates(cognito_client, user, cognito_user_response_factory): cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '2@example.org') + cognito_user_response_factory('2ihg2ox304po', '2@example.org') ] out = StringIO() call_command('cognito_sync', verbosity=2, stdout=out) - assert 'updating user 1' in out.getvalue() + assert 'updating user 2ihg2ox304po' in out.getvalue() assert '1 user(s) updated' in out.getvalue() - assert call().update_user('1', '1@example.org') in cognito_client.mock_calls + assert call().update_user('2ihg2ox304po', '1', '1@example.org') in cognito_client.mock_calls @patch('cognito.management.commands.cognito_sync.Client') @@ -80,35 +85,35 @@ def test_command_updates_disabled(cognito_client, user, cognito_user_response_fa user.deleted_at = timezone.now() user.save() cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '1@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org') ] out = StringIO() call_command('cognito_sync', verbosity=2, stdout=out) - assert 'disabling user 1' in out.getvalue() + assert 'disabling user 2ihg2ox304po' in out.getvalue() assert '1 user(s) disabled' in out.getvalue() - assert call().disable_user('1') in cognito_client.mock_calls + assert call().disable_user('2ihg2ox304po') in cognito_client.mock_calls @patch('cognito.management.commands.cognito_sync.Client') def test_command_updates_enabled(cognito_client, user, cognito_user_response_factory): cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '1@example.org', False) + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org', False) ] out = StringIO() call_command('cognito_sync', verbosity=2, stdout=out) - assert 'enabling user 1' in out.getvalue() + assert 'enabling user 2ihg2ox304po' in out.getvalue() assert '1 user(s) enabled' in out.getvalue() - assert call().enable_user('1') in cognito_client.mock_calls + assert call().enable_user('2ihg2ox304po') in cognito_client.mock_calls @patch('cognito.management.commands.cognito_sync.Client') def test_command_does_not_updates_if_unchanged(cognito_client, user, cognito_user_response_factory): cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '1@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org') ] out = StringIO() @@ -122,19 +127,19 @@ def test_command_does_not_updates_if_unchanged(cognito_client, user, cognito_use def test_command_clears_if_confirmed(cognito_client, input_, user, cognito_user_response_factory): input_.side_effect = ['yes'] cognito_client.return_value.list_users.side_effect = [[ - cognito_user_response_factory('1', '1@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org') ], []] out = StringIO() call_command('cognito_sync', clear=True, verbosity=2, stdout=out) assert 'This action will delete all managed users from cognito' in out.getvalue() - assert 'deleting user 1' in out.getvalue() + assert 'deleting user 2ihg2ox304po' in out.getvalue() assert '1 user(s) deleted' in out.getvalue() - assert 'adding user 1' in out.getvalue() + assert 'adding user 2ihg2ox304po' in out.getvalue() assert '1 user(s) added' in out.getvalue() - assert call().delete_user('1') in cognito_client.mock_calls - assert call().create_user('1', '1@example.org') in cognito_client.mock_calls + assert call().delete_user('2ihg2ox304po') in cognito_client.mock_calls + assert call().create_user('2ihg2ox304po', '1', '1@example.org') in cognito_client.mock_calls @patch('builtins.input') @@ -144,7 +149,7 @@ def test_command_does_not_clears_if_not_confirmed( ): input_.side_effect = ['no'] cognito_client.return_value.list_users.side_effect = [[ - cognito_user_response_factory('1', '1@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '1@example.org') ], []] out = StringIO() @@ -159,20 +164,25 @@ def test_command_does_not_clears_if_not_confirmed( @patch('cognito.management.commands.cognito_sync.Client') def test_command_runs_dry(cognito_client, provider, user, cognito_user_response_factory): User.objects.create( - username='2', first_name='2', last_name='2', email='2@example.org', provider=provider + user_id='goho4o3ggg2o', + username='2', + first_name='2', + last_name='2', + email='2@example.org', + provider=provider ) cognito_client.return_value.list_users.return_value = [ - cognito_user_response_factory('1', '10@example.org'), - cognito_user_response_factory('3', '3@example.org') + cognito_user_response_factory('2ihg2ox304po', '1', '10@example.org'), + cognito_user_response_factory('04i4p3g4iggh', '3', '3@example.org') ] out = StringIO() call_command('cognito_sync', dry_run=True, verbosity=2, stdout=out) - assert 'adding user 2' in out.getvalue() - assert 'deleting user 3' in out.getvalue() - assert 'updating user 1' in out.getvalue() + assert 'adding user goho4o3ggg2o' in out.getvalue() + assert 'deleting user 04i4p3g4iggh' in out.getvalue() + assert 'updating user 2ihg2ox304po' in out.getvalue() assert 'dry run' in out.getvalue() assert cognito_client.return_value.list_users.called assert not cognito_client.return_value.create_user.called diff --git a/app/tests/conftest.py b/app/tests/conftest.py index 9122e27..3faa7ff 100644 --- a/app/tests/conftest.py +++ b/app/tests/conftest.py @@ -41,6 +41,7 @@ def cognito_user_response_factory(): Returns a callable that accepts the following parameters: - username: The username of the user. + - preferred_username: The preferred username of the user. - email: The email address of the user. - enabled (bool): A flag indicating if the user is enabled. - managed (bool): A flag indicating if the user is managed. @@ -52,19 +53,26 @@ def cognito_user_response_factory(): def test_something(cognito_user_response_factory): response = cognito_user_response_factory( - 'user', 'user@example.org', enabled=False, managed=True, attributes_key='Attributes' + '2ihg2ox304po', 'user', 'user@example.org', enabled=False, managed=True, + attributes_key='Attributes' ) """ + # pylint: disable=too-many-positional-arguments def create_cognito_user_response( username, + preferred_username, email='test@example.org', enabled=True, managed=True, attributes_key='Attributes' ): - attributes = [{'Name': 'email', 'Value': email}] + attributes = [{ + 'Name': 'email', 'Value': email + }, { + 'Name': 'preferred_username', 'Value': preferred_username + }] if managed: attributes.append({'Name': settings.COGNITO_MANAGED_FLAG_NAME, 'Value': 'true'}) return {'Username': username, attributes_key: attributes, 'Enabled': enabled} diff --git a/app/tests/utils/test_short_id.py b/app/tests/utils/test_short_id.py new file mode 100644 index 0000000..a4c758e --- /dev/null +++ b/app/tests/utils/test_short_id.py @@ -0,0 +1,16 @@ +from utils.short_id import generate_short_id + + +def test_generate_short_id_default(): + short_id_1 = generate_short_id() + short_id_2 = generate_short_id() + assert short_id_1 != short_id_2 + assert len(short_id_1) == 12 + assert len(short_id_2) == 12 + + +def test_generate_short_id_settings(settings): + settings.SHORT_ID_SIZE = 4 + settings.SHORT_ID_ALPHABET = 'a' + short_id = generate_short_id() + assert short_id == 'aaaa' diff --git a/app/utils/short_id.py b/app/utils/short_id.py new file mode 100644 index 0000000..2c8a6dc --- /dev/null +++ b/app/utils/short_id.py @@ -0,0 +1,13 @@ +from nanoid import generate + +from django.conf import settings + + +def generate_short_id() -> str: + """ Generate a Short ID + + By default, a short ID consists of 12 lowercase letters or digits. This format is identical + to the one used in the service-shortlink. + + """ + return generate(settings.SHORT_ID_ALPHABET, int(settings.SHORT_ID_SIZE))