Skip to content

Commit

Permalink
removed unecessary group duplication in rego
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ptroger
Ptroger committed Nov 6, 2024
1 parent b6906ea commit c8c34c4
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 440 deletions.
2 changes: 0 additions & 2 deletions apps/policy-engine/src/open-policy-agent/core/type/open-policy-agent.type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ 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)
})
Expand Down
34 changes: 0 additions & 34 deletions apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/evaluation.util.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,39 +360,5 @@ describe('toData', () => {
)
})
})

it('indexes legacy groups with members by lower case id', () => {
const { entities } = toData({
...FIXTURE.ENTITIES,
userGroupMembers: [
...FIXTURE.ENTITIES.userGroupMembers!,
{ userId: 'test-legacy-alice', groupId: 'test-legacy-user-group' }
],
accountGroupMembers: [
...FIXTURE.ENTITIES.accountGroupMembers!,
{ accountId: 'test-legacy-account', groupId: 'test-legacy-account-group' }
]
})

expect(entities.userGroups['test-legacy-user-group']).toEqual({
id: 'test-legacy-user-group',
users: ['test-legacy-alice']
})
expect(entities.accountGroups['test-legacy-account-group']).toEqual({
id: 'test-legacy-account-group',
accounts: ['test-legacy-account']
})
const group = FIXTURE.GROUP.Engineering

expect(entities.groups[group.id.toLowerCase()]).toEqual({
id: group.id.toLowerCase(),
users: FIXTURE.USER_GROUP_MEMBER.filter(({ groupId }) => groupId === group.id).map(({ userId }) =>
userId.toLowerCase()
),
accounts: FIXTURE.ACCOUNT_GROUP_MEMBER.filter(({ groupId }) => groupId === group.id).map(({ accountId }) =>
accountId.toLowerCase()
)
})
})
})
})
44 changes: 24 additions & 20 deletions apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,19 +192,33 @@ export const toInput = (params: {
}

export const toData = (entities: Entities): Data => {
const userGroups = entities.userGroupMembers.reduce((groups, { userId, groupId }) => {
const groups = new Map<string, Group>()

// Process user group members
entities.userGroupMembers.forEach(({ userId, groupId }) => {
const id = groupId.toLowerCase()
const group = groups.get(id)
const group = groups.get(id) || {
id: groupId,
users: [],
accounts: []
}

if (group) {
return groups.set(id, {
id: groupId,
users: group.users.concat(userId)
})
} else {
return groups.set(groupId, { id: groupId, users: [userId] })
group.users.push(userId)
groups.set(id, group)
})

// Process account group members
entities.accountGroupMembers.forEach(({ accountId, groupId }) => {
const id = groupId.toLowerCase()
const group = groups.get(id) || {
id: groupId,
users: [],
accounts: []
}
}, new Map<string, UserGroup>())

group.accounts.push(accountId)
groups.set(id, group)
})

const accountAssignees = entities.userAccounts.reduce((assignees, { userId, accountId }) => {
const account = assignees.get(accountId)
Expand Down Expand Up @@ -234,23 +248,13 @@ export const toData = (entities: Entities): Data => {
}
}, new Map<string, AccountGroup>())

const groups = (entities.groups || []).reduce((groups, { id }) => {
const users = userGroups.get(id)?.users || []
const accounts = accountGroups.get(id)?.accounts || []
groups.set(id, { id, users, accounts })

return groups
}, new Map<string, Group>())

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)),
userGroups: Object.fromEntries(userGroups),
accounts: indexBy('id', accounts),
accountGroups: Object.fromEntries(accountGroups)
}
}

Expand Down
263 changes: 0 additions & 263 deletions ...gine/src/resource/open-policy-agent/rego/armory/entities/__test__/entityQueries_test.rego
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,34 +130,6 @@ test_user if {
user == userUpper
}

test_userGroupWorkWithDataInBothDeprecatedAndNewSchema if {
entities := {
"users": {"test-bob-uid": {
"id": "test-BOB-uid",
"role": "root",
"groups": ["test-GROUP-one-uid", "test-GROUP-two-uid"],
"userGroups": ["test-GROUP-one-uid"],
}},
"groups": {"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"users": ["test-BOB-uid"],
"accounts": [],
}},
"userGroups": {"test-GROUP-two-uid": {
"id": "test-GROUP-two-uid",
"users": ["test-BOB-uid"],
}},
}

user := getUser("test-BOB-uid") with data.entities as entities

user == {
"id": "test-BOB-uid",
"role": "root",
"groups": {"test-GROUP-one-uid", "test-GROUP-two-uid"},
}
}

test_usersByRole if {
root := getUsersByRole("root") with data.entities as testData.entities

Expand All @@ -170,238 +142,3 @@ test_usersByRole if {
"0xAAA8ee1cbaa1856f4550c6fc24abb16c5c9b2a43",
}
}

test_accountGroupsWorkWithDataInBothDeprecatedAndNewSchema if {
entities := {
"accounts": {"eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e": {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"groups": ["test-GROUP-one-uid"],
"accountGroups": ["test-GROUP-two-uid"],
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}},
"groups": {"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
}},
"accountGroups": {
"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
},
"test-GROUP-two-uid": {
"id": "test-GROUP-two-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
},
},
}

account := getAccount("eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e") with data.entities as entities
expected := {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"groups": {"test-GROUP-one-uid", "test-GROUP-two-uid"},
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}
account == expected
}

test_accountGroupWorkWithDataDuplicatedInBothSchema if {
account := getAccount("eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e") with data.entities as {
"accounts": {"eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e": {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"groups": ["test-GROUP-one-uid"],
"accountGroups": ["test-GROUP-one-uid", "test-GROUP-two-uid"],
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}},
"accountGroups": {
"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
},
"test-GROUP-two-uid": {
"id": "test-GROUP-two-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
},
},
"groups": {"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
}},
}

account == {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"groups": {"test-GROUP-one-uid", "test-GROUP-two-uid"},
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}
}

test_accountGroupWorkWithDeprecatedSchema if {
account := getAccount("eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e") with data.entities as {
"accounts": {"eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e": {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"accountGroups": ["test-GROUP-one-uid"],
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}},
"accountGroups": {"test-GROUP-one-uid": {
"id": "test-GROUP-one-uid",
"accounts": ["eip155:eoa:0xddcf208f219a6e6af072f2cfdc615b2c1805f98e"],
}},
}

account == {
"accountType": "eoa",
"address": "0xddcf208F219a6e6af072f2cfdc615b2c1805f98e",
"assignees": ["test-bOb-uid", "test-alicE-uid", "test-foo-uid", "test-bar-uid"],
"groups": {"test-GROUP-one-uid"},
"id": "eip155:eoa:0xDDcf208f219a6e6af072f2cfdc615b2c1805f98e",
}
}

test_mixOfLegacyAndNewGroups if {
entities := {
"users": {
"test-alice-uid": {
"id": "test-alice-uid",
"role": "admin",
"groups": ["shared-GROUP-one", "user-only-GROUP-one", "legacy-user-GROUP-migrated"],
"userGroups": ["legacy-user-GROUP-one", "legacy-user-GROUP-migrated"],
},
"test-bob-uid": {
"id": "test-bob-uid",
"role": "user",
"userGroups": ["legacy-user-GROUP-one", "legacy-user-GROUP-two", "legacy-user-GROUP-migrated"],
},
"test-charlie-uid": {
"id": "test-charlie-uid",
"role": "root",
"groups": ["shared-GROUP-one", "shared-GROUP-two"],
},
},
"accounts": {
"eip155:eoa:0x123": {
"id": "eip155:eoa:0x123",
"accountType": "eoa",
"address": "0x123",
"assignees": ["test-alice-uid", "test-bob-uid"],
"groups": ["shared-GROUP-one", "legacy-user-GROUP-migrated"],
"accountGroups": ["legacy-account-GROUP-one"],
},
"eip155:eoa:0x456": {
"id": "eip155:eoa:0x456",
"accountType": "eoa",
"address": "0x456",
"assignees": ["test-charlie-uid"],
"accountGroups": ["legacy-account-GROUP-one", "legacy-account-GROUP-two"],
},
},
"groups": {
"shared-GROUP-one": {
"id": "shared-GROUP-one",
"users": ["test-alice-uid", "test-charlie-uid"],
"accounts": ["eip155:eoa:0x123"],
},
"user-only-GROUP-one": {
"id": "user-only-GROUP-one",
"users": ["test-alice-uid"],
"accounts": [],
},
"shared-GROUP-two": {
"id": "shared-GROUP-two",
"users": ["test-charlie-uid"],
"accounts": [],
},
"legacy-user-GROUP-migrated": {
"id": "legacy-user-GROUP-migrated",
"users": ["test-alice-uid"],
"accounts": ["eip155:eoa:0x123"],
},
},
"userGroups": {
"legacy-user-GROUP-one": {
"id": "legacy-user-GROUP-one",
"users": ["test-alice-uid", "test-bob-uid"],
},
"legacy-user-GROUP-two": {
"id": "legacy-user-GROUP-two",
"users": ["test-bob-uid"],
},
"legacy-user-GROUP-migrated": {
"id": "legacy-user-GROUP-migrated",
"users": ["test-bob-uid"],
},
},
"accountGroups": {
"legacy-account-GROUP-one": {
"id": "legacy-account-GROUP-one",
"accounts": ["eip155:eoa:0x123", "eip155:eoa:0x456"],
},
"legacy-account-GROUP-two": {
"id": "legacy-account-GROUP-two",
"accounts": ["eip155:eoa:0x456"],
},
},
}

# Test user with both new and legacy groups
alice := getUser("test-alice-uid") with data.entities as entities

alice == {
"id": "test-alice-uid",
"role": "admin",
"groups": {
"shared-GROUP-one",
"user-only-GROUP-one",
"legacy-user-GROUP-one",
"legacy-user-GROUP-migrated",
},
}

# Test user with only legacy groups, but still has a membership in a migrated group
bob := getUser("test-bob-uid") with data.entities as entities
bob == {
"id": "test-bob-uid",
"role": "user",
"groups": {
"legacy-user-GROUP-one",
"legacy-user-GROUP-two",
"legacy-user-GROUP-migrated",
},
}

# # Test account with both new and legacy groups
account1 := getAccount("eip155:eoa:0x123") with data.entities as entities
account1 == {
"id": "eip155:eoa:0x123",
"accountType": "eoa",
"address": "0x123",
"assignees": ["test-alice-uid", "test-bob-uid"],
"groups": {
"shared-GROUP-one",
"legacy-account-GROUP-one",
"legacy-user-GROUP-migrated",
},
}

# # Test account with only legacy groups
account2 := getAccount("eip155:eoa:0x456") with data.entities as entities
account2 == {
"id": "eip155:eoa:0x456",
"accountType": "eoa",
"address": "0x456",
"assignees": ["test-charlie-uid"],
"groups": {
"legacy-account-GROUP-one",
"legacy-account-GROUP-two",
},
}
}
Loading

0 comments on commit c8c34c4

Please sign in to comment.