Merge pull request #38 from geoadmin/feat-PB-1074-user-deactivate

PB-1074: deactivate user

app/access/admin.py

@@ -1,8 +1,20 @@
 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 = ["disable"]
+
+    @admin.action(description="Disable selected users")
+    def disable(self, request: HttpRequest, queryset: models.QuerySet[User]) -> None:
+        for u in queryset:
+            u.disable()
+
+    def get_queryset(self, request: HttpRequest) -> models.QuerySet[User]:
+        return User.all_objects.get_queryset()

app/access/api.py

@@ -1,7 +1,7 @@
 from http import HTTPStatus
 
 from cognito.utils.user import create_cognito_user
-from cognito.utils.user import delete_cognito_user
+from cognito.utils.user import disable_cognito_user
 from cognito.utils.user import update_cognito_user
 from ninja import Router
 from ninja.errors import HttpError
@@ -45,7 +45,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, username=username)
     response = user_to_response(model)
     return response
 
@@ -110,10 +110,10 @@ def delete(request: HttpRequest, username: str) -> HttpResponse:
         user_to_delete = User.objects.select_for_update().filter(username=username).first()
         if not user_to_delete:
             raise Http404("Not Found")
-        deleted = delete_cognito_user(user_to_delete)
+        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)
 
 

app/access/migrations/0002_user_deleted_at.py

@@ -0,0 +1,19 @@
+# Generated by Django 5.0.9 on 2024-11-11 16:07
+
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('access', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='user',
+            name='deleted_at',
+            field=models.DateTimeField(blank=True, null=True, verbose_name='deleted at'),
+        ),
+    ]

app/access/models.py

@@ -2,9 +2,18 @@
 
 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 ActiveUserManager(models.Manager["User"]):
+    """ActiveUserManager filters out soft deleted users.
+    """
+
+    def get_queryset(self) -> models.QuerySet["User"]:
+        return super().get_queryset().filter(deleted_at__isnull=True)
+
+
 class User(models.Model):
 
     _context = "User model"
@@ -16,9 +25,18 @@ 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)
 
+    # By default only return active users
+    objects = ActiveUserManager()
+    all_objects = models.Manager()
+
+    @property
+    def is_active(self) -> bool:
+        return self.deleted_at is None
+
     def save(
         self,
         force_insert: bool | tuple[ModelBase, ...] = False,
@@ -29,3 +47,8 @@ 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:
+        # use django.utils.timezone over datetime to use timezone aware objects.
+        self.deleted_at = timezone.now()
+        self.save()

app/access/tests/test_api.py

@@ -333,9 +333,9 @@ def test_post_user_returns_403_if_no_permission(self, create_cognito_user):
         assert not create_cognito_user.called
         assert User.objects.count() == 1
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_deletes_user(self, delete_cognito_user):
-        delete_cognito_user.return_value = True
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_deletes_user(self, disable_cognito_user):
+        disable_cognito_user.return_value = True
 
         create_user_with_permissions('test', 'test', [('access', 'user', 'delete_user')])
         self.client.login(username='test', password='test')
@@ -345,11 +345,11 @@ def test_delete_user_deletes_user(self, delete_cognito_user):
         assert response.status_code == 204
         assert response.content == b''
         assert User.objects.count() == 0
-        assert delete_cognito_user.called
+        assert disable_cognito_user.called
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_returns_404_if_nonexisting(self, delete_cognito_user):
-        delete_cognito_user.return_value = False
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_returns_404_if_nonexisting(self, disable_cognito_user):
+        disable_cognito_user.return_value = False
 
         create_user_with_permissions('test', 'test', [('access', 'user', 'delete_user')])
         self.client.login(username='test', password='test')
@@ -359,11 +359,11 @@ def test_delete_user_returns_404_if_nonexisting(self, delete_cognito_user):
         assert response.status_code == 404
         assert response.json() == {"code": 404, "description": "Resource not found"}
         assert User.objects.count() == 1
-        assert not delete_cognito_user.called
+        assert not disable_cognito_user.called
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_returns_500_if_cognito_inconsistent(self, delete_cognito_user):
-        delete_cognito_user.return_value = False
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_returns_500_if_cognito_inconsistent(self, disable_cognito_user):
+        disable_cognito_user.return_value = False
 
         create_user_with_permissions('test', 'test', [('access', 'user', 'delete_user')])
         self.client.login(username='test', password='test')
@@ -373,11 +373,11 @@ def test_delete_user_returns_500_if_cognito_inconsistent(self, delete_cognito_us
         assert response.status_code == 500
         assert response.json() == {"code": 500, "description": "Internal Server Error"}
         assert User.objects.count() == 1
-        assert delete_cognito_user.called
+        assert disable_cognito_user.called
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_returns_503_if_cognito_down(self, delete_cognito_user):
-        delete_cognito_user.side_effect = EndpointConnectionError(endpoint_url='http://localhost')
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_returns_503_if_cognito_down(self, disable_cognito_user):
+        disable_cognito_user.side_effect = EndpointConnectionError(endpoint_url='http://localhost')
 
         create_user_with_permissions('test', 'test', [('access', 'user', 'delete_user')])
         self.client.login(username='test', password='test')
@@ -387,22 +387,22 @@ def test_delete_user_returns_503_if_cognito_down(self, delete_cognito_user):
         assert response.status_code == 503
         assert response.json() == {"code": 503, "description": "Service Unavailable"}
         assert User.objects.count() == 1
-        assert delete_cognito_user.called
+        assert disable_cognito_user.called
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_returns_401_if_not_logged_in(self, delete_cognito_user):
-        delete_cognito_user.return_value = True
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_returns_401_if_not_logged_in(self, disable_cognito_user):
+        disable_cognito_user.return_value = True
 
         response = self.client.delete("/api/users/dude", data={}, content_type='application/json')
 
         assert response.status_code == 401
         assert response.json() == {"code": 401, "description": "Unauthorized"}
         assert User.objects.count() == 1
-        assert not delete_cognito_user.called
+        assert not disable_cognito_user.called
 
-    @patch('access.api.delete_cognito_user')
-    def test_delete_user_returns_403_if_no_permission(self, delete_cognito_user):
-        delete_cognito_user.return_value = True
+    @patch('access.api.disable_cognito_user')
+    def test_delete_user_returns_403_if_no_permission(self, disable_cognito_user):
+        disable_cognito_user.return_value = True
 
         create_user_with_permissions('test', 'test', [])
         self.client.login(username='test', password='test')
@@ -412,7 +412,7 @@ def test_delete_user_returns_403_if_no_permission(self, delete_cognito_user):
         assert response.status_code == 403
         assert response.json() == {"code": 403, "description": "Forbidden"}
         assert User.objects.count() == 1
-        assert not delete_cognito_user.called
+        assert not disable_cognito_user.called
 
     def test_update_user_returns_401_if_not_logged_in(self):
         response = self.client.put("/api/users/dude")

app/cognito/management/commands/cognito_sync.py

@@ -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. """
@@ -75,11 +75,27 @@ 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. """
 
         # Get all remote and local users
-        local_users = {user.username: user for user in User.objects.all()}
+        local_users = {user.username: user for user in User.all_objects.all()}
         local_usernames = set(local_users.keys())
         remote_users = {user['Username']: user for user in self.client.list_users()}
         remote_usernames = set(remote_users.keys())

app/cognito/tests/test_client.py

@@ -222,3 +222,51 @@ def test_update_user_does_not_update_unmanaged(self, boto3):
             ),
             boto3.mock_calls
         )
+
+    @patch('cognito.utils.client.client')
+    def test_disable_user_disables_managed(self, boto3):
+        boto3.return_value.admin_get_user.return_value = cognito_user('1234', True, 'get_user')
+
+        client = Client()
+        disabled = client.disable_user('1234')
+        self.assertEqual(disabled, True)
+        self.assertIn(
+            call().admin_disable_user(UserPoolId=client.user_pool_id, Username='1234'),
+            boto3.mock_calls
+        )
+
+    @patch('cognito.utils.client.client')
+    def test_disable_user_does_not_disable_unmanaged(self, boto3):
+        boto3.return_value.admin_get_user.return_value = cognito_user('1234', False, 'get_user')
+
+        client = Client()
+        disabled = client.disable_user('1234')
+        self.assertEqual(disabled, False)
+        self.assertNotIn(
+            call().admin_disable_user(UserPoolId=client.user_pool_id, Username='1234'),
+            boto3.mock_calls
+        )
+
+    @patch('cognito.utils.client.client')
+    def test_enable_user_enables_managed(self, boto3):
+        boto3.return_value.admin_get_user.return_value = cognito_user('1234', True, 'get_user')
+
+        client = Client()
+        enabled = client.enable_user('1234')
+        self.assertEqual(enabled, True)
+        self.assertIn(
+            call().admin_enable_user(UserPoolId=client.user_pool_id, Username='1234'),
+            boto3.mock_calls
+        )
+
+    @patch('cognito.utils.client.client')
+    def test_enable_user_does_not_enable_unmanaged(self, boto3):
+        boto3.return_value.admin_get_user.return_value = cognito_user('1234', False, 'get_user')
+
+        client = Client()
+        enabled = client.enable_user('1234')
+        self.assertEqual(enabled, False)
+        self.assertNotIn(
+            call().admin_enable_user(UserPoolId=client.user_pool_id, Username='1234'),
+            boto3.mock_calls
+        )

app/cognito/tests/test_sync_command.py

@@ -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):
@@ -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')
@@ -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]')