Skip to content

Commit e2b330f

Browse files
EmyrkEmyrk
authored
chore: change sql parameter for custom roles to be a (name,org_id) tuple (#13480)
* chore: sql parameter to custom roles to be a (name,org) tuple CustomRole lookup takes (name,org_id) tuples as the search criteria.
1 parent 1adc19b commit e2b330f

12 files changed

+339
-26
lines changed

‎coderd/database/dbauthz/customroles_test.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestUpsertCustomRoles(t *testing.T) {
3838
Name: "can-assign",
3939
DisplayName: "",
4040
Site: rbac.Permissions(map[string][]policy.Action{
41-
rbac.ResourceAssignRole.Type: {policy.ActionCreate},
41+
rbac.ResourceAssignRole.Type: {policy.ActionRead, policy.ActionCreate},
4242
}),
4343
}
4444

@@ -243,6 +243,20 @@ func TestUpsertCustomRoles(t *testing.T) {
243243
require.ErrorContains(t, err, tc.errorContains)
244244
} else {
245245
require.NoError(t, err)
246+
247+
// Verify we can fetch the role
248+
roles, err := az.CustomRoles(ctx, database.CustomRolesParams{
249+
LookupRoles: []database.NameOrganizationPair{
250+
{
251+
Name: "test-role",
252+
OrganizationID: tc.organizationID.UUID,
253+
},
254+
},
255+
ExcludeOrgRoles: false,
256+
OrganizationID: uuid.UUID{},
257+
})
258+
require.NoError(t, err)
259+
require.Len(t, roles, 1)
246260
}
247261
})
248262
}

‎coderd/database/dbmem/dbmem.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -1187,12 +1187,17 @@ func (q *FakeQuerier) CustomRoles(_ context.Context, arg database.CustomRolesPar
11871187
for _, role := range q.data.customRoles {
11881188
role := role
11891189
if len(arg.LookupRoles) > 0 {
1190-
if !slices.ContainsFunc(arg.LookupRoles, func(s string) bool {
1191-
roleName := rbac.RoleName(role.Name, "")
1192-
if role.OrganizationID.UUID != uuid.Nil {
1193-
roleName = rbac.RoleName(role.Name, role.OrganizationID.UUID.String())
1190+
if !slices.ContainsFunc(arg.LookupRoles, func(pair database.NameOrganizationPair) bool {
1191+
if pair.Name != role.Name {
1192+
return false
11941193
}
1195-
return strings.EqualFold(s, roleName)
1194+
1195+
if role.OrganizationID.Valid {
1196+
// Expect org match
1197+
return role.OrganizationID.UUID == pair.OrganizationID
1198+
}
1199+
// Expect no org
1200+
return pair.OrganizationID == uuid.Nil
11961201
}) {
11971202
continue
11981203
}

‎coderd/database/dbtestutil/postgres.go

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ func Open() (string, func(), error) {
2828
if err != nil {
2929
return "", nil, xerrors.Errorf("connect to ci postgres: %w", err)
3030
}
31+
3132
defer db.Close()
3233

3334
dbName, err := cryptorand.StringCharset(cryptorand.Lower, 10)

‎coderd/database/dump.sql

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000216_custom_role_pair_parameter.down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DROP TYPE name_organization_pair;

‎coderd/database/migrations/000216_custom_role_pair_parameter.up.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CREATE TYPE name_organization_pair AS (name text, organization_id uuid);

‎coderd/database/querier_test.go

+230
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"database/sql"
88
"encoding/json"
9+
"fmt"
910
"sort"
1011
"testing"
1112
"time"
@@ -14,6 +15,7 @@ import (
1415
"github.com/stretchr/testify/require"
1516

1617
"github.com/coder/coder/v2/coderd/database"
18+
"github.com/coder/coder/v2/coderd/database/db2sdk"
1719
"github.com/coder/coder/v2/coderd/database/dbgen"
1820
"github.com/coder/coder/v2/coderd/database/dbtime"
1921
"github.com/coder/coder/v2/coderd/database/migrations"
@@ -514,6 +516,234 @@ func TestDefaultOrg(t *testing.T) {
514516
require.True(t, all[0].IsDefault, "first org should always be default")
515517
}
516518

519+
// TestReadCustomRoles tests the input params returns the correct set of roles.
520+
func TestReadCustomRoles(t *testing.T) {
521+
t.Parallel()
522+
523+
if testing.Short() {
524+
t.SkipNow()
525+
}
526+
527+
sqlDB := testSQLDB(t)
528+
err := migrations.Up(sqlDB)
529+
require.NoError(t, err)
530+
531+
db := database.New(sqlDB)
532+
ctx := testutil.Context(t, testutil.WaitLong)
533+
534+
// Make a few site roles, and a few org roles
535+
orgIDs := make([]uuid.UUID, 3)
536+
for i := range orgIDs {
537+
orgIDs[i] = uuid.New()
538+
}
539+
540+
allRoles := make([]database.CustomRole, 0)
541+
siteRoles := make([]database.CustomRole, 0)
542+
orgRoles := make([]database.CustomRole, 0)
543+
for i := 0; i < 15; i++ {
544+
orgID := uuid.NullUUID{
545+
UUID: orgIDs[i%len(orgIDs)],
546+
Valid: true,
547+
}
548+
if i%4 == 0 {
549+
// Some should be site wide
550+
orgID = uuid.NullUUID{}
551+
}
552+
553+
role, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
554+
Name: fmt.Sprintf("role-%d", i),
555+
OrganizationID: orgID,
556+
})
557+
require.NoError(t, err)
558+
allRoles = append(allRoles, role)
559+
if orgID.Valid {
560+
orgRoles = append(orgRoles, role)
561+
} else {
562+
siteRoles = append(siteRoles, role)
563+
}
564+
}
565+
566+
// normalizedRoleName allows for the simple ElementsMatch to work properly.
567+
normalizedRoleName := func(role database.CustomRole) string {
568+
return role.Name + ":" + role.OrganizationID.UUID.String()
569+
}
570+
571+
roleToLookup := func(role database.CustomRole) database.NameOrganizationPair {
572+
return database.NameOrganizationPair{
573+
Name: role.Name,
574+
OrganizationID: role.OrganizationID.UUID,
575+
}
576+
}
577+
578+
testCases := []struct {
579+
Name string
580+
Params database.CustomRolesParams
581+
Match func(role database.CustomRole) bool
582+
}{
583+
{
584+
Name: "NilRoles",
585+
Params: database.CustomRolesParams{
586+
LookupRoles: nil,
587+
ExcludeOrgRoles: false,
588+
OrganizationID: uuid.UUID{},
589+
},
590+
Match: func(role database.CustomRole) bool {
591+
return true
592+
},
593+
},
594+
{
595+
// Empty params should return all roles
596+
Name: "Empty",
597+
Params: database.CustomRolesParams{
598+
LookupRoles: []database.NameOrganizationPair{},
599+
ExcludeOrgRoles: false,
600+
OrganizationID: uuid.UUID{},
601+
},
602+
Match: func(role database.CustomRole) bool {
603+
return true
604+
},
605+
},
606+
{
607+
Name: "Organization",
608+
Params: database.CustomRolesParams{
609+
LookupRoles: []database.NameOrganizationPair{},
610+
ExcludeOrgRoles: false,
611+
OrganizationID: orgIDs[1],
612+
},
613+
Match: func(role database.CustomRole) bool {
614+
return role.OrganizationID.UUID == orgIDs[1]
615+
},
616+
},
617+
{
618+
Name: "SpecificOrgRole",
619+
Params: database.CustomRolesParams{
620+
LookupRoles: []database.NameOrganizationPair{
621+
{
622+
Name: orgRoles[0].Name,
623+
OrganizationID: orgRoles[0].OrganizationID.UUID,
624+
},
625+
},
626+
},
627+
Match: func(role database.CustomRole) bool {
628+
return role.Name == orgRoles[0].Name && role.OrganizationID.UUID == orgRoles[0].OrganizationID.UUID
629+
},
630+
},
631+
{
632+
Name: "SpecificSiteRole",
633+
Params: database.CustomRolesParams{
634+
LookupRoles: []database.NameOrganizationPair{
635+
{
636+
Name: siteRoles[0].Name,
637+
OrganizationID: siteRoles[0].OrganizationID.UUID,
638+
},
639+
},
640+
},
641+
Match: func(role database.CustomRole) bool {
642+
return role.Name == siteRoles[0].Name && role.OrganizationID.UUID == siteRoles[0].OrganizationID.UUID
643+
},
644+
},
645+
{
646+
Name: "FewSpecificRoles",
647+
Params: database.CustomRolesParams{
648+
LookupRoles: []database.NameOrganizationPair{
649+
{
650+
Name: orgRoles[0].Name,
651+
OrganizationID: orgRoles[0].OrganizationID.UUID,
652+
},
653+
{
654+
Name: orgRoles[1].Name,
655+
OrganizationID: orgRoles[1].OrganizationID.UUID,
656+
},
657+
{
658+
Name: siteRoles[0].Name,
659+
OrganizationID: siteRoles[0].OrganizationID.UUID,
660+
},
661+
},
662+
},
663+
Match: func(role database.CustomRole) bool {
664+
return (role.Name == orgRoles[0].Name && role.OrganizationID.UUID == orgRoles[0].OrganizationID.UUID) ||
665+
(role.Name == orgRoles[1].Name && role.OrganizationID.UUID == orgRoles[1].OrganizationID.UUID) ||
666+
(role.Name == siteRoles[0].Name && role.OrganizationID.UUID == siteRoles[0].OrganizationID.UUID)
667+
},
668+
},
669+
{
670+
Name: "AllRolesByLookup",
671+
Params: database.CustomRolesParams{
672+
LookupRoles: db2sdk.List(allRoles, roleToLookup),
673+
},
674+
Match: func(role database.CustomRole) bool {
675+
return true
676+
},
677+
},
678+
{
679+
Name: "NotExists",
680+
Params: database.CustomRolesParams{
681+
LookupRoles: []database.NameOrganizationPair{
682+
{
683+
Name: "not-exists",
684+
OrganizationID: uuid.New(),
685+
},
686+
{
687+
Name: "not-exists",
688+
OrganizationID: uuid.Nil,
689+
},
690+
},
691+
},
692+
Match: func(role database.CustomRole) bool {
693+
return false
694+
},
695+
},
696+
{
697+
Name: "Mixed",
698+
Params: database.CustomRolesParams{
699+
LookupRoles: []database.NameOrganizationPair{
700+
{
701+
Name: "not-exists",
702+
OrganizationID: uuid.New(),
703+
},
704+
{
705+
Name: "not-exists",
706+
OrganizationID: uuid.Nil,
707+
},
708+
{
709+
Name: orgRoles[0].Name,
710+
OrganizationID: orgRoles[0].OrganizationID.UUID,
711+
},
712+
{
713+
Name: siteRoles[0].Name,
714+
},
715+
},
716+
},
717+
Match: func(role database.CustomRole) bool {
718+
return (role.Name == orgRoles[0].Name && role.OrganizationID.UUID == orgRoles[0].OrganizationID.UUID) ||
719+
(role.Name == siteRoles[0].Name && role.OrganizationID.UUID == siteRoles[0].OrganizationID.UUID)
720+
},
721+
},
722+
}
723+
724+
for _, tc := range testCases {
725+
tc := tc
726+
727+
t.Run(tc.Name, func(t *testing.T) {
728+
t.Parallel()
729+
730+
ctx := testutil.Context(t, testutil.WaitLong)
731+
found, err := db.CustomRoles(ctx, tc.Params)
732+
require.NoError(t, err)
733+
filtered := make([]database.CustomRole, 0)
734+
for _, role := range allRoles {
735+
if tc.Match(role) {
736+
filtered = append(filtered, role)
737+
}
738+
}
739+
740+
a := db2sdk.List(filtered, normalizedRoleName)
741+
b := db2sdk.List(found, normalizedRoleName)
742+
require.Equal(t, a, b)
743+
})
744+
}
745+
}
746+
517747
type tvArgs struct {
518748
Status database.ProvisionerJobStatus
519749
// CreateWorkspace is true if we should create a workspace for the template version

‎coderd/database/queries.sql.go

+11-11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)