Skip to content

Commit 144f374

Browse files
johnstcnjohnstcn
authored
refactor(dbauthz): add authz for system-level functions (coder#6513)
- Introduces rbac.ResourceSystem - Grants system.* to system and provisionerd rbac subjects - Updates dbauthz system queries where applicable - coderd: Avoid index out of bounds in api.workspaceBuilds - dbauthz: move GetUsersByIDs out of system, modify RBAC check to ResourceUser - workspaceapps: Add test case for when owner of app is not found
1 parent 1db2b12 commit 144f374

File tree

17 files changed

+470
-200
lines changed

17 files changed

+470
-200
lines changed

‎coderd/coderd.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,8 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
795795
}()
796796

797797
name := namesgenerator.GetRandomName(1)
798-
daemon, err := api.Database.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{
798+
// nolint:gocritic // Inserting a provisioner daemon is a system function.
799+
daemon, err := api.Database.InsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.InsertProvisionerDaemonParams{
799800
ID: uuid.New(),
800801
CreatedAt: database.Now(),
801802
Name: name,

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -115,17 +115,17 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
115115
return a, ok
116116
}
117117

118-
// AsProvisionerd returns a context with an actor that has permissions required
119-
// for provisionerd to function.
120-
func AsProvisionerd(ctx context.Context) context.Context {
121-
return context.WithValue(ctx, authContextKey{}, rbac.Subject{
118+
var (
119+
subjectProvisionerd = rbac.Subject{
122120
ID: uuid.Nil.String(),
123121
Roles: rbac.Roles([]rbac.Role{
124122
{
125123
Name: "provisionerd",
126124
DisplayName: "Provisioner Daemon",
127125
Site: rbac.Permissions(map[string][]rbac.Action{
126+
// TODO: Add ProvisionerJob resource type.
128127
rbac.ResourceFile.Type: {rbac.ActionRead},
128+
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
129129
rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate},
130130
rbac.ResourceUser.Type: {rbac.ActionRead},
131131
rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
@@ -135,20 +135,15 @@ func AsProvisionerd(ctx context.Context) context.Context {
135135
},
136136
}),
137137
Scope: rbac.ScopeAll,
138-
},
139-
)
140-
}
141-
142-
// AsAutostart returns a context with an actor that has permissions required
143-
// for autostart to function.
144-
func AsAutostart(ctx context.Context) context.Context {
145-
return context.WithValue(ctx, authContextKey{}, rbac.Subject{
138+
}
139+
subjectAutostart = rbac.Subject{
146140
ID: uuid.Nil.String(),
147141
Roles: rbac.Roles([]rbac.Role{
148142
{
149143
Name: "autostart",
150144
DisplayName: "Autostart Daemon",
151145
Site: rbac.Permissions(map[string][]rbac.Action{
146+
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
152147
rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate},
153148
rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate},
154149
}),
@@ -157,14 +152,8 @@ func AsAutostart(ctx context.Context) context.Context {
157152
},
158153
}),
159154
Scope: rbac.ScopeAll,
160-
},
161-
)
162-
}
163-
164-
// AsSystemRestricted returns a context with an actor that has permissions
165-
// required for various system operations (login, logout, metrics cache).
166-
func AsSystemRestricted(ctx context.Context) context.Context {
167-
return context.WithValue(ctx, authContextKey{}, rbac.Subject{
155+
}
156+
subjectSystemRestricted = rbac.Subject{
168157
ID: uuid.Nil.String(),
169158
Roles: rbac.Roles([]rbac.Role{
170159
{
@@ -175,6 +164,7 @@ func AsSystemRestricted(ctx context.Context) context.Context {
175164
rbac.ResourceAPIKey.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
176165
rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate},
177166
rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate},
167+
rbac.ResourceSystem.Type: {rbac.WildcardSymbol},
178168
rbac.ResourceOrganization.Type: {rbac.ActionCreate},
179169
rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate},
180170
rbac.ResourceOrgRoleAssignment.Type: {rbac.ActionCreate},
@@ -187,8 +177,25 @@ func AsSystemRestricted(ctx context.Context) context.Context {
187177
},
188178
}),
189179
Scope: rbac.ScopeAll,
190-
},
191-
)
180+
}
181+
)
182+
183+
// AsProvisionerd returns a context with an actor that has permissions required
184+
// for provisionerd to function.
185+
func AsProvisionerd(ctx context.Context) context.Context {
186+
return context.WithValue(ctx, authContextKey{}, subjectProvisionerd)
187+
}
188+
189+
// AsAutostart returns a context with an actor that has permissions required
190+
// for autostart to function.
191+
func AsAutostart(ctx context.Context) context.Context {
192+
return context.WithValue(ctx, authContextKey{}, subjectAutostart)
193+
}
194+
195+
// AsSystemRestricted returns a context with an actor that has permissions
196+
// required for various system operations (login, logout, metrics cache).
197+
func AsSystemRestricted(ctx context.Context) context.Context {
198+
return context.WithValue(ctx, authContextKey{}, subjectSystemRestricted)
192199
}
193200

194201
var AsRemoveActor = rbac.Subject{

‎coderd/database/dbauthz/querier.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,18 @@ func (q *querier) GetUserByID(ctx context.Context, id uuid.UUID) (database.User,
963963
return fetch(q.log, q.auth, q.db.GetUserByID)(ctx, id)
964964
}
965965

966+
// GetUsersByIDs is only used for usernames on workspace return data.
967+
// This function should be replaced by joining this data to the workspace query
968+
// itself.
969+
func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
970+
for _, uid := range ids {
971+
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil {
972+
return nil, err
973+
}
974+
}
975+
return q.db.GetUsersByIDs(ctx, ids)
976+
}
977+
966978
func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
967979
return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
968980
}

‎coderd/database/dbauthz/querier_test.go

Lines changed: 7 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -282,11 +282,6 @@ func (s *MethodTestSuite) TestProvsionerJob() {
282282
check.Args(database.UpdateProvisionerJobWithCancelByIDParams{ID: j.ID}).
283283
Asserts(v.RBACObject(tpl), []rbac.Action{rbac.ActionRead, rbac.ActionUpdate}).Returns()
284284
}))
285-
s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) {
286-
a := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{})
287-
b := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{})
288-
check.Args([]uuid.UUID{a.ID, b.ID}).Asserts().Returns(slice.New(a, b))
289-
}))
290285
s.Run("GetProvisionerLogsByIDBetween", s.Subtest(func(db database.Store, check *expects) {
291286
w := dbgen.Workspace(s.T(), db, database.Workspace{})
292287
j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{
@@ -619,22 +614,6 @@ func (s *MethodTestSuite) TestTemplate() {
619614
})
620615
check.Args(tv.ID).Asserts(t1, rbac.ActionRead).Returns(tv)
621616
}))
622-
s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) {
623-
t1 := dbgen.Template(s.T(), db, database.Template{})
624-
t2 := dbgen.Template(s.T(), db, database.Template{})
625-
tv1 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{
626-
TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true},
627-
})
628-
tv2 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{
629-
TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true},
630-
})
631-
tv3 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{
632-
TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true},
633-
})
634-
check.Args([]uuid.UUID{tv1.ID, tv2.ID, tv3.ID}).
635-
Asserts( /*t1, rbac.ActionRead, t2, rbac.ActionRead*/ ).
636-
Returns(slice.New(tv1, tv2, tv3))
637-
}))
638617
s.Run("GetTemplateVersionsByTemplateID", s.Subtest(func(db database.Store, check *expects) {
639618
t1 := dbgen.Template(s.T(), db, database.Template{})
640619
a := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{
@@ -784,6 +763,13 @@ func (s *MethodTestSuite) TestUser() {
784763
u := dbgen.User(s.T(), db, database.User{})
785764
check.Args(u.ID).Asserts(u, rbac.ActionRead).Returns(u)
786765
}))
766+
s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) {
767+
a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
768+
b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
769+
check.Args([]uuid.UUID{a.ID, b.ID}).
770+
Asserts(a, rbac.ActionRead, b, rbac.ActionRead).
771+
Returns(slice.New(a, b))
772+
}))
787773
s.Run("GetAuthorizedUserCount", s.Subtest(func(db database.Store, check *expects) {
788774
_ = dbgen.User(s.T(), db, database.User{})
789775
check.Args(database.GetFilteredUserCountParams{}, emptyPreparedAuthorized{}).Asserts().Returns(int64(1))
@@ -803,13 +789,6 @@ func (s *MethodTestSuite) TestUser() {
803789
b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
804790
check.Args(database.GetUsersParams{}).Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
805791
}))
806-
s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) {
807-
a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
808-
b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
809-
check.Args([]uuid.UUID{a.ID, b.ID}).
810-
Asserts( /*a, rbac.ActionRead, b, rbac.ActionRead*/ ).
811-
Returns(slice.New(a, b))
812-
}))
813792
s.Run("InsertUser", s.Subtest(func(db database.Store, check *expects) {
814793
check.Args(database.InsertUserParams{
815794
ID: uuid.New(),
@@ -977,14 +956,6 @@ func (s *MethodTestSuite) TestWorkspace() {
977956
agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
978957
check.Args(agt.AuthInstanceID.String).Asserts(ws, rbac.ActionRead).Returns(agt)
979958
}))
980-
s.Run("GetWorkspaceAgentsByResourceIDs", s.Subtest(func(db database.Store, check *expects) {
981-
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
982-
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
983-
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
984-
agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
985-
check.Args([]uuid.UUID{res.ID}).Asserts( /*ws, rbac.ActionRead*/ ).
986-
Returns([]database.WorkspaceAgent{agt})
987-
}))
988959
s.Run("UpdateWorkspaceAgentLifecycleStateByID", s.Subtest(func(db database.Store, check *expects) {
989960
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
990961
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
@@ -1026,23 +997,6 @@ func (s *MethodTestSuite) TestWorkspace() {
1026997

1027998
check.Args(agt.ID).Asserts(ws, rbac.ActionRead).Returns(slice.New(a, b))
1028999
}))
1029-
s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) {
1030-
aWs := dbgen.Workspace(s.T(), db, database.Workspace{})
1031-
aBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: aWs.ID, JobID: uuid.New()})
1032-
aRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: aBuild.JobID})
1033-
aAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: aRes.ID})
1034-
a := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: aAgt.ID})
1035-
1036-
bWs := dbgen.Workspace(s.T(), db, database.Workspace{})
1037-
bBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: bWs.ID, JobID: uuid.New()})
1038-
bRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: bBuild.JobID})
1039-
bAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: bRes.ID})
1040-
b := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: bAgt.ID})
1041-
1042-
check.Args([]uuid.UUID{a.AgentID, b.AgentID}).
1043-
Asserts( /*aWs, rbac.ActionRead, bWs, rbac.ActionRead*/ ).
1044-
Returns([]database.WorkspaceApp{a, b})
1045-
}))
10461000
s.Run("GetWorkspaceBuildByID", s.Subtest(func(db database.Store, check *expects) {
10471001
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
10481002
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})
@@ -1096,15 +1050,6 @@ func (s *MethodTestSuite) TestWorkspace() {
10961050
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
10971051
check.Args(res.ID).Asserts(ws, rbac.ActionRead).Returns(res)
10981052
}))
1099-
s.Run("GetWorkspaceResourceMetadataByResourceIDs", s.Subtest(func(db database.Store, check *expects) {
1100-
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1101-
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1102-
_ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1103-
a := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1104-
b := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1105-
check.Args([]uuid.UUID{a.ID, b.ID}).
1106-
Asserts( /*ws, []rbac.Action{rbac.ActionRead, rbac.ActionRead}*/ )
1107-
}))
11081053
s.Run("Build/GetWorkspaceResourcesByJobID", s.Subtest(func(db database.Store, check *expects) {
11091054
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
11101055
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
@@ -1117,18 +1062,6 @@ func (s *MethodTestSuite) TestWorkspace() {
11171062
job := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: v.JobID, Type: database.ProvisionerJobTypeTemplateVersionImport})
11181063
check.Args(job.ID).Asserts(v.RBACObject(tpl), []rbac.Action{rbac.ActionRead, rbac.ActionRead}).Returns([]database.WorkspaceResource{})
11191064
}))
1120-
s.Run("GetWorkspaceResourcesByJobIDs", s.Subtest(func(db database.Store, check *expects) {
1121-
tpl := dbgen.Template(s.T(), db, database.Template{})
1122-
v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, JobID: uuid.New()})
1123-
tJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: v.JobID, Type: database.ProvisionerJobTypeTemplateVersionImport})
1124-
1125-
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1126-
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1127-
wJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1128-
check.Args([]uuid.UUID{tJob.ID, wJob.ID}).
1129-
Asserts( /*v.RBACObject(tpl), rbac.ActionRead, ws, rbac.ActionRead*/ ).
1130-
Returns([]database.WorkspaceResource{})
1131-
}))
11321065
s.Run("InsertWorkspace", s.Subtest(func(db database.Store, check *expects) {
11331066
u := dbgen.User(s.T(), db, database.User{})
11341067
o := dbgen.Organization(s.T(), db, database.Organization{})

0 commit comments

Comments
 (0)