From 4da9a94bea96999a5676314d8e11bae971fd236d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 8 Mar 2023 16:11:05 +0000 Subject: [PATCH 01/15] refactor(dbauthz): add authz for system-level functions - Introduces rbac.ResourceSystem - Grants system.* to system and provisionerd rbac subjects --- coderd/database/dbauthz/dbauthz.go | 49 ++++--- coderd/database/dbauthz/querier_test.go | 74 ---------- coderd/database/dbauthz/system.go | 175 +++++++++++++++++++++--- coderd/database/dbauthz/system_test.go | 166 ++++++++++++++++------ coderd/rbac/object.go | 5 + 5 files changed, 312 insertions(+), 157 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 4f139bdc0b409..19bfc385b7e24 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -115,10 +115,8 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) { return a, ok } -// AsProvisionerd returns a context with an actor that has permissions required -// for provisionerd to function. -func AsProvisionerd(ctx context.Context) context.Context { - return context.WithValue(ctx, authContextKey{}, rbac.Subject{ +var ( + subjectProvisionerd = rbac.Subject{ ID: uuid.Nil.String(), Roles: rbac.Roles([]rbac.Role{ { @@ -126,6 +124,7 @@ func AsProvisionerd(ctx context.Context) context.Context { DisplayName: "Provisioner Daemon", Site: rbac.Permissions(map[string][]rbac.Action{ rbac.ResourceFile.Type: {rbac.ActionRead}, + rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate}, rbac.ResourceUser.Type: {rbac.ActionRead}, rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, @@ -135,14 +134,8 @@ func AsProvisionerd(ctx context.Context) context.Context { }, }), Scope: rbac.ScopeAll, - }, - ) -} - -// AsAutostart returns a context with an actor that has permissions required -// for autostart to function. -func AsAutostart(ctx context.Context) context.Context { - return context.WithValue(ctx, authContextKey{}, rbac.Subject{ + } + subjectAutostart = rbac.Subject{ ID: uuid.Nil.String(), Roles: rbac.Roles([]rbac.Role{ { @@ -157,14 +150,8 @@ func AsAutostart(ctx context.Context) context.Context { }, }), Scope: rbac.ScopeAll, - }, - ) -} - -// AsSystemRestricted returns a context with an actor that has permissions -// required for various system operations (login, logout, metrics cache). -func AsSystemRestricted(ctx context.Context) context.Context { - return context.WithValue(ctx, authContextKey{}, rbac.Subject{ + } + subjectSystemRestricted = rbac.Subject{ ID: uuid.Nil.String(), Roles: rbac.Roles([]rbac.Role{ { @@ -175,6 +162,7 @@ func AsSystemRestricted(ctx context.Context) context.Context { rbac.ResourceAPIKey.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate}, rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate}, + rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, rbac.ResourceOrganization.Type: {rbac.ActionCreate}, rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate}, rbac.ResourceOrgRoleAssignment.Type: {rbac.ActionCreate}, @@ -187,8 +175,25 @@ func AsSystemRestricted(ctx context.Context) context.Context { }, }), Scope: rbac.ScopeAll, - }, - ) + } +) + +// AsProvisionerd returns a context with an actor that has permissions required +// for provisionerd to function. +func AsProvisionerd(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, subjectProvisionerd) +} + +// AsAutostart returns a context with an actor that has permissions required +// for autostart to function. +func AsAutostart(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, subjectAutostart) +} + +// AsSystemRestricted returns a context with an actor that has permissions +// required for various system operations (login, logout, metrics cache). +func AsSystemRestricted(ctx context.Context) context.Context { + return context.WithValue(ctx, authContextKey{}, subjectSystemRestricted) } var AsRemoveActor = rbac.Subject{ diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index d89a93319d9e0..5c5374a181eed 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -282,11 +282,6 @@ func (s *MethodTestSuite) TestProvsionerJob() { check.Args(database.UpdateProvisionerJobWithCancelByIDParams{ID: j.ID}). Asserts(v.RBACObject(tpl), []rbac.Action{rbac.ActionRead, rbac.ActionUpdate}).Returns() })) - s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { - a := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) - b := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) - check.Args([]uuid.UUID{a.ID, b.ID}).Asserts().Returns(slice.New(a, b)) - })) s.Run("GetProvisionerLogsByIDBetween", s.Subtest(func(db database.Store, check *expects) { w := dbgen.Workspace(s.T(), db, database.Workspace{}) j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ @@ -619,22 +614,6 @@ func (s *MethodTestSuite) TestTemplate() { }) check.Args(tv.ID).Asserts(t1, rbac.ActionRead).Returns(tv) })) - s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) { - t1 := dbgen.Template(s.T(), db, database.Template{}) - t2 := dbgen.Template(s.T(), db, database.Template{}) - tv1 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ - TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, - }) - tv2 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ - TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true}, - }) - tv3 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ - TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true}, - }) - check.Args([]uuid.UUID{tv1.ID, tv2.ID, tv3.ID}). - Asserts( /*t1, rbac.ActionRead, t2, rbac.ActionRead*/ ). - Returns(slice.New(tv1, tv2, tv3)) - })) s.Run("GetTemplateVersionsByTemplateID", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) a := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ @@ -803,13 +782,6 @@ func (s *MethodTestSuite) TestUser() { b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) check.Args(database.GetUsersParams{}).Asserts(a, rbac.ActionRead, b, rbac.ActionRead) })) - s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) { - a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)}) - b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) - check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts( /*a, rbac.ActionRead, b, rbac.ActionRead*/ ). - Returns(slice.New(a, b)) - })) s.Run("InsertUser", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertUserParams{ ID: uuid.New(), @@ -977,14 +949,6 @@ func (s *MethodTestSuite) TestWorkspace() { agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) check.Args(agt.AuthInstanceID.String).Asserts(ws, rbac.ActionRead).Returns(agt) })) - s.Run("GetWorkspaceAgentsByResourceIDs", s.Subtest(func(db database.Store, check *expects) { - ws := dbgen.Workspace(s.T(), db, database.Workspace{}) - build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) - res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) - agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) - check.Args([]uuid.UUID{res.ID}).Asserts( /*ws, rbac.ActionRead*/ ). - Returns([]database.WorkspaceAgent{agt}) - })) s.Run("UpdateWorkspaceAgentLifecycleStateByID", s.Subtest(func(db database.Store, check *expects) { ws := dbgen.Workspace(s.T(), db, database.Workspace{}) build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) @@ -1026,23 +990,6 @@ func (s *MethodTestSuite) TestWorkspace() { check.Args(agt.ID).Asserts(ws, rbac.ActionRead).Returns(slice.New(a, b)) })) - s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) { - aWs := dbgen.Workspace(s.T(), db, database.Workspace{}) - aBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: aWs.ID, JobID: uuid.New()}) - aRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: aBuild.JobID}) - aAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: aRes.ID}) - a := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: aAgt.ID}) - - bWs := dbgen.Workspace(s.T(), db, database.Workspace{}) - bBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: bWs.ID, JobID: uuid.New()}) - bRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: bBuild.JobID}) - bAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: bRes.ID}) - b := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: bAgt.ID}) - - check.Args([]uuid.UUID{a.AgentID, b.AgentID}). - Asserts( /*aWs, rbac.ActionRead, bWs, rbac.ActionRead*/ ). - Returns([]database.WorkspaceApp{a, b}) - })) s.Run("GetWorkspaceBuildByID", s.Subtest(func(db database.Store, check *expects) { ws := dbgen.Workspace(s.T(), db, database.Workspace{}) build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID}) @@ -1096,15 +1043,6 @@ func (s *MethodTestSuite) TestWorkspace() { res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) check.Args(res.ID).Asserts(ws, rbac.ActionRead).Returns(res) })) - s.Run("GetWorkspaceResourceMetadataByResourceIDs", s.Subtest(func(db database.Store, check *expects) { - ws := dbgen.Workspace(s.T(), db, database.Workspace{}) - build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) - _ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) - a := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) - b := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) - check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts( /*ws, []rbac.Action{rbac.ActionRead, rbac.ActionRead}*/ ) - })) s.Run("Build/GetWorkspaceResourcesByJobID", s.Subtest(func(db database.Store, check *expects) { ws := dbgen.Workspace(s.T(), db, database.Workspace{}) build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) @@ -1117,18 +1055,6 @@ func (s *MethodTestSuite) TestWorkspace() { job := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: v.JobID, Type: database.ProvisionerJobTypeTemplateVersionImport}) check.Args(job.ID).Asserts(v.RBACObject(tpl), []rbac.Action{rbac.ActionRead, rbac.ActionRead}).Returns([]database.WorkspaceResource{}) })) - s.Run("GetWorkspaceResourcesByJobIDs", s.Subtest(func(db database.Store, check *expects) { - tpl := dbgen.Template(s.T(), db, database.Template{}) - v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, JobID: uuid.New()}) - tJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: v.JobID, Type: database.ProvisionerJobTypeTemplateVersionImport}) - - ws := dbgen.Workspace(s.T(), db, database.Workspace{}) - build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) - wJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) - check.Args([]uuid.UUID{tJob.ID, wJob.ID}). - Asserts( /*v.RBACObject(tpl), rbac.ActionRead, ws, rbac.ActionRead*/ ). - Returns([]database.WorkspaceResource{}) - })) s.Run("InsertWorkspace", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) o := dbgen.Organization(s.T(), db, database.Organization{}) diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index e83b0b0771f9d..16a5848033d39 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -7,53 +7,59 @@ import ( "github.com/google/uuid" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/rbac" ) -// TODO: All these system functions should have rbac objects created to allow -// only system roles to call them. No user roles should ever have the permission -// to these objects. Might need a negative permission on the `Owner` role to -// prevent owners. - // GetWorkspaceAppsByAgentIDs // The workspace/job is already fetched. -// TODO: This function should be removed/replaced with something with proper auth. func (q *querier) GetWorkspaceAppsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceApp, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceAppsByAgentIDs(ctx, ids) } // GetWorkspaceAgentsByResourceIDs // The workspace/job is already fetched. -// TODO: This function should be removed/replaced with something with proper auth. func (q *querier) GetWorkspaceAgentsByResourceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceAgent, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceAgentsByResourceIDs(ctx, ids) } // GetWorkspaceResourceMetadataByResourceIDs is only used for build data. // The workspace/job is already fetched. -// TODO: This function should be removed/replaced with something with proper auth. func (q *querier) GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceResourceMetadatum, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceResourceMetadataByResourceIDs(ctx, ids) } // GetUsersByIDs is only used for usernames on workspace return data. // This function should be replaced by joining this data to the workspace query // itself. -// TODO: This function should be removed/replaced with something with proper auth. -// A SQL compiled filter is an option. func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetUsersByIDs(ctx, ids) } func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) { - // TODO: This is missing authorization and is incorrect. This call is used by telemetry, and by 1 http route. - // That http handler should find a better way to fetch these jobs with easier rbac authz. + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetProvisionerJobsByIDs(ctx, ids) } // GetTemplateVersionsByIDs is only used for workspace build data. // The workspace is already fetched. -// TODO: Find a way to replace this with proper authz. func (q *querier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.TemplateVersion, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetTemplateVersionsByIDs(ctx, ids) } @@ -61,18 +67,30 @@ func (q *querier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UUID) // The workspace is already fetched. // TODO: Find a way to replace this with proper authz. func (q *querier) GetWorkspaceResourcesByJobIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceResource, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceResourcesByJobIDs(ctx, ids) } func (q *querier) UpdateUserLinkedID(ctx context.Context, arg database.UpdateUserLinkedIDParams) (database.UserLink, error) { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return database.UserLink{}, err + } return q.db.UpdateUserLinkedID(ctx, arg) } func (q *querier) GetUserLinkByLinkedID(ctx context.Context, linkedID string) (database.UserLink, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return database.UserLink{}, err + } return q.db.GetUserLinkByLinkedID(ctx, linkedID) } func (q *querier) GetUserLinkByUserIDLoginType(ctx context.Context, arg database.GetUserLinkByUserIDLoginTypeParams) (database.UserLink, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return database.UserLink{}, err + } return q.db.GetUserLinkByUserIDLoginType(ctx, arg) } @@ -81,96 +99,148 @@ func (q *querier) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.Work // This is because we need to query for all related workspaces to the returned builds. // This is a very inefficient method of fetching the latest workspace builds. // We should just join the rbac properties. + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetLatestWorkspaceBuilds(ctx) } // GetWorkspaceAgentByAuthToken is used in http middleware to get the workspace agent. // This should only be used by a system user in that middleware. func (q *querier) GetWorkspaceAgentByAuthToken(ctx context.Context, authToken uuid.UUID) (database.WorkspaceAgent, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return database.WorkspaceAgent{}, err + } return q.db.GetWorkspaceAgentByAuthToken(ctx, authToken) } func (q *querier) GetActiveUserCount(ctx context.Context) (int64, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return 0, err + } return q.db.GetActiveUserCount(ctx) } func (q *querier) GetUnexpiredLicenses(ctx context.Context) ([]database.License, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetUnexpiredLicenses(ctx) } func (q *querier) GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (database.GetAuthorizationUserRolesRow, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return database.GetAuthorizationUserRolesRow{}, err + } return q.db.GetAuthorizationUserRoles(ctx, userID) } func (q *querier) GetDERPMeshKey(ctx context.Context) (string, error) { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return "", err + } return q.db.GetDERPMeshKey(ctx) } func (q *querier) InsertDERPMeshKey(ctx context.Context, value string) error { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return err + } return q.db.InsertDERPMeshKey(ctx, value) } func (q *querier) InsertDeploymentID(ctx context.Context, value string) error { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return err + } return q.db.InsertDeploymentID(ctx, value) } func (q *querier) InsertReplica(ctx context.Context, arg database.InsertReplicaParams) (database.Replica, error) { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.Replica{}, err + } return q.db.InsertReplica(ctx, arg) } func (q *querier) UpdateReplica(ctx context.Context, arg database.UpdateReplicaParams) (database.Replica, error) { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return database.Replica{}, err + } return q.db.UpdateReplica(ctx, arg) } func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceSystem); err != nil { + return err + } return q.db.DeleteReplicasUpdatedBefore(ctx, updatedAt) } func (q *querier) GetReplicasUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]database.Replica, error) { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetReplicasUpdatedAfter(ctx, updatedAt) } func (q *querier) GetUserCount(ctx context.Context) (int64, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return 0, err + } return q.db.GetUserCount(ctx) } func (q *querier) GetTemplates(ctx context.Context) ([]database.Template, error) { - // TODO Implement authz check for system user. + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetTemplates(ctx) } // Only used by metrics cache. func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return database.GetTemplateAverageBuildTimeRow{}, err + } return q.db.GetTemplateAverageBuildTime(ctx, arg) } // Only used by metrics cache. func (q *querier) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]database.GetTemplateDAUsRow, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetTemplateDAUs(ctx, templateID) } // Only used by metrics cache. func (q *querier) GetDeploymentDAUs(ctx context.Context) ([]database.GetDeploymentDAUsRow, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetDeploymentDAUs(ctx) } // UpdateWorkspaceBuildCostByID is used by the provisioning system to update the cost of a workspace build. func (q *querier) UpdateWorkspaceBuildCostByID(ctx context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) (database.WorkspaceBuild, error) { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return database.WorkspaceBuild{}, err + } return q.db.UpdateWorkspaceBuildCostByID(ctx, arg) } func (q *querier) InsertOrUpdateLastUpdateCheck(ctx context.Context, value string) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } return q.db.InsertOrUpdateLastUpdateCheck(ctx, value) } func (q *querier) GetLastUpdateCheck(ctx context.Context) (string, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return "", err + } return q.db.GetLastUpdateCheck(ctx) } @@ -178,26 +248,44 @@ func (q *querier) GetLastUpdateCheck(ctx context.Context) (string, error) { // telemetry data. Never called by a user. func (q *querier) GetWorkspaceBuildsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceBuild, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceBuildsCreatedAfter(ctx, createdAt) } func (q *querier) GetWorkspaceAgentsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceAgent, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceAgentsCreatedAfter(ctx, createdAt) } func (q *querier) GetWorkspaceAppsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceApp, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceAppsCreatedAfter(ctx, createdAt) } func (q *querier) GetWorkspaceResourcesCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceResource, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceResourcesCreatedAfter(ctx, createdAt) } func (q *querier) GetWorkspaceResourceMetadataCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.WorkspaceResourceMetadatum, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetWorkspaceResourceMetadataCreatedAfter(ctx, createdAt) } func (q *querier) DeleteOldWorkspaceAgentStats(ctx context.Context) error { + if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceSystem); err != nil { + return err + } return q.db.DeleteOldWorkspaceAgentStats(ctx) } @@ -210,63 +298,108 @@ func (q *querier) GetDeploymentWorkspaceStats(ctx context.Context) (database.Get } func (q *querier) GetParameterSchemasCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.ParameterSchema, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetParameterSchemasCreatedAfter(ctx, createdAt) } func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.ProvisionerJob, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt) } // Provisionerd server functions func (q *querier) InsertWorkspaceAgent(ctx context.Context, arg database.InsertWorkspaceAgentParams) (database.WorkspaceAgent, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.WorkspaceAgent{}, err + } return q.db.InsertWorkspaceAgent(ctx, arg) } func (q *querier) InsertWorkspaceApp(ctx context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.WorkspaceApp{}, err + } return q.db.InsertWorkspaceApp(ctx, arg) } func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg database.InsertWorkspaceResourceMetadataParams) ([]database.WorkspaceResourceMetadatum, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.InsertWorkspaceResourceMetadata(ctx, arg) } func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return database.ProvisionerJob{}, err + } return q.db.AcquireProvisionerJob(ctx, arg) } func (q *querier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteByIDParams) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } return q.db.UpdateProvisionerJobWithCompleteByID(ctx, arg) } func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } return q.db.UpdateProvisionerJobByID(ctx, arg) } func (q *querier) InsertProvisionerJob(ctx context.Context, arg database.InsertProvisionerJobParams) (database.ProvisionerJob, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.ProvisionerJob{}, err + } return q.db.InsertProvisionerJob(ctx, arg) } func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.InsertProvisionerJobLogsParams) ([]database.ProvisionerJobLog, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return nil, err + } return q.db.InsertProvisionerJobLogs(ctx, arg) } func (q *querier) InsertProvisionerDaemon(ctx context.Context, arg database.InsertProvisionerDaemonParams) (database.ProvisionerDaemon, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.ProvisionerDaemon{}, err + } return q.db.InsertProvisionerDaemon(ctx, arg) } func (q *querier) InsertTemplateVersionParameter(ctx context.Context, arg database.InsertTemplateVersionParameterParams) (database.TemplateVersionParameter, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.TemplateVersionParameter{}, err + } return q.db.InsertTemplateVersionParameter(ctx, arg) } func (q *querier) InsertTemplateVersionVariable(ctx context.Context, arg database.InsertTemplateVersionVariableParams) (database.TemplateVersionVariable, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.TemplateVersionVariable{}, err + } return q.db.InsertTemplateVersionVariable(ctx, arg) } func (q *querier) InsertWorkspaceResource(ctx context.Context, arg database.InsertWorkspaceResourceParams) (database.WorkspaceResource, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.WorkspaceResource{}, err + } return q.db.InsertWorkspaceResource(ctx, arg) } func (q *querier) InsertParameterSchema(ctx context.Context, arg database.InsertParameterSchemaParams) (database.ParameterSchema, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return database.ParameterSchema{}, err + } return q.db.InsertParameterSchema(ctx, arg) } diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go index d64727f1b6227..fcbc329ba3c49 100644 --- a/coderd/database/dbauthz/system_test.go +++ b/coderd/database/dbauthz/system_test.go @@ -10,9 +10,18 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbgen" + "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/util/slice" ) func (s *MethodTestSuite) TestSystemFunctions() { + var ( + sys = rbac.ResourceSystem + create = rbac.ActionCreate + read = rbac.ActionRead + update = rbac.ActionUpdate + delete = rbac.ActionDelete + ) s.Run("UpdateUserLinkedID", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) l := dbgen.UserLink(s.T(), db, database.UserLink{UserID: u.ID}) @@ -20,51 +29,51 @@ func (s *MethodTestSuite) TestSystemFunctions() { UserID: u.ID, LinkedID: l.LinkedID, LoginType: database.LoginTypeGithub, - }).Asserts().Returns(l) + }).Asserts(sys, update).Returns(l) })) s.Run("GetUserLinkByLinkedID", s.Subtest(func(db database.Store, check *expects) { l := dbgen.UserLink(s.T(), db, database.UserLink{}) - check.Args(l.LinkedID).Asserts().Returns(l) + check.Args(l.LinkedID).Asserts(sys, read).Returns(l) })) s.Run("GetUserLinkByUserIDLoginType", s.Subtest(func(db database.Store, check *expects) { l := dbgen.UserLink(s.T(), db, database.UserLink{}) check.Args(database.GetUserLinkByUserIDLoginTypeParams{ UserID: l.UserID, LoginType: l.LoginType, - }).Asserts().Returns(l) + }).Asserts(sys, read).Returns(l) })) s.Run("GetLatestWorkspaceBuilds", s.Subtest(func(db database.Store, check *expects) { dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) - check.Args().Asserts() + check.Args().Asserts(sys, read) })) s.Run("GetWorkspaceAgentByAuthToken", s.Subtest(func(db database.Store, check *expects) { agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{}) - check.Args(agt.AuthToken).Asserts().Returns(agt) + check.Args(agt.AuthToken).Asserts(sys, read).Returns(agt) })) s.Run("GetActiveUserCount", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts().Returns(int64(0)) + check.Args().Asserts(sys, read).Returns(int64(0)) })) s.Run("GetUnexpiredLicenses", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts() + check.Args().Asserts(sys, read) })) s.Run("GetAuthorizationUserRoles", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) - check.Args(u.ID).Asserts() + check.Args(u.ID).Asserts(sys, read) })) s.Run("GetDERPMeshKey", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts() + check.Args().Asserts(sys, read) })) s.Run("InsertDERPMeshKey", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts().Returns() + check.Args("value").Asserts(sys, create).Returns() })) s.Run("InsertDeploymentID", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts().Returns() + check.Args("value").Asserts(sys, create).Returns() })) s.Run("InsertReplica", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertReplicaParams{ ID: uuid.New(), - }).Asserts() + }).Asserts(sys, create) })) s.Run("UpdateReplica", s.Subtest(func(db database.Store, check *expects) { replica, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New()}) @@ -72,24 +81,24 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args(database.UpdateReplicaParams{ ID: replica.ID, DatabaseLatency: 100, - }).Asserts() + }).Asserts(sys, update) })) s.Run("DeleteReplicasUpdatedBefore", s.Subtest(func(db database.Store, check *expects) { _, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New(), UpdatedAt: time.Now()}) require.NoError(s.T(), err) - check.Args(time.Now().Add(time.Hour)).Asserts() + check.Args(time.Now().Add(time.Hour)).Asserts(sys, delete) })) s.Run("GetReplicasUpdatedAfter", s.Subtest(func(db database.Store, check *expects) { _, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New(), UpdatedAt: time.Now()}) require.NoError(s.T(), err) - check.Args(time.Now().Add(time.Hour * -1)).Asserts() + check.Args(time.Now().Add(time.Hour*-1)).Asserts(sys, read) })) s.Run("GetUserCount", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts().Returns(int64(0)) + check.Args().Asserts(sys, read).Returns(int64(0)) })) s.Run("GetTemplates", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.Template(s.T(), db, database.Template{}) - check.Args().Asserts() + check.Args().Asserts(sys, read) })) s.Run("UpdateWorkspaceBuildCostByID", s.Subtest(func(db database.Store, check *expects) { b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) @@ -98,83 +107,160 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args(database.UpdateWorkspaceBuildCostByIDParams{ ID: b.ID, DailyCost: 10, - }).Asserts().Returns(o) + }).Asserts(sys, update).Returns(o) })) s.Run("InsertOrUpdateLastUpdateCheck", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts() + check.Args("value").Asserts(sys, update) })) s.Run("GetLastUpdateCheck", s.Subtest(func(db database.Store, check *expects) { err := db.InsertOrUpdateLastUpdateCheck(context.Background(), "value") require.NoError(s.T(), err) - check.Args().Asserts() + check.Args().Asserts(sys, read) })) s.Run("GetWorkspaceBuildsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("GetWorkspaceAgentsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("GetWorkspaceAppsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("GetWorkspaceResourcesCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("GetWorkspaceResourceMetadataCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceResourceMetadatums(s.T(), db, database.WorkspaceResourceMetadatum{}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("DeleteOldWorkspaceAgentStats", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts() + check.Args().Asserts(sys, delete) })) s.Run("GetParameterSchemasCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.ParameterSchema(s.T(), db, database.ParameterSchema{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) })) s.Run("GetProvisionerJobsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts() + check.Args(time.Now()).Asserts(sys, read) + })) + s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) { + t1 := dbgen.Template(s.T(), db, database.Template{}) + t2 := dbgen.Template(s.T(), db, database.Template{}) + tv1 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, + }) + tv2 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true}, + }) + tv3 := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true}, + }) + check.Args([]uuid.UUID{tv1.ID, tv2.ID, tv3.ID}). + Asserts(sys, read). + Returns(slice.New(tv1, tv2, tv3)) + })) + s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) { + a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)}) + b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) + check.Args([]uuid.UUID{a.ID, b.ID}). + Asserts(sys, read). + Returns(slice.New(a, b)) + })) + s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) { + aWs := dbgen.Workspace(s.T(), db, database.Workspace{}) + aBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: aWs.ID, JobID: uuid.New()}) + aRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: aBuild.JobID}) + aAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: aRes.ID}) + a := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: aAgt.ID}) + + bWs := dbgen.Workspace(s.T(), db, database.Workspace{}) + bBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: bWs.ID, JobID: uuid.New()}) + bRes := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: bBuild.JobID}) + bAgt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: bRes.ID}) + b := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: bAgt.ID}) + + check.Args([]uuid.UUID{a.AgentID, b.AgentID}). + Asserts(sys, read). + Returns([]database.WorkspaceApp{a, b}) + })) + s.Run("GetWorkspaceResourcesByJobIDs", s.Subtest(func(db database.Store, check *expects) { + tpl := dbgen.Template(s.T(), db, database.Template{}) + v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, JobID: uuid.New()}) + tJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: v.JobID, Type: database.ProvisionerJobTypeTemplateVersionImport}) + + ws := dbgen.Workspace(s.T(), db, database.Workspace{}) + build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) + wJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) + check.Args([]uuid.UUID{tJob.ID, wJob.ID}). + Asserts(sys, read). + Returns([]database.WorkspaceResource{}) + })) + s.Run("GetWorkspaceResourceMetadataByResourceIDs", s.Subtest(func(db database.Store, check *expects) { + ws := dbgen.Workspace(s.T(), db, database.Workspace{}) + build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) + _ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) + a := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) + b := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) + check.Args([]uuid.UUID{a.ID, b.ID}). + Asserts(sys, read) + })) + s.Run("GetWorkspaceAgentsByResourceIDs", s.Subtest(func(db database.Store, check *expects) { + ws := dbgen.Workspace(s.T(), db, database.Workspace{}) + build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) + res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) + agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) + check.Args([]uuid.UUID{res.ID}). + Asserts(sys, read). + Returns([]database.WorkspaceAgent{agt}) + })) + s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { + a := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) + b := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) + check.Args([]uuid.UUID{a.ID, b.ID}). + Asserts(sys, read). + Returns(slice.New(a, b)) })) s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceAgentParams{ ID: uuid.New(), - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceAppParams{ ID: uuid.New(), Health: database.WorkspaceAppHealthDisabled, SharingLevel: database.AppSharingLevelOwner, - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertWorkspaceResourceMetadata", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceResourceMetadataParams{ WorkspaceResourceID: uuid.New(), - }).Asserts() + }).Asserts(sys, create) })) s.Run("AcquireProvisionerJob", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ StartedAt: sql.NullTime{Valid: false}, }) check.Args(database.AcquireProvisionerJobParams{Types: []database.ProvisionerType{j.Provisioner}}). - Asserts() + Asserts(sys, update) })) s.Run("UpdateProvisionerJobWithCompleteByID", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobWithCompleteByIDParams{ ID: j.ID, - }).Asserts() + }).Asserts(sys, update) })) s.Run("UpdateProvisionerJobByID", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobByIDParams{ ID: j.ID, UpdatedAt: time.Now(), - }).Asserts() + }).Asserts(sys, update) })) s.Run("InsertProvisionerJob", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertProvisionerJobParams{ @@ -182,31 +268,31 @@ func (s *MethodTestSuite) TestSystemFunctions() { Provisioner: database.ProvisionerTypeEcho, StorageMethod: database.ProvisionerStorageMethodFile, Type: database.ProvisionerJobTypeWorkspaceBuild, - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertProvisionerJobLogs", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.InsertProvisionerJobLogsParams{ JobID: j.ID, - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertProvisionerDaemon", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertProvisionerDaemonParams{ ID: uuid.New(), - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertTemplateVersionParameter", s.Subtest(func(db database.Store, check *expects) { v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{}) check.Args(database.InsertTemplateVersionParameterParams{ TemplateVersionID: v.ID, - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertWorkspaceResource", s.Subtest(func(db database.Store, check *expects) { r := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{}) check.Args(database.InsertWorkspaceResourceParams{ ID: r.ID, Transition: database.WorkspaceTransitionStart, - }).Asserts() + }).Asserts(sys, create) })) s.Run("InsertParameterSchema", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertParameterSchemaParams{ @@ -214,6 +300,6 @@ func (s *MethodTestSuite) TestSystemFunctions() { DefaultSourceScheme: database.ParameterSourceSchemeNone, DefaultDestinationScheme: database.ParameterDestinationSchemeNone, ValidationTypeSystem: database.ParameterTypeSystemNone, - }).Asserts() + }).Asserts(sys, create) })) } diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 5e70fc02beaee..d83d2fa36c4b4 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -159,6 +159,11 @@ var ( ResourceDebugInfo = Object{ Type: "debug_info", } + + // ResourceSystem is a pseudo-resource only used for system-level actions. + ResourceSystem = Object{ + Type: "system", + } ) // Object is used to create objects for authz checks when you have none in From a11fed533f8f8a821a1af747036459a522901649 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 8 Mar 2023 17:14:37 +0000 Subject: [PATCH 02/15] replace short names --- coderd/database/dbauthz/system_test.go | 101 ++++++++++++------------- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go index fcbc329ba3c49..536bafa32cd0f 100644 --- a/coderd/database/dbauthz/system_test.go +++ b/coderd/database/dbauthz/system_test.go @@ -15,13 +15,6 @@ import ( ) func (s *MethodTestSuite) TestSystemFunctions() { - var ( - sys = rbac.ResourceSystem - create = rbac.ActionCreate - read = rbac.ActionRead - update = rbac.ActionUpdate - delete = rbac.ActionDelete - ) s.Run("UpdateUserLinkedID", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) l := dbgen.UserLink(s.T(), db, database.UserLink{UserID: u.ID}) @@ -29,51 +22,51 @@ func (s *MethodTestSuite) TestSystemFunctions() { UserID: u.ID, LinkedID: l.LinkedID, LoginType: database.LoginTypeGithub, - }).Asserts(sys, update).Returns(l) + }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns(l) })) s.Run("GetUserLinkByLinkedID", s.Subtest(func(db database.Store, check *expects) { l := dbgen.UserLink(s.T(), db, database.UserLink{}) - check.Args(l.LinkedID).Asserts(sys, read).Returns(l) + check.Args(l.LinkedID).Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(l) })) s.Run("GetUserLinkByUserIDLoginType", s.Subtest(func(db database.Store, check *expects) { l := dbgen.UserLink(s.T(), db, database.UserLink{}) check.Args(database.GetUserLinkByUserIDLoginTypeParams{ UserID: l.UserID, LoginType: l.LoginType, - }).Asserts(sys, read).Returns(l) + }).Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(l) })) s.Run("GetLatestWorkspaceBuilds", s.Subtest(func(db database.Store, check *expects) { dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) - check.Args().Asserts(sys, read) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceAgentByAuthToken", s.Subtest(func(db database.Store, check *expects) { agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{}) - check.Args(agt.AuthToken).Asserts(sys, read).Returns(agt) + check.Args(agt.AuthToken).Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(agt) })) s.Run("GetActiveUserCount", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(sys, read).Returns(int64(0)) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(int64(0)) })) s.Run("GetUnexpiredLicenses", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(sys, read) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetAuthorizationUserRoles", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) - check.Args(u.ID).Asserts(sys, read) + check.Args(u.ID).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetDERPMeshKey", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(sys, read) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("InsertDERPMeshKey", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts(sys, create).Returns() + check.Args("value").Asserts(rbac.ResourceSystem, rbac.ActionCreate).Returns() })) s.Run("InsertDeploymentID", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts(sys, create).Returns() + check.Args("value").Asserts(rbac.ResourceSystem, rbac.ActionCreate).Returns() })) s.Run("InsertReplica", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertReplicaParams{ ID: uuid.New(), - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("UpdateReplica", s.Subtest(func(db database.Store, check *expects) { replica, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New()}) @@ -81,24 +74,24 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args(database.UpdateReplicaParams{ ID: replica.ID, DatabaseLatency: 100, - }).Asserts(sys, update) + }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate) })) s.Run("DeleteReplicasUpdatedBefore", s.Subtest(func(db database.Store, check *expects) { _, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New(), UpdatedAt: time.Now()}) require.NoError(s.T(), err) - check.Args(time.Now().Add(time.Hour)).Asserts(sys, delete) + check.Args(time.Now().Add(time.Hour)).Asserts(rbac.ResourceSystem, rbac.ActionDelete) })) s.Run("GetReplicasUpdatedAfter", s.Subtest(func(db database.Store, check *expects) { _, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{ID: uuid.New(), UpdatedAt: time.Now()}) require.NoError(s.T(), err) - check.Args(time.Now().Add(time.Hour*-1)).Asserts(sys, read) + check.Args(time.Now().Add(time.Hour*-1)).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetUserCount", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(sys, read).Returns(int64(0)) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(int64(0)) })) s.Run("GetTemplates", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.Template(s.T(), db, database.Template{}) - check.Args().Asserts(sys, read) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("UpdateWorkspaceBuildCostByID", s.Subtest(func(db database.Store, check *expects) { b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) @@ -107,46 +100,46 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args(database.UpdateWorkspaceBuildCostByIDParams{ ID: b.ID, DailyCost: 10, - }).Asserts(sys, update).Returns(o) + }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns(o) })) s.Run("InsertOrUpdateLastUpdateCheck", s.Subtest(func(db database.Store, check *expects) { - check.Args("value").Asserts(sys, update) + check.Args("value").Asserts(rbac.ResourceSystem, rbac.ActionUpdate) })) s.Run("GetLastUpdateCheck", s.Subtest(func(db database.Store, check *expects) { err := db.InsertOrUpdateLastUpdateCheck(context.Background(), "value") require.NoError(s.T(), err) - check.Args().Asserts(sys, read) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceBuildsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceAgentsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceAppsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceResourcesCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceResourceMetadataCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.WorkspaceResourceMetadatums(s.T(), db, database.WorkspaceResourceMetadatum{}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("DeleteOldWorkspaceAgentStats", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts(sys, delete) + check.Args().Asserts(rbac.ResourceSystem, rbac.ActionDelete) })) s.Run("GetParameterSchemasCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.ParameterSchema(s.T(), db, database.ParameterSchema{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetProvisionerJobsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(sys, read) + check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) @@ -161,14 +154,14 @@ func (s *MethodTestSuite) TestSystemFunctions() { TemplateID: uuid.NullUUID{UUID: t2.ID, Valid: true}, }) check.Args([]uuid.UUID{tv1.ID, tv2.ID, tv3.ID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns(slice.New(tv1, tv2, tv3)) })) s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) { a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)}) b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns(slice.New(a, b)) })) s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) { @@ -185,7 +178,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { b := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: bAgt.ID}) check.Args([]uuid.UUID{a.AgentID, b.AgentID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns([]database.WorkspaceApp{a, b}) })) s.Run("GetWorkspaceResourcesByJobIDs", s.Subtest(func(db database.Store, check *expects) { @@ -197,7 +190,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) wJob := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) check.Args([]uuid.UUID{tJob.ID, wJob.ID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns([]database.WorkspaceResource{}) })) s.Run("GetWorkspaceResourceMetadataByResourceIDs", s.Subtest(func(db database.Store, check *expects) { @@ -207,7 +200,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { a := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) b := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts(sys, read) + Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetWorkspaceAgentsByResourceIDs", s.Subtest(func(db database.Store, check *expects) { ws := dbgen.Workspace(s.T(), db, database.Workspace{}) @@ -215,52 +208,52 @@ func (s *MethodTestSuite) TestSystemFunctions() { res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID}) agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) check.Args([]uuid.UUID{res.ID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns([]database.WorkspaceAgent{agt}) })) s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { a := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) b := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts(sys, read). + Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns(slice.New(a, b)) })) s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceAgentParams{ ID: uuid.New(), - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceAppParams{ ID: uuid.New(), Health: database.WorkspaceAppHealthDisabled, SharingLevel: database.AppSharingLevelOwner, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertWorkspaceResourceMetadata", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceResourceMetadataParams{ WorkspaceResourceID: uuid.New(), - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("AcquireProvisionerJob", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ StartedAt: sql.NullTime{Valid: false}, }) check.Args(database.AcquireProvisionerJobParams{Types: []database.ProvisionerType{j.Provisioner}}). - Asserts(sys, update) + Asserts(rbac.ResourceSystem, rbac.ActionUpdate) })) s.Run("UpdateProvisionerJobWithCompleteByID", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobWithCompleteByIDParams{ ID: j.ID, - }).Asserts(sys, update) + }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate) })) s.Run("UpdateProvisionerJobByID", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobByIDParams{ ID: j.ID, UpdatedAt: time.Now(), - }).Asserts(sys, update) + }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate) })) s.Run("InsertProvisionerJob", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertProvisionerJobParams{ @@ -268,31 +261,31 @@ func (s *MethodTestSuite) TestSystemFunctions() { Provisioner: database.ProvisionerTypeEcho, StorageMethod: database.ProvisionerStorageMethodFile, Type: database.ProvisionerJobTypeWorkspaceBuild, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertProvisionerJobLogs", s.Subtest(func(db database.Store, check *expects) { j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.InsertProvisionerJobLogsParams{ JobID: j.ID, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertProvisionerDaemon", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertProvisionerDaemonParams{ ID: uuid.New(), - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertTemplateVersionParameter", s.Subtest(func(db database.Store, check *expects) { v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{}) check.Args(database.InsertTemplateVersionParameterParams{ TemplateVersionID: v.ID, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertWorkspaceResource", s.Subtest(func(db database.Store, check *expects) { r := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{}) check.Args(database.InsertWorkspaceResourceParams{ ID: r.ID, Transition: database.WorkspaceTransitionStart, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("InsertParameterSchema", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertParameterSchemaParams{ @@ -300,6 +293,6 @@ func (s *MethodTestSuite) TestSystemFunctions() { DefaultSourceScheme: database.ParameterSourceSchemeNone, DefaultDestinationScheme: database.ParameterDestinationSchemeNone, ValidationTypeSystem: database.ParameterTypeSystemNone, - }).Asserts(sys, create) + }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) } From 6f40cf68a799549ab0a460fc8271c4c391496e55 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 8 Mar 2023 17:57:55 +0000 Subject: [PATCH 03/15] fix: add dbauthz.AsSystemRestricted in updatecheck, when creating first user, and when registering InMemoryProvisionerd --- coderd/coderd.go | 2 +- coderd/updatecheck/updatecheck.go | 3 ++- coderd/users.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index b0edab9b782a2..6745d51fee560 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -795,7 +795,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti }() name := namesgenerator.GetRandomName(1) - daemon, err := api.Database.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ + daemon, err := api.Database.InsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.InsertProvisionerDaemonParams{ ID: uuid.New(), CreatedAt: database.Now(), Name: name, diff --git a/coderd/updatecheck/updatecheck.go b/coderd/updatecheck/updatecheck.go index 132405c6bbd1b..177cc4f744b3b 100644 --- a/coderd/updatecheck/updatecheck.go +++ b/coderd/updatecheck/updatecheck.go @@ -20,6 +20,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" ) const ( @@ -224,7 +225,7 @@ func (c *Checker) notifyIfNewer(prev, next Result) { } func (c *Checker) lastUpdateCheck(ctx context.Context) (r Result, err error) { - s, err := c.db.GetLastUpdateCheck(ctx) + s, err := c.db.GetLastUpdateCheck(dbauthz.AsSystemRestricted(ctx)) if err != nil { return r, err } diff --git a/coderd/users.go b/coderd/users.go index 69571eb32d937..16fa763a79127 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -37,7 +37,7 @@ import ( // @Router /users/first [get] func (api *API) firstUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - userCount, err := api.Database.GetUserCount(ctx) + userCount, err := api.Database.GetUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user count.", @@ -78,7 +78,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { } // This should only function for the first user. - userCount, err := api.Database.GetUserCount(ctx) + userCount, err := api.Database.GetUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user count.", From 7fd66f4838e124c3859d019ae585ca36fad4cc46 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 10:34:09 +0000 Subject: [PATCH 04/15] replicasync: use dbauthz.AsSystemRestricted for system funcs --- enterprise/replicasync/replicasync.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/enterprise/replicasync/replicasync.go b/enterprise/replicasync/replicasync.go index 57aedbc10a07b..e2de2e9663042 100644 --- a/enterprise/replicasync/replicasync.go +++ b/enterprise/replicasync/replicasync.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/buildinfo" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" ) var PubsubEvent = "replica" @@ -61,7 +62,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data if err != nil { return nil, xerrors.Errorf("ping database: %w", err) } - replica, err := db.InsertReplica(ctx, database.InsertReplicaParams{ + replica, err := db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{ ID: options.ID, CreatedAt: database.Now(), StartedAt: database.Now(), @@ -141,7 +142,7 @@ func (m *Manager) loop(ctx context.Context) { case <-ctx.Done(): return case <-deleteTicker.C: - err := m.db.DeleteReplicasUpdatedBefore(ctx, m.updateInterval()) + err := m.db.DeleteReplicasUpdatedBefore(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { m.logger.Warn(ctx, "delete old replicas", slog.Error(err)) } @@ -218,7 +219,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { defer m.closeWait.Done() // Expect replicas to update once every three times the interval... // If they don't, assume death! - replicas, err := m.db.GetReplicasUpdatedAfter(ctx, m.updateInterval()) + replicas, err := m.db.GetReplicasUpdatedAfter(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { return xerrors.Errorf("get replicas: %w", err) } @@ -366,7 +367,7 @@ func (m *Manager) Close() error { defer m.mutex.Unlock() ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) defer cancelFunc() - _, err := m.db.UpdateReplica(ctx, database.UpdateReplicaParams{ + _, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), StartedAt: m.self.StartedAt, From 2a0746de260d525acbd9b67ecdd6f6f787d2af6e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 10:59:23 +0000 Subject: [PATCH 05/15] make linter happy --- coderd/coderd.go | 1 + coderd/updatecheck/updatecheck.go | 1 + coderd/users.go | 3 ++- enterprise/replicasync/replicasync.go | 4 ++++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 6745d51fee560..b17927203d2a9 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -795,6 +795,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti }() name := namesgenerator.GetRandomName(1) + // nolint:gocritic // Inserting a provisioner daemon is a system function. daemon, err := api.Database.InsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.InsertProvisionerDaemonParams{ ID: uuid.New(), CreatedAt: database.Now(), diff --git a/coderd/updatecheck/updatecheck.go b/coderd/updatecheck/updatecheck.go index 177cc4f744b3b..f4de324ea1812 100644 --- a/coderd/updatecheck/updatecheck.go +++ b/coderd/updatecheck/updatecheck.go @@ -225,6 +225,7 @@ func (c *Checker) notifyIfNewer(prev, next Result) { } func (c *Checker) lastUpdateCheck(ctx context.Context) (r Result, err error) { + // nolint:gocritic // Getting the last update check is a system function. s, err := c.db.GetLastUpdateCheck(dbauthz.AsSystemRestricted(ctx)) if err != nil { return r, err diff --git a/coderd/users.go b/coderd/users.go index 16fa763a79127..54c225e6f53f8 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -37,6 +37,7 @@ import ( // @Router /users/first [get] func (api *API) firstUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() + // nolint:gocritic // Getting user count is a system function. userCount, err := api.Database.GetUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -70,7 +71,6 @@ func (api *API) firstUser(rw http.ResponseWriter, r *http.Request) { // @Success 201 {object} codersdk.CreateFirstUserResponse // @Router /users/first [post] func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { - // TODO: Should this admin system context be in a middleware? ctx := r.Context() var createUser codersdk.CreateFirstUserRequest if !httpapi.Read(ctx, rw, r, &createUser) { @@ -78,6 +78,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { } // This should only function for the first user. + // nolint:gocritic // Getting user count is a system function. userCount, err := api.Database.GetUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/enterprise/replicasync/replicasync.go b/enterprise/replicasync/replicasync.go index e2de2e9663042..0da9c7f34a389 100644 --- a/enterprise/replicasync/replicasync.go +++ b/enterprise/replicasync/replicasync.go @@ -62,6 +62,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data if err != nil { return nil, xerrors.Errorf("ping database: %w", err) } + // nolint:gocritic // Inserting a replica is a system function. replica, err := db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{ ID: options.ID, CreatedAt: database.Now(), @@ -142,6 +143,7 @@ func (m *Manager) loop(ctx context.Context) { case <-ctx.Done(): return case <-deleteTicker.C: + // nolint:gocritic // Deleting a replica is a system function err := m.db.DeleteReplicasUpdatedBefore(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { m.logger.Warn(ctx, "delete old replicas", slog.Error(err)) @@ -219,6 +221,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { defer m.closeWait.Done() // Expect replicas to update once every three times the interval... // If they don't, assume death! + // nolint:gocritic // Reading replicas is a system function replicas, err := m.db.GetReplicasUpdatedAfter(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { return xerrors.Errorf("get replicas: %w", err) @@ -367,6 +370,7 @@ func (m *Manager) Close() error { defer m.mutex.Unlock() ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) defer cancelFunc() + // nolint:gocritic // Updating a replica is a sytsem function. _, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), From 970b717ad59f67f8997e622c8361290a1c882c9e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 13:47:14 +0000 Subject: [PATCH 06/15] add required authz system inovcations in enterprise/coderd --- enterprise/coderd/coderd.go | 4 ++- enterprise/coderd/license/license.go | 7 +++-- enterprise/coderd/provisionerdaemons_test.go | 30 +++++++++++++++++--- enterprise/replicasync/replicasync.go | 10 +++---- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index aedcd49b1a6cd..76832b9405e1d 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -18,6 +18,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd" agplaudit "github.com/coder/coder/coderd/audit" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" @@ -181,7 +182,8 @@ func New(ctx context.Context, options *Options) (*API, error) { ServerName: options.AccessURL.Hostname(), } var err error - api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, &replicasync.Options{ + // nolint:gocritic // ReplicaManager needs system permissions. + api.replicaManager, err = replicasync.New(dbauthz.AsSystemRestricted(ctx), options.Logger, options.Database, options.Pubsub, &replicasync.Options{ ID: api.AGPL.ID, RelayAddress: options.DERPServerRelayAddress, RegionID: int32(options.DERPServerRegionID), diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index 39bb0bb65ffd6..d29dad402e613 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -12,6 +12,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/codersdk" ) @@ -39,12 +40,14 @@ func Entitlements( } } - licenses, err := db.GetUnexpiredLicenses(ctx) + // nolint:gocritic // Getting unexpired licenses is a system function. + licenses, err := db.GetUnexpiredLicenses(dbauthz.AsSystemRestricted(ctx)) if err != nil { return entitlements, err } - activeUserCount, err := db.GetActiveUserCount(ctx) + // nolint:gocritic // Getting active user count is a system function. + activeUserCount, err := db.GetActiveUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { return entitlements, xerrors.Errorf("query active user count: %w", err) } diff --git a/enterprise/coderd/provisionerdaemons_test.go b/enterprise/coderd/provisionerdaemons_test.go index e04a7a9c4d39d..04f3b5cfe6470 100644 --- a/enterprise/coderd/provisionerdaemons_test.go +++ b/enterprise/coderd/provisionerdaemons_test.go @@ -11,6 +11,7 @@ import ( "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/provisionerdserver" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/enterprise/coderd/coderdenttest" "github.com/coder/coder/enterprise/coderd/license" @@ -20,6 +21,22 @@ import ( func TestProvisionerDaemonServe(t *testing.T) { t.Parallel() + t.Run("OK", func(t *testing.T) { + t.Parallel() + client := coderdenttest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureExternalProvisionerDaemons: 1, + }, + }) + srv, err := client.ServeProvisionerDaemon(context.Background(), user.OrganizationID, []codersdk.ProvisionerType{ + codersdk.ProvisionerTypeEcho, + }, map[string]string{}) + require.NoError(t, err) + srv.DRPCConn().Close() + }) + t.Run("NoLicense", func(t *testing.T) { t.Parallel() client := coderdenttest.New(t, nil) @@ -42,11 +59,16 @@ func TestProvisionerDaemonServe(t *testing.T) { codersdk.FeatureExternalProvisionerDaemons: 1, }, }) - srv, err := client.ServeProvisionerDaemon(context.Background(), user.OrganizationID, []codersdk.ProvisionerType{ + another, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOrgAdmin(user.OrganizationID)) + _, err := another.ServeProvisionerDaemon(context.Background(), user.OrganizationID, []codersdk.ProvisionerType{ codersdk.ProvisionerTypeEcho, - }, map[string]string{}) - require.NoError(t, err) - srv.DRPCConn().Close() + }, map[string]string{ + provisionerdserver.TagScope: provisionerdserver.ScopeOrganization, + }) + require.Error(t, err) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusForbidden, apiError.StatusCode()) }) t.Run("OrganizationNoPerms", func(t *testing.T) { diff --git a/enterprise/replicasync/replicasync.go b/enterprise/replicasync/replicasync.go index 0da9c7f34a389..7c0eb5056f1bb 100644 --- a/enterprise/replicasync/replicasync.go +++ b/enterprise/replicasync/replicasync.go @@ -19,7 +19,6 @@ import ( "github.com/coder/coder/buildinfo" "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/database/dbauthz" ) var PubsubEvent = "replica" @@ -63,7 +62,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data return nil, xerrors.Errorf("ping database: %w", err) } // nolint:gocritic // Inserting a replica is a system function. - replica, err := db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{ + replica, err := db.InsertReplica(ctx, database.InsertReplicaParams{ ID: options.ID, CreatedAt: database.Now(), StartedAt: database.Now(), @@ -144,7 +143,7 @@ func (m *Manager) loop(ctx context.Context) { return case <-deleteTicker.C: // nolint:gocritic // Deleting a replica is a system function - err := m.db.DeleteReplicasUpdatedBefore(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) + err := m.db.DeleteReplicasUpdatedBefore(ctx, m.updateInterval()) if err != nil { m.logger.Warn(ctx, "delete old replicas", slog.Error(err)) } @@ -222,7 +221,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { // Expect replicas to update once every three times the interval... // If they don't, assume death! // nolint:gocritic // Reading replicas is a system function - replicas, err := m.db.GetReplicasUpdatedAfter(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) + replicas, err := m.db.GetReplicasUpdatedAfter(ctx, m.updateInterval()) if err != nil { return xerrors.Errorf("get replicas: %w", err) } @@ -280,6 +279,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { m.mutex.Lock() defer m.mutex.Unlock() + // nolint:gocritic // Updating a replica is a system function. replica, err := m.db.UpdateReplica(ctx, database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), @@ -371,7 +371,7 @@ func (m *Manager) Close() error { ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) defer cancelFunc() // nolint:gocritic // Updating a replica is a sytsem function. - _, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{ + _, err := m.db.UpdateReplica(ctx, database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), StartedAt: m.self.StartedAt, From e5a7cad2b5e0350cda60918f0a6d40521208d27b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 14:10:34 +0000 Subject: [PATCH 07/15] dbauthz: move GetUsersByIDs out of system, modify RBAC check to ResourceUser --- coderd/database/dbauthz/querier.go | 12 ++++++++++++ coderd/database/dbauthz/querier_test.go | 7 +++++++ coderd/database/dbauthz/system.go | 11 ----------- coderd/database/dbauthz/system_test.go | 7 ------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 052863e2edc74..4de696222a828 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -963,6 +963,18 @@ func (q *querier) GetUserByID(ctx context.Context, id uuid.UUID) (database.User, return fetch(q.log, q.auth, q.db.GetUserByID)(ctx, id) } +// GetUsersByIDs is only used for usernames on workspace return data. +// This function should be replaced by joining this data to the workspace query +// itself. +func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) { + for _, uid := range ids { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil { + return nil, err + } + } + return q.db.GetUsersByIDs(ctx, ids) +} + func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) { return q.db.GetAuthorizedUserCount(ctx, arg, prepared) } diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index 5c5374a181eed..f31ec4876d000 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -763,6 +763,13 @@ func (s *MethodTestSuite) TestUser() { u := dbgen.User(s.T(), db, database.User{}) check.Args(u.ID).Asserts(u, rbac.ActionRead).Returns(u) })) + s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) { + a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)}) + b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) + check.Args([]uuid.UUID{a.ID, b.ID}). + Asserts(a, rbac.ActionRead, b, rbac.ActionRead). + Returns(slice.New(a, b)) + })) s.Run("GetAuthorizedUserCount", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.User(s.T(), db, database.User{}) check.Args(database.GetFilteredUserCountParams{}, emptyPreparedAuthorized{}).Asserts().Returns(int64(1)) diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index 16a5848033d39..8a646020626c1 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -36,17 +36,6 @@ func (q *querier) GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context, } return q.db.GetWorkspaceResourceMetadataByResourceIDs(ctx, ids) } - -// GetUsersByIDs is only used for usernames on workspace return data. -// This function should be replaced by joining this data to the workspace query -// itself. -func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err - } - return q.db.GetUsersByIDs(ctx, ids) -} - func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) { if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { return nil, err diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go index 536bafa32cd0f..9926dacd3e552 100644 --- a/coderd/database/dbauthz/system_test.go +++ b/coderd/database/dbauthz/system_test.go @@ -157,13 +157,6 @@ func (s *MethodTestSuite) TestSystemFunctions() { Asserts(rbac.ResourceSystem, rbac.ActionRead). Returns(slice.New(tv1, tv2, tv3)) })) - s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) { - a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)}) - b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()}) - check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts(rbac.ResourceSystem, rbac.ActionRead). - Returns(slice.New(a, b)) - })) s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) { aWs := dbgen.Workspace(s.T(), db, database.Workspace{}) aBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: aWs.ID, JobID: uuid.New()}) From 29f1d5d42c4a81c67dc256012eb231b1b8c632ef Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 14:30:16 +0000 Subject: [PATCH 08/15] fix updatecheck --- coderd/updatecheck/updatecheck.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/updatecheck/updatecheck.go b/coderd/updatecheck/updatecheck.go index f4de324ea1812..0f5d2ce913d36 100644 --- a/coderd/updatecheck/updatecheck.go +++ b/coderd/updatecheck/updatecheck.go @@ -210,7 +210,8 @@ func (c *Checker) update() (r Result, err error) { return r, xerrors.Errorf("json marshal result: %w", err) } - err = c.db.InsertOrUpdateLastUpdateCheck(ctx, string(b)) + // nolint:gocritic // Inserting the last update check is a system function. + err = c.db.InsertOrUpdateLastUpdateCheck(dbauthz.AsSystemRestricted(ctx), string(b)) if err != nil { return r, err } From 8a975fc0c9934ff0856bfa4c67149e22aca3115a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 14:39:06 +0000 Subject: [PATCH 09/15] avoid index out of bounds in api.workspaceBuild --- coderd/workspaceapps_test.go | 2 ++ coderd/workspacebuilds.go | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index a248f14d2f168..690f8ba35bb26 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -1309,6 +1309,8 @@ func TestAppSharing(t *testing.T) { // Verify that the apps have the correct sharing levels set. workspaceBuild, err := client.WorkspaceBuild(ctx, workspace.LatestBuild.ID) require.NoError(t, err) + require.NotEmpty(t, workspaceBuild.Resources, "workspace build has no resources") + require.NotEmpty(t, workspaceBuild.Resources[0].Agents, "workspace build has no agents") agnt = workspaceBuild.Resources[0].Agents[0] found := map[string]codersdk.WorkspaceAppSharingLevel{} expected := map[string]codersdk.WorkspaceAppSharingLevel{ diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 064d966941473..f4245769d2a23 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -50,6 +50,24 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { return } + // Ensure we have the job and template version for the workspace build. + // Otherwise we risk a panic in the api.convertWorkspaceBuild call below. + if len(data.jobs) == 0 { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Internal error getting workspace build data.", + Detail: "No job found for workspace build.", + }) + return + } + + if len(data.templateVersions) == 0 { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Internal error getting workspace build data.", + Detail: "No template version found for workspace build.", + }) + return + } + apiBuild, err := api.convertWorkspaceBuild( workspaceBuild, workspace, From 018b804fe552c9f6fb6ab6e597e49b5ac00d45d9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 14:40:01 +0000 Subject: [PATCH 10/15] use dbauthz system in workspaceBuildsData --- coderd/workspaceapps/auth.go | 34 +++++++++++++++++++++++----------- coderd/workspacebuilds.go | 17 ++++++++++++----- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go index 66bd8cf8b7174..acba68a4cfaf9 100644 --- a/coderd/workspaceapps/auth.go +++ b/coderd/workspaceapps/auth.go @@ -14,6 +14,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" @@ -38,6 +39,12 @@ const ( // // Upstream code should avoid any database calls ever. func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { + // nolint:gocritic // We need to make a number of database calls. Setting a system context here + // // is simpler than calling dbauthz.AsSystemRestricted on every call. + // // dangerousSystemCtx is only used for database calls. The actual authentication + // // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's + // // permissions. + dangerousSystemCtx := dbauthz.AsSystemRestricted(r.Context()) err := appReq.Validate() if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "invalid app request") @@ -108,13 +115,14 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe userErr error ) if userID, uuidErr := uuid.Parse(appReq.UsernameOrID); uuidErr == nil { - user, userErr = p.Database.GetUserByID(r.Context(), userID) + user, userErr = p.Database.GetUserByID(dangerousSystemCtx, userID) } else { - user, userErr = p.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{ + user, userErr = p.Database.GetUserByEmailOrUsername(dangerousSystemCtx, database.GetUserByEmailOrUsernameParams{ Username: appReq.UsernameOrID, }) } if xerrors.Is(userErr, sql.ErrNoRows) { + // TODO: add coverage p.writeWorkspaceApp404(rw, r, &appReq, fmt.Sprintf("user %q not found", appReq.UsernameOrID)) return nil, false } else if userErr != nil { @@ -129,9 +137,9 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe workspaceErr error ) if workspaceID, uuidErr := uuid.Parse(appReq.WorkspaceNameOrID); uuidErr == nil { - workspace, workspaceErr = p.Database.GetWorkspaceByID(r.Context(), workspaceID) + workspace, workspaceErr = p.Database.GetWorkspaceByID(dangerousSystemCtx, workspaceID) } else { - workspace, workspaceErr = p.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ + workspace, workspaceErr = p.Database.GetWorkspaceByOwnerIDAndName(dangerousSystemCtx, database.GetWorkspaceByOwnerIDAndNameParams{ OwnerID: user.ID, Name: appReq.WorkspaceNameOrID, Deleted: false, @@ -153,15 +161,16 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe trustAgent = false ) if agentID, uuidErr := uuid.Parse(appReq.AgentNameOrID); uuidErr == nil { - agent, agentErr = p.Database.GetWorkspaceAgentByID(r.Context(), agentID) + agent, agentErr = p.Database.GetWorkspaceAgentByID(dangerousSystemCtx, agentID) } else { - build, err := p.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) + build, err := p.Database.GetLatestWorkspaceBuildByWorkspaceID(dangerousSystemCtx, workspace.ID) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get latest workspace build") return nil, false } - resources, err := p.Database.GetWorkspaceResourcesByJobID(r.Context(), build.JobID) + // nolint:gocritic // We need to fetch the agent to authenticate the request. This is a system function. + resources, err := p.Database.GetWorkspaceResourcesByJobID(dangerousSystemCtx, build.JobID) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get workspace resources") return nil, false @@ -171,7 +180,8 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe resourcesIDs = append(resourcesIDs, resource.ID) } - agents, err := p.Database.GetWorkspaceAgentsByResourceIDs(r.Context(), resourcesIDs) + // nolint:gocritic // We need to fetch the agent to authenticate the request. This is a system function. + agents, err := p.Database.GetWorkspaceAgentsByResourceIDs(dangerousSystemCtx, resourcesIDs) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get workspace agents") return nil, false @@ -209,12 +219,13 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe // Verify the agent belongs to the workspace. if !trustAgent { - agentResource, err := p.Database.GetWorkspaceResourceByID(r.Context(), agent.ResourceID) + //nolint:gocritic // We need to fetch the agent to authenticate the request. This is a system function. + agentResource, err := p.Database.GetWorkspaceResourceByID(dangerousSystemCtx, agent.ResourceID) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get agent resource") return nil, false } - build, err := p.Database.GetWorkspaceBuildByJobID(r.Context(), agentResource.JobID) + build, err := p.Database.GetWorkspaceBuildByJobID(dangerousSystemCtx, agentResource.JobID) if err != nil { p.writeWorkspaceApp500(rw, r, &appReq, err, "get agent workspace build") return nil, false @@ -324,7 +335,8 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe // error while looking it up, an HTML error page is returned and false is // returned so the caller can return early. func (p *Provider) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agentID uuid.UUID, appSlug string) (database.WorkspaceApp, bool) { - app, err := p.Database.GetWorkspaceAppByAgentIDAndSlug(r.Context(), database.GetWorkspaceAppByAgentIDAndSlugParams{ + // nolint:gocritic // We need to fetch the workspace app to authorize the request. + app, err := p.Database.GetWorkspaceAppByAgentIDAndSlug(dbauthz.AsSystemRestricted(r.Context()), database.GetWorkspaceAppByAgentIDAndSlugParams{ AgentID: agentID, Slug: appSlug, }) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index f4245769d2a23..6acee2d6fff21 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -16,6 +16,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/provisionerdserver" @@ -967,12 +968,15 @@ func (api *API) workspaceBuildsData(ctx context.Context, workspaces []database.W for _, build := range workspaceBuilds { templateVersionIDs = append(templateVersionIDs, build.TemplateVersionID) } - templateVersions, err := api.Database.GetTemplateVersionsByIDs(ctx, templateVersionIDs) + + // nolint:gocritic // Getting template versions by ID is a system function. + templateVersions, err := api.Database.GetTemplateVersionsByIDs(dbauthz.AsSystemRestricted(ctx), templateVersionIDs) if err != nil && !errors.Is(err, sql.ErrNoRows) { return workspaceBuildsData{}, xerrors.Errorf("get template versions: %w", err) } - resources, err := api.Database.GetWorkspaceResourcesByJobIDs(ctx, jobIDs) + // nolint:gocritic // Getting workspace resources by job ID is a system function. + resources, err := api.Database.GetWorkspaceResourcesByJobIDs(dbauthz.AsSystemRestricted(ctx), jobIDs) if err != nil && !errors.Is(err, sql.ErrNoRows) { return workspaceBuildsData{}, xerrors.Errorf("get workspace resources by job: %w", err) } @@ -990,12 +994,14 @@ func (api *API) workspaceBuildsData(ctx context.Context, workspaces []database.W resourceIDs = append(resourceIDs, resource.ID) } - metadata, err := api.Database.GetWorkspaceResourceMetadataByResourceIDs(ctx, resourceIDs) + // nolint:gocritic // Getting workspace resource metadata by resource ID is a system function. + metadata, err := api.Database.GetWorkspaceResourceMetadataByResourceIDs(dbauthz.AsSystemRestricted(ctx), resourceIDs) if err != nil && !errors.Is(err, sql.ErrNoRows) { return workspaceBuildsData{}, xerrors.Errorf("fetching resource metadata: %w", err) } - agents, err := api.Database.GetWorkspaceAgentsByResourceIDs(ctx, resourceIDs) + // nolint:gocritic // Getting workspace agents by resource IDs is a system function. + agents, err := api.Database.GetWorkspaceAgentsByResourceIDs(dbauthz.AsSystemRestricted(ctx), resourceIDs) if err != nil && !errors.Is(err, sql.ErrNoRows) { return workspaceBuildsData{}, xerrors.Errorf("get workspace agents: %w", err) } @@ -1015,7 +1021,8 @@ func (api *API) workspaceBuildsData(ctx context.Context, workspaces []database.W agentIDs = append(agentIDs, agent.ID) } - apps, err := api.Database.GetWorkspaceAppsByAgentIDs(ctx, agentIDs) + // nolint:gocritic // Getting workspace apps by agent IDs is a system function. + apps, err := api.Database.GetWorkspaceAppsByAgentIDs(dbauthz.AsSystemRestricted(ctx), agentIDs) if err != nil && !errors.Is(err, sql.ErrNoRows) { return workspaceBuildsData{}, xerrors.Errorf("fetching workspace apps: %w", err) } From 7ecb2f0edd038fb36055121efea3b93dccb254d9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 14:42:56 +0000 Subject: [PATCH 11/15] remove system auth requirement on provisionerjob / provisionerdaemon functions as this breaks external provisionerds --- coderd/database/dbauthz/dbauthz.go | 2 + coderd/database/dbauthz/system.go | 57 +++++++++++++++----------- coderd/database/dbauthz/system_test.go | 24 +++++++---- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 19bfc385b7e24..2d177ef6fb79c 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -123,6 +123,7 @@ var ( Name: "provisionerd", DisplayName: "Provisioner Daemon", Site: rbac.Permissions(map[string][]rbac.Action{ + // TODO: Add ProvisionerJob resource type. rbac.ResourceFile.Type: {rbac.ActionRead}, rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate}, @@ -142,6 +143,7 @@ var ( Name: "autostart", DisplayName: "Autostart Daemon", Site: rbac.Permissions(map[string][]rbac.Action{ + rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, rbac.ResourceTemplate.Type: {rbac.ActionRead, rbac.ActionUpdate}, rbac.ResourceWorkspace.Type: {rbac.ActionRead, rbac.ActionUpdate}, }), diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index 8a646020626c1..5418c57c96b94 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -36,10 +36,12 @@ func (q *querier) GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context, } return q.db.GetWorkspaceResourceMetadataByResourceIDs(ctx, ids) } + +// TODO: we need to add a provisioner job resource func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err - } + // if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + // return nil, err + // } return q.db.GetProvisionerJobsByIDs(ctx, ids) } @@ -293,10 +295,11 @@ func (q *querier) GetParameterSchemasCreatedAfter(ctx context.Context, createdAt return q.db.GetParameterSchemasCreatedAfter(ctx, createdAt) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.ProvisionerJob, error) { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err - } + // if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + // return nil, err + // } return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt) } @@ -323,45 +326,51 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab return q.db.InsertWorkspaceResourceMetadata(ctx, arg) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) { - if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { - return database.ProvisionerJob{}, err - } + // if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + // return database.ProvisionerJob{}, err + // } return q.db.AcquireProvisionerJob(ctx, arg) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteByIDParams) error { - if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { - return err - } + // if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + // return err + // } return q.db.UpdateProvisionerJobWithCompleteByID(ctx, arg) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error { - if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { - return err - } + // if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + // return err + // } return q.db.UpdateProvisionerJobByID(ctx, arg) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) InsertProvisionerJob(ctx context.Context, arg database.InsertProvisionerJobParams) (database.ProvisionerJob, error) { - if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { - return database.ProvisionerJob{}, err - } + // if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + // return database.ProvisionerJob{}, err + // } return q.db.InsertProvisionerJob(ctx, arg) } +// TODO: We need to create a ProvisionerJob resource type func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.InsertProvisionerJobLogsParams) ([]database.ProvisionerJobLog, error) { - if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { - return nil, err - } + // if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + // return nil, err + // } return q.db.InsertProvisionerJobLogs(ctx, arg) } +// TODO: We need to create a ProvisionerDaemon resource type func (q *querier) InsertProvisionerDaemon(ctx context.Context, arg database.InsertProvisionerDaemonParams) (database.ProvisionerDaemon, error) { - if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { - return database.ProvisionerDaemon{}, err - } + // if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + // return database.ProvisionerDaemon{}, err + // } return q.db.InsertProvisionerDaemon(ctx, arg) } diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go index 9926dacd3e552..4ce8d6a8a1066 100644 --- a/coderd/database/dbauthz/system_test.go +++ b/coderd/database/dbauthz/system_test.go @@ -138,8 +138,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) })) s.Run("GetProvisionerJobsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { + // TODO: add provisioner job resource type _ = dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts(rbac.ResourceSystem, rbac.ActionRead) + check.Args(time.Now()).Asserts( /*rbac.ResourceSystem, rbac.ActionRead*/ ) })) s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) @@ -205,10 +206,11 @@ func (s *MethodTestSuite) TestSystemFunctions() { Returns([]database.WorkspaceAgent{agt}) })) s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { + // TODO: add a ProvisionerJob resource type a := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) b := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts(rbac.ResourceSystem, rbac.ActionRead). + Asserts( /*rbac.ResourceSystem, rbac.ActionRead*/ ). Returns(slice.New(a, b)) })) s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) { @@ -229,43 +231,49 @@ func (s *MethodTestSuite) TestSystemFunctions() { }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) })) s.Run("AcquireProvisionerJob", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{ StartedAt: sql.NullTime{Valid: false}, }) check.Args(database.AcquireProvisionerJobParams{Types: []database.ProvisionerType{j.Provisioner}}). - Asserts(rbac.ResourceSystem, rbac.ActionUpdate) + Asserts( /*rbac.ResourceSystem, rbac.ActionUpdate*/ ) })) s.Run("UpdateProvisionerJobWithCompleteByID", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobWithCompleteByIDParams{ ID: j.ID, - }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate) + }).Asserts( /*rbac.ResourceSystem, rbac.ActionUpdate*/ ) })) s.Run("UpdateProvisionerJobByID", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobByIDParams{ ID: j.ID, UpdatedAt: time.Now(), - }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate) + }).Asserts( /*rbac.ResourceSystem, rbac.ActionUpdate*/ ) })) s.Run("InsertProvisionerJob", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerJob resource check.Args(database.InsertProvisionerJobParams{ ID: uuid.New(), Provisioner: database.ProvisionerTypeEcho, StorageMethod: database.ProvisionerStorageMethodFile, Type: database.ProvisionerJobTypeWorkspaceBuild, - }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) + }).Asserts( /*rbac.ResourceSystem, rbac.ActionCreate*/ ) })) s.Run("InsertProvisionerJobLogs", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{}) check.Args(database.InsertProvisionerJobLogsParams{ JobID: j.ID, - }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) + }).Asserts( /*rbac.ResourceSystem, rbac.ActionCreate*/ ) })) s.Run("InsertProvisionerDaemon", s.Subtest(func(db database.Store, check *expects) { + // TODO: we need to create a ProvisionerDaemon resource check.Args(database.InsertProvisionerDaemonParams{ ID: uuid.New(), - }).Asserts(rbac.ResourceSystem, rbac.ActionCreate) + }).Asserts( /*rbac.ResourceSystem, rbac.ActionCreate*/ ) })) s.Run("InsertTemplateVersionParameter", s.Subtest(func(db database.Store, check *expects) { v := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{}) From 14a25a6f0c4941c31bfc21f08488c2e2cc0a123e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 9 Mar 2023 17:24:29 +0000 Subject: [PATCH 12/15] Add dbauthz system to api.provisionerJobResources --- coderd/provisionerjobs.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index aac27cfd95d9b..d324e9551f690 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -201,7 +201,9 @@ func (api *API) provisionerJobResources(rw http.ResponseWriter, r *http.Request, }) return } - resources, err := api.Database.GetWorkspaceResourcesByJobID(ctx, job.ID) + + // nolint:gocritic // GetWorkspaceResourcesByJobID is a system function. + resources, err := api.Database.GetWorkspaceResourcesByJobID(dbauthz.AsSystemRestricted(ctx), job.ID) if errors.Is(err, sql.ErrNoRows) { err = nil } @@ -216,7 +218,9 @@ func (api *API) provisionerJobResources(rw http.ResponseWriter, r *http.Request, for _, resource := range resources { resourceIDs = append(resourceIDs, resource.ID) } - resourceAgents, err := api.Database.GetWorkspaceAgentsByResourceIDs(ctx, resourceIDs) + + // nolint:gocritic // GetWorkspaceAgentsByResourceIDs is a system function. + resourceAgents, err := api.Database.GetWorkspaceAgentsByResourceIDs(dbauthz.AsSystemRestricted(ctx), resourceIDs) if errors.Is(err, sql.ErrNoRows) { err = nil } @@ -231,6 +235,8 @@ func (api *API) provisionerJobResources(rw http.ResponseWriter, r *http.Request, for _, agent := range resourceAgents { resourceAgentIDs = append(resourceAgentIDs, agent.ID) } + + // nolint:gocritic // GetWorkspaceAppsByAgentIDs is a system function. apps, err := api.Database.GetWorkspaceAppsByAgentIDs(ctx, resourceAgentIDs) if errors.Is(err, sql.ErrNoRows) { err = nil @@ -242,7 +248,9 @@ func (api *API) provisionerJobResources(rw http.ResponseWriter, r *http.Request, }) return } - resourceMetadata, err := api.Database.GetWorkspaceResourceMetadataByResourceIDs(ctx, resourceIDs) + + // nolint:gocritic // GetWorkspaceResourceMetadataByResourceIDs is a system function. + resourceMetadata, err := api.Database.GetWorkspaceResourceMetadataByResourceIDs(dbauthz.AsSystemRestricted(ctx), resourceIDs) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspace metadata.", From 177cbe135c86d40ce99939ee81f705c27d39d6d1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 10 Mar 2023 14:18:20 +0000 Subject: [PATCH 13/15] workspaceapps: add test case for when owner of app is not found --- coderd/workspaceapps/auth_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/coderd/workspaceapps/auth_test.go b/coderd/workspaceapps/auth_test.go index e0a092b85e7c2..d4815f72d80d6 100644 --- a/coderd/workspaceapps/auth_test.go +++ b/coderd/workspaceapps/auth_test.go @@ -540,6 +540,26 @@ func Test_ResolveRequest(t *testing.T) { require.Nil(t, ticket) }) + t.Run("UserNotFound", func(t *testing.T) { + t.Parallel() + req := workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: "thisuserdoesnotexist", + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: appNameOwner, + } + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + ticket, ok := api.WorkspaceAppsProvider.ResolveRequest(rw, r, req) + require.False(t, ok) + require.Nil(t, ticket) + }) + t.Run("RedirectSubdomainAuth", func(t *testing.T) { t.Parallel() From 3a003e714cc6b540b576553d7e2bd616b7e9ddff Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 10 Mar 2023 14:26:13 +0000 Subject: [PATCH 14/15] replicasync: use dbauthz system ctx directly in package instead of passing in elevated ctx --- coderd/workspaceapps/auth.go | 1 - enterprise/coderd/coderd.go | 4 +--- enterprise/replicasync/replicasync.go | 11 ++++++----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go index acba68a4cfaf9..b0c40a5ad22f9 100644 --- a/coderd/workspaceapps/auth.go +++ b/coderd/workspaceapps/auth.go @@ -122,7 +122,6 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe }) } if xerrors.Is(userErr, sql.ErrNoRows) { - // TODO: add coverage p.writeWorkspaceApp404(rw, r, &appReq, fmt.Sprintf("user %q not found", appReq.UsernameOrID)) return nil, false } else if userErr != nil { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 76832b9405e1d..aedcd49b1a6cd 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -18,7 +18,6 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd" agplaudit "github.com/coder/coder/coderd/audit" - "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" @@ -182,8 +181,7 @@ func New(ctx context.Context, options *Options) (*API, error) { ServerName: options.AccessURL.Hostname(), } var err error - // nolint:gocritic // ReplicaManager needs system permissions. - api.replicaManager, err = replicasync.New(dbauthz.AsSystemRestricted(ctx), options.Logger, options.Database, options.Pubsub, &replicasync.Options{ + api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, &replicasync.Options{ ID: api.AGPL.ID, RelayAddress: options.DERPServerRelayAddress, RegionID: int32(options.DERPServerRegionID), diff --git a/enterprise/replicasync/replicasync.go b/enterprise/replicasync/replicasync.go index 7c0eb5056f1bb..e9df4c3c58f93 100644 --- a/enterprise/replicasync/replicasync.go +++ b/enterprise/replicasync/replicasync.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/buildinfo" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" ) var PubsubEvent = "replica" @@ -62,7 +63,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data return nil, xerrors.Errorf("ping database: %w", err) } // nolint:gocritic // Inserting a replica is a system function. - replica, err := db.InsertReplica(ctx, database.InsertReplicaParams{ + replica, err := db.InsertReplica(dbauthz.AsSystemRestricted(ctx), database.InsertReplicaParams{ ID: options.ID, CreatedAt: database.Now(), StartedAt: database.Now(), @@ -143,7 +144,7 @@ func (m *Manager) loop(ctx context.Context) { return case <-deleteTicker.C: // nolint:gocritic // Deleting a replica is a system function - err := m.db.DeleteReplicasUpdatedBefore(ctx, m.updateInterval()) + err := m.db.DeleteReplicasUpdatedBefore(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { m.logger.Warn(ctx, "delete old replicas", slog.Error(err)) } @@ -221,7 +222,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { // Expect replicas to update once every three times the interval... // If they don't, assume death! // nolint:gocritic // Reading replicas is a system function - replicas, err := m.db.GetReplicasUpdatedAfter(ctx, m.updateInterval()) + replicas, err := m.db.GetReplicasUpdatedAfter(dbauthz.AsSystemRestricted(ctx), m.updateInterval()) if err != nil { return xerrors.Errorf("get replicas: %w", err) } @@ -280,7 +281,7 @@ func (m *Manager) syncReplicas(ctx context.Context) error { m.mutex.Lock() defer m.mutex.Unlock() // nolint:gocritic // Updating a replica is a system function. - replica, err := m.db.UpdateReplica(ctx, database.UpdateReplicaParams{ + replica, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), StartedAt: m.self.StartedAt, @@ -371,7 +372,7 @@ func (m *Manager) Close() error { ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) defer cancelFunc() // nolint:gocritic // Updating a replica is a sytsem function. - _, err := m.db.UpdateReplica(ctx, database.UpdateReplicaParams{ + _, err := m.db.UpdateReplica(dbauthz.AsSystemRestricted(ctx), database.UpdateReplicaParams{ ID: m.self.ID, UpdatedAt: database.Now(), StartedAt: m.self.StartedAt, From ebd0ef5a21181fe9785677ff470567f2b0f8d823 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 10 Mar 2023 15:59:33 +0000 Subject: [PATCH 15/15] fix tabs vs spaces --- coderd/workspaceapps/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go index b0c40a5ad22f9..6c0b66d011b00 100644 --- a/coderd/workspaceapps/auth.go +++ b/coderd/workspaceapps/auth.go @@ -41,7 +41,7 @@ const ( func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appReq Request) (*Ticket, bool) { // nolint:gocritic // We need to make a number of database calls. Setting a system context here // // is simpler than calling dbauthz.AsSystemRestricted on every call. - // // dangerousSystemCtx is only used for database calls. The actual authentication + // // dangerousSystemCtx is only used for database calls. The actual authentication // // logic is handled in Provider.authorizeWorkspaceApp which directly checks the actor's // // permissions. dangerousSystemCtx := dbauthz.AsSystemRestricted(r.Context())