made deprecated and new fields optional

narval-xyz · Nov 7, 2024 · 2924b76 · 2924b76
1 parent a9fce26
commit 2924b76
Show file tree

Hide file tree

Showing 16 changed files with 523 additions and 72 deletions.
diff --git a/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts b/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts
@@ -178,7 +178,7 @@ describe(DataStoreService.name, () => {
     })
 
     it('throws DataStoreException when entity domain is invalid', async () => {
-      const duplicateGroups = [
+      const duplicateUserGroups = [
         {
           id: '1'
         },
@@ -187,14 +187,15 @@ describe(DataStoreService.name, () => {
         }
       ]
       const entities = {
-        groups: duplicateGroups,
+        userGroups: duplicateUserGroups,
         addressBook: [],
         credentials: [],
         tokens: [],
         userGroupMembers: [],
         userAccounts: [],
         users: [],
         accountGroupMembers: [],
+        accountGroups: [],
         accounts: []
       }
 

diff --git a/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts b/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts
@@ -73,7 +73,8 @@ describe('OpenPolicyAgentEngine', () => {
         credentials: [],
         tokens: [],
         userGroupMembers: [],
-        groups: [],
+        accountGroups: [],
+        userGroups: [],
         userAccounts: [],
         users: [],
         accountGroupMembers: [],
@@ -437,9 +438,10 @@ describe('OpenPolicyAgentEngine', () => {
             }
           }
         ],
+        userGroups: [],
+        accountGroups: [],
         tokens: [],
         userGroupMembers: [],
-        groups: [],
         userAccounts: [],
         users: [
           {

diff --git a/apps/policy-engine/src/open-policy-agent/core/type/open-policy-agent.type.ts b/apps/policy-engine/src/open-policy-agent/core/type/open-policy-agent.type.ts
@@ -70,6 +70,8 @@ export const Data = z.object({
     addressBook: z.record(Id, addressBookAccountEntitySchema.extend({ id: Id })),
     tokens: z.record(Id, tokenEntitySchema.extend({ id: Id })),
     users: z.record(Id, userEntitySchema.extend({ id: Id })),
+    accountGroups: z.record(Id, AccountGroup),
+    userGroups: z.record(Id, UserGroup),
     groups: z.record(Id, Group),
     accounts: z.record(Id, Account)
   })

diff --git a/apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/evaluation.util.spec.ts b/apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/evaluation.util.spec.ts
@@ -360,5 +360,34 @@ describe('toData', () => {
         )
       })
     })
+
+    it('indexes legacy groups with members by lower case id', () => {
+      expect.assertions(3)
+
+      const { entities } = toData({
+        ...FIXTURE.ENTITIES,
+        userGroupMembers: [
+          ...FIXTURE.ENTITIES.userGroupMembers,
+          { userId: 'test-legacy-alice', groupId: 'test-legacy-group' }
+        ],
+        accountGroupMembers: [
+          ...FIXTURE.ENTITIES.accountGroupMembers,
+          { accountId: 'test-legacy-account', groupId: 'test-legacy-group' }
+        ],
+        accountGroups: [{ id: 'test-legacy-group' }],
+        userGroups: [{ id: 'test-legacy-group' }]
+      })
+
+      expect(entities.userGroups['test-legacy-group']).toEqual({
+        id: 'test-legacy-group',
+        users: ['test-legacy-alice']
+      })
+      expect(entities.accountGroups['test-legacy-group']).toEqual({
+        id: 'test-legacy-group',
+        accounts: ['test-legacy-account']
+      })
+
+      expect(entities.groups['test-legacy-group']).toEqual(undefined)
+    })
   })
 })
diff --git a/apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts b/apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts
@@ -18,7 +18,7 @@ import { InputType, safeDecode } from '@narval/transaction-request-intent'
 import { HttpStatus } from '@nestjs/common'
 import { indexBy } from 'lodash/fp'
 import { OpenPolicyAgentException } from '../exception/open-policy-agent.exception'
-import { Account, AccountGroup, Data, Group, Input } from '../type/open-policy-agent.type'
+import { Account, AccountGroup, Data, Group, Input, UserGroup } from '../type/open-policy-agent.type'
 
 type Mapping<R extends Request> = (
   request: R,
@@ -193,31 +193,41 @@ export const toInput = (params: {
 
 export const toData = (entities: Entities): Data => {
   const groups = new Map<string, Group>()
+  const userGroups = new Map<string, UserGroup>()
+  const accountGroups = new Map<string, AccountGroup>()
+
+  const {
+    userGroups: legacyUserGroupsEntities = [],
+    accountGroups: legacyAccountGroupsEntities = [],
+    groups: newGroups = []
+  } = entities
 
   // Process user group members
   entities.userGroupMembers.forEach(({ userId, groupId }) => {
     const id = groupId.toLowerCase()
-    const group = groups.get(id) || {
-      id: groupId,
-      users: [],
-      accounts: []
-    }
+    const isInNewGroups = newGroups.some((group) => group.id === id)
+    const isInLegacyGroups = legacyUserGroupsEntities.some((group) => group.id === id)
 
-    group.users.push(userId)
-    groups.set(id, group)
+    if (isInNewGroups) {
+      groups.get(id)?.users.push(userId) || groups.set(id, { id, users: [userId], accounts: [] })
+    }
+    if (isInLegacyGroups) {
+      userGroups.get(id)?.users.push(userId) || userGroups.set(id, { id, users: [userId] })
+    }
   })
 
   // Process account group members
   entities.accountGroupMembers.forEach(({ accountId, groupId }) => {
     const id = groupId.toLowerCase()
-    const group = groups.get(id) || {
-      id: groupId,
-      users: [],
-      accounts: []
-    }
+    const isInNewGroups = newGroups.some((group) => group.id === id)
+    const isInLegacyGroups = legacyAccountGroupsEntities.some((group) => group.id === id)
 
-    group.accounts.push(accountId)
-    groups.set(id, group)
+    if (isInNewGroups) {
+      groups.get(id)?.accounts.push(accountId) || groups.set(id, { id, users: [], accounts: [accountId] })
+    }
+    if (isInLegacyGroups) {
+      accountGroups.get(id)?.accounts.push(accountId) || accountGroups.set(id, { id, accounts: [accountId] })
+    }
   })
 
   const accountAssignees = entities.userAccounts.reduce((assignees, { userId, accountId }) => {
@@ -235,25 +245,14 @@ export const toData = (entities: Entities): Data => {
     assignees: accountAssignees.get(account.id) || []
   }))
 
-  const accountGroups = entities.accountGroupMembers.reduce((groups, { accountId, groupId }) => {
-    const group = groups.get(groupId)
-
-    if (group) {
-      return groups.set(groupId, {
-        id: groupId,
-        accounts: group.accounts.concat(accountId)
-      })
-    } else {
-      return groups.set(groupId, { id: groupId, accounts: [accountId] })
-    }
-  }, new Map<string, AccountGroup>())
-
   const data: Data = {
     entities: {
       addressBook: indexBy('id', entities.addressBook),
       tokens: indexBy('id', entities.tokens),
       users: indexBy('id', entities.users),
       groups: indexBy('id', Object.fromEntries(groups)),
+      accountGroups: Object.fromEntries(accountGroups),
+      userGroups: Object.fromEntries(userGroups),
       accounts: indexBy('id', accounts)
     }
   }

diff --git a/apps/policy-engine/src/resource/open-policy-agent/rego/armory/criteria/checkApprovals.rego b/apps/policy-engine/src/resource/open-policy-agent/rego/armory/criteria/checkApprovals.rego
@@ -48,7 +48,7 @@ checkApproval(approval) := result if {
 	approval.approvalEntityType == "Narval::UserGroup"
 	possibleApprovers = {user |
 		some entity in approval.entityIds
-		users = entities.getGroup(entity).users
+		users = entities.getUserGroup(entity).users
 		some user in users
 	} | {principal.id}
 
@@ -62,7 +62,8 @@ checkApproval(approval) := result if {
 	approval.approvalEntityType == "Narval::UserGroup"
 	possibleApprovers = {user |
 		some entity in approval.entityIds
-		users = entities.getGroup(entity).users
+		users = entities.getUserGroup(entity).users
+
 		some user in users
 		not lib.caseInsensitiveEqual(user, principal.id)
 	}