From a42e77ce10dc97db06781238958c74ed3a9e7693 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 9 Aug 2024 11:27:35 -0500 Subject: [PATCH 1/4] test: add unit test to verify group permission behavior --- coderd/database/dbauthz/groupsauth_test.go | 167 +++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 coderd/database/dbauthz/groupsauth_test.go diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go new file mode 100644 index 0000000000000..496be268089f5 --- /dev/null +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -0,0 +1,167 @@ +package dbauthz_test + +import ( + "context" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmem" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/rbac" +) + +// nolint:tparallel +func TestGroupsAuth(t *testing.T) { + t.Parallel() + + if dbtestutil.WillUsePostgres() { + t.Skip("this test would take too long to run on postgres") + } + + authz := rbac.NewAuthorizer(prometheus.NewRegistry()) + + db := dbmem.New() + db = dbauthz.New(db, authz, slogtest.Make(t, &slogtest.Options{ + IgnoreErrors: true, + }), coderdtest.AccessControlStorePointer()) + + ownerCtx := dbauthz.As(context.Background(), rbac.Subject{ + ID: "owner", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleOwner()}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }) + + org := dbgen.Organization(t, db, database.Organization{}) + group := dbgen.Group(t, db, database.Group{ + OrganizationID: org.ID, + }) + + var users []database.User + for i := 0; i < 5; i++ { + user := dbgen.User(t, db, database.User{}) + users = append(users, user) + err := db.InsertGroupMember(ownerCtx, database.InsertGroupMemberParams{ + UserID: user.ID, + GroupID: group.ID, + }) + require.NoError(t, err) + } + + totalMembers := len(users) + testCases := []struct { + Name string + Subject rbac.Subject + ReadGroup bool + ReadMembers bool + MembersExpected int + }{ + { + Name: "Owner", + Subject: rbac.Subject{ + ID: "owner", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleOwner()}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + ReadGroup: true, + ReadMembers: true, + MembersExpected: totalMembers, + }, + { + Name: "UserAdmin", + Subject: rbac.Subject{ + ID: "useradmin", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleUserAdmin()}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + ReadGroup: true, + ReadMembers: true, + MembersExpected: totalMembers, + }, + { + Name: "OrgAdmin", + Subject: rbac.Subject{ + ID: "orgadmin", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + ReadGroup: true, + ReadMembers: true, + MembersExpected: totalMembers, + }, + { + Name: "OrgUserAdmin", + Subject: rbac.Subject{ + ID: "orgUserAdmin", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.ScopedRoleOrgUserAdmin(org.ID)}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + ReadGroup: true, + ReadMembers: true, + MembersExpected: totalMembers, + }, + { + Name: "GroupMember", + Subject: rbac.Subject{ + ID: users[0].ID.String(), + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.ScopedRoleOrgMember(org.ID)}.Expand())), + Groups: []string{ + group.Name, + }, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + // TODO: currently group members cannot see their own groups. + // If this is fixed, these booleans should be flipped to true. + ReadGroup: false, + ReadMembers: false, + // TODO: If fixed, they should only be able to see themselves + //MembersExpected: 1, + }, + { + // Org admin in the incorrect organization + Name: "DifferentOrgAdmin", + Subject: rbac.Subject{ + ID: "orgadmin", + Roles: rbac.Roles(must(rbac.RoleIdentifiers{}.Expand())), + Groups: []string{}, + Scope: rbac.ExpandableScope(rbac.ScopeAll), + }, + ReadGroup: false, + ReadMembers: false, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + actorCtx := dbauthz.As(context.Background(), tc.Subject) + _, err := db.GetGroupByID(actorCtx, group.ID) + if tc.ReadGroup { + require.NoError(t, err, "group read") + } else { + require.Error(t, err, "group read") + } + + members, err := db.GetGroupMembersByGroupID(actorCtx, group.ID) + if tc.ReadMembers { + require.NoError(t, err, "member read") + require.Len(t, members, tc.MembersExpected, "member count found does not match") + } else { + require.Error(t, err, "member read") + } + }) + } +} From 0b9711e17a458757fc509b4fbd5f878c2c6db2bd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 9 Aug 2024 12:52:23 -0500 Subject: [PATCH 2/4] fmt --- coderd/database/dbauthz/groupsauth_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go index 496be268089f5..caeff897a39c6 100644 --- a/coderd/database/dbauthz/groupsauth_test.go +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -126,7 +126,7 @@ func TestGroupsAuth(t *testing.T) { ReadGroup: false, ReadMembers: false, // TODO: If fixed, they should only be able to see themselves - //MembersExpected: 1, + // MembersExpected: 1, }, { // Org admin in the incorrect organization From e14728ee862e838d63273e2ec9f887443b80e096 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 12 Aug 2024 08:09:06 -0500 Subject: [PATCH 3/4] Update coderd/database/dbauthz/groupsauth_test.go Co-authored-by: Cian Johnston --- coderd/database/dbauthz/groupsauth_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go index caeff897a39c6..13df48438423e 100644 --- a/coderd/database/dbauthz/groupsauth_test.go +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -27,8 +27,7 @@ func TestGroupsAuth(t *testing.T) { authz := rbac.NewAuthorizer(prometheus.NewRegistry()) - db := dbmem.New() - db = dbauthz.New(db, authz, slogtest.Make(t, &slogtest.Options{ + db := dbauthz.New(dbmem.New(), authz, slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, }), coderdtest.AccessControlStorePointer()) From 80bbcac4e8463445df6802c40fe5a59e875001dd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 12 Aug 2024 08:13:21 -0500 Subject: [PATCH 4/4] pre suggestions --- coderd/database/dbauthz/groupsauth_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go index 13df48438423e..d742e15148fa9 100644 --- a/coderd/database/dbauthz/groupsauth_test.go +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" @@ -132,7 +133,7 @@ func TestGroupsAuth(t *testing.T) { Name: "DifferentOrgAdmin", Subject: rbac.Subject{ ID: "orgadmin", - Roles: rbac.Roles(must(rbac.RoleIdentifiers{}.Expand())), + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.ScopedRoleOrgUserAdmin(uuid.New())}.Expand())), Groups: []string{}, Scope: rbac.ExpandableScope(rbac.ScopeAll), }, @@ -160,6 +161,7 @@ func TestGroupsAuth(t *testing.T) { require.Len(t, members, tc.MembersExpected, "member count found does not match") } else { require.Error(t, err, "member read") + require.True(t, dbauthz.IsNotAuthorizedError(err), "not authorized error") } }) }