Skip to content

Commit e2a3d15

Browse files
ethanndicksonethanndickson
committed
auth-filter query
1 parent a279cef commit e2a3d15

File tree

12 files changed

+178
-55
lines changed

12 files changed

+178
-55
lines changed

‎coderd/database/dbauthz/dbauthz.go

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

27682768
func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
2769-
return q.db.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
2769+
prep, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceWorkspace.Type)
2770+
if err != nil {
2771+
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
2772+
}
2773+
return q.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, prep)
27702774
}
27712775

27722776
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
@@ -4222,6 +4226,10 @@ func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetW
42224226
return q.GetWorkspaces(ctx, arg)
42234227
}
42244228

4229+
func (q *querier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
4230+
return q.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
4231+
}
4232+
42254233
// GetAuthorizedUsers is not required for dbauthz since GetUsers is already
42264234
// authenticated.
42274235
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
@@ -6840,57 +6840,8 @@ func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspa
68406840
}
68416841

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

68966847
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.WorkspaceTable, error) {
@@ -11272,6 +11223,67 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
1127211223
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
1127311224
}
1127411225

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

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 14 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/000272_workspace_updates.down.sql

File renamed without changes.

‎coderd/database/migrations/000267_workspace_updates.up.sql renamed to ‎coderd/database/migrations/000272_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)