From 97b1c52f70c318323f74ad481326196770f30f04 Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Wed, 16 Oct 2024 22:08:07 +0100
Subject: [PATCH] Protect against old roles with differences when it comes to
 loops.

---
 packages/shared-core/src/helpers/roles.ts        | 16 ++++++++++++----
 .../shared-core/src/helpers/tests/roles.spec.ts  |  7 +++++++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/packages/shared-core/src/helpers/roles.ts b/packages/shared-core/src/helpers/roles.ts
index 24b65501ce..8dee928edd 100644
--- a/packages/shared-core/src/helpers/roles.ts
+++ b/packages/shared-core/src/helpers/roles.ts
@@ -1,4 +1,11 @@
-import { Role, SEPARATOR, DocumentType } from "@budibase/types"
+import { Role, DocumentType, SEPARATOR } from "@budibase/types"
+
+// need to have a way to prefix, so we can check if the ID has its prefix or not
+// all new IDs should be the same in the future, but old roles they are never prefixed
+// while the role IDs always are - best to check both, also we can't access backend-core here
+function prefixForCheck(id: string) {
+  return `${DocumentType.ROLE}${SEPARATOR}${id}`
+}
 
 // Function to detect loops in roles
 export function checkForRoleInheritanceLoops(roles: Role[]): boolean {
@@ -11,16 +18,17 @@ export function checkForRoleInheritanceLoops(roles: Role[]): boolean {
   const checking = new Set<string>()
 
   function hasLoop(roleId: string): boolean {
-    if (checking.has(roleId)) {
+    const prefixed = prefixForCheck(roleId)
+    if (checking.has(roleId) || checking.has(prefixed)) {
       return true
     }
-    if (checked.has(roleId)) {
+    if (checked.has(roleId) || checked.has(prefixed)) {
       return false
     }
 
     checking.add(roleId)
 
-    const role = roleMap.get(roleId)
+    const role = roleMap.get(prefixed) || roleMap.get(roleId)
     if (!role) {
       // role not found - ignore
       checking.delete(roleId)
diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts
index 7e52ded3f6..7b67e4e9e9 100644
--- a/packages/shared-core/src/helpers/tests/roles.spec.ts
+++ b/packages/shared-core/src/helpers/tests/roles.spec.ts
@@ -57,5 +57,12 @@ describe("role utilities", () => {
       ]
       check(true)
     })
+
+    it("should handle new and old inherits structure", () => {
+      const role1 = role("role_role_1", "role_1")
+      role("role_role_2", ["role_1"])
+      role1.inherits = "role_2"
+      check(true)
+    })
   })
 })