Merge pull request RedHatInsights#1472 from astrozzc/lower

Store lower case username for principal

rbac/internal/specs/openapi.json

@@ -1481,6 +1481,84 @@
           }
         }
       }
+    },
+    "/api/utils/username_lower/": {
+      "get": {
+        "tags": [
+          "List username"
+        ],
+        "summary": "List uppercase username",
+        "description": "List uppercase username.",
+        "operationId": "ListUsername",
+        "responses": {
+          "200": {
+            "description": "List of uppercase username."
+          },
+          "405": {
+            "description": "Invalid method.",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                }
+              }
+            }
+          },
+          "500": {
+            "description": "Unexpected Error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                }
+              }
+            }
+          }
+        }
+      },
+      "post": {
+        "tags": [
+          "Update username"
+        ],
+        "summary": "Update uppercase username to lowercase",
+        "description": "Update uppercase username to lowercase.",
+        "operationId": "LowerUsername",
+        "responses": {
+          "200": {
+            "description": "All uppercase username updated to lowercase."
+          },
+          "400":{
+            "description": "Invalid request.",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                }
+              }
+            }
+          },
+          "405": {
+            "description": "Invalid method.",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                }
+              }
+            }
+          },
+          "500": {
+            "description": "Unexpected Error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                }
+              }
+            }
+          }
+        }
+      }
     }
   },
   "servers": [

rbac/internal/urls.py

@@ -80,6 +80,7 @@
     path("api/utils/get_org_admin/<int:org_or_account>/", views.get_org_admin),
     path("api/utils/role/", views.role_removal),
     path("api/utils/permission/", views.permission_removal),
+    path("api/utils/username_lower/", views.username_lower),
     path("api/utils/data_migration/", views.data_migration),
     path("api/utils/bindings/<role_uuid>/", views.list_or_delete_bindings_for_role),
     path("api/utils/bootstrap_tenant/", views.bootstrap_tenant),

rbac/internal/views.py

@@ -25,12 +25,13 @@
 from django.conf import settings
 from django.db import connection, transaction
 from django.db.migrations.recorder import MigrationRecorder
+from django.db.models import Func
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import get_object_or_404
 from django.utils.html import escape
 from internal.utils import delete_bindings
 from management.cache import TenantCache
-from management.models import Group, Permission, ResourceDefinition, Role
+from management.models import Group, Permission, Principal, ResourceDefinition, Role
 from management.principal.proxy import (
     API_TOKEN_HEADER,
     CLIENT_ID_HEADER,
@@ -865,3 +866,33 @@ def correct_resource_definitions(request):
         return HttpResponse(f"Updated {count} bad resource definitions", status=200)
 
     return HttpResponse('Invalid method, only "GET" or "PATCH" are allowed.', status=405)
+
+
+class Upper(Func):
+    """Upper class function."""
+
+    function = "UPPER"
+
+
+def username_lower(request):
+    """Update the username for the principal to be lowercase."""
+    if request.method not in ["POST", "GET"]:
+        return HttpResponse("Invalid request method, only POST/GET are allowed.", status=405)
+    if request.method == "POST" and not destructive_ok("api"):
+        return HttpResponse("Destructive operations disallowed.", status=400)
+
+    pre_names = []
+    updated_names = []
+    with transaction.atomic():
+        principals = Principal.objects.filter(type="user").filter(username=Upper("username"))
+        for principal in principals:
+            pre_names.append(principal.username)
+            principal.username = principal.username.lower()
+            updated_names.append(principal.username)
+        if request.method == "GET":
+            return HttpResponse(
+                f"Usernames to be updated: {pre_names} to {updated_names}",
+                status=200,
+            )
+        Principal.objects.bulk_update(principals, ["username"])
+        return HttpResponse(f"Updated {len(principals)} usernames", status=200)

rbac/management/principal/model.py

@@ -51,6 +51,11 @@ def principal_resource_id(self) -> Optional[str]:
             return None
         return Principal.user_id_to_principal_resource_id(self.user_id)
 
+    def save(self, *args, **kwargs):
+        """Override save to only store lower case username."""
+        self.username = self.username.lower()
+        super(Principal, self).save(*args, **kwargs)
+
     class Meta:
         ordering = ["username"]
         constraints = [

tests/internal/test_views.py

@@ -1316,6 +1316,44 @@ def test_fetch_role(self):
         self.assertFalse(role["admin_default"])
         self.assertEqual(response.status_code, 200)
 
+    @override_settings(INTERNAL_DESTRUCTIVE_API_OK_UNTIL=valid_destructive_time())
+    def test_update_username_to_lowercase(self):
+        """Test that the uppercase username would be updated to lowercase."""
+        # Only POST is allowed
+        response = self.client.delete(
+            f"/_private/api/utils/username_lower/",
+            **self.request.META,
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED)
+
+        Principal.objects.bulk_create(
+            [
+                Principal(username="12345", tenant=self.tenant),
+                Principal(username="ABCDE", tenant=self.tenant),
+                Principal(username="user", tenant=self.tenant),
+            ]
+        )
+
+        response = self.client.get(
+            f"/_private/api/utils/username_lower/",
+            **self.request.META,
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(
+            response.content.decode(), "Usernames to be updated: ['12345', 'ABCDE'] to ['12345', 'abcde']"
+        )
+
+        response = self.client.post(
+            f"/_private/api/utils/username_lower/",
+            **self.request.META,
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        usernames = Principal.objects.values_list("username", flat=True)
+        self.assertEqual({"12345", "abcde", "user"}, set(usernames))
+
 
 class InternalViewsetResourceDefinitionTests(IdentityRequest):
     def setUp(self):

tests/management/principal/test_model.py

@@ -30,10 +30,10 @@ def test_principal_creation(self):
         """Test that we can create principal correctly."""
         # Default value for cross_account is False.
         principalA = Principal.objects.create(username="principalA", tenant=self.tenant)
-        self.assertEqual(principalA.username, "principalA")
+        self.assertEqual(principalA.username, "principala")
         self.assertEqual(principalA.cross_account, False)
 
         # Explicitly set cross_account.
         principalB = Principal.objects.create(username="principalB", cross_account=True, tenant=self.tenant)
-        self.assertEqual(principalB.username, "principalB")
+        self.assertEqual(principalB.username, "principalb")
         self.assertEqual(principalB.cross_account, True)