Skip to content

Commit 3a95c19

Browse files
ethanndicksonethanndickson
committed
auth-filter query
1 parent 298797e commit 3a95c19

File tree

12 files changed

+171
-55
lines changed

12 files changed

+171
-55
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2720,7 +2720,11 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP
27202720
}
27212721

27222722
func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
2723-
return q.db.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
2723+
prep, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceWorkspace.Type)
2724+
if err != nil {
2725+
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
2726+
}
2727+
return q.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, prep)
27242728
}
27252729

27262730
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
@@ -4176,6 +4180,10 @@ func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetW
41764180
return q.GetWorkspaces(ctx, arg)
41774181
}
41784182

4183+
func (q *querier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
4184+
return q.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
4185+
}
4186+
41794187
// GetAuthorizedUsers is not required for dbauthz since GetUsers is already
41804188
// authenticated.
41814189
func (q *querier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, _ rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,6 +1478,14 @@ func (s *MethodTestSuite) TestWorkspace() {
14781478
_ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
14791479
check.Args(ws.OwnerID).Asserts()
14801480
}))
1481+
s.Run("GetAuthorizedWorkspacesAndAgentsByOwnerID", s.Subtest(func(db database.Store, check *expects) {
1482+
ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{})
1483+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1484+
_ = dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1485+
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1486+
_ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
1487+
check.Args(ws.OwnerID, emptyPreparedAuthorized{}).Asserts()
1488+
}))
14811489
s.Run("GetLatestWorkspaceBuildByWorkspaceID", s.Subtest(func(db database.Store, check *expects) {
14821490
ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{})
14831491
b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})

‎coderd/database/dbmem/dbmem.go

Lines changed: 63 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6837,57 +6837,8 @@ func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspa
68376837
}
68386838

68396839
func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
6840-
q.mutex.RLock()
6841-
defer q.mutex.RUnlock()
6842-
6843-
workspaces := make([]database.WorkspaceTable, 0)
6844-
for _, workspace := range q.workspaces {
6845-
if workspace.OwnerID == ownerID && !workspace.Deleted {
6846-
workspaces = append(workspaces, workspace)
6847-
}
6848-
}
6849-
6850-
out := make([]database.GetWorkspacesAndAgentsByOwnerIDRow, 0, len(workspaces))
6851-
for _, w := range workspaces {
6852-
// these always exist
6853-
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
6854-
if err != nil {
6855-
return nil, xerrors.Errorf("get latest build: %w", err)
6856-
}
6857-
6858-
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
6859-
if err != nil {
6860-
return nil, xerrors.Errorf("get provisioner job: %w", err)
6861-
}
6862-
6863-
outAgents := make([]database.AgentIDNamePair, 0)
6864-
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
6865-
if err != nil {
6866-
return nil, xerrors.Errorf("get workspace resources: %w", err)
6867-
}
6868-
if len(resources) > 0 {
6869-
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
6870-
if err != nil {
6871-
return nil, xerrors.Errorf("get workspace agents: %w", err)
6872-
}
6873-
for _, a := range agents {
6874-
outAgents = append(outAgents, database.AgentIDNamePair{
6875-
ID: a.ID,
6876-
Name: a.Name,
6877-
})
6878-
}
6879-
}
6880-
6881-
out = append(out, database.GetWorkspacesAndAgentsByOwnerIDRow{
6882-
ID: w.ID,
6883-
Name: w.Name,
6884-
JobStatus: job.JobStatus,
6885-
Transition: build.Transition,
6886-
Agents: outAgents,
6887-
})
6888-
}
6889-
6890-
return out, nil
6840+
// No auth filter.
6841+
return q.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, nil)
68916842
}
68926843

68936844
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
@@ -11269,6 +11220,67 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
1126911220
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
1127011221
}
1127111222

11223+
func (q *FakeQuerier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
11224+
q.mutex.RLock()
11225+
defer q.mutex.RUnlock()
11226+
11227+
if prepared != nil {
11228+
// Call this to match the same function calls as the SQL implementation.
11229+
_, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
11230+
if err != nil {
11231+
return nil, err
11232+
}
11233+
}
11234+
workspaces := make([]database.WorkspaceTable, 0)
11235+
for _, workspace := range q.workspaces {
11236+
if workspace.OwnerID == ownerID && !workspace.Deleted {
11237+
workspaces = append(workspaces, workspace)
11238+
}
11239+
}
11240+
11241+
out := make([]database.GetWorkspacesAndAgentsByOwnerIDRow, 0, len(workspaces))
11242+
for _, w := range workspaces {
11243+
// these always exist
11244+
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
11245+
if err != nil {
11246+
return nil, xerrors.Errorf("get latest build: %w", err)
11247+
}
11248+
11249+
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
11250+
if err != nil {
11251+
return nil, xerrors.Errorf("get provisioner job: %w", err)
11252+
}
11253+
11254+
outAgents := make([]database.AgentIDNamePair, 0)
11255+
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
11256+
if err != nil {
11257+
return nil, xerrors.Errorf("get workspace resources: %w", err)
11258+
}
11259+
if len(resources) > 0 {
11260+
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
11261+
if err != nil {
11262+
return nil, xerrors.Errorf("get workspace agents: %w", err)
11263+
}
11264+
for _, a := range agents {
11265+
outAgents = append(outAgents, database.AgentIDNamePair{
11266+
ID: a.ID,
11267+
Name: a.Name,
11268+
})
11269+
}
11270+
}
11271+
11272+
out = append(out, database.GetWorkspacesAndAgentsByOwnerIDRow{
11273+
ID: w.ID,
11274+
Name: w.Name,
11275+
JobStatus: job.JobStatus,
11276+
Transition: build.Transition,
11277+
Agents: outAgents,
11278+
})
11279+
}
11280+
11281+
return out, nil
11282+
}
11283+
1127211284
func (q *FakeQuerier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
1127311285
if err := validateDatabaseType(arg); err != nil {
1127411286
return nil, err

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 0 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: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000267_workspace_updates.down.sql renamed to ‎coderd/database/migrations/000270_workspace_updates.down.sql

File renamed without changes.

‎coderd/database/migrations/000267_workspace_updates.up.sql renamed to ‎coderd/database/migrations/000270_workspace_updates.up.sql

File renamed without changes.

‎coderd/database/modelqueries.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ 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+
GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsByOwnerIDRow, error)
224225
}
225226

226227
// GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access.
@@ -320,6 +321,49 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
320321
return items, nil
321322
}
322323

324+
func (q *sqlQuerier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsByOwnerIDRow, error) {
325+
authorizedFilter, err := prepared.CompileToSQL(ctx, rbac.ConfigWorkspaces())
326+
if err != nil {
327+
return nil, xerrors.Errorf("compile authorized filter: %w", err)
328+
}
329+
330+
// In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the
331+
// authorizedFilter between the end of the where clause and those statements.
332+
filtered, err := insertAuthorizedFilter(getWorkspacesAndAgentsByOwnerID, fmt.Sprintf(" AND %s", authorizedFilter))
333+
if err != nil {
334+
return nil, xerrors.Errorf("insert authorized filter: %w", err)
335+
}
336+
337+
// The name comment is for metric tracking
338+
query := fmt.Sprintf("-- name: GetAuthorizedWorkspacesAndAgentsByOwnerID :many\n%s", filtered)
339+
rows, err := q.db.QueryContext(ctx, query, ownerID)
340+
if err != nil {
341+
return nil, err
342+
}
343+
defer rows.Close()
344+
var items []GetWorkspacesAndAgentsByOwnerIDRow
345+
for rows.Next() {
346+
var i GetWorkspacesAndAgentsByOwnerIDRow
347+
if err := rows.Scan(
348+
&i.ID,
349+
&i.Name,
350+
&i.JobStatus,
351+
&i.Transition,
352+
pq.Array(&i.Agents),
353+
); err != nil {
354+
return nil, err
355+
}
356+
items = append(items, i)
357+
}
358+
if err := rows.Close(); err != nil {
359+
return nil, err
360+
}
361+
if err := rows.Err(); err != nil {
362+
return nil, err
363+
}
364+
return items, nil
365+
}
366+
323367
type userQuerier interface {
324368
GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error)
325369
}

‎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: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ 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"
2728
"github.com/coder/coder/v2/coderd/rbac"
29+
"github.com/coder/coder/v2/coderd/rbac/policy"
2830
"github.com/coder/coder/v2/testutil"
2931
)
3032

@@ -612,7 +614,7 @@ func TestGetWorkspaceAgentUsageStatsAndLabels(t *testing.T) {
612614
})
613615
}
614616

615-
func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
617+
func TestGetAuthorizedWorkspacesAndAgentsByOwnerID(t *testing.T) {
616618
t.Parallel()
617619
if testing.Short() {
618620
t.SkipNow()
@@ -628,6 +630,7 @@ func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
628630
owner := dbgen.User(t, db, database.User{
629631
RBACRoles: []string{rbac.RoleOwner().String()},
630632
})
633+
user := dbgen.User(t, db, database.User{})
631634
tpl := dbgen.Template(t, db, database.Template{
632635
OrganizationID: org.ID,
633636
CreatedBy: owner.ID,
@@ -666,7 +669,22 @@ func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
666669
CreateAgent: false,
667670
})
668671

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

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/workspaces.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,8 @@ WHERE
724724
-- Filter by owner_id
725725
workspaces.owner_id = @owner_id :: uuid
726726
AND workspaces.deleted = false
727+
-- Authorize Filter clause will be injected below in GetAuthorizedWorkspacesAndAgentsByOwnerID
728+
-- @authorize_filter
727729
GROUP BY workspaces.id, workspaces.name, latest_build.job_status, latest_build.job_id, latest_build.transition;
728730

729731

0 commit comments

Comments
 (0)