Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit839bb9a

Browse files
committed
fixup tests and include migrations
1 parent8b9dfc9 commit839bb9a

File tree

6 files changed

+223
-110
lines changed

6 files changed

+223
-110
lines changed

‎coderd/database/dbauthz/customroles_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import (
1919
"github.com/coder/coder/v2/testutil"
2020
)
2121

22-
//TestUpsertCustomRoles verifies creating custom roles cannot escalate permissions.
23-
funcTestUpsertCustomRoles(t*testing.T) {
22+
//TestInsertCustomRoles verifies creating custom roles cannot escalate permissions.
23+
funcTestInsertCustomRoles(t*testing.T) {
2424
t.Parallel()
2525

2626
userID:=uuid.New()
@@ -231,7 +231,7 @@ func TestUpsertCustomRoles(t *testing.T) {
231231
ctx:=testutil.Context(t,testutil.WaitMedium)
232232
ctx=dbauthz.As(ctx,subject)
233233

234-
_,err:=az.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
234+
_,err:=az.InsertCustomRole(ctx, database.InsertCustomRoleParams{
235235
Name:"test-role",
236236
DisplayName:"",
237237
OrganizationID:tc.organizationID,

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 104 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,86 @@ func (q *querier) customRoleEscalationCheck(ctx context.Context, actor rbac.Subj
815815
returnnil
816816
}
817817

818+
// customRoleCheck will validate a custom role for inserting or updating.
819+
// If the role is not valid, an error will be returned.
820+
// - Check custom roles are valid for their resource types + actions
821+
// - Check the actor can create the custom role
822+
// - Check the custom role does not grant perms the actor does not have
823+
// - Prevent negative perms
824+
// - Prevent roles with site and org permissions.
825+
func (q*querier)customRoleCheck(ctx context.Context,role database.CustomRole)error {
826+
act,ok:=ActorFromContext(ctx)
827+
if!ok {
828+
returnNoActorError
829+
}
830+
831+
// Org permissions require an org role
832+
ifrole.OrganizationID.UUID==uuid.Nil&&len(role.OrgPermissions)>0 {
833+
returnxerrors.Errorf("organization permissions require specifying an organization id")
834+
}
835+
836+
// Org roles can only specify org permissions
837+
ifrole.OrganizationID.UUID!=uuid.Nil&& (len(role.SitePermissions)>0||len(role.UserPermissions)>0) {
838+
returnxerrors.Errorf("organization roles specify site or user permissions")
839+
}
840+
841+
// The rbac.Role has a 'Valid()' function on it that will do a lot
842+
// of checks.
843+
rbacRole,err:=rolestore.ConvertDBRole(database.CustomRole{
844+
Name:role.Name,
845+
DisplayName:role.DisplayName,
846+
SitePermissions:role.SitePermissions,
847+
OrgPermissions:role.OrgPermissions,
848+
UserPermissions:role.UserPermissions,
849+
OrganizationID:role.OrganizationID,
850+
})
851+
iferr!=nil {
852+
returnxerrors.Errorf("invalid args: %w",err)
853+
}
854+
855+
err=rbacRole.Valid()
856+
iferr!=nil {
857+
returnxerrors.Errorf("invalid role: %w",err)
858+
}
859+
860+
iflen(rbacRole.Org)>0&&len(rbacRole.Site)>0 {
861+
// This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can
862+
// do what gets more complicated.
863+
returnxerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time")
864+
}
865+
866+
iflen(rbacRole.Org)>1 {
867+
// Again to avoid more complexity in our roles
868+
returnxerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time")
869+
}
870+
871+
// Prevent escalation
872+
for_,sitePerm:=rangerbacRole.Site {
873+
err:=q.customRoleEscalationCheck(ctx,act,sitePerm, rbac.Object{Type:sitePerm.ResourceType})
874+
iferr!=nil {
875+
returnxerrors.Errorf("site permission: %w",err)
876+
}
877+
}
878+
879+
fororgID,perms:=rangerbacRole.Org {
880+
for_,orgPerm:=rangeperms {
881+
err:=q.customRoleEscalationCheck(ctx,act,orgPerm, rbac.Object{OrgID:orgID,Type:orgPerm.ResourceType})
882+
iferr!=nil {
883+
returnxerrors.Errorf("org=%q: %w",orgID,err)
884+
}
885+
}
886+
}
887+
888+
for_,userPerm:=rangerbacRole.User {
889+
err:=q.customRoleEscalationCheck(ctx,act,userPerm, rbac.Object{Type:userPerm.ResourceType,Owner:act.ID})
890+
iferr!=nil {
891+
returnxerrors.Errorf("user permission: %w",err)
892+
}
893+
}
894+
895+
returnnil
896+
}
897+
818898
func (q*querier)AcquireLock(ctx context.Context,idint64)error {
819899
returnq.db.AcquireLock(ctx,id)
820900
}
@@ -2571,86 +2651,6 @@ func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCusto
25712651
returnq.db.InsertCustomRole(ctx,arg)
25722652
}
25732653

2574-
// customRoleCheck will validate a custom role for inserting or updating.
2575-
// If the role is not valid, an error will be returned.
2576-
// - Check custom roles are valid for their resource types + actions
2577-
// - Check the actor can create the custom role
2578-
// - Check the custom role does not grant perms the actor does not have
2579-
// - Prevent negative perms
2580-
// - Prevent roles with site and org permissions.
2581-
func (q*querier)customRoleCheck(ctx context.Context,role database.CustomRole)error {
2582-
act,ok:=ActorFromContext(ctx)
2583-
if!ok {
2584-
returnNoActorError
2585-
}
2586-
2587-
// Org permissions require an org role
2588-
ifrole.OrganizationID.UUID==uuid.Nil&&len(role.OrgPermissions)>0 {
2589-
returnxerrors.Errorf("organization permissions require specifying an organization id")
2590-
}
2591-
2592-
// Org roles can only specify org permissions
2593-
ifrole.OrganizationID.UUID!=uuid.Nil&& (len(role.SitePermissions)>0||len(role.UserPermissions)>0) {
2594-
returnxerrors.Errorf("organization roles specify site or user permissions")
2595-
}
2596-
2597-
// The rbac.Role has a 'Valid()' function on it that will do a lot
2598-
// of checks.
2599-
rbacRole,err:=rolestore.ConvertDBRole(database.CustomRole{
2600-
Name:role.Name,
2601-
DisplayName:role.DisplayName,
2602-
SitePermissions:role.SitePermissions,
2603-
OrgPermissions:role.OrgPermissions,
2604-
UserPermissions:role.UserPermissions,
2605-
OrganizationID:role.OrganizationID,
2606-
})
2607-
iferr!=nil {
2608-
returnxerrors.Errorf("invalid args: %w",err)
2609-
}
2610-
2611-
err=rbacRole.Valid()
2612-
iferr!=nil {
2613-
returnxerrors.Errorf("invalid role: %w",err)
2614-
}
2615-
2616-
iflen(rbacRole.Org)>0&&len(rbacRole.Site)>0 {
2617-
// This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can
2618-
// do what gets more complicated.
2619-
returnxerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time")
2620-
}
2621-
2622-
iflen(rbacRole.Org)>1 {
2623-
// Again to avoid more complexity in our roles
2624-
returnxerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time")
2625-
}
2626-
2627-
// Prevent escalation
2628-
for_,sitePerm:=rangerbacRole.Site {
2629-
err:=q.customRoleEscalationCheck(ctx,act,sitePerm, rbac.Object{Type:sitePerm.ResourceType})
2630-
iferr!=nil {
2631-
returnxerrors.Errorf("site permission: %w",err)
2632-
}
2633-
}
2634-
2635-
fororgID,perms:=rangerbacRole.Org {
2636-
for_,orgPerm:=rangeperms {
2637-
err:=q.customRoleEscalationCheck(ctx,act,orgPerm, rbac.Object{OrgID:orgID,Type:orgPerm.ResourceType})
2638-
iferr!=nil {
2639-
returnxerrors.Errorf("org=%q: %w",orgID,err)
2640-
}
2641-
}
2642-
}
2643-
2644-
for_,userPerm:=rangerbacRole.User {
2645-
err:=q.customRoleEscalationCheck(ctx,act,userPerm, rbac.Object{Type:userPerm.ResourceType,Owner:act.ID})
2646-
iferr!=nil {
2647-
returnxerrors.Errorf("user permission: %w",err)
2648-
}
2649-
}
2650-
2651-
returnnil
2652-
}
2653-
26542654
func (q*querier)InsertDBCryptKey(ctx context.Context,arg database.InsertDBCryptKeyParams)error {
26552655
iferr:=q.authorizeContext(ctx,policy.ActionCreate,rbac.ResourceSystem);err!=nil {
26562656
returnerr
@@ -3103,7 +3103,30 @@ func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKe
31033103
}
31043104

31053105
func (q*querier)UpdateCustomRole(ctx context.Context,arg database.UpdateCustomRoleParams) (database.CustomRole,error) {
3106-
panic("not implemented")
3106+
ifarg.OrganizationID.UUID!=uuid.Nil {
3107+
iferr:=q.authorizeContext(ctx,policy.ActionUpdate,rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID));err!=nil {
3108+
return database.CustomRole{},err
3109+
}
3110+
}else {
3111+
iferr:=q.authorizeContext(ctx,policy.ActionUpdate,rbac.ResourceAssignRole);err!=nil {
3112+
return database.CustomRole{},err
3113+
}
3114+
}
3115+
3116+
iferr:=q.customRoleCheck(ctx, database.CustomRole{
3117+
Name:arg.Name,
3118+
DisplayName:arg.DisplayName,
3119+
SitePermissions:arg.SitePermissions,
3120+
OrgPermissions:arg.OrgPermissions,
3121+
UserPermissions:arg.UserPermissions,
3122+
CreatedAt:time.Now(),
3123+
UpdatedAt:time.Now(),
3124+
OrganizationID:arg.OrganizationID,
3125+
ID:uuid.New(),
3126+
});err!=nil {
3127+
return database.CustomRole{},err
3128+
}
3129+
returnq.db.UpdateCustomRole(ctx,arg)
31073130
}
31083131

31093132
func (q*querier)UpdateExternalAuthLink(ctx context.Context,arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink,error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 75 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,18 +1272,86 @@ func (s *MethodTestSuite) TestUser() {
12721272
}).Asserts(
12731273
rbac.ResourceAssignRole,policy.ActionDelete)
12741274
}))
1275-
s.Run("Blank/UpsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
1275+
s.Run("Blank/UpdateCustomRole",s.Subtest(func(db database.Store,check*expects) {
1276+
customRole:=dbgen.CustomRole(s.T(),db, database.CustomRole{})
12761277
// Blank is no perms in the role
1277-
check.Args(database.UpsertCustomRoleParams{
1278+
check.Args(database.UpdateCustomRoleParams{
1279+
Name:customRole.Name,
1280+
DisplayName:"Test Name",
1281+
SitePermissions:nil,
1282+
OrgPermissions:nil,
1283+
UserPermissions:nil,
1284+
}).Asserts(rbac.ResourceAssignRole,policy.ActionUpdate)
1285+
}))
1286+
s.Run("SitePermissions/UpdateCustomRole",s.Subtest(func(db database.Store,check*expects) {
1287+
customRole:=dbgen.CustomRole(s.T(),db, database.CustomRole{
1288+
OrganizationID: uuid.NullUUID{
1289+
UUID:uuid.Nil,
1290+
Valid:false,
1291+
},
1292+
})
1293+
check.Args(database.UpdateCustomRoleParams{
1294+
Name:customRole.Name,
1295+
OrganizationID:customRole.OrganizationID,
1296+
DisplayName:"Test Name",
1297+
SitePermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
1298+
codersdk.ResourceTemplate: {codersdk.ActionCreate,codersdk.ActionRead,codersdk.ActionUpdate,codersdk.ActionDelete,codersdk.ActionViewInsights},
1299+
}),convertSDKPerm),
1300+
OrgPermissions:nil,
1301+
UserPermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
1302+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
1303+
}),convertSDKPerm),
1304+
}).Asserts(
1305+
// First check
1306+
rbac.ResourceAssignRole,policy.ActionUpdate,
1307+
// Escalation checks
1308+
rbac.ResourceTemplate,policy.ActionCreate,
1309+
rbac.ResourceTemplate,policy.ActionRead,
1310+
rbac.ResourceTemplate,policy.ActionUpdate,
1311+
rbac.ResourceTemplate,policy.ActionDelete,
1312+
rbac.ResourceTemplate,policy.ActionViewInsights,
1313+
1314+
rbac.ResourceWorkspace.WithOwner(testActorID.String()),policy.ActionRead,
1315+
)
1316+
}))
1317+
s.Run("OrgPermissions/UpdateCustomRole",s.Subtest(func(db database.Store,check*expects) {
1318+
orgID:=uuid.New()
1319+
customRole:=dbgen.CustomRole(s.T(),db, database.CustomRole{
1320+
OrganizationID: uuid.NullUUID{
1321+
UUID:orgID,
1322+
Valid:true,
1323+
},
1324+
})
1325+
1326+
check.Args(database.UpdateCustomRoleParams{
1327+
Name:customRole.Name,
1328+
DisplayName:"Test Name",
1329+
OrganizationID:customRole.OrganizationID,
1330+
SitePermissions:nil,
1331+
OrgPermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
1332+
codersdk.ResourceTemplate: {codersdk.ActionCreate,codersdk.ActionRead},
1333+
}),convertSDKPerm),
1334+
UserPermissions:nil,
1335+
}).Asserts(
1336+
// First check
1337+
rbac.ResourceAssignOrgRole.InOrg(orgID),policy.ActionUpdate,
1338+
// Escalation checks
1339+
rbac.ResourceTemplate.InOrg(orgID),policy.ActionCreate,
1340+
rbac.ResourceTemplate.InOrg(orgID),policy.ActionRead,
1341+
)
1342+
}))
1343+
s.Run("Blank/InsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
1344+
// Blank is no perms in the role
1345+
check.Args(database.InsertCustomRoleParams{
12781346
Name:"test",
12791347
DisplayName:"Test Name",
12801348
SitePermissions:nil,
12811349
OrgPermissions:nil,
12821350
UserPermissions:nil,
12831351
}).Asserts(rbac.ResourceAssignRole,policy.ActionCreate)
12841352
}))
1285-
s.Run("SitePermissions/UpsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
1286-
check.Args(database.UpsertCustomRoleParams{
1353+
s.Run("SitePermissions/InsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
1354+
check.Args(database.InsertCustomRoleParams{
12871355
Name:"test",
12881356
DisplayName:"Test Name",
12891357
SitePermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
@@ -1306,9 +1374,9 @@ func (s *MethodTestSuite) TestUser() {
13061374
rbac.ResourceWorkspace.WithOwner(testActorID.String()),policy.ActionRead,
13071375
)
13081376
}))
1309-
s.Run("OrgPermissions/UpsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
1377+
s.Run("OrgPermissions/InsertCustomRole",s.Subtest(func(db database.Store,check*expects) {
13101378
orgID:=uuid.New()
1311-
check.Args(database.UpsertCustomRoleParams{
1379+
check.Args(database.InsertCustomRoleParams{
13121380
Name:"test",
13131381
DisplayName:"Test Name",
13141382
OrganizationID: uuid.NullUUID{
@@ -1319,17 +1387,13 @@ func (s *MethodTestSuite) TestUser() {
13191387
OrgPermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
13201388
codersdk.ResourceTemplate: {codersdk.ActionCreate,codersdk.ActionRead},
13211389
}),convertSDKPerm),
1322-
UserPermissions:db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
1323-
codersdk.ResourceWorkspace: {codersdk.ActionRead},
1324-
}),convertSDKPerm),
1390+
UserPermissions:nil,
13251391
}).Asserts(
13261392
// First check
13271393
rbac.ResourceAssignOrgRole.InOrg(orgID),policy.ActionCreate,
13281394
// Escalation checks
13291395
rbac.ResourceTemplate.InOrg(orgID),policy.ActionCreate,
13301396
rbac.ResourceTemplate.InOrg(orgID),policy.ActionRead,
1331-
1332-
rbac.ResourceWorkspace.WithOwner(testActorID.String()),policy.ActionRead,
13331397
)
13341398
}))
13351399
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp