Skip to content

Commit

Permalink
Must check group deletion both up and down the hierarchy chain. Add t…
Browse files Browse the repository at this point in the history 
…ests for the GetAllGroupMemberships function. Switch to the ElementsMatch assertion since the order in the slice isn't important
  • Loading branch information
@jacobsee
jacobsee committed Mar 23, 2024
1 parent 59644bd commit c3618b2
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 25 deletions.
12 changes: 6 additions & 6 deletions internal/dbtools/membership_enumeration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ const (
FROM
membership_query AS a
INNER JOIN group_hierarchies AS b ON a.group_id = b.member_group_id
INNER JOIN groups ON groups.id = b.member_group_id
WHERE groups.deleted_at IS NULL
INNER JOIN groups as parentgroup ON parentgroup.id = b.parent_group_id AND parentgroup.deleted_at IS NULL
INNER JOIN groups as membergroup ON membergroup.id = b.member_group_id AND membergroup.deleted_at IS NULL
)
SELECT
group_id,
Expand Down Expand Up @@ -92,8 +92,8 @@ const (
TRUE AS direct
FROM
group_memberships
INNER JOIN groups ON groups.id = group_memberships.group_id
WHERE user_id = $1 AND groups.deleted_at IS NULL
INNER JOIN groups ON groups.id = group_memberships.group_id
WHERE user_id = $1 AND groups.deleted_at IS NULL
UNION ALL
SELECT
b.parent_group_id,
Expand All @@ -105,8 +105,8 @@ const (
FROM
membership_query AS a
INNER JOIN group_hierarchies AS b ON a.group_id = b.member_group_id
INNER JOIN groups ON groups.id = b.member_group_id
WHERE groups.deleted_at IS NULL
INNER JOIN groups as parentgroup ON parentgroup.id = b.parent_group_id AND parentgroup.deleted_at IS NULL
INNER JOIN groups as membergroup ON membergroup.id = b.member_group_id AND membergroup.deleted_at IS NULL
)
SELECT
group_id,
Expand Down
140 changes: 121 additions & 19 deletions internal/dbtools/membership_enumeration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,87 @@ func TestServerRunning(t *testing.T) {
assert.Equal(t, c, 5)
}

func TestGetAllGroupMemberships(t *testing.T) {
expect := []EnumeratedMembership{
{
UserID: "00000001-0000-0000-0000-000000000001",
GroupID: "00000002-0000-0000-0000-000000000001",
IsAdmin: true,
Direct: true,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000002",
GroupID: "00000002-0000-0000-0000-000000000002",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000002",
GroupID: "00000002-0000-0000-0000-000000000001",
IsAdmin: false,
Direct: false,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000003",
GroupID: "00000002-0000-0000-0000-000000000003",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000003",
GroupID: "00000002-0000-0000-0000-000000000002",
IsAdmin: false,
Direct: false,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000003",
GroupID: "00000002-0000-0000-0000-000000000001",
IsAdmin: false,
Direct: false,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000004",
GroupID: "00000002-0000-0000-0000-000000000001",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000004",
GroupID: "00000002-0000-0000-0000-000000000003",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000004",
GroupID: "00000002-0000-0000-0000-000000000002",
IsAdmin: false,
Direct: false,
ExpiresAt: null.Time{},
},
{
UserID: "00000001-0000-0000-0000-000000000005",
GroupID: "00000002-0000-0000-0000-000000000005",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
}

allMemberships, err := GetAllGroupMemberships(context.TODO(), db, false)

if assert.NoError(t, err) {
assert.True(t, assert.ElementsMatch(t, allMemberships, expect))
}
}

func TestGetMembershipsForUser(t *testing.T) {
testCases := map[string][]EnumeratedMembership{
"00000001-0000-0000-0000-000000000001": {
Expand Down Expand Up @@ -127,15 +208,23 @@ func TestGetMembershipsForUser(t *testing.T) {
ExpiresAt: null.Time{},
},
},
"00000001-0000-0000-0000-000000000005": {},
"00000001-0000-0000-0000-000000000005": {
{
UserID: "00000001-0000-0000-0000-000000000005",
GroupID: "00000002-0000-0000-0000-000000000005",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
},
}

for user, expect := range testCases {
t.Run(fmt.Sprintf("groups for user: %s", user), func(t *testing.T) {
enumeratedMemberships, err := GetMembershipsForUser(context.TODO(), db, user, false)

if assert.NoError(t, err) {
assert.True(t, assert.ObjectsAreEqualValues(enumeratedMemberships, expect))
assert.True(t, assert.ElementsMatch(t, enumeratedMemberships, expect))
}
})
}
Expand Down Expand Up @@ -212,14 +301,23 @@ func TestGetMembersOfGroup(t *testing.T) {
ExpiresAt: null.Time{},
},
},
"00000002-0000-0000-0000-000000000005": {
{
UserID: "00000001-0000-0000-0000-000000000005",
GroupID: "00000002-0000-0000-0000-000000000005",
IsAdmin: false,
Direct: true,
ExpiresAt: null.Time{},
},
},
}

for user, expect := range testCases {
t.Run(fmt.Sprintf("users for group: %s", user), func(t *testing.T) {
enumeratedMemberships, err := GetMembersOfGroup(context.TODO(), db, user, false)

if assert.NoError(t, err) {
assert.True(t, assert.ObjectsAreEqualValues(enumeratedMemberships, expect))
assert.True(t, assert.ElementsMatch(t, enumeratedMemberships, expect))
}
})
}
Expand Down Expand Up @@ -253,22 +351,24 @@ func TestHierarchyWouldCreateCycle(t *testing.T) {

// nolint:all
// Sets this up:
// ┌──────┐
// ┌───┤Group1│
// │ └─┬─┬──┘
// ▼ │ │
// ┌──────┐ │ ▼
// ┌─────────────────┬───┤Group2│ │User1
// │ │ └───┬──┘ │
// ▼ ▼ │ │
// ┌────────────────┐ ┌──────┐ ▼ │
// │Group4 (Deleted)│ │Group3│ User2 │
// └───┬────────┬───┘ └┬──┬──┘ │
// │ │ │ │ │
// ▼ ▼ │ ▼ │
// ┌──────┐ User5 │ User3 ▼
// │Group5│ └────────────► User4
// └──────┘
// ┌──────┐
// ┌───┤Group1│
// │ └─┬─┬──┘
// ▼ │ │
// ┌──────┐ │ ▼
// ┌─────────────────┬───┤Group2│ │User1
// │ │ └───┬──┘ │
// ▼ ▼ │ │
// ┌────────────────┐ ┌──────┐ ▼ │
// │Group4 (Deleted)│ │Group3│ User2 │
// └───┬────────┬───┘ └┬──┬──┘ │
// │ │ │ │ │
// ▼ │ │ ▼ │
// ┌──────┐ │ │ User3 ▼
// │Group5│ │ └────────────► User4
// └───┬──┘ │
// │ ▼
// └────► User5

func seedTestDB(db *sql.DB) error {
testData := []string{
Expand Down Expand Up @@ -306,6 +406,8 @@ func seedTestDB(db *sql.DB) error {
('00000003-0000-0000-0000-000000000005', '00000002-0000-0000-0000-000000000003', '00000001-0000-0000-0000-000000000004', 'f', '2023-07-12 12:00:00.000000+00', '2023-07-12 12:00:00.000000+00', NULL);`,
`INSERT INTO "group_memberships" ("id", "group_id", "user_id", "is_admin", "created_at", "updated_at", "expires_at") VALUES
('00000003-0000-0000-0000-000000000006', '00000002-0000-0000-0000-000000000004', '00000001-0000-0000-0000-000000000005', 'f', '2023-07-12 12:00:00.000000+00', '2023-07-12 12:00:00.000000+00', NULL);`,
`INSERT INTO "group_memberships" ("id", "group_id", "user_id", "is_admin", "created_at", "updated_at", "expires_at") VALUES
('00000003-0000-0000-0000-000000000007', '00000002-0000-0000-0000-000000000005', '00000001-0000-0000-0000-000000000005', 'f', '2023-07-12 12:00:00.000000+00', '2023-07-12 12:00:00.000000+00', NULL);`,

`INSERT INTO "group_hierarchies" ("id", "parent_group_id", "member_group_id", "created_at", "updated_at", "expires_at") VALUES
('00000004-0000-0000-0000-000000000001', '00000002-0000-0000-0000-000000000001', '00000002-0000-0000-0000-000000000002', '2023-07-12 12:00:00.000000+00', '2023-07-12 12:00:00.000000+00', NULL);`,
Expand Down

0 comments on commit c3618b2

Please sign in to comment.