From dcb02281059e1fb0003fdb2e0b2afaadb756b2d3 Mon Sep 17 00:00:00 2001
From: Fredrik Strand Oseberg <fredrik.no@gmail.com>
Date: Tue, 1 Oct 2024 09:03:46 +0200
Subject: [PATCH] fix: handle empty strings on permissions gracefully in access
 service (#8306)

Splitting #8271 into smaller pieces. This first PR will focus on making
access service handle empty string inputs gracefully and converting them
to null before inserting them into the database.
---
 src/lib/services/access-service.ts            |  15 ++-
 .../clean-permission-environment.test.ts      | 117 ++++++++++++++++++
 src/lib/types/model.ts                        |   2 +-
 3 files changed, 131 insertions(+), 3 deletions(-)
 create mode 100644 src/lib/services/clean-permission-environment.test.ts

diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts
index 9842085b7807..12023cdba67d 100644
--- a/src/lib/services/access-service.ts
+++ b/src/lib/services/access-service.ts
@@ -108,6 +108,17 @@ export interface AccessWithRoles {
 
 const isProjectPermission = (permission) => PROJECT_ADMIN.includes(permission);
 
+export const cleanPermissionEnvironment = (
+    permissions: PermissionRef[] | undefined,
+) => {
+    return permissions?.map((permission) => {
+        if (permission.environment === '') {
+            return { ...permission, environment: null };
+        }
+        return permission;
+    });
+};
+
 export class AccessService {
     private store: IAccessStore;
 
@@ -721,7 +732,7 @@ export class AccessService {
             roleType,
         };
 
-        const rolePermissions = role.permissions;
+        const rolePermissions = cleanPermissionEnvironment(role.permissions);
         const newRole = await this.roleStore.create(baseRole);
         if (rolePermissions) {
             if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
@@ -770,7 +781,7 @@ export class AccessService {
             description: role.description,
             roleType,
         };
-        const rolePermissions = role.permissions;
+        const rolePermissions = cleanPermissionEnvironment(role.permissions);
         const updatedRole = await this.roleStore.update(baseRole);
         const existingPermissions = await this.store.getPermissionsForRole(
             role.id,
diff --git a/src/lib/services/clean-permission-environment.test.ts b/src/lib/services/clean-permission-environment.test.ts
new file mode 100644
index 000000000000..62b73b780bc3
--- /dev/null
+++ b/src/lib/services/clean-permission-environment.test.ts
@@ -0,0 +1,117 @@
+import { cleanPermissionEnvironment } from './access-service';
+
+test('should convert all empty strings to null', () => {
+    const permissions = [
+        {
+            name: 'UPDATE_PROJECT',
+            environment: '',
+        },
+        {
+            name: 'UPDATE_FEATURE_VARIANTS',
+            environment: '',
+        },
+        {
+            name: 'READ_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'CREATE_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'DELETE_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'UPDATE_PROJECT_SEGMENT',
+            environment: '',
+        },
+    ];
+
+    const result = cleanPermissionEnvironment(permissions);
+
+    expect(result).toEqual([
+        {
+            name: 'UPDATE_PROJECT',
+            environment: null,
+        },
+        {
+            name: 'UPDATE_FEATURE_VARIANTS',
+            environment: null,
+        },
+        {
+            name: 'READ_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'CREATE_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'DELETE_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'UPDATE_PROJECT_SEGMENT',
+            environment: null,
+        },
+    ]);
+});
+
+test('should handle some permissions already having null values', () => {
+    const permissions = [
+        {
+            name: 'UPDATE_PROJECT',
+            environment: null,
+        },
+        {
+            name: 'UPDATE_FEATURE_VARIANTS',
+            environment: null,
+        },
+        {
+            name: 'READ_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'CREATE_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'DELETE_PROJECT_API_TOKEN',
+            environment: '',
+        },
+        {
+            name: 'UPDATE_PROJECT_SEGMENT',
+            environment: '',
+        },
+    ];
+
+    const result = cleanPermissionEnvironment(permissions);
+
+    expect(result).toEqual([
+        {
+            name: 'UPDATE_PROJECT',
+            environment: null,
+        },
+        {
+            name: 'UPDATE_FEATURE_VARIANTS',
+            environment: null,
+        },
+        {
+            name: 'READ_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'CREATE_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'DELETE_PROJECT_API_TOKEN',
+            environment: null,
+        },
+        {
+            name: 'UPDATE_PROJECT_SEGMENT',
+            environment: null,
+        },
+    ]);
+});
diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts
index 6a8ddbb7c767..46da47981400 100644
--- a/src/lib/types/model.ts
+++ b/src/lib/types/model.ts
@@ -419,7 +419,7 @@ export interface IPermission {
     name: string;
     displayName: string;
     type: string;
-    environment?: string;
+    environment?: string | null;
 }
 
 export interface IEnvironmentPermission {