Skip to content

Commit

Permalink
PB-1074: Disable users
Browse files Browse the repository at this point in the history
Allow disabling and deleting users via admin UI. API only disables users.
Cognito sync job also considers disabled/enabled users.
  • Loading branch information
benschs committed Nov 12, 2024
1 parent ac026da commit 52f1b79
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 55 deletions.
9 changes: 9 additions & 0 deletions app/access/admin.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
from django.contrib import admin
from django.db import models
from django.http import HttpRequest

from .models import User


@admin.register(User)
class UserAdmin(admin.ModelAdmin): # type:ignore[type-arg]
'''Admin View for User'''
list_display = ('provider', 'username', 'deleted_at')
actions = ["make_disabled"]

@admin.action(description="Disable selected users")
def make_disabled(self, request: HttpRequest, queryset: models.QuerySet[User]) -> None:
for u in queryset:
u.disable()
8 changes: 4 additions & 4 deletions app/access/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def user(request: HttpRequest, username: str) -> UserSchema:
"""
Get the user with the given username.
"""
model = get_object_or_404(User, username=username)
model = get_object_or_404(User.objects_api(), username=username)
response = user_to_response(model)
return response

Expand All @@ -46,7 +46,7 @@ def users(request: HttpRequest) -> dict[str, list[UserSchema]]:
"""
Get all users.
"""
models = User.objects.all()
models = User.objects_api().all()
responses = [user_to_response(model) for model in models]
return {"items": responses}

Expand Down Expand Up @@ -93,11 +93,11 @@ def delete(request: HttpRequest, username: str) -> HttpResponse:
"""

with transaction.atomic():
user_to_delete = User.objects.select_for_update().filter(username=username).first()
user_to_delete = User.objects_api().select_for_update().filter(username=username).first()
if not user_to_delete:
raise Http404("Not Found")
deleted = disable_cognito_user(user_to_delete)
if not deleted:
raise HttpError(500, "Internal Server Error")
user_to_delete.delete()
user_to_delete.disable()
return HttpResponse(status=204)
19 changes: 14 additions & 5 deletions app/access/models.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
from typing import Iterable

from utils.soft_delete_model import SoftDeleteModel
from utils.soft_delete_model import SoftDeleteModelManager

from django.db import models
from django.db.models.base import ModelBase
from django.utils import timezone
from django.utils.translation import pgettext_lazy as _


class User(SoftDeleteModel):
class User(models.Model):

_context = "User model"

Expand All @@ -19,10 +17,17 @@ def __str__(self) -> str:
first_name = models.CharField(_(_context, "First name"))
last_name = models.CharField(_(_context, "Last name"))
email = models.EmailField(_(_context, "Email"))
deleted_at = models.DateTimeField(_(_context, "deleted at"), null=True, blank=True)

provider = models.ForeignKey("provider.Provider", on_delete=models.CASCADE)

objects = SoftDeleteModelManager()
@staticmethod
def objects_api() -> models.QuerySet["User"]:
return User.objects.filter(deleted_at__isnull=True)

@property
def is_active(self) -> bool:
return self.deleted_at is None

def save(
self,
Expand All @@ -34,3 +39,7 @@ def save(
"""Validates the model before writing it to the database."""
self.full_clean()
super().save(force_insert, force_update, using, update_fields)

def disable(self) -> None:
self.deleted_at = timezone.now()
self.save()
18 changes: 9 additions & 9 deletions app/access/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ def setUp(self):
"email": "[email protected]",
"provider": provider,
}
User.objects.create(**model_fields)
User.objects_api().create(**model_fields)

self.client = TestClient(router)

def test_user_to_response_maps_fields_correctly(self):

model = User.objects.last()
model = User.objects_api().last()

actual = user_to_response(model)

Expand Down Expand Up @@ -86,7 +86,7 @@ def test_get_users_returns_users_ordered_by_id(self):
"email": "[email protected]",
"provider": Provider.objects.last(),
}
User.objects.create(**model_fields)
User.objects_api().create(**model_fields)

response = self.client.get("users")

Expand Down Expand Up @@ -228,7 +228,7 @@ def test_post_users_returns_500_if_cognito_inconsistent(self, create_cognito_use

assert response.status_code == 500
assert response.data == {'code': 500, 'description': 'Internal Server Error'}
assert User.objects.count() == 1
assert User.objects_api().count() == 1
assert create_cognito_user.called

@patch('access.api.create_cognito_user')
Expand All @@ -247,7 +247,7 @@ def test_post_users_returns_503_if_cognito_down(self, create_cognito_user):

assert response.status_code == 503
assert response.data == {'code': 503, 'description': 'Service Unavailable'}
assert User.objects.count() == 1
assert User.objects_api().count() == 1
assert create_cognito_user.called

@patch('access.api.disable_cognito_user')
Expand All @@ -258,7 +258,7 @@ def test_delete_user_deletes_user(self, disable_cognito_user):

assert response.status_code == 204
assert response.content == b''
assert User.objects.count() == 0
assert User.objects_api().count() == 0
assert disable_cognito_user.called

@patch('access.api.disable_cognito_user')
Expand All @@ -269,7 +269,7 @@ def test_delete_user_returns_404_if_nonexisting(self, disable_cognito_user):

assert response.status_code == 404
assert response.data == {"code": 404, "description": "Resource not found"}
assert User.objects.count() == 1
assert User.objects_api().count() == 1
assert not disable_cognito_user.called

@patch('access.api.disable_cognito_user')
Expand All @@ -280,7 +280,7 @@ def test_delete_user_returns_500_if_cognito_inconsistent(self, disable_cognito_u

assert response.status_code == 500
assert response.data == {"code": 500, "description": "Internal Server Error"}
assert User.objects.count() == 1
assert User.objects_api().count() == 1
assert disable_cognito_user.called

@patch('access.api.disable_cognito_user')
Expand All @@ -291,5 +291,5 @@ def test_delete_user_returns_503_if_cognito_down(self, disable_cognito_user):

assert response.status_code == 503
assert response.data == {"code": 503, "description": "Service Unavailable"}
assert User.objects.count() == 1
assert User.objects_api().count() == 1
assert disable_cognito_user.called
18 changes: 17 additions & 1 deletion app/cognito/management/commands/cognito_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(self, command: CustomBaseCommand, options: dict[str, Any]) -> None:
self.clear = options['clear']
self.dry_run = options['dry_run']
self.client = Client()
self.counts = {'added': 0, 'deleted': 0, 'updated': 0}
self.counts = {'added': 0, 'deleted': 0, 'updated': 0, 'enabled': 0, 'disabled': 0}

def clear_users(self) -> None:
""" Remove all existing cognito users. """
Expand Down Expand Up @@ -75,6 +75,22 @@ def update_user(self, local_user: User, remote_user: 'UserTypeTypeDef') -> None:
local_user.username
)

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}')
if not self.dry_run:
enabled = self.client.enable_user(local_user.username)
if not enabled:
self.print_error('Could not enable %s', local_user.username)
else:
self.counts['disabled'] += 1
self.print(f'disabling user {local_user.username}')
if not self.dry_run:
disabled = self.client.disable_user(local_user.username)
if not disabled:
self.print_error('Could not disable %s', local_user.username)

def sync_users(self) -> None:
""" Synchronizes local and cognito users. """

Expand Down
38 changes: 34 additions & 4 deletions app/cognito/tests/test_sync_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@

from django.core.management import call_command
from django.test import TestCase
from django.utils import timezone


def cognito_user(username, email):
return {'Username': username, 'Attributes': [{'Name': 'email', 'Value': email}]}
def cognito_user(username, email, enabled=True):
return {
'Username': username, 'Attributes': [{
'Name': 'email', 'Value': email
}], 'Enabled': enabled
}


class CognitoSyncCommandTest(TestCase):
Expand All @@ -29,13 +34,14 @@ def setUp(self):
name_rm="Uffizi federal per l'ambient",
)

def add_user(self, username, email):
def add_user(self, username, email, deleted_at=None):
User.objects.create(
username=username,
first_name=username,
last_name=username,
email=email,
provider=self.provider
provider=self.provider,
deleted_at=deleted_at
)

@patch('cognito.management.commands.cognito_sync.Client')
Expand Down Expand Up @@ -73,6 +79,30 @@ def test_command_updates(self, client):
self.assertIn('1 user(s) updated', out.getvalue())
self.assertIn(call().update_user('1', '[email protected]'), client.mock_calls)

@patch('cognito.management.commands.cognito_sync.Client')
def test_command_updates_disabled(self, client):
self.add_user('1', '[email protected]', timezone.now())
client.return_value.list_users.return_value = [cognito_user('1', '[email protected]')]

out = StringIO()
call_command('cognito_sync', verbosity=2, stdout=out)

self.assertIn('disabling user 1', out.getvalue())
self.assertIn('1 user(s) disabled', out.getvalue())
self.assertIn(call().disable_user('1'), client.mock_calls)

@patch('cognito.management.commands.cognito_sync.Client')
def test_command_updates_enabled(self, client):
self.add_user('1', '[email protected]')
client.return_value.list_users.return_value = [cognito_user('1', '[email protected]', False)]

out = StringIO()
call_command('cognito_sync', verbosity=2, stdout=out)

self.assertIn('enabling user 1', out.getvalue())
self.assertIn('1 user(s) enabled', out.getvalue())
self.assertIn(call().enable_user('1'), client.mock_calls)

@patch('cognito.management.commands.cognito_sync.Client')
def test_command_does_not_updates_if_unchanged(self, client):
self.add_user('1', '[email protected]')
Expand Down
32 changes: 0 additions & 32 deletions app/utils/soft_delete_model.py

This file was deleted.

0 comments on commit 52f1b79

Please sign in to comment.