Skip to content

Commit db4f308

Browse files
ethanndicksonethanndickson
committed
remove auth, filter by owner id
1 parent 5ce9631 commit db4f308

File tree

10 files changed

+88
-188
lines changed

10 files changed

+88
-188
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,12 +2712,8 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP
27122712
return q.db.GetAuthorizedWorkspaces(ctx, arg, prep)
27132713
}
27142714

2715-
func (q *querier) GetWorkspacesAndAgents(ctx context.Context) ([]database.GetWorkspacesAndAgentsRow, error) {
2716-
prep, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceWorkspace.Type)
2717-
if err != nil {
2718-
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
2719-
}
2720-
return q.db.GetAuthorizedWorkspacesAndAgents(ctx, prep)
2715+
func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
2716+
return q.db.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
27212717
}
27222718

27232719
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
@@ -4157,10 +4153,6 @@ func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetW
41574153
return q.GetWorkspaces(ctx, arg)
41584154
}
41594155

4160-
func (q *querier) GetAuthorizedWorkspacesAndAgents(ctx context.Context, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsRow, error) {
4161-
return q.GetWorkspacesAndAgents(ctx)
4162-
}
4163-
41644156
// GetAuthorizedUsers is not required for dbauthz since GetUsers is already
41654157
// authenticated.
41664158
func (q *querier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, _ rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,13 +1484,13 @@ func (s *MethodTestSuite) TestWorkspace() {
14841484
// No asserts here because SQLFilter.
14851485
check.Args(database.GetWorkspacesParams{}, emptyPreparedAuthorized{}).Asserts()
14861486
}))
1487-
s.Run("GetWorkspacesAndAgents", s.Subtest(func(db database.Store, check *expects) {
1488-
// No asserts here because SQLFilter.
1489-
check.Args().Asserts()
1490-
}))
1491-
s.Run("GetAuthorizedWorkspacesAndAgents", s.Subtest(func(db database.Store, check *expects) {
1492-
// No asserts here because SQLFilter.
1493-
check.Args(emptyPreparedAuthorized{}).Asserts()
1487+
s.Run("GetWorkspacesAndAgentsByOwnerID", s.Subtest(func(db database.Store, check *expects) {
1488+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1489+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1490+
_ = dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1491+
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1492+
_ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
1493+
check.Args(ws.OwnerID).Asserts()
14941494
}))
14951495
s.Run("GetLatestWorkspaceBuildByWorkspaceID", s.Subtest(func(db database.Store, check *expects) {
14961496
ws := dbgen.Workspace(s.T(), db, database.Workspace{})

‎coderd/database/dbmem/dbmem.go

Lines changed: 53 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -6731,9 +6731,59 @@ func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspa
67316731
return workspaceRows, err
67326732
}
67336733

6734-
func (q *FakeQuerier) GetWorkspacesAndAgents(ctx context.Context) ([]database.GetWorkspacesAndAgentsRow, error) {
6735-
// No auth filter.
6736-
return q.GetAuthorizedWorkspacesAndAgents(ctx, nil)
6734+
func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
6735+
q.mutex.RLock()
6736+
defer q.mutex.RUnlock()
6737+
6738+
workspaces := make([]database.Workspace, 0)
6739+
for _, workspace := range q.workspaces {
6740+
if workspace.OwnerID == ownerID {
6741+
workspaces = append(workspaces, workspace)
6742+
}
6743+
}
6744+
6745+
out := make([]database.GetWorkspacesAndAgentsByOwnerIDRow, 0, len(workspaces))
6746+
for _, w := range workspaces {
6747+
// these always exist
6748+
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
6749+
if err != nil {
6750+
return nil, xerrors.Errorf("get latest build: %w", err)
6751+
}
6752+
6753+
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
6754+
if err != nil {
6755+
return nil, xerrors.Errorf("get provisioner job: %w", err)
6756+
}
6757+
6758+
outAgents := make([]database.AgentIDNamePair, 0)
6759+
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
6760+
if err != nil {
6761+
return nil, xerrors.Errorf("get workspace resources: %w", err)
6762+
}
6763+
if len(resources) > 0 {
6764+
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
6765+
if err != nil {
6766+
return nil, xerrors.Errorf("get workspace agents: %w", err)
6767+
}
6768+
for _, a := range agents {
6769+
outAgents = append(outAgents, database.AgentIDNamePair{
6770+
ID: a.ID,
6771+
Name: a.Name,
6772+
})
6773+
}
6774+
}
6775+
6776+
out = append(out, database.GetWorkspacesAndAgentsByOwnerIDRow{
6777+
ID: w.ID,
6778+
Name: w.Name,
6779+
OwnerID: w.OwnerID,
6780+
JobStatus: job.JobStatus,
6781+
Transition: build.Transition,
6782+
Agents: outAgents,
6783+
})
6784+
}
6785+
6786+
return out, nil
67376787
}
67386788

67396789
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
@@ -11102,68 +11152,6 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
1110211152
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
1110311153
}
1110411154

11105-
func (q *FakeQuerier) GetAuthorizedWorkspacesAndAgents(ctx context.Context, prepared rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsRow, error) {
11106-
q.mutex.RLock()
11107-
defer q.mutex.RUnlock()
11108-
11109-
if prepared != nil {
11110-
// Call this to match the same function calls as the SQL implementation.
11111-
_, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
11112-
if err != nil {
11113-
return nil, err
11114-
}
11115-
}
11116-
workspaces := make([]database.Workspace, 0)
11117-
for _, workspace := range q.workspaces {
11118-
if prepared != nil && prepared.Authorize(ctx, workspace.RBACObject()) == nil {
11119-
workspaces = append(workspaces, workspace)
11120-
}
11121-
}
11122-
11123-
out := make([]database.GetWorkspacesAndAgentsRow, 0, len(workspaces))
11124-
for _, w := range workspaces {
11125-
// these always exist
11126-
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
11127-
if err != nil {
11128-
return nil, xerrors.Errorf("get latest build: %w", err)
11129-
}
11130-
11131-
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
11132-
if err != nil {
11133-
return nil, xerrors.Errorf("get provisioner job: %w", err)
11134-
}
11135-
11136-
outAgents := make([]database.AgentIDNamePair, 0)
11137-
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
11138-
if err != nil {
11139-
return nil, xerrors.Errorf("get workspace resources: %w", err)
11140-
}
11141-
if len(resources) > 0 {
11142-
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
11143-
if err != nil {
11144-
return nil, xerrors.Errorf("get workspace agents: %w", err)
11145-
}
11146-
for _, a := range agents {
11147-
outAgents = append(outAgents, database.AgentIDNamePair{
11148-
ID: a.ID,
11149-
Name: a.Name,
11150-
})
11151-
}
11152-
}
11153-
11154-
out = append(out, database.GetWorkspacesAndAgentsRow{
11155-
ID: w.ID,
11156-
Name: w.Name,
11157-
OwnerID: w.OwnerID,
11158-
JobStatus: job.JobStatus,
11159-
Transition: build.Transition,
11160-
Agents: outAgents,
11161-
})
11162-
}
11163-
11164-
return out, nil
11165-
}
11166-
1116711155
func (q *FakeQuerier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
1116811156
if err := validateDatabaseType(arg); err != nil {
1116911157
return nil, err

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 3 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 7 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/modelqueries.go

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,6 @@ func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([
221221

222222
type workspaceQuerier interface {
223223
GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, prepared rbac.PreparedAuthorized) ([]GetWorkspacesRow, error)
224-
GetAuthorizedWorkspacesAndAgents(ctx context.Context, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsRow, error)
225224
}
226225

227226
// GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access.
@@ -313,46 +312,6 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
313312
return items, nil
314313
}
315314

316-
func (q *sqlQuerier) GetAuthorizedWorkspacesAndAgents(ctx context.Context, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsRow, error) {
317-
authorizedFilter, err := prepared.CompileToSQL(ctx, rbac.ConfigWorkspaces())
318-
if err != nil {
319-
return nil, xerrors.Errorf("compile authorized filter: %w", err)
320-
}
321-
filtered, err := insertAuthorizedFilter(getWorkspacesAndAgents, fmt.Sprintf(" WHERE %s", authorizedFilter))
322-
if err != nil {
323-
return nil, xerrors.Errorf("insert authorized filter: %w", err)
324-
}
325-
326-
query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", filtered)
327-
rows, err := q.db.QueryContext(ctx, query)
328-
if err != nil {
329-
return nil, err
330-
}
331-
defer rows.Close()
332-
var items []GetWorkspacesAndAgentsRow
333-
for rows.Next() {
334-
var i GetWorkspacesAndAgentsRow
335-
if err := rows.Scan(
336-
&i.ID,
337-
&i.Name,
338-
&i.OwnerID,
339-
&i.JobStatus,
340-
&i.Transition,
341-
pq.Array(&i.Agents),
342-
); err != nil {
343-
return nil, err
344-
}
345-
items = append(items, i)
346-
}
347-
if err := rows.Close(); err != nil {
348-
return nil, err
349-
}
350-
if err := rows.Err(); err != nil {
351-
return nil, err
352-
}
353-
return items, nil
354-
}
355-
356315
type userQuerier interface {
357316
GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error)
358317
}

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier_test.go

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ import (
2424
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2525
"github.com/coder/coder/v2/coderd/database/dbtime"
2626
"github.com/coder/coder/v2/coderd/database/migrations"
27-
"github.com/coder/coder/v2/coderd/httpmw"
2827
"github.com/coder/coder/v2/coderd/rbac"
29-
"github.com/coder/coder/v2/coderd/rbac/policy"
3028
"github.com/coder/coder/v2/testutil"
3129
)
3230

@@ -614,7 +612,7 @@ func TestGetWorkspaceAgentUsageStatsAndLabels(t *testing.T) {
614612
})
615613
}
616614

617-
func TestGetAuthorizedWorkspacesAndAgents(t *testing.T) {
615+
func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
618616
t.Parallel()
619617
if testing.Short() {
620618
t.SkipNow()
@@ -630,7 +628,6 @@ func TestGetAuthorizedWorkspacesAndAgents(t *testing.T) {
630628
owner := dbgen.User(t, db, database.User{
631629
RBACRoles: []string{rbac.RoleOwner().String()},
632630
})
633-
user := dbgen.User(t, db, database.User{})
634631
tpl := dbgen.Template(t, db, database.Template{
635632
OrganizationID: org.ID,
636633
CreatedBy: owner.ID,
@@ -669,23 +666,7 @@ func TestGetAuthorizedWorkspacesAndAgents(t *testing.T) {
669666
CreateAgent: false,
670667
})
671668

672-
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
673-
674-
userSubject, _, err := httpmw.UserRBACSubject(ctx, db, user.ID, rbac.ExpandableScope(rbac.ScopeAll))
675-
require.NoError(t, err)
676-
preparedUser, err := authorizer.Prepare(ctx, userSubject, policy.ActionRead, rbac.ResourceWorkspace.Type)
677-
require.NoError(t, err)
678-
userCtx := dbauthz.As(ctx, userSubject)
679-
userRows, err := db.GetAuthorizedWorkspacesAndAgents(userCtx, preparedUser)
680-
require.NoError(t, err)
681-
require.Len(t, userRows, 0)
682-
683-
ownerSubject, _, err := httpmw.UserRBACSubject(ctx, db, owner.ID, rbac.ExpandableScope(rbac.ScopeAll))
684-
require.NoError(t, err)
685-
preparedOwner, err := authorizer.Prepare(ctx, ownerSubject, policy.ActionRead, rbac.ResourceWorkspace.Type)
686-
require.NoError(t, err)
687-
ownerCtx := dbauthz.As(ctx, ownerSubject)
688-
ownerRows, err := db.GetAuthorizedWorkspacesAndAgents(ownerCtx, preparedOwner)
669+
ownerRows, err := db.GetWorkspacesAndAgentsByOwnerID(ctx, owner.ID)
689670
require.NoError(t, err)
690671
require.Len(t, ownerRows, 4)
691672
for _, row := range ownerRows {

0 commit comments

Comments
 (0)