Skip to content

Commit

Permalink
PB-1106: Use cognito's preferred username
Browse files Browse the repository at this point in the history
  • Loading branch information
msom committed Nov 26, 2024
1 parent c619fd2 commit 270bcfb
Show file tree
Hide file tree
Showing 14 changed files with 245 additions and 131 deletions.
2 changes: 2 additions & 0 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "*"
Expand All @@ -34,6 +35,7 @@ debugpy = "*"
boto3-stubs = {extras = ["cognito-idp"], version = "~=1.35"}
bandit = "*"
pytest-xdist = "*"
types-nanoid = "*"

[requires]
python_version = "3.12"
19 changes: 18 additions & 1 deletion Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion app/access/admin.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -8,7 +9,7 @@
@admin.register(User)
class UserAdmin(admin.ModelAdmin): # type:ignore[type-arg]
'''Admin View for User'''
list_display = ('provider', 'username', 'deleted_at')
list_display = ('user_id', 'username', 'deleted_at', 'provider')
actions = ["disable"]

@admin.action(description="Disable selected users")
Expand All @@ -18,3 +19,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 []
36 changes: 36 additions & 0 deletions app/access/migrations/0003_user_user_id.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
18 changes: 10 additions & 8 deletions app/access/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand All @@ -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()
66 changes: 36 additions & 30 deletions app/cognito/management/commands/cognito_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. """
Expand Down
18 changes: 11 additions & 7 deletions app/cognito/utils/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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"
}],
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
"""
Expand All @@ -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.
"""
Expand Down
4 changes: 4 additions & 0 deletions app/config/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Loading

0 comments on commit 270bcfb

Please sign in to comment.