Skip to content

Commit dda1b4f

Browse files
EmyrkEmyrk
committed
cleanup
1 parent 1c99808 commit dda1b4f

File tree

16 files changed

+70
-56
lines changed

16 files changed

+70
-56
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,12 @@ var (
165165
Site: rbac.Permissions(map[string][]policy.Action{
166166
// TODO: Add ProvisionerJob resource type.
167167
rbac.ResourceFile.Type: {policy.ActionRead},
168-
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
168+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
169169
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
170170
// Unsure why provisionerd needs update and read personal
171171
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
172172
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild},
173-
rbac.ResourceApiKey.Type: {rbac.WildcardSymbol},
173+
rbac.ResourceApiKey.Type: {policy.WildcardSymbol},
174174
// When org scoped provisioner credentials are implemented,
175175
// this can be reduced to read a specific org.
176176
rbac.ResourceOrganization.Type: {policy.ActionRead},
@@ -191,7 +191,7 @@ var (
191191
Name: "autostart",
192192
DisplayName: "Autostart Daemon",
193193
Site: rbac.Permissions(map[string][]policy.Action{
194-
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
194+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
195195
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
196196
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceBuild},
197197
rbac.ResourceUser.Type: {policy.ActionRead},
@@ -212,7 +212,7 @@ var (
212212
Name: "hangdetector",
213213
DisplayName: "Hang Detector Daemon",
214214
Site: rbac.Permissions(map[string][]policy.Action{
215-
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
215+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
216216
rbac.ResourceTemplate.Type: {policy.ActionRead},
217217
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate},
218218
}),
@@ -234,10 +234,11 @@ var (
234234
rbac.ResourceWildcard.Type: {policy.ActionRead},
235235
rbac.ResourceApiKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
236236
rbac.ResourceGroup.Type: {policy.ActionCreate, policy.ActionUpdate},
237-
rbac.ResourceAssignRole.Type: {policy.ActionCreate, policy.ActionDelete},
238-
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
237+
rbac.ResourceAssignRole.Type: rbac.ResourceAssignRole.AvailableActions(),
238+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
239239
rbac.ResourceOrganization.Type: {policy.ActionCreate, policy.ActionRead},
240240
rbac.ResourceOrganizationMember.Type: {policy.ActionCreate},
241+
rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionDelete},
241242
rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionUpdate},
242243
rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(),
243244
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild, policy.ActionSSH},
@@ -607,7 +608,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
607608
}
608609

609610
if len(added) > 0 {
610-
if err := q.authorizeContext(ctx, policy.ActionCreate, roleAssign); err != nil {
611+
if err := q.authorizeContext(ctx, policy.ActionAssign, roleAssign); err != nil {
611612
return err
612613
}
613614
}
@@ -1775,7 +1776,7 @@ func (q *querier) GetUserActivityInsights(ctx context.Context, arg database.GetU
17751776
return nil, err
17761777
}
17771778

1778-
if err := q.authorizeContext(ctx, policy.ActionViewInsights, template.RBACObject()); err != nil {
1779+
if err := q.authorizeContext(ctx, policy.ActionViewInsights, template); err != nil {
17791780
return nil, err
17801781
}
17811782
}

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ func (s *MethodTestSuite) TestOrganization() {
636636
UserID: u.ID,
637637
Roles: []string{rbac.RoleOrgAdmin(o.ID)},
638638
}).Asserts(
639-
rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionCreate,
639+
rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionAssign,
640640
rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID), policy.ActionCreate)
641641
}))
642642
s.Run("UpdateMemberRoles", s.Subtest(func(db database.Store, check *expects) {
@@ -656,7 +656,7 @@ func (s *MethodTestSuite) TestOrganization() {
656656
OrgID: o.ID,
657657
}).Asserts(
658658
mem, policy.ActionRead,
659-
rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionCreate, // org-mem
659+
rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionAssign, // org-mem
660660
rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionDelete, // org-admin
661661
).Returns(out)
662662
}))
@@ -1023,7 +1023,7 @@ func (s *MethodTestSuite) TestUser() {
10231023
check.Args(database.InsertUserParams{
10241024
ID: uuid.New(),
10251025
LoginType: database.LoginTypePassword,
1026-
}).Asserts(rbac.ResourceAssignRole, policy.ActionCreate, rbac.ResourceUser, policy.ActionCreate)
1026+
}).Asserts(rbac.ResourceAssignRole, policy.ActionAssign, rbac.ResourceUser, policy.ActionCreate)
10271027
}))
10281028
s.Run("InsertUserLink", s.Subtest(func(db database.Store, check *expects) {
10291029
u := dbgen.User(s.T(), db, database.User{})
@@ -1158,7 +1158,7 @@ func (s *MethodTestSuite) TestUser() {
11581158
ID: u.ID,
11591159
}).Asserts(
11601160
u, policy.ActionRead,
1161-
rbac.ResourceAssignRole, policy.ActionCreate,
1161+
rbac.ResourceAssignRole, policy.ActionAssign,
11621162
rbac.ResourceAssignRole, policy.ActionDelete,
11631163
).Returns(o)
11641164
}))

‎coderd/rbac/authz_internal_test.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/coder/coder/v2/coderd/rbac/policy"
1717
"github.com/coder/coder/v2/coderd/rbac/regosql"
18+
"github.com/coder/coder/v2/coderd/util/slice"
1819
"github.com/coder/coder/v2/testutil"
1920
)
2021

@@ -310,7 +311,7 @@ func TestAuthorizeDomain(t *testing.T) {
310311
},
311312
{
312313
resource: ResourceWorkspace.WithOwner(unuseID.String()).InOrg(unuseID).WithACLUserList(map[string][]policy.Action{
313-
user.ID: {WildcardSymbol},
314+
user.ID: {policy.WildcardSymbol},
314315
}),
315316
actions: ResourceWorkspace.AvailableActions(),
316317
allow: true,
@@ -342,7 +343,7 @@ func TestAuthorizeDomain(t *testing.T) {
342343
},
343344
{
344345
resource: ResourceWorkspace.WithOwner(unuseID.String()).InOrg(defOrg).WithGroupACL(map[string][]policy.Action{
345-
allUsersGroup: {WildcardSymbol},
346+
allUsersGroup: {policy.WildcardSymbol},
346347
}),
347348
actions: ResourceWorkspace.AvailableActions(),
348349
allow: true,
@@ -398,8 +399,8 @@ func TestAuthorizeDomain(t *testing.T) {
398399
Site: []Permission{
399400
{
400401
Negate: true,
401-
ResourceType: WildcardSymbol,
402-
Action: WildcardSymbol,
402+
ResourceType: policy.WildcardSymbol,
403+
Action: policy.WildcardSymbol,
403404
},
404405
},
405406
}},
@@ -439,10 +440,13 @@ func TestAuthorizeDomain(t *testing.T) {
439440
},
440441
}
441442

443+
workspaceExceptConnect := slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH)
444+
workspaceConnect := []policy.Action{policy.ActionApplicationConnect, policy.ActionSSH}
442445
testAuthorize(t, "OrgAdmin", user, []authTestCase{
443446
// Org + me
444447
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
445-
{resource: ResourceWorkspace.InOrg(defOrg), actions: ResourceWorkspace.AvailableActions(), allow: true},
448+
{resource: ResourceWorkspace.InOrg(defOrg), actions: workspaceExceptConnect, allow: true},
449+
{resource: ResourceWorkspace.InOrg(defOrg), actions: workspaceConnect, allow: false},
446450

447451
{resource: ResourceWorkspace.WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
448452

@@ -453,7 +457,8 @@ func TestAuthorizeDomain(t *testing.T) {
453457
{resource: ResourceWorkspace.InOrg(unuseID), actions: ResourceWorkspace.AvailableActions(), allow: false},
454458

455459
// Other org + other user
456-
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: true},
460+
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: workspaceExceptConnect, allow: true},
461+
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: workspaceConnect, allow: false},
457462

458463
{resource: ResourceWorkspace.WithOwner("not-me"), actions: ResourceWorkspace.AvailableActions(), allow: false},
459464

@@ -713,8 +718,8 @@ func TestAuthorizeLevels(t *testing.T) {
713718
User: []Permission{
714719
{
715720
Negate: true,
716-
ResourceType: WildcardSymbol,
717-
Action: WildcardSymbol,
721+
ResourceType: policy.WildcardSymbol,
722+
Action: policy.WildcardSymbol,
718723
},
719724
},
720725
},
@@ -761,7 +766,7 @@ func TestAuthorizeLevels(t *testing.T) {
761766
{
762767
Negate: true,
763768
ResourceType: "random",
764-
Action: WildcardSymbol,
769+
Action: policy.WildcardSymbol,
765770
},
766771
},
767772
},
@@ -772,8 +777,8 @@ func TestAuthorizeLevels(t *testing.T) {
772777
User: []Permission{
773778
{
774779
Negate: true,
775-
ResourceType: WildcardSymbol,
776-
Action: WildcardSymbol,
780+
ResourceType: policy.WildcardSymbol,
781+
Action: policy.WildcardSymbol,
777782
},
778783
},
779784
},
@@ -782,7 +787,8 @@ func TestAuthorizeLevels(t *testing.T) {
782787

783788
testAuthorize(t, "OrgAllowAll", user,
784789
cases(func(c authTestCase) authTestCase {
785-
c.actions = ResourceWorkspace.AvailableActions()
790+
// SSH and app connect are not implied here.
791+
c.actions = slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH)
786792
return c
787793
}, []authTestCase{
788794
// Org + me

‎coderd/rbac/object.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import (
88
"github.com/coder/coder/v2/coderd/rbac/policy"
99
)
1010

11-
const WildcardSymbol = "*"
12-
1311
// ResourceUserObject is a helper function to create a user object for authz checks.
1412
func ResourceUserObject(userID uuid.UUID) Object {
1513
return ResourceUser.WithID(userID).WithOwner(userID.String())

‎coderd/rbac/policy/policy.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,8 @@ var RBACPermissions = map[string]PermissionDefinition{
7575
ActionUpdate: actDef("update an existing user"),
7676
ActionDelete: actDef("delete an existing user"),
7777

78-
ActionReadPersonal: actDef("read personal user data like password"),
78+
ActionReadPersonal: actDef("read personal user data like user settings and auth links"),
7979
ActionUpdatePersonal: actDef("update personal data"),
80-
// ActionReadPublic: actDef(fieldOwner, "read public user data"),
8180
},
8281
},
8382
"workspace": {
@@ -168,7 +167,7 @@ var RBACPermissions = map[string]PermissionDefinition{
168167
Actions: map[Action]ActionDefinition{
169168
ActionCreate: actDef("create an organization member"),
170169
ActionRead: actDef("read member"),
171-
ActionUpdate: actDef("update a organization member"),
170+
ActionUpdate: actDef("update an organization member"),
172171
ActionDelete: actDef("delete member"),
173172
},
174173
},

‎coderd/rbac/roles.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func allPermsExcept(excepts ...Objecter) []Permission {
9292
perms = append(perms, Permission{
9393
Negate: false,
9494
ResourceType: r.RBACObject().Type,
95-
Action: WildcardSymbol,
95+
Action: policy.WildcardSymbol,
9696
})
9797
}
9898
return perms

‎coderd/rbac/roles_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ func TestOwnerExec(t *testing.T) {
5656
})
5757
}
5858

59+
// nolint:tparallel,paralleltest -- subtests share a map, just run sequentially.
5960
func TestRolePermissions(t *testing.T) {
6061
t.Parallel()
6162

@@ -523,9 +524,10 @@ func TestRolePermissions(t *testing.T) {
523524
}
524525

525526
passed := true
527+
// nolint:tparallel,paralleltest
526528
for _, c := range testCases {
527529
c := c
528-
// nolint:tparallel -- These share the same remainingPermissions map
530+
// nolint:tparallel,paralleltest -- These share the same remainingPermissions map
529531
t.Run(c.Name, func(t *testing.T) {
530532
remainingSubjs := make(map[string]struct{})
531533
for _, subj := range requiredSubjects {
@@ -568,7 +570,7 @@ func TestRolePermissions(t *testing.T) {
568570
// Only run these if the tests on top passed. Otherwise, the error output is too noisy.
569571
if passed {
570572
for rtype, v := range remainingPermissions {
571-
// nolint:tparallel -- Making a subtest for easier diagnosing failures.
573+
// nolint:tparallel,paralleltest -- Making a subtest for easier diagnosing failures.
572574
t.Run(fmt.Sprintf("%s-AllActions", rtype), func(t *testing.T) {
573575
if len(v) > 0 {
574576
assert.Equal(t, map[policy.Action]bool{}, v, "remaining permissions should be empty for type %q", rtype)

‎coderd/rbac/scopes.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ var builtinScopes = map[ScopeName]Scope{
6161
Name: fmt.Sprintf("Scope_%s", ScopeAll),
6262
DisplayName: "All operations",
6363
Site: Permissions(map[string][]policy.Action{
64-
ResourceWildcard.Type: {WildcardSymbol},
64+
ResourceWildcard.Type: {policy.WildcardSymbol},
6565
}),
6666
Org: map[string][]Permission{},
6767
User: []Permission{},
6868
},
69-
AllowIDList: []string{WildcardSymbol},
69+
AllowIDList: []string{policy.WildcardSymbol},
7070
},
7171

7272
ScopeApplicationConnect: {
@@ -79,7 +79,7 @@ var builtinScopes = map[ScopeName]Scope{
7979
Org: map[string][]Permission{},
8080
User: []Permission{},
8181
},
82-
AllowIDList: []string{WildcardSymbol},
82+
AllowIDList: []string{policy.WildcardSymbol},
8383
},
8484
}
8585

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -541,32 +541,31 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
541541
appTokenAPIClient.HTTPClient.Transport = appDetails.SDKClient.HTTPClient.Transport
542542

543543
var (
544-
canCreateApplicationConnect = "can-create-application_connect"
545-
canReadUserMe = "can-read-user-me"
544+
canApplicationConnect = "can-create-application_connect"
545+
canReadUserMe = "can-read-user-me"
546546
)
547547
authRes, err := appTokenAPIClient.AuthCheck(ctx, codersdk.AuthorizationRequest{
548548
Checks: map[string]codersdk.AuthorizationCheck{
549-
canCreateApplicationConnect: {
549+
canApplicationConnect: {
550550
Object: codersdk.AuthorizationObject{
551-
ResourceType: "application_connect",
552-
OwnerID: "me",
551+
ResourceType: "workspace",
552+
OwnerID: appDetails.FirstUser.UserID.String(),
553553
OrganizationID: appDetails.FirstUser.OrganizationID.String(),
554554
},
555-
Action: "create",
555+
Action: codersdk.ActionApplicationConnect,
556556
},
557557
canReadUserMe: {
558558
Object: codersdk.AuthorizationObject{
559559
ResourceType: "user",
560-
OwnerID: "me",
561560
ResourceID: appDetails.FirstUser.UserID.String(),
562561
},
563-
Action: "read",
562+
Action: codersdk.ActionRead,
564563
},
565564
},
566565
})
567566
require.NoError(t, err)
568567

569-
require.True(t, authRes[canCreateApplicationConnect])
568+
require.True(t, authRes[canApplicationConnect])
570569
require.False(t, authRes[canReadUserMe])
571570

572571
// Load the application page with the API key set.

‎codersdk/authorization.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type AuthorizationCheck struct {
3232
// Omitting the 'OrganizationID' could produce the incorrect value, as
3333
// workspaces have both `user` and `organization` owners.
3434
Object AuthorizationObject `json:"object"`
35-
Action string `json:"action" enums:"create,read,update,delete"`
35+
Action RBACAction `json:"action" enums:"create,read,update,delete"`
3636
}
3737

3838
// AuthorizationObject can represent a "set" of objects, such as: all workspaces in an organization, all workspaces owned by me,

‎enterprise/coderd/authorize_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestCheckACLPermissions(t *testing.T) {
5959
ResourceType: codersdk.ResourceTemplate,
6060
ResourceID: template.ID.String(),
6161
},
62-
Action: "write",
62+
Action: codersdk.ActionUpdate,
6363
},
6464
}
6565

‎enterprise/coderd/coderd_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ func testDBAuthzRole(ctx context.Context) context.Context {
500500
Name: "testing",
501501
DisplayName: "Unit Tests",
502502
Site: rbac.Permissions(map[string][]policy.Action{
503-
rbac.ResourceWildcard.Type: {rbac.WildcardSymbol},
503+
rbac.ResourceWildcard.Type: {policy.WildcardSymbol},
504504
}),
505505
Org: map[string][]rbac.Permission{},
506506
User: []rbac.Permission{},

‎enterprise/coderd/templates.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/coder/coder/v2/coderd/database/dbauthz"
1616
"github.com/coder/coder/v2/coderd/httpapi"
1717
"github.com/coder/coder/v2/coderd/httpmw"
18-
"github.com/coder/coder/v2/coderd/rbac"
1918
"github.com/coder/coder/v2/coderd/rbac/policy"
2019
"github.com/coder/coder/v2/codersdk"
2120
)
@@ -320,7 +319,7 @@ func convertToTemplateRole(actions []policy.Action) codersdk.TemplateRole {
320319
func convertSDKTemplateRole(role codersdk.TemplateRole) []policy.Action {
321320
switch role {
322321
case codersdk.TemplateRoleAdmin:
323-
return []policy.Action{rbac.WildcardSymbol}
322+
return []policy.Action{policy.WildcardSymbol}
324323
case codersdk.TemplateRoleUse:
325324
return []policy.Action{policy.ActionRead}
326325
}

‎enterprise/tailnet/pgcoord.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ var pgCoordSubject = rbac.Subject{
103103
Name: "tailnetcoordinator",
104104
DisplayName: "Tailnet Coordinator",
105105
Site: rbac.Permissions(map[string][]policy.Action{
106-
rbac.ResourceTailnetCoordinator.Type: {rbac.WildcardSymbol},
106+
rbac.ResourceTailnetCoordinator.Type: {policy.WildcardSymbol},
107107
}),
108108
Org: map[string][]rbac.Permission{},
109109
User: []rbac.Permission{},

0 commit comments

Comments
 (0)